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

classic Classic list List threaded Threaded
5 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.