Execute OS command - Java

On Wed, 12 Sep 2007 22:43:55 -0000, Earl Lewis
<earl.lewis.1> wrote, quoted or indirectly quoted someone
who said :

>What I want to do is put a timer in this code that will only allow the
>command to attempt to run for a set amount of time, and when that time
>elapses the program will decide that the command isn't going to work
>and it quits with an error message.

See http://mindprod.com/jgloss/timer.html
--
Roedy Green Canadian Mind Products
The Java Glossary
http://mindprod.com

Earl Lewis wrote:

> You java gurus probably hate it when a posting starts like this, but
> here goes anyway. Sorry, in advance!
>
> Disclaimer: I'm not a java guru. I know how to spell it and I've
> written some basic OO stuff. I now have a real problem that I'm trying
> to solve but have no idea how best to implement it.
>
> Here's the situation. I have a small java routine (in Oracle) that
> executes an operating system command. Here's that code:
>
> import java.io.*;
> public class Host {
> public static void executeCommand(String command) {
> try {
> String[] finalCommand;
> if (isWindows()) {
> finalCommand = new String[4];
> // Use the appropriate path for your windows version.
> finalCommand[0] = "C:\\windows\\system32\\cmd.exe"; //
> Windows XP/2003
> //finalCommand[0] = "C:\\winnt\\system32\\cmd.exe"; //
> Windows NT/2000
> finalCommand[1] = "/y";
> finalCommand[2] = "/c";
> finalCommand[3] = command;
> }
> else {
> finalCommand = new String[3];
> finalCommand[0] = "/bin/sh";
> finalCommand[1] = "-c";
> finalCommand[2] = command;
> }
>
> final Process pr = Runtime.getRuntime().exec(finalCommand);
> pr.waitFor();
>
> new Thread(new Runnable(){
> public void run() {
> BufferedReader br_in = null;
> try {
> br_in = new BufferedReader(new
> InputStreamReader(pr.getInputStream()));
> String buff = null;
> while ((buff = br_in.readLine()) != null) {
> System.out.println("Process out :" + buff);
> try {Thread.sleep(100); } catch(Exception e) {}
> }
> br_in.close();
> }
> catch (IOException ioe) {
> System.out.println("Exception caught printing process
> output.");
> ioe.printStackTrace();
> }
> finally {
> try {
> br_in.close();
> } catch (Exception ex) {}
> }
> }
> }).start();
>
> new Thread(new Runnable(){
> public void run() {
> BufferedReader br_err = null;
> try {
> br_err = new BufferedReader(new
> InputStreamReader(pr.getErrorStream()));
> String buff = null;
> while ((buff = br_err.readLine()) != null) {
> System.out.println("Process err :" + buff);
> try {Thread.sleep(100); } catch(Exception e) {}
> }
> br_err.close();
> }
> catch (IOException ioe) {
> System.out.println("Exception caught printing process
> error.");
> ioe.printStackTrace();
> }
> finally {
> try {
> br_err.close();
> } catch (Exception ex) {}
> }
> }
> }).start();
> }
> catch (Exception ex) {
> System.out.println(ex.getLocalizedMessage());
> }
> }
>
> public static boolean isWindows() {
> if
> (System.getProperty("os.name").toLowerCase().indexOf("windows") != -1)
> return true;
> else
> return false;
> }
>
> };
>
> What I want to do is put a timer in this code that will only allow the
> command to attempt to run for a set amount of time, and when that time
> elapses the program will decide that the command isn't going to work
> and it quits with an error message.
>
> The way I understand the documentation, as the code is written right
> now (with the pr.waitFor(); line) it will wait for the process (in
> this case an OS command) to complete before the code will continue.
> That seems to bear out in my testing so far.
>
> I've seen also in the javadocs where you can call Thread.join(50000);
> or something similar, to join the running thread and if it doesn't
> complete in the specified time that it will kill the thread. Is this
> the right approach to solving this problem? Is the problem even clear?
> Any suggestions about how to do this and where to put the suggested
> code snippets will be greatly appreciated. Thank you.
>
> Sincerely,
>
> Earl

I believe there is a flaw in the logic in the above program.

You call Process.waitFor() before you start the threads which read the process's
standard output/error channels. If the process generates a lot of output, such
that either the output or error channel fills and blocks the process, the
waitFor() will not return. The threads to read that output, which should
prevent the process from blocking, haven't been started yet. Start the threads
to read the process output *before* you invoke Process.waitFor().

This may prevent you from needing to terminate if the process doesn't complete
in a fixed time.

--
Nigel Wade, System Administrator, Space Plasma Physics Group,
University of Leicester, Leicester, LE1 7RH, UK
E-mail : nmw@ion.le.ac.uk
Phone : +44 (0)116 2523548, Fax : +44 (0)116 2523555

On Sep 13, 3:11 am, Nigel Wade <n...@ion.le.ac.uk> wrote:
> Earl Lewis wrote:
> > You java gurus probably hate it when a posting starts like this, but
> > here goes anyway. Sorry, in advance!
>
> > Disclaimer: I'm not a java guru. I know how to spell it and I've
> > written some basic OO stuff. I now have a real problem that I'm trying
> > to solve but have no idea how best to implement it.
>
> > Here's the situation. I have a small java routine (in Oracle) that
> > executes an operating system command. Here's that code:
>
> > import java.io.*;
> > public class Host {
> > public static void executeCommand(String command) {
> > try {
> > String[] finalCommand;
> > if (isWindows()) {
> > finalCommand = new String[4];
> > // Use the appropriate path for your windows version.
> > finalCommand[0] = "C:\\windows\\system32\\cmd.exe"; //
> > Windows XP/2003
> > //finalCommand[0] = "C:\\winnt\\system32\\cmd.exe"; //
> > Windows NT/2000
> > finalCommand[1] = "/y";
> > finalCommand[2] = "/c";
> > finalCommand[3] = command;
> > }
> > else {
> > finalCommand = new String[3];
> > finalCommand[0] = "/bin/sh";
> > finalCommand[1] = "-c";
> > finalCommand[2] = command;
> > }
>
> > final Process pr = Runtime.getRuntime().exec(finalCommand);
> > pr.waitFor();
>
> > new Thread(new Runnable(){
> > public void run() {
> > BufferedReader br_in = null;
> > try {
> > br_in = new BufferedReader(new
> > InputStreamReader(pr.getInputStream()));
> > String buff = null;
> > while ((buff = br_in.readLine()) != null) {
> > System.out.println("Process out :" + buff);
> > try {Thread.sleep(100); } catch(Exception e) {}
> > }
> > br_in.close();
> > }
> > catch (IOException ioe) {
> > System.out.println("Exception caught printing process
> > output.");
> > ioe.printStackTrace();
> > }
> > finally {
> > try {
> > br_in.close();
> > } catch (Exception ex) {}
> > }
> > }
> > }).start();
>
> > new Thread(new Runnable(){
> > public void run() {
> > BufferedReader br_err = null;
> > try {
> > br_err = new BufferedReader(new
> > InputStreamReader(pr.getErrorStream()));
> > String buff = null;
> > while ((buff = br_err.readLine()) != null) {
> > System.out.println("Process err :" + buff);
> > try {Thread.sleep(100); } catch(Exception e) {}
> > }
> > br_err.close();
> > }
> > catch (IOException ioe) {
> > System.out.println("Exception caught printing process
> > error.");
> > ioe.printStackTrace();
> > }
> > finally {
> > try {
> > br_err.close();
> > } catch (Exception ex) {}
> > }
> > }
> > }).start();
> > }
> > catch (Exception ex) {
> > System.out.println(ex.getLocalizedMessage());
> > }
> > }
>
> > public static boolean isWindows() {
> > if
> > (System.getProperty("os.name").toLowerCase().indexOf("windows") != -1)
> > return true;
> > else
> > return false;
> > }
>
> > };
>
> > What I want to do is put a timer in this code that will only allow the
> > command to attempt to run for a set amount of time, and when that time
> > elapses the program will decide that the command isn't going to work
> > and it quits with an error message.
>
> > The way I understand the documentation, as the code is written right
> > now (with the pr.waitFor(); line) it will wait for the process (in
> > this case an OS command) to complete before the code will continue.
> > That seems to bear out in my testing so far.
>
> > I've seen also in the javadocs where you can call Thread.join(50000);
> > or something similar, to join the running thread and if it doesn't
> > complete in the specified time that it will kill the thread. Is this
> > the right approach to solving this problem? Is the problem even clear?
> > Any suggestions about how to do this and where to put the suggested
> > code snippets will be greatly appreciated. Thank you.
>
> > Sincerely,
>
> > Earl
>
> I believe there is a flaw in the logic in the above program.
>
> You call Process.waitFor() before you start the threads which read the process's
> standard output/error channels. If the process generates a lot of output, such
> that either the output or error channel fills and blocks the process, the
> waitFor() will not return. The threads to read that output, which should
> prevent the process from blocking, haven't been started yet. Start the threads
> to read the process output *before* you invoke Process.waitFor().
>
> This may prevent you from needing to terminate if the process doesn't complete
> in a fixed time.
>
> --
> Nigel Wade, System Administrator, Space Plasma Physics Group,
> University of Leicester, Leicester, LE1 7RH, UK
> E-mail : n...@ion.le.ac.uk
> Phone : +44 (0)116 2523548, Fax : +44 (0)116 2523555


You may well be right. I can tell you that the code 'works' for the
requirements I've had
to date. Up to this point I've only used this for OS commands that
return some sort of
output almost immediately. Now I'm trying to use it to execute a
ping. For security reasons
many hosts are disallowing ping/icmp and subsequently ping will just
sit there waiting for
a reply. That's why I need to only allow this to run for some fixed
period of time before
it is forcibly terminated and returns an error.

I'll try your suggestion and see if I can get the results I want.
Thanks.

Earl

On Thu, 13 Sep 2007 14:49:59 -0000, Earl Lewis wrote:
> Now I'm trying to use it to execute a ping. For security reasons
> many hosts are disallowing ping/icmp and subsequently ping will just
> sit there waiting for a reply. That's why I need to only allow this
> to run for some fixed period of time before it is forcibly
> terminated and returns an error.

Perhaps the easiest solution in this case would be to specify a
timeout as part of the ping command itself (read the documentation).

Exactly how does disallowing icmp improve security?

/gordon

--