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.gerasi...@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
