RFR: 8282008: Incorrect handling of quoted arguments in ProcessBuilder [v3]

[Roger Riggs]

On Fri, 18 Feb 2022 17:21:48 GMT, Olga Mikhaltsova <omikhaltcova at openjdk.org> wrote:

>> This fix made equal processing of strings such as ""C:\\Program Files\\Git\\"" before and after JDK-8250568.
>> 
>> For example, it's needed to execute the following command on Windows:
>> `C:\Windows\SysWOW64\WScript.exe "MyVB.vbs" "C:\Program Files\Git" "Test"`
>> it's equal to:
>> `new ProcessBuilder("C:\\Windows\\SysWOW64\\WScript.exe", "MyVB.vbs", ""C:\\Program Files\\Git\\"", "Test").start();`
>> 
>> While processing, the 3rd argument ""C:\\Program Files\\Git\\"" treated as unquoted due to the condition added in JDK-8250568.
>> 
>>     private static String unQuote(String str) {
>>     .. 
>>        if (str.endsWith("\\"")) {
>>             return str;    // not properly quoted, treat as unquoted
>>         }
>>     ..
>>     }
>> 
>> 
>> that leads to the additional surrounding by quotes in ProcessImpl::createCommandLine(..) because needsEscaping(..) returns true due to the space inside the string argument.
>> As a result the native function CreateProcessW (src/java.base/windows/native/libjava/ProcessImpl_md.c) gets the incorrectly quoted argument: 
>> 
>> pcmd = C:\Windows\SysWOW64\WScript.exe MyVB.vbs ""C:\Program Files\Git"" Test
>> (jdk.lang.Process.allowAmbiguousCommands = true)
>> pcmd = "C:\Windows\SysWOW64\WScript.exe" MyVB.vbs ""C:\Program Files\Git\\"" Test
>> (jdk.lang.Process.allowAmbiguousCommands = false)
>> 
>> 
>> Obviously, a string ending with `"\\""` must not be started with `"""` to treat as unquoted overwise it’s should be treated as properly quoted.
>
> Olga Mikhaltsova has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add a new line to the end of test file for JDK-8282008

(I'm still working on a more nuanced fix that works with .exe, .cmd, and with allowAmbiguousCommands both true and false).

The suggested workaround was to remove the application quotes and let ProcessBuilder do the quoting.
That resulted in an extra backslash "\" at the end of a file path. In my investigation, the extra "\" doesn't prevent the 
string from being correctly used as a directory path in either VisualBasic or cmd.exe.
So I'm curious, in the original application that uncovered this problem, what is/was reported as the error?
Was the original application retested with the workaround?

The case of the backslash at the end of an argument occurs mainly in a directory path.
Yes, the argument is different, but does it make a difference that matters in the context in which it appears.

-------------

PR: https://git.openjdk.java.net/jdk/pull/7504