RFR: 8067951: System.loadLibrary cannot find library when path contains quoted entry

[Stuart Marks]

Hi Ivan,

It's not clear to me that quoting should be applied at this level. On Unix, 
quotes are stripped by the shell at the time the command line is split into 
arguments, so the values should be treated literally without further quote 
processing.

The referenced blog post at MSDN talks about Windows, which passes the entire 
command string to the program, which then is responsible for splitting it into 
arguments or otherwise parsing it. This seems to occur in the file

     jdk/src/java.base/windows/native/libjli/cmdtoargs.c

If the logic in that file is mishandling quotes, it seems like the fix ought to 
be applied there.

In addition, this change simply strips quotes that happen to occur at the 
boundaries of substrings between path separators. It doesn't do any actual 
quoting, that is, protecting the path separator character so that it can be used 
as part of an actual path instead of as a separator. (But I'm not sure that's 
actually a supported case anyway.) Simply stripping quotes seems wrong.

Unfortunately there are no details in the bug report JDK-8067951, so I can't 
tell what the real problem is.

s'marks


On 12/24/14 2:45 AM, Ivan Gerasimov wrote:
> Thank you Max!
>
> On 24.12.2014 3:51, Wang Weijun wrote:
>>> On Dec 23, 2014, at 21:31, Ivan Gerasimov <ivan.gerasimov at oracle.com> wrote:
>>>
>>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8067951
>>> WEBREV: http://cr.openjdk.java.net/~igerasim/8067951/0/webrev/
>> I am not sure why you want to rewrite the loop entirely. Isn't it OK to just
>> add that long if check before the old line 1770?
>>
> Unfortunately no.
> First, it would miss a quoted entry if it was last in the list.
> Second, the loop, as it is currently implemented, doesn't correctly handle empty
> entries:  if an empty entry was at the beginning of the list, it is replaced
> with a dot, but if it was at the end, it produces an empty string in the
> result.  Not a big deal, given how the function is used, but still better have
> if fixed.
>
>>   I always find myself confusing when a for line contains "," inside. :-)
>
> Okay, I reformatted it a bit, no more commas.
>
>> And how about a white box test with
>> getDeclaredMethod("initializePath").setAccessible(true)? I think you can even
>> add a few non-supported case (say, "A:B":C) and comment them out at the moment.
>
> Good idea! Please take a look at the updated webrev.
> I've also added some checks for correct handling of empty entries.
>
> Here's the updated webrev:
> http://cr.openjdk.java.net/~igerasim/8067951/1/webrev/
>
> Tested with JPRT on all available platforms with jdk_lang subset of tests. No
> failures.
>
> Sincerely yours,
> Ivan
>
>
>> Thanks
>> Max
>>
>>
>>
>>
>