RFR: 8273922: (fs) UserDefinedFileAttributeView doesn't handle file names that are just under the MAX_PATH limit (win) [v3]

Mike Hearn duke at openjdk.java.net
Mon Oct 25 15:38:07 UTC 2021


On Tue, 21 Sep 2021 19:28:58 GMT, Brian Burkhalter <bpb at openjdk.org> wrote:

>> Modify `sun.nio.fs.WindowsUserDefinedFileAttributeView.join(WindowsPath,String)` to handle file names which exceed the limit.
>
> Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8273922: Make join() reject absolute path in 1nth in 'name' parameter

Thanks for fixing this Brian. I made my own attempt at a fix (I reported the bug originally) and came up with a different approach which simply removes the attempt to use short paths when possible, and always uses "long mode" paths. It passes tier1 and as far as I can tell should always work.


diff --git a/src/java.base/windows/classes/sun/nio/fs/WindowsFileCopy.java b/src/java.base/windows/classes/sun/nio/fs/WindowsFileCopy.java
index 54ecaf152c1..11c0e6b929c 100644
--- a/src/java.base/windows/classes/sun/nio/fs/WindowsFileCopy.java
+++ b/src/java.base/windows/classes/sun/nio/fs/WindowsFileCopy.java
@@ -228,7 +228,7 @@ class WindowsFileCopy {
                 String linkTarget = WindowsLinkSupport.readLink(source);
                 int flags = SYMBOLIC_LINK_FLAG_DIRECTORY;
                 CreateSymbolicLink(targetPath,
-                                   WindowsPath.addPrefixIfNeeded(linkTarget),
+                                   WindowsPath.addLongPathPrefix(linkTarget),
                                    flags);
             }
         } catch (WindowsException x) {
@@ -431,7 +431,7 @@ class WindowsFileCopy {
             } else {
                 String linkTarget = WindowsLinkSupport.readLink(source);
                 CreateSymbolicLink(targetPath,
-                                   WindowsPath.addPrefixIfNeeded(linkTarget),
+                                   WindowsPath.addLongPathPrefix(linkTarget),
                                    SYMBOLIC_LINK_FLAG_DIRECTORY);
             }
         } catch (WindowsException x) {
diff --git a/src/java.base/windows/classes/sun/nio/fs/WindowsFileSystemProvider.java b/src/java.base/windows/classes/sun/nio/fs/WindowsFileSystemProvider.java
index 84658566873..e83e47505ba 100644
--- a/src/java.base/windows/classes/sun/nio/fs/WindowsFileSystemProvider.java
+++ b/src/java.base/windows/classes/sun/nio/fs/WindowsFileSystemProvider.java
@@ -589,7 +589,7 @@ class WindowsFileSystemProvider
         // create the link
         try {
             CreateSymbolicLink(link.getPathForWin32Calls(),
-                               WindowsPath.addPrefixIfNeeded(target.toString()),
+                               WindowsPath.addLongPathPrefix(target.toString()),
                                flags);
         } catch (WindowsException x) {
             if (x.lastError() == ERROR_INVALID_REPARSE_DATA) {
diff --git a/src/java.base/windows/classes/sun/nio/fs/WindowsLinkSupport.java b/src/java.base/windows/classes/sun/nio/fs/WindowsLinkSupport.java
index 864663a1738..4bcd8a75222 100644
--- a/src/java.base/windows/classes/sun/nio/fs/WindowsLinkSupport.java
+++ b/src/java.base/windows/classes/sun/nio/fs/WindowsLinkSupport.java
@@ -230,7 +230,7 @@ class WindowsLinkSupport {
             int end = (next == -1) ? path.length() : next;
             String search = sb.toString() + path.substring(curr, end);
             try {
-                FirstFile fileData = FindFirstFile(WindowsPath.addPrefixIfNeeded(search));
+                FirstFile fileData = FindFirstFile(WindowsPath.addLongPathPrefix(search));
                 FindClose(fileData.handle());
 
                 // if a reparse point is encountered then we must return the
diff --git a/src/java.base/windows/classes/sun/nio/fs/WindowsPath.java b/src/java.base/windows/classes/sun/nio/fs/WindowsPath.java
index 8500646a96f..b93b31368b5 100644
--- a/src/java.base/windows/classes/sun/nio/fs/WindowsPath.java
+++ b/src/java.base/windows/classes/sun/nio/fs/WindowsPath.java
@@ -172,12 +172,8 @@ class WindowsPath implements Path {
     }
 
     // use this path for Win32 calls
-    // This method will prefix long paths with \?\ or \?\UNC as required.
+    // This method will prefix paths with \?\ or \?\UNC in order to enable long path mode.
     String getPathForWin32Calls() throws WindowsException {
-        // short absolute paths can be used directly
-        if (isAbsolute() && path.length() <= MAX_PATH)
-            return path;
-
         // return cached values if available
         WeakReference<String> ref = pathForWin32Calls;
         String resolved = (ref != null) ? ref.get() : null;
@@ -189,18 +185,18 @@ class WindowsPath implements Path {
         // resolve against default directory
         resolved = getAbsolutePath();
 
-        // Long paths need to have "." and ".." removed and be prefixed with
-        // "\?". Note that it is okay to remove ".." even when it follows
+        // Paths need to have "." and ".." removed and be prefixed with
+        // "\?", as otherwise paths can be at most 247 characters long.
+        //
+        // Note that it is okay to remove ".." even when it follows
         // a link - for example, it is okay for foo/link/../bar to be changed
         // to foo/bar. The reason is that Win32 APIs to access foo/link/../bar
         // will access foo/bar anyway (which differs to Unix systems)
-        if (resolved.length() > MAX_PATH) {
-            if (resolved.length() > MAX_LONG_PATH) {
-                throw new WindowsException("Cannot access file with path exceeding "
-                    + MAX_LONG_PATH + " characters");
-            }
-            resolved = addPrefixIfNeeded(GetFullPathName(resolved));
+        if (resolved.length() > MAX_LONG_PATH) {
+            throw new WindowsException("Cannot access file with path exceeding "
+                + MAX_LONG_PATH + " characters");
         }
+        resolved = addLongPathPrefix(GetFullPathName(resolved));
 
         // cache the resolved path (except drive relative paths as the working
         // directory on removal media devices can change during the lifetime
@@ -279,16 +275,13 @@ class WindowsPath implements Path {
                Character.toUpperCase(root2.charAt(0));
     }
 
-    // Add long path prefix to path if required
-    static String addPrefixIfNeeded(String path) {
-        if (path.length() > MAX_PATH) {
-            if (path.startsWith("\\\")) {
-                path = "\\\?\\UNC" + path.substring(1, path.length());
-            } else {
-                path = "\\\?\" + path;
-            }
+    // Add long path prefix to path.
+    static String addLongPathPrefix(String path) {
+        if (path.startsWith("\\\")) {
+            return "\\\?\\UNC" + path.substring(1, path.length());
+        } else {
+            return "\\\?\" + path;
         }
-        return path;
     }
 
     @Override

-------------

PR: https://git.openjdk.java.net/jdk/pull/5594


More information about the nio-dev mailing list