Review request: JDK-8012453 (process) Runtime.exec(String) fails if command contains spaces [win] (original) (raw)

Alan Bateman Alan.Bateman at oracle.com
Wed Apr 24 14:23:55 UTC 2013

On 24/04/2013 13:58, Alexey Utkin wrote:

I changed in the ProcessBuilder class to restore the compatibility with Java documentation. In accordance with spec, the IllegalArgumentException exception could not be thrown from the start method. I made it a cause for declared IOException. This part looks good to me.

Thanks for your attention. The test was extended to cover the case. New version: http://cr.openjdk.java.net/~uta/openjdk-webrevs/JDK-8012453/webrev.01/ Lines 381-382 in ProcessImpl.java file are responsible for the second call to Security Manager. The call with [".\Program Files\doNot.cmd" arg] param does not pass the second Security Manager verification in the ExecCommand.java test. Overall I think this looks okay.

In getTokensFromCommand then I guess I would have used a regex rather than legacy StringTokenizer (I realize Runtime is specified to use StringTokenizer for the initial split). I think I agree with Martin on wondering why LinkedList is used but it's not an issue as this is the fallback. Minor nit is that the brace on L161 can be moved to the end of the previous line.

A minor comment is that -Djdk.lang.Process.allowAmbigousCommands=false will not do what is intended (the value of the property is not examined).

The test needs a copyright header but otherwise looks comprehensive. One thing I see is that it doesn't read from the child's output/error streams so if there is a problem then it will be hard to diagnose from the logs.

Minor comments on the test is that the creation of the commands could use try-with-resources.

-Alan.

More information about the core-libs-dev mailing list