RFR: 8194154 patch for crash at File.getCanonicalPath()

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8194154 patch for crash at File.getCanonicalPath()

Wenqian Pei

Hi:

Bug: https://bugs.openjdk.java.net/browse/JDK-8194154

We found that if user defines -Duser.dir like "/home/a/b/c/", jvm will crash at File.getCanonicalPath() in java process bootstrap (before invoking user's java code). The native implematation of canonicalize_md.c:collapsible(char *names) has problem in processing double '/', parameter 'names' need normalized before JNI_CALL.

This patch normalize parameters before call canonicalize0() in this call path

Patch and test are in mailbox attachments.

Can I please have a review for this patch?


Thanks


Wenqian Pei
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8194154 patch for crash at File.getCanonicalPath()

David Holmes
Hi,

Attachments get stripped (usually). You'll need to include it inline.

Cheers,
David

On 25/12/2017 7:40 PM, Wenqian Pei wrote:

>
> Hi:
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8194154
>
> We found that if user defines -Duser.dir like "/home/a/b/c/", jvm will crash at File.getCanonicalPath() in java process bootstrap (before invoking user's java code). The native implematation of canonicalize_md.c:collapsible(char *names) has problem in processing double '/', parameter 'names' need normalized before JNI_CALL.
>
> This patch normalize parameters before call canonicalize0() in this call path
>
> Patch and test are in mailbox attachments.
>
> Can I please have a review for this patch?
>
>
> Thanks
>
>
> Wenqian Pei
>
Reply | Threaded
Open this post in threaded view
|

回复:RFR: 8194154 patch for crash at File.getCanonicalPath()

Wenqian Pei
In reply to this post by Wenqian Pei
Sorry, the patch and test is below:

diff -r 777356696811 src/java.base/unix/classes/java/io/UnixFileSystem.java
--- a/src/java.base/unix/classes/java/io/UnixFileSystem.java    Fri Sep 08 18:24:17 2017 +0000
+++ b/src/java.base/unix/classes/java/io/UnixFileSystem.java    Mon Dec 25 16:48:36 2017 +0800
@@ -100,10 +100,10 @@
         if (child.equals("")) return parent;
         if (child.charAt(0) == '/') {
             if (parent.equals("/")) return child;
-            return parent + child;
+            return normalize(parent + child);
         }
         if (parent.equals("/")) return parent + child;
-        return parent + '/' + child;
+        return normalize(parent + '/' + child);
     }
 
     public String getDefaultParent() {
diff -r 777356696811 test/java/io/File/GetCanonicalPath.java
--- a/test/java/io/File/GetCanonicalPath.java   Fri Sep 08 18:24:17 2017 +0000
+++ b/test/java/io/File/GetCanonicalPath.java   Mon Dec 25 16:48:36 2017 +0800
@@ -23,20 +23,35 @@
 
 /* @test
    @bug 4899022
+   @library /lib/testlibrary
+   @run main/othervm GetCanonicalPath 
    @summary Look for erroneous representation of drive letter
  */
 
 import java.io.*;
+import java.lang.reflect.Field;
+import java.util.Properties;
 
 public class GetCanonicalPath {
     public static void main(String[] args) throws Exception {
         if (File.separatorChar == '\\') {
             testDriveLetter();
         }
+        String osName = System.getProperty("os.name");
+        if (osName.startsWith("Linux")) {
+            testDuplicateSeparators();
+        }
     }
     private static void testDriveLetter() throws Exception {
         String path = new File("c:/").getCanonicalPath();
         if (path.length() > 3)
             throw new RuntimeException("Drive letter incorrectly represented");
     }
+    private static void testDuplicateSeparators() throws Exception {
+        Field f = System.class.getDeclaredField("props");
+        f.setAccessible(true);
+        Properties p = (Properties) f.get(null);
+        p.setProperty("user.dir", "/home/a/b/c/");
+        System.out.println(new File("./a").getCanonicalPath());
+   }
 } 
ps: only test on Linux :)
------------------------------------------------------------------发件人:David Holmes <[hidden email]>发送时间:2017年12月27日(星期三) 14:14收件人:裴文谦(右席) <[hidden email]>; core-libs-dev <[hidden email]>抄 送:陆传胜(传胜) <[hidden email]>; 李三红(三红) <[hidden email]>主 题:Re: RFR: 8194154 patch for crash at File.getCanonicalPath()
Hi,

Attachments get stripped (usually). You'll need to include it inline.

Cheers,
David

On 25/12/2017 7:40 PM, Wenqian Pei wrote:


> Hi:

> Bug: https://bugs.openjdk.java.net/browse/JDK-8194154

> We found that if user defines -Duser.dir like "/home/a/b/c/", jvm will crash at File.getCanonicalPath() in java process bootstrap (before invoking user's java code). The native implematation of canonicalize_md.c:collapsible(char *names) has problem in processing double '/', parameter 'names' need normalized before JNI_CALL.

> This patch normalize parameters before call canonicalize0() in this call path

> Patch and test are in mailbox attachments.

> Can I please have a review for this patch?


> Thanks


> Wenqian Pei
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8194154 patch for crash at File.getCanonicalPath()

Alan Bateman
In reply to this post by Wenqian Pei
On 25/12/2017 09:40, Wenqian Pei wrote:
> Hi:
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8194154
>
> We found that if user defines -Duser.dir like "/home/a/b/c/", jvm will crash at File.getCanonicalPath() in java process bootstrap (before invoking user's java code). The native implematation of canonicalize_md.c:collapsible(char *names) has problem in processing double '/', parameter 'names' need normalized before JNI_CALL.
>
user.dir is a "read-only" property and should never be changed on the
command-line or in a running VM (it breaks APIs in other areas to have
user.dir be different to the actual working directory). Someday we need
to figure out how to enforce this.

In the mean-time, the runtime shouldn't crash so I agree it should be fixed.

-Alan.
Reply | Threaded
Open this post in threaded view
|

回复:RFR: 8194154 patch for crash at File.getCanonicalPath()

Wenqian Pei
In reply to this post by Wenqian Pei

Can you please review this patch?

Thanks in advance!

Wenqian Pei
------------------------------------------------------------------发件人:Alan Bateman <[hidden email]>发送时间:2018年1月3日(星期三) 01:30收件人:裴文谦(右席) <[hidden email]>; core-libs-dev <[hidden email]>抄 送:陆传胜(传胜) <[hidden email]>; 李三红(三红) <[hidden email]>主 题:Re: RFR: 8194154 patch for crash at File.getCanonicalPath()
On 25/12/2017 09:40, Wenqian Pei wrote:
> Hi:
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8194154
>
> We found that if user defines -Duser.dir like "/home/a/b/c/", jvm will crash at File.getCanonicalPath() in java process bootstrap (before invoking user's java code). The native implematation of canonicalize_md.c:collapsible(char *names) has problem in processing double '/', parameter 'names' need normalized before JNI_CALL.
>
user.dir is a "read-only" property and should never be changed on the 
command-line or in a running VM (it breaks APIs in other areas to have 
user.dir be different to the actual working directory). Someday we need 
to figure out how to enforce this.

In the mean-time, the runtime shouldn't crash so I agree it should be fixed.

-Alan.
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8194154 patch for crash at File.getCanonicalPath()

yumin qi-2
HI, Alan

  As in your email to RFR of 8194154 which now has a new fix in
canonicalize_md.c, switch back to discuss solution here again.
  The current fix is not in java, instead, I put it in C function. In the
function before the fix, it assumes no more "//" pattern in the string so
failed to get correct substrings with the case as the test case.
  The fix should not cause other problem since it does double check if
double or more slashes in sequential.

  The APIs indicates 'user.dir' etc system property should not be
rewritten, but did not prevent modifying them in practise.

Thanks
Yumin


On Tue, Jan 2, 2018 at 6:09 PM, Wenqian Pei <[hidden email]>
wrote:

>
> Can you please review this patch?
>
> Thanks in advance!
>
> Wenqian Pei
> ------------------------------------------------------------------发件人:Alan
> Bateman <[hidden email]>发送时间:2018年1月3日(星期三) 01:30收件人:裴文谦(右席) <
> [hidden email]>; core-libs-dev <[hidden email].
> net>抄 送:陆传胜(传胜) <[hidden email]>; 李三红(三红) <
> [hidden email]>主 题:Re: RFR: 8194154 patch for crash at
> File.getCanonicalPath()
> On 25/12/2017 09:40, Wenqian Pei wrote:
> > Hi:
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8194154
> >
> > We found that if user defines -Duser.dir like "/
> home/a/b/c/", jvm will crash at File.getCanonicalPath() in
> java process bootstrap (before invoking user's java code). The native
> implematation of canonicalize_md.c:collapsible(char *names)
> has problem in processing double '/', parameter 'names'
> need normalized before JNI_CALL.
> >
> user.dir is a "read-only" property and should never be changed on the
> command-line or in a running VM (it breaks APIs in other areas to have
> user.dir be different to the actual working directory). Someday we need
> to figure out how to enforce this.
>
> In the mean-time, the runtime shouldn't crash so I agree it
> should be fixed.
>
> -Alan.
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8194154 patch for crash at File.getCanonicalPath()

Alan Bateman
On 08/02/2018 17:35, yumin qi wrote:

> HI, Alan
>
>   As in your email to RFR of 8194154 which now has a new fix in
> canonicalize_md.c, switch back to discuss solution here again.
>   The current fix is not in java, instead, I put it in C function. In
> the function before the fix, it assumes no more "//" pattern in the
> string so failed to get correct substrings with the case as the test case.
>   The fix should not cause other problem since it does double check if
> double or more slashes in sequential.
>   The APIs indicates 'user.dir' etc system property should not be
> rewritten, but did not prevent modifying them in practise.
>
Sure, but it's really hairy for anything to be depend on that.  We can
improve the reliability by changing java.io.UnixFileSystem to read the
system property at most once. Second/subsequent usages should only need
to do a permission check (for the security manager case). Going further
then we need to get to the point where the APIs never read a modified
property, this goes for java.home and several others too.

I'll study the patch you have but I think we also need to create issues
to get us to the point where changing this system property in a running
VM doesn't impact running code.

-Alan
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8194154 patch for crash at File.getCanonicalPath()

Alan Bateman
On 09/02/2018 18:01, Alan Bateman wrote:
> :
>
> I'll study the patch you have but I think we also need to create
> issues to get us to the point where changing this system property in a
> running VM doesn't impact running code.
Looking at it again, I think we should change java.io.UnixFileSystem
(and the Windows equivalent) to cache the value of user.dir to avoid
difficult to diagnose issues with bad code changing the value of this
property in a running VM. This should reduce the issue down to cases
where user.dir is changed on the command line (never supported either of
course) to a value that is not "/" but has trailing or duplicate slashes.

When reduced down then the alternatives are to change the native
canonicalize method as you have done or alternatively do it once at
UnixFileSystem initialization time so that canonicalize does not have to
deal with this case. The former would require changing the description
of the function (it currently reads "The input path is assumed to
contain no duplicate slashes"), the latter avoids any changes to the
native implementation.

-Alan


diff -r 0937e5f799df src/java.base/unix/classes/java/io/UnixFileSystem.java
--- a/src/java.base/unix/classes/java/io/UnixFileSystem.java    Sat Feb
10 07:06:16 2018 -0500
+++ b/src/java.base/unix/classes/java/io/UnixFileSystem.java    Mon Feb
12 10:49:40 2018 +0000
@@ -34,12 +34,14 @@
      private final char slash;
      private final char colon;
      private final String javaHome;
+    private final String userDir;

      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 = props.getProperty("user.dir");
      }


@@ -128,7 +130,11 @@

      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());
      }

      // Caches for canonicalization results to improve startup performance.

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8194154 patch for crash at File.getCanonicalPath()

yumin qi-2
Alan,

  In fact, if property "user.dir" is cached, the problem will go away
without any changes to native or UnixFileSystem.java, since it won't
canonicalize the string supplied from user. The output will be
  getProperty("user.dir") + "/" + file. (The result is not as expected,
user can not change the property, means user has to work around the problem
to reach their targeted directory to do things like loading classes)

  Any change to "user.dir" in java program does not have any effect.
  For Windows:

--- a/src/java.base/windows/classes/java/io/WinNTFileSystem.java Fri Feb 02
10:32:59 2018 -0800
+++ b/src/java.base/windows/classes/java/io/WinNTFileSystem.java Tue Feb 13
23:40:21 2018 +0000
@@ -43,12 +43,14 @@
     private final char slash;
     private final char altSlash;
     private final char semicolon;
+    private final String userDir;

     public WinNTFileSystem() {
         Properties props = GetPropertyAction.privilegedGetProperties();
         slash = props.getProperty("file.separator").charAt(0);
         semicolon = props.getProperty("path.separator").charAt(0);
         altSlash = (this.slash == '\\') ? '/' : '\\';
+        userDir = props.getProperty("user.dir");
     }

     private boolean isSlash(char c) {
@@ -347,7 +349,7 @@
     private String getUserPath() {
         /* For both compatibility and security,
            we must look this up every time */
-        return normalize(System.getProperty("user.dir"));
+        return normalize(userDir);
     }

     private String getDrive(String path) {

  Does it need normalize(userDir) in getUserPath()? I think we need here.

  I will update the webrev and send for another round of review: the change
in native will be reverted, and made changes to UnixFileSystem.java.


Thanks
Yumin


On Mon, Feb 12, 2018 at 3:09 AM, Alan Bateman <[hidden email]>
wrote:

> On 09/02/2018 18:01, Alan Bateman wrote:
>
>> :
>>
>> I'll study the patch you have but I think we also need to create issues
>> to get us to the point where changing this system property in a running VM
>> doesn't impact running code.
>>
> Looking at it again, I think we should change java.io.UnixFileSystem (and
> the Windows equivalent) to cache the value of user.dir to avoid difficult
> to diagnose issues with bad code changing the value of this property in a
> running VM. This should reduce the issue down to cases where user.dir is
> changed on the command line (never supported either of course) to a value
> that is not "/" but has trailing or duplicate slashes.
>
> When reduced down then the alternatives are to change the native
> canonicalize method as you have done or alternatively do it once at
> UnixFileSystem initialization time so that canonicalize does not have to
> deal with this case. The former would require changing the description of
> the function (it currently reads "The input path is assumed to contain no
> duplicate slashes"), the latter avoids any changes to the native
> implementation.
>
> -Alan
>
>
> diff -r 0937e5f799df src/java.base/unix/classes/jav
> a/io/UnixFileSystem.java
> --- a/src/java.base/unix/classes/java/io/UnixFileSystem.java    Sat Feb
> 10 07:06:16 2018 -0500
> +++ b/src/java.base/unix/classes/java/io/UnixFileSystem.java    Mon Feb
> 12 10:49:40 2018 +0000
> @@ -34,12 +34,14 @@
>      private final char slash;
>      private final char colon;
>      private final String javaHome;
> +    private final String userDir;
>
>      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 = props.getProperty("user.dir");
>      }
>
>
> @@ -128,7 +130,11 @@
>
>      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());
>      }
>
>      // Caches for canonicalization results to improve startup performance.
>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8194154 patch for crash at File.getCanonicalPath()

roger riggs
Hi,

In the test, the message can be improved; though the exception should
never occur
it will be misleading if it does.
Instead:
     "Changing property user.dir should have no effect on the
getCanonicalPath"

It is cleaner to throw AssertionError for the test failure.
Throwing FileSystemException makes it look like it came from inside the
file system implementation.

Regards, Roger

On 2/13/2018 6:45 PM, yumin qi wrote:

> Alan,
>
>    In fact, if property "user.dir" is cached, the problem will go away
> without any changes to native or UnixFileSystem.java, since it won't
> canonicalize the string supplied from user. The output will be
>    getProperty("user.dir") + "/" + file. (The result is not as expected,
> user can not change the property, means user has to work around the problem
> to reach their targeted directory to do things like loading classes)
>
>    Any change to "user.dir" in java program does not have any effect.
>    For Windows:
>
> --- a/src/java.base/windows/classes/java/io/WinNTFileSystem.java Fri Feb 02
> 10:32:59 2018 -0800
> +++ b/src/java.base/windows/classes/java/io/WinNTFileSystem.java Tue Feb 13
> 23:40:21 2018 +0000
> @@ -43,12 +43,14 @@
>       private final char slash;
>       private final char altSlash;
>       private final char semicolon;
> +    private final String userDir;
>
>       public WinNTFileSystem() {
>           Properties props = GetPropertyAction.privilegedGetProperties();
>           slash = props.getProperty("file.separator").charAt(0);
>           semicolon = props.getProperty("path.separator").charAt(0);
>           altSlash = (this.slash == '\\') ? '/' : '\\';
> +        userDir = props.getProperty("user.dir");
>       }
>
>       private boolean isSlash(char c) {
> @@ -347,7 +349,7 @@
>       private String getUserPath() {
>           /* For both compatibility and security,
>              we must look this up every time */
> -        return normalize(System.getProperty("user.dir"));
> +        return normalize(userDir);
>       }
>
>       private String getDrive(String path) {
>
>    Does it need normalize(userDir) in getUserPath()? I think we need here.
>
>    I will update the webrev and send for another round of review: the change
> in native will be reverted, and made changes to UnixFileSystem.java.
>
>
> Thanks
> Yumin
>
>
> On Mon, Feb 12, 2018 at 3:09 AM, Alan Bateman <[hidden email]>
> wrote:
>
>> On 09/02/2018 18:01, Alan Bateman wrote:
>>
>>> :
>>>
>>> I'll study the patch you have but I think we also need to create issues
>>> to get us to the point where changing this system property in a running VM
>>> doesn't impact running code.
>>>
>> Looking at it again, I think we should change java.io.UnixFileSystem (and
>> the Windows equivalent) to cache the value of user.dir to avoid difficult
>> to diagnose issues with bad code changing the value of this property in a
>> running VM. This should reduce the issue down to cases where user.dir is
>> changed on the command line (never supported either of course) to a value
>> that is not "/" but has trailing or duplicate slashes.
>>
>> When reduced down then the alternatives are to change the native
>> canonicalize method as you have done or alternatively do it once at
>> UnixFileSystem initialization time so that canonicalize does not have to
>> deal with this case. The former would require changing the description of
>> the function (it currently reads "The input path is assumed to contain no
>> duplicate slashes"), the latter avoids any changes to the native
>> implementation.
>>
>> -Alan
>>
>>
>> diff -r 0937e5f799df src/java.base/unix/classes/jav
>> a/io/UnixFileSystem.java
>> --- a/src/java.base/unix/classes/java/io/UnixFileSystem.java    Sat Feb
>> 10 07:06:16 2018 -0500
>> +++ b/src/java.base/unix/classes/java/io/UnixFileSystem.java    Mon Feb
>> 12 10:49:40 2018 +0000
>> @@ -34,12 +34,14 @@
>>       private final char slash;
>>       private final char colon;
>>       private final String javaHome;
>> +    private final String userDir;
>>
>>       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 = props.getProperty("user.dir");
>>       }
>>
>>
>> @@ -128,7 +130,11 @@
>>
>>       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());
>>       }
>>
>>       // Caches for canonicalization results to improve startup performance.
>>
>>

Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8194154 patch for crash at File.getCanonicalPath()

yumin qi-2
HI, Roger and Brian

  Updated again in same link.
  I could not run jtreg so please have a look if the options are good for
@run

Thanks
Yumin

On Wed, Feb 14, 2018 at 11:13 AM, Roger Riggs <[hidden email]>
wrote:

> Hi,
>
> In the test, the message can be improved; though the exception should
> never occur
> it will be misleading if it does.
> Instead:
>     "Changing property user.dir should have no effect on the
> getCanonicalPath"
>
> It is cleaner to throw AssertionError for the test failure.
> Throwing FileSystemException makes it look like it came from inside the
> file system implementation.
>
> Regards, Roger
>
>
> On 2/13/2018 6:45 PM, yumin qi wrote:
>
>> Alan,
>>
>>    In fact, if property "user.dir" is cached, the problem will go away
>> without any changes to native or UnixFileSystem.java, since it won't
>> canonicalize the string supplied from user. The output will be
>>    getProperty("user.dir") + "/" + file. (The result is not as expected,
>> user can not change the property, means user has to work around the
>> problem
>> to reach their targeted directory to do things like loading classes)
>>
>>    Any change to "user.dir" in java program does not have any effect.
>>    For Windows:
>>
>> --- a/src/java.base/windows/classes/java/io/WinNTFileSystem.java Fri Feb
>> 02
>> 10:32:59 2018 -0800
>> +++ b/src/java.base/windows/classes/java/io/WinNTFileSystem.java Tue Feb
>> 13
>> 23:40:21 2018 +0000
>> @@ -43,12 +43,14 @@
>>       private final char slash;
>>       private final char altSlash;
>>       private final char semicolon;
>> +    private final String userDir;
>>
>>       public WinNTFileSystem() {
>>           Properties props = GetPropertyAction.privilegedGetProperties();
>>           slash = props.getProperty("file.separator").charAt(0);
>>           semicolon = props.getProperty("path.separator").charAt(0);
>>           altSlash = (this.slash == '\\') ? '/' : '\\';
>> +        userDir = props.getProperty("user.dir");
>>       }
>>
>>       private boolean isSlash(char c) {
>> @@ -347,7 +349,7 @@
>>       private String getUserPath() {
>>           /* For both compatibility and security,
>>              we must look this up every time */
>> -        return normalize(System.getProperty("user.dir"));
>> +        return normalize(userDir);
>>       }
>>
>>       private String getDrive(String path) {
>>
>>    Does it need normalize(userDir) in getUserPath()? I think we need here.
>>
>>    I will update the webrev and send for another round of review: the
>> change
>> in native will be reverted, and made changes to UnixFileSystem.java.
>>
>>
>> Thanks
>> Yumin
>>
>>
>> On Mon, Feb 12, 2018 at 3:09 AM, Alan Bateman <[hidden email]>
>> wrote:
>>
>> On 09/02/2018 18:01, Alan Bateman wrote:
>>>
>>> :
>>>>
>>>> I'll study the patch you have but I think we also need to create issues
>>>> to get us to the point where changing this system property in a running
>>>> VM
>>>> doesn't impact running code.
>>>>
>>>> Looking at it again, I think we should change java.io.UnixFileSystem
>>> (and
>>> the Windows equivalent) to cache the value of user.dir to avoid difficult
>>> to diagnose issues with bad code changing the value of this property in a
>>> running VM. This should reduce the issue down to cases where user.dir is
>>> changed on the command line (never supported either of course) to a value
>>> that is not "/" but has trailing or duplicate slashes.
>>>
>>> When reduced down then the alternatives are to change the native
>>> canonicalize method as you have done or alternatively do it once at
>>> UnixFileSystem initialization time so that canonicalize does not have to
>>> deal with this case. The former would require changing the description of
>>> the function (it currently reads "The input path is assumed to contain no
>>> duplicate slashes"), the latter avoids any changes to the native
>>> implementation.
>>>
>>> -Alan
>>>
>>>
>>> diff -r 0937e5f799df src/java.base/unix/classes/jav
>>> a/io/UnixFileSystem.java
>>> --- a/src/java.base/unix/classes/java/io/UnixFileSystem.java    Sat Feb
>>> 10 07:06:16 2018 -0500
>>> +++ b/src/java.base/unix/classes/java/io/UnixFileSystem.java    Mon Feb
>>> 12 10:49:40 2018 +0000
>>> @@ -34,12 +34,14 @@
>>>       private final char slash;
>>>       private final char colon;
>>>       private final String javaHome;
>>> +    private final String userDir;
>>>
>>>       public UnixFileSystem() {
>>>           Properties props = GetPropertyAction.privilegedGe
>>> tProperties();
>>>           slash = props.getProperty("file.separator").charAt(0);
>>>           colon = props.getProperty("path.separator").charAt(0);
>>>           javaHome = props.getProperty("java.home");
>>> +        userDir = props.getProperty("user.dir");
>>>       }
>>>
>>>
>>> @@ -128,7 +130,11 @@
>>>
>>>       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());
>>>       }
>>>
>>>       // Caches for canonicalization results to improve startup
>>> performance.
>>>
>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8194154 patch for crash at File.getCanonicalPath()

Brian Burkhalter-2
Yumin,

As for the test options you would need to make this change:

--- a/test/jdk/java/io/File/ParseCanonicalPath.java
+++ b/test/jdk/java/io/File/ParseCanonicalPath.java
@@ -25,7 +25,7 @@
                   (os.family == "solaris") | (os.family == "aix")
    @bug 8194154
    @summary Test parsing path with double slashes on unix like platforms.
-   @run -ea main ParseCanonicalPath
+   @run main/othervm -ea ParseCanonicalPath
  */

A better alternative might be to throw a RuntimeException instead of using assert.

Thanks,

Brian

On Feb 14, 2018, at 2:36 PM, yumin qi <[hidden email]> wrote:

>   Updated again in same link.
>   I could not run jtreg so please have a look if the options are good for @run