Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-21 Thread yumin qi
Roger,

 Thanks. Pushed.

On Wed, Feb 21, 2018 at 2:00 PM, Roger Riggs  wrote:

> Hi Yumin,
>
> Use your OpenJDK id  "minqi" for the Author instead of your email.
>
> Roger
>
>
>
> On 2/21/2018 4:54 PM, yumin qi wrote:
>
>> Alan,
>>
>>Somehow there is a problem for me to push the changeset:
>> pushing to ssh://mi...@hg.openjdk.java.net/jdk/jdk
>> searching for changes
>> remote: adding changesets
>> remote: adding manifests
>> remote: adding file changes
>> remote: added 1 changesets with 3 changes to 3 files
>> remote: [jcheck fc6df5c6d4d2 2018-01-23 14:21:17 -0800]
>> remote:
>> remote: > Changeset: 48927:0736b1e12c70
>> remote: > Author:Yumin Qi 
>> remote: > Date:  2018-02-21 12:01
>> remote: >
>> remote: > 8194154: System property user.dir should not be changed
>> remote: > Summary: Cached user.dir so getCanonicalPath uses the cached
>> value.
>> remote: > Reviewed-by: alanb, bpb, rriggs
>> remote:
>> remote: Invalid changeset author: Yumin Qi 
>> remote:
>> remote: transaction abort!
>> remote: rollback completed
>> remote: abort: pretxnchangegroup.0.jcheck hook failed
>>
>> What is the real reason for the failure?
>> I am in following projects:
>>
>> Projects
>> hsx  HotSpot Express Project – Author
>> jdk  JDK Project – Committer
>> jdk-updates  JDK Updates
>> Project – Committer
>> jdk10  JDK 10 Project – Committer
>>
>> Thanks
>> Yumin
>>
>> On Mon, Feb 19, 2018 at 10:11 AM, yumin qi  wrote:
>>
>> Yes, I am committer.
>>>
>>> Brian, do you okay with the version? If no objection, I will push it into
>>> jdk.
>>>
>>> Thanks
>>> Yumin
>>>
>>> On Mon, Feb 19, 2018 at 2:28 AM, Alan Bateman 
>>> wrote:
>>>
>>> On 19/02/2018 05:09, yumin qi wrote:

 Thanks!
>
> I need a sponsor for pushing it to jdk. Can you or someone else help to
> push it?
>
> minqi is a jdk committer. If this is you then you should be able to
 push
 it yourself.

 -Alan.


>>>
>


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-21 Thread Roger Riggs

Hi Yumin,

Use your OpenJDK id  "minqi" for the Author instead of your email.

Roger


On 2/21/2018 4:54 PM, yumin qi wrote:

Alan,

   Somehow there is a problem for me to push the changeset:
pushing to ssh://mi...@hg.openjdk.java.net/jdk/jdk
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 3 changes to 3 files
remote: [jcheck fc6df5c6d4d2 2018-01-23 14:21:17 -0800]
remote:
remote: > Changeset: 48927:0736b1e12c70
remote: > Author:Yumin Qi 
remote: > Date:  2018-02-21 12:01
remote: >
remote: > 8194154: System property user.dir should not be changed
remote: > Summary: Cached user.dir so getCanonicalPath uses the cached
value.
remote: > Reviewed-by: alanb, bpb, rriggs
remote:
remote: Invalid changeset author: Yumin Qi 
remote:
remote: transaction abort!
remote: rollback completed
remote: abort: pretxnchangegroup.0.jcheck hook failed

What is the real reason for the failure?
I am in following projects:

Projects
hsx  HotSpot Express Project – Author
jdk  JDK Project – Committer
jdk-updates  JDK Updates
Project – Committer
jdk10  JDK 10 Project – Committer

Thanks
Yumin

On Mon, Feb 19, 2018 at 10:11 AM, yumin qi  wrote:


Yes, I am committer.

Brian, do you okay with the version? If no objection, I will push it into
jdk.

Thanks
Yumin

On Mon, Feb 19, 2018 at 2:28 AM, Alan Bateman 
wrote:


On 19/02/2018 05:09, yumin qi wrote:


Thanks!

I need a sponsor for pushing it to jdk. Can you or someone else help to
push it?


minqi is a jdk committer. If this is you then you should be able to push
it yourself.

-Alan.







Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-21 Thread yumin qi
Alan,

  Somehow there is a problem for me to push the changeset:
pushing to ssh://mi...@hg.openjdk.java.net/jdk/jdk
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 3 changes to 3 files
remote: [jcheck fc6df5c6d4d2 2018-01-23 14:21:17 -0800]
remote:
remote: > Changeset: 48927:0736b1e12c70
remote: > Author:Yumin Qi 
remote: > Date:  2018-02-21 12:01
remote: >
remote: > 8194154: System property user.dir should not be changed
remote: > Summary: Cached user.dir so getCanonicalPath uses the cached
value.
remote: > Reviewed-by: alanb, bpb, rriggs
remote:
remote: Invalid changeset author: Yumin Qi 
remote:
remote: transaction abort!
remote: rollback completed
remote: abort: pretxnchangegroup.0.jcheck hook failed

What is the real reason for the failure?
I am in following projects:

Projects
hsx  HotSpot Express Project – Author
jdk  JDK Project – Committer
jdk-updates  JDK Updates
Project – Committer
jdk10  JDK 10 Project – Committer

Thanks
Yumin

On Mon, Feb 19, 2018 at 10:11 AM, yumin qi  wrote:

> Yes, I am committer.
>
> Brian, do you okay with the version? If no objection, I will push it into
> jdk.
>
> Thanks
> Yumin
>
> On Mon, Feb 19, 2018 at 2:28 AM, Alan Bateman 
> wrote:
>
>> On 19/02/2018 05:09, yumin qi wrote:
>>
>>> Thanks!
>>>
>>> I need a sponsor for pushing it to jdk. Can you or someone else help to
>>> push it?
>>>
>> minqi is a jdk committer. If this is you then you should be able to push
>> it yourself.
>>
>> -Alan.
>>
>
>


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-19 Thread yumin qi
Yes, I am committer.

Brian, do you okay with the version? If no objection, I will push it into
jdk.

Thanks
Yumin

On Mon, Feb 19, 2018 at 2:28 AM, Alan Bateman 
wrote:

> On 19/02/2018 05:09, yumin qi wrote:
>
>> Thanks!
>>
>> I need a sponsor for pushing it to jdk. Can you or someone else help to
>> push it?
>>
> minqi is a jdk committer. If this is you then you should be able to push
> it yourself.
>
> -Alan.
>


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-19 Thread Alan Bateman

On 19/02/2018 05:09, yumin qi wrote:

Thanks!

I need a sponsor for pushing it to jdk. Can you or someone else help 
to push it?
minqi is a jdk committer. If this is you then you should be able to push 
it yourself.


-Alan.


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-18 Thread yumin qi
Thanks!

I need a sponsor for pushing it to jdk. Can you or someone else help to
push it?

Yumin

On Fri, Feb 16, 2018 at 11:45 PM, Alan Bateman 
wrote:

> On 16/02/2018 20:35, yumin qi wrote:
>
> :
>
>>
>> Updated bug,  and update webrev at same link:
> http://cr.openjdk.java.net/~minqi/8194154/webrev1/
>
> I think this version is good to go.
>
> -Alan
>


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-16 Thread Alan Bateman

On 16/02/2018 20:35, yumin qi wrote:

:


Updated bug,  and update webrev at same link:
http://cr.openjdk.java.net/~minqi/8194154/webrev1/ 




I think this version is good to go.

-Alan


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-16 Thread yumin qi
On Fri, Feb 16, 2018 at 3:52 AM, Alan Bateman 
wrote:

> On 15/02/2018 20:28, yumin qi wrote:
>
>> :
>> Since the property string contains non-normalized characters, it
>> crashed in native canonicalize.
>> I believe user.dir from the system is normalized, so it is OK but
>> after it is changed like "/home/a/b/c/", it crashed.
>>
>> Now with using cached "user.dir", the problem is gone.
>> :
>>
>> So the changes in resolve should be removed.
>>
>> Since the bug is talking about the crash, the real reason is user.dir
>> should not be changed, how about changing description to
>> 8194154: System property user.dir should not be changed.
>>
>>  The test case renamed to: UserDirChangedTest.java   ?
>>
>> Thanks for the confirming. Yes, changing the bug description and test
> should be good and if you can post an updated webrev then I assume we can
> wrap this one up quickly.
>
> Updated bug,  and update webrev at same link:
http://cr.openjdk.java.net/~minqi/8194154/webrev1/

Thanks
Yumin


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-16 Thread Alan Bateman

On 15/02/2018 20:28, yumin qi wrote:

:
    Since the property string contains non-normalized characters, it 
crashed in native canonicalize.
    I believe user.dir from the system is normalized, so it is OK but 
after it is changed like "/home/a/b/c/", it crashed.


    Now with using cached "user.dir", the problem is gone.
    :

    So the changes in resolve should be removed.

    Since the bug is talking about the crash, the real reason is 
user.dir should not be changed, how about changing description to

    8194154: System property user.dir should not be changed.

     The test case renamed to: UserDirChangedTest.java   ?

Thanks for the confirming. Yes, changing the bug description and test 
should be good and if you can post an updated webrev then I assume we 
can wrap this one up quickly.


-Alan


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-15 Thread yumin qi
Hi, Alan

  The real reason, for why File.getCanonicalPath() will fail after
"user.dir" property changed is, in File.java:

 public String getCanonicalPath() throws IOException {
if (isInvalid()) {
throw new IOException("Invalid file path");
}
return fs.canonicalize(fs.resolve(this));
}

It passed this File to FileSystem.resolve(File):

public String resolve(File f) {
if (isAbsolute(f)) return f.getPath();
return resolve(System.getProperty("user.dir"), f.getPath());
}
Note, here it get property of "user.dir" and passed the string directly
into resolve(String, String). Checked all the usage of this function in all
other places, they all normalize the string first then pass it to resolve.
Like:

File.java:

public File(String parent, String child) {
if (child == null) {
throw new NullPointerException();
}
if (parent != null) {
if (parent.equals("")) {
this.path = fs.resolve(fs.getDefaultParent(),
   fs.normalize(child));
} else {
this.path = fs.resolve(fs.normalize(parent),
   fs.normalize(child));
}
} else {
this.path = fs.normalize(child);
}
this.prefixLength = fs.prefixLength(this.path);
}

Since the property string contains non-normalized characters, it
crashed in native canonicalize.
I believe user.dir from the system is normalized, so it is OK but after
it is changed like "/home/a/b/c/", it crashed.

Now with using cached "user.dir", the problem is gone.
The ultimate guaranteed solution may be:

public UnixFileSystem() {
Properties props = GetPropertyAction.privilegedGetProperties();
slash = props.getProperty("file.separator").charAt(0);
colon = props.getProperty("path.separator").charAt(0);
javaHome = props.getProperty("java.home");
+userDir = normalize(props.getProperty("user.dir")); // is it
necessary?
}

 public String resolve(File f) {
 if (isAbsolute(f)) return f.getPath();-return
resolve(System.getProperty("user.dir"), f.getPath());+
SecurityManager sm = System.getSecurityManager();+if (sm !=
null) {+sm.checkPropertyAccess("user.dir");+}+
   return resolve(userDir, f.getPath());
 }


So the changes in resolve should be removed.

Since the bug is talking about the crash, the real reason is user.dir
should not be changed, how about changing description to
8194154: System property user.dir should not be changed.

 The test case renamed to: UserDirChangedTest.java   ?

Thanks
Yumin



On Thu, Feb 15, 2018 at 10:41 AM, yumin qi  wrote:

> There are two problems here, so become complex.
> 1) crash on parsing "//", which included in file path, on linux. This is
> fixed in UnixFileSystem.java in resolve function.
> 2) user.dir should be read only. This is fixed in both UnixFileSystem.java
> and WinNTFileSystem.java.
>
> The test case covers two of them.
> Should we handle them in two separate bugs?
>
> Yumin
>
>
> On Thu, Feb 15, 2018 at 10:00 AM, yumin qi  wrote:
>
>> OK, let's work on a suitable test case.
>>
>> Thanks
>> Yumin
>>
>> On Thu, Feb 15, 2018 at 9:41 AM, Alan Bateman 
>> wrote:
>>
>>> On 15/02/2018 17:25, yumin qi wrote:
>>>
 Alan,

   The real reason is if we do not change on resolve, the string passed
 into native canonicalize will be like "//./a/" which is not expected in
 native part. In native part, we resume that no more "//" in the path string
 (already normalized in java).

 If that is the case then the test doesn't match the issue we have been
>>> discussing on this thread. Would it be possible to create a test case for
>>> the issue that you are actually trying to address?
>>>
>>> -Alan
>>>
>>
>>
>


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-15 Thread Alan Bateman



On 15/02/2018 18:41, yumin qi wrote:

There are two problems here, so become complex.
1) crash on parsing "//", which included in file path, on linux. This 
is fixed in UnixFileSystem.java in resolve function.
2) user.dir should be read only. This is fixed in both 
UnixFileSystem.java and WinNTFileSystem.java.


The test case covers two of them.
Should we handle them in two separate bugs?

Assuming #2 is fixed and user.dir cannot be changed, do you still have 
#1? The current test just ensures that bad code changing user.dir in a 
running VM doesn't change getCanonicalPath. If there is more to #1 then 
I think it will need further tests.


-Alan


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-15 Thread yumin qi
There are two problems here, so become complex.
1) crash on parsing "//", which included in file path, on linux. This is
fixed in UnixFileSystem.java in resolve function.
2) user.dir should be read only. This is fixed in both UnixFileSystem.java
and WinNTFileSystem.java.

The test case covers two of them.
Should we handle them in two separate bugs?

Yumin


On Thu, Feb 15, 2018 at 10:00 AM, yumin qi  wrote:

> OK, let's work on a suitable test case.
>
> Thanks
> Yumin
>
> On Thu, Feb 15, 2018 at 9:41 AM, Alan Bateman 
> wrote:
>
>> On 15/02/2018 17:25, yumin qi wrote:
>>
>>> Alan,
>>>
>>>   The real reason is if we do not change on resolve, the string passed
>>> into native canonicalize will be like "//./a/" which is not expected in
>>> native part. In native part, we resume that no more "//" in the path string
>>> (already normalized in java).
>>>
>>> If that is the case then the test doesn't match the issue we have been
>> discussing on this thread. Would it be possible to create a test case for
>> the issue that you are actually trying to address?
>>
>> -Alan
>>
>
>


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-15 Thread yumin qi
OK, let's work on a suitable test case.

Thanks
Yumin

On Thu, Feb 15, 2018 at 9:41 AM, Alan Bateman 
wrote:

> On 15/02/2018 17:25, yumin qi wrote:
>
>> Alan,
>>
>>   The real reason is if we do not change on resolve, the string passed
>> into native canonicalize will be like "//./a/" which is not expected in
>> native part. In native part, we resume that no more "//" in the path string
>> (already normalized in java).
>>
>> If that is the case then the test doesn't match the issue we have been
> discussing on this thread. Would it be possible to create a test case for
> the issue that you are actually trying to address?
>
> -Alan
>


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-15 Thread Alan Bateman

On 15/02/2018 17:25, yumin qi wrote:

Alan,

  The real reason is if we do not change on resolve, the string passed 
into native canonicalize will be like "//./a/" which is not expected 
in native part. In native part, we resume that no more "//" in the 
path string (already normalized in java).


If that is the case then the test doesn't match the issue we have been 
discussing on this thread. Would it be possible to create a test case 
for the issue that you are actually trying to address?


-Alan


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-15 Thread yumin qi
Alan,

  The real reason is if we do not change on resolve, the string passed into
native canonicalize will be like "//./a/" which is not expected in native
part. In native part, we resume that no more "//" in the path string
(already normalized in java).
   We can fix this by  either:
   1) in java part, like the webrev here
or
2) in native canonicalize_md.c as webrev0.
   If the test run on Windows, should it run in unix like shell on Windows?
The separator "/" is not for Windows.

 Thanks
  Yumin



On Thu, Feb 15, 2018 at 12:23 AM, Alan Bateman 
wrote:

> On 14/02/2018 23:52, yumin qi wrote:
>
>> Brian,
>>
>>   Updated using RuntimeException, which will not need -ea so removed.
>> http://cr.openjdk.java.net/~minqi/8194154/webrev1/ <
>> http://cr.openjdk.java.net/%7Eminqi/8194154/webrev1/>
>>
>> Can you explain why resolve has been changed to call normalize
> parent+child? I can't help thinking there is something else going on if
> this change is needed.
>
> I see the mails about the test so I'll skip that except to say that it
> should not need "require". The test should be able to run on all platforms.
>
> -Alan
>
>


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-15 Thread Alan Bateman

On 14/02/2018 23:52, yumin qi wrote:

Brian,

  Updated using RuntimeException, which will not need -ea so removed.
http://cr.openjdk.java.net/~minqi/8194154/webrev1/ 



Can you explain why resolve has been changed to call normalize 
parent+child? I can't help thinking there is something else going on if 
this change is needed.


I see the mails about the test so I'll skip that except to say that it 
should not need "require". The test should be able to run on all platforms.


-Alan



Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-14 Thread yumin qi
Brian,

  Updated using RuntimeException, which will not need -ea so removed.
  http://cr.openjdk.java.net/~minqi/8194154/webrev1/

Thanks
Yumin

On Wed, Feb 14, 2018 at 11:17 AM, Brian Burkhalter <
brian.burkhal...@oracle.com> wrote:

> Hello,
>
> In the test you have
>
> /* @test requires (os.family == "linux") | (os.family == "mac") | (os.family 
> == "solaris") | (os.family == "aix")
>@bug 8194154
>@summary Test parsing path with double slashes on unix like platforms.
>  */
>
> but I think you need to have the @requires annotation on a separate line
> for example as:
>
> /* @test
>  * @requires (os.family == "linux") | (os.family == "mac”)
>  * | (os.family == "solaris") | (os.family == "aix")
>  * @bug 8194154
>  * @summary Test parsing path with double slashes on unix like platforms.
>  */
>
> Thanks,
>
> Brian
>
> On Feb 14, 2018, at 10:48 AM, yumin qi  wrote:
>
>  Thanks. Updated on same link
>   http://cr.openjdk.java.net/~minqi/8194154/webrev1/
>  as your recommendation.
>
>
>


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-14 Thread Brian Burkhalter
Hello,

In the test you have
/* @test requires (os.family == "linux") | (os.family == "mac") | (os.family == 
"solaris") | (os.family == "aix")
   @bug 8194154
   @summary Test parsing path with double slashes on unix like platforms.
 */
but I think you need to have the @requires annotation on a separate line for 
example as:
/* @test
 * @requires (os.family == "linux") | (os.family == "mac”)
 * | (os.family == "solaris") | (os.family == "aix")
 * @bug 8194154
 * @summary Test parsing path with double slashes on unix like platforms.
 */
Thanks,

Brian

On Feb 14, 2018, at 10:48 AM, yumin qi  wrote:

>  Thanks. Updated on same link
>   http://cr.openjdk.java.net/~minqi/8194154/webrev1/
>  as your recommendation.



Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-14 Thread yumin qi
Hi, Alan

  Thanks. Updated on same link
   http://cr.openjdk.java.net/~minqi/8194154/webrev1/
  as your recommendation.

Yumin

On Wed, Feb 14, 2018 at 4:42 AM, Alan Bateman 
wrote:

> On 14/02/2018 01:23, yumin qi wrote:
>
>> Hi,
>>
>>   I have update the webrev:
>> http://cr.openjdk.java.net/~minqi/8194154/webrev1/ <
>> http://cr.openjdk.java.net/%7Eminqi/8194154/webrev1/>
>>
>>   In this version, as suggested by Alan(thanks!), property of "user.dir"
>> is cached and behave like it is 'read only'. The change made to *inux as
>> well as windows. Since property of "user.dir" is cached, any changes via
>> property setting for it has no effect.
>>
>> This looks much better but you've removed a permission check from the
> Windows getUserPath implementation. That code the same permission check as
> the resolve method in the Unix implementation.
>
> I think the test needs works. The simplest would be to call
> getCanonicalFile before changing the system property, then call it after
> and check that you an equal result. Also no need to hack the Properties
> object, you can use System.setProperty instead.
>
> -Alan
>
>
>


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-14 Thread Alan Bateman

On 14/02/2018 01:23, yumin qi wrote:

Hi,

  I have update the webrev:
http://cr.openjdk.java.net/~minqi/8194154/webrev1/ 



  In this version, as suggested by Alan(thanks!), property of 
"user.dir" is cached and behave like it is 'read only'. The change 
made to *inux as well as windows. Since property of "user.dir" is 
cached, any changes via property setting for it has no effect.


This looks much better but you've removed a permission check from the 
Windows getUserPath implementation. That code the same permission check 
as the resolve method in the Unix implementation.


I think the test needs works. The simplest would be to call 
getCanonicalFile before changing the system property, then call it after 
and check that you an equal result. Also no need to hack the Properties 
object, you can use System.setProperty instead.


-Alan




Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-13 Thread yumin qi
HI, Brian

  Thanks, updated on same link. I don't have platforms other than linux so
could not test on those platforms.
  BTW, tried jtreg, it seems the version has not modified to run with
current jdk repo.

Thanks
Yumin

On Tue, Feb 13, 2018 at 5:52 PM, Brian Burkhalter <
brian.burkhal...@oracle.com> wrote:

> Hi,
>
> As for the test alone you could add the @requires annotation [1]
>
> @requires (os.family == "linux") | (os.family == "mac") | (os.family ==
> "solaris") | (os.family == "aix")
>
> instead of programmatically exiting if the platform is not Windows. (I am
> assuming that this should be run on all Unix variants, not just Linux.).
>
> Thanks,
>
> Brian
>
> [1] http://openjdk.java.net/jtreg/tag-spec.html#requires_names
>
> On Feb 13, 2018, at 5:23 PM, yumin qi  wrote:
>
>  I have update the webrev:
>  http://cr.openjdk.java.net/~minqi/8194154/webrev1/
>
>
>


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-13 Thread Brian Burkhalter
Hi,

As for the test alone you could add the @requires annotation [1]

@requires (os.family == "linux") | (os.family == "mac") | (os.family == 
"solaris") | (os.family == "aix")

instead of programmatically exiting if the platform is not Windows. (I am 
assuming that this should be run on all Unix variants, not just Linux.).

Thanks,

Brian

[1] http://openjdk.java.net/jtreg/tag-spec.html#requires_names

On Feb 13, 2018, at 5:23 PM, yumin qi  wrote:

>  I have update the webrev:
>  http://cr.openjdk.java.net/~minqi/8194154/webrev1/



Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-13 Thread yumin qi
Hi,

  I have update the webrev:
  http://cr.openjdk.java.net/~minqi/8194154/webrev1/

  In this version, as suggested by Alan(thanks!), property of "user.dir" is
cached and behave like it is 'read only'. The change made to *inux as well
as windows. Since property of "user.dir" is cached, any changes via
property setting for it has no effect.

Thanks
Yumin


On Wed, Feb 7, 2018 at 10:54 PM, Alan Bateman 
wrote:

> On 07/02/2018 20:10, yumin qi wrote:
>
>> Hi,
>>
>>Please review the fix (extra small) for:
>>bug 8194154: https://bugs.openjdk.java.net/browse/JDK-8194154
>>webrev: http://cr.openjdk.java.net/~minqi/8194154/
>>
>>Summary: canonicalize will check and fold double (or more) slashes, but
>> in function collapsible(char*) and splitNames(char*, char**), it failed to
>> process strings like "//". This results in the former does not give a
>> correct number of substrings and the later does not give a correct array
>> of
>> substrings. The fix add a check if the character is '/' after a '/'.
>>
>> The JDK has never supported setting user.dir in this way (it breaks
> several things). There is a thread discussing this on core-libs-dev so best
> to bring it to that mailing list to discuss again.
>
> -Alan
>


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-07 Thread yumin qi
David, Thanks!

On Wed, Feb 7, 2018 at 7:32 PM, David Holmes 
wrote:

> Moving to core-libs-dev. Code reviews don't take place on jdk-dev but on
> the component area mailing lists.
>
> Thanks,
> David
>
>
> On 8/02/2018 6:10 AM, yumin qi wrote:
>
>> Hi,
>>
>>Please review the fix (extra small) for:
>>bug 8194154: https://bugs.openjdk.java.net/browse/JDK-8194154
>>webrev: http://cr.openjdk.java.net/~minqi/8194154/
>>
>>Summary: canonicalize will check and fold double (or more) slashes, but
>> in function collapsible(char*) and splitNames(char*, char**), it failed to
>> process strings like "//". This results in the former does not give a
>> correct number of substrings and the later does not give a correct array
>> of
>> substrings. The fix add a check if the character is '/' after a '/'.
>>
>>  The test case added will crash without the fix.
>>
>>   Thanks
>>   Yumin
>>
>>


Re: RFR: 8194154: JDK crashes parsing path string contains '//' on linux

2018-02-07 Thread David Holmes
Moving to core-libs-dev. Code reviews don't take place on jdk-dev but on 
the component area mailing lists.


Thanks,
David

On 8/02/2018 6:10 AM, yumin qi wrote:

Hi,

   Please review the fix (extra small) for:
   bug 8194154: https://bugs.openjdk.java.net/browse/JDK-8194154
   webrev: http://cr.openjdk.java.net/~minqi/8194154/

   Summary: canonicalize will check and fold double (or more) slashes, but
in function collapsible(char*) and splitNames(char*, char**), it failed to
process strings like "//". This results in the former does not give a
correct number of substrings and the later does not give a correct array of
substrings. The fix add a check if the character is '/' after a '/'.

 The test case added will crash without the fix.

  Thanks
  Yumin