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

[Roger Riggs]

On Thu, 3 Mar 2022 15:38:44 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:
> 
>   Reverted addition of the test via echo

As an alternative fix, please take a look at Draft PR: https://github.com/openjdk/jdk/pull/7709.

In the default handling of arguments, the check for what is quoted is reverted to prior to 8255068. First and last quotes are sufficient to identify a "quoted" string. The check for a backslash ("\") is removed.
This original check is sufficient for `jdk.lang.Process.allowAmbiguousCommands = true`.

For the case where the system property `jdk.lang.Process.allowAmbiguousCommands = false`
and the argument has first and last quotes, a backslash ("\") before the final quote must not allow the quote to interpreted as a literal quote and merge the following argument.  The backslashes will doubled to prevent the interpretation of the quote as a literal.  This is the correct encoding if the command uses the ".exe" encoding, when reparsing the arguments the doubled backslashes are reduced to the original contents.
When the command is using the simpler parsing that does not support literal quotes, the backslash before the quote is typically is a trailing backslash on a file path and in that case the additional backslash is redundant and has no effect on the interpretation of the argument as a directory path.

The PR includes a test of the 12 combinations of invoking an "java"/.exe program, a .cmd script, and a Visual Basic script (which uses the .exe rules but different command line parser); with and without application quotes and compares the actual results with the expected arguments.

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

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