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

[Alan Bateman]

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.