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

Alexey Utkin alexey.utkin at
Thu Apr 25 14:41:55 UTC 2013


Thanks for your comments.
I did the changes in code.
Please, read my answers below.

Here is fixed version:


On 24.04.2013 18:23, Alan Bateman wrote:
> 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:
>> Lines 381-382 in 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 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.
The diagnostic includes logging of the command line  in problem. It 
should not be a problem to localize issue.

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

More information about the core-libs-dev mailing list