zipfs and relative paths

Joel Uckelman uckelman at nomic.net
Fri Nov 27 15:48:14 PST 2009


Thus spake Alan Bateman:
> Joel Uckelman wrote:
> >   
> Path operations with the demo zip provider should be similar to the 
> default provider on Solaris/Linux so if you have access to either then 
> it should be easy to compare. So fs.getPath("foo/").toString() should 
> return "foo" and fs.getPath("foo/../../bar").normalize() should result 
> in a Path that represent "../bar". Another idea is look at 
> test/java/nio/file/Path/PathOps.java - that test exercises all of the 
> path operations and could be easily modified to verify the zip provider 
> (the sanity.sh test for the zip provider doesn't provide good coverage).
> 
> -Alan.

More patches for zipfs:

* s/pathForprint/pathForPrint/g
* Fixed comment typos.
* Use String ctor directly, instead of substring(), no need to keep larger
  array around.
* Replaced StringBuffer with StringBuilder.
* ZipFilePath.normalize() now works.
* ZipPathParser.normalize() now chops trailing slashes.
* ZipPathParser.resolve() now handles leading '..' in relative paths properly.
* Introduced checkPath() to throw NPE and ProviderMismatchExceptions before
  using Paths passed as arguments.
* Fixed endsWith() to handle absolute other path properly (/foo !endsWith /).
* Fixed startsWith(): Absolute paths do not start with relative ones.

There's still more to do after this... I don't have zipfs passing all of
the tests I wrote yet.

-------------- next part --------------
# HG changeset patch
# User uckelman at adsl-208-39.dsl.uva.nl
# Date 1258402555 -3600
# Node ID 1a3ffe00a8e0a2eeef40f1237f69b87da910256a
# Parent  221e7d8e0dfd1bceca8f5b847b5326f4c25279c7
s/pathForprint/pathForPrint/g

diff --git a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
--- a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
+++ b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
@@ -73,13 +73,13 @@
     private final byte[] pathForZip;
     private final ReadLock readLock = new ReentrantReadWriteLock().readLock();
     private ZipFilePath pathToZip;
-    private final byte[] pathForprint;
+    private final byte[] pathForPrint;
 
     // package-private
     ZipFilePath(ZipFileSystem fileSystem, byte[] pathInZip) {
         this.fileSystem = fileSystem;
         this.path = pathInZip;
-        this.pathForprint = pathInZip;
+        this.pathForPrint = pathInZip;
         boolean isAbs = (path[0] == '/');
         String toResolve = new String(path);
         if (!isAbs) {
@@ -100,7 +100,7 @@
         this.fileSystem = fileSystem;
         this.path = pathForZip;
         this.pathForZip = pathForZip;
-        this.pathForprint = pathInZip; //given path
+        this.pathForPrint = pathInZip; //given path
     }
 
     public boolean isNestedZip() {
@@ -650,7 +650,7 @@
 
     @Override
     public String toString() {
-        return new String(pathForprint);
+        return new String(pathForPrint);
     }
 
     @Override
-------------- next part --------------
# HG changeset patch
# User uckelman at adsl-208-39.dsl.uva.nl
# Date 1258405833 -3600
# Node ID f627e54903f3c8e8f78bd454c814a983927e9b73
# Parent  1a3ffe00a8e0a2eeef40f1237f69b87da910256a
Fixed comment typos.
Use String ctor directly, instead of substring(), no need to keep larger
array around.

diff --git a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipPathParser.java b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipPathParser.java
--- a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipPathParser.java
+++ b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipPathParser.java
@@ -208,14 +208,14 @@
         pathInZip.getChars(0, len, pathArr, 0);
 
         //Repleace all Separators \\ with Zip Separator /
-        //throws InavalidPathException in Windows and Unux if char is not valid
+        //throws InavalidPathException in Windows and Unix if char is not valid
         // in respective file systems.
 
         for (int i = 0; i < len; i++) {
             if (pathArr[i] == '\\') {
                 pathArr[i] = '/';
             }
-            if (fileSepa == '\\') { //If Path is In Windows
+            if (fileSepa == '\\') { //If Path is in Windows
                 if ((pathArr[i] < '\u0020') || (invalidChars.indexOf(pathArr[i]) != -1)) {
                     throw new InvalidPathException(pathInZip, "Invalid char at " + i);
                 }
@@ -223,7 +223,7 @@
                         ((i == len - 1) && pathArr[i] == ' '))) {
                     throw new InvalidPathException(pathInZip, "Trailing space at" + (i - 1));
                 }
-            } else if (fileSepa == '/') { //If path In In Unix
+            } else if (fileSepa == '/') { //If path is in Unix
                 if (pathArr[i] == '\u0000') {
                     throw new InvalidPathException(pathInZip, "Null char at" + i);
                 }
@@ -240,9 +240,8 @@
             }
 
         }
-        return new String(pathArr).substring(0, size);
 
-
+        return new String(pathArr, 0, size);
     }
 
     // Remove DotSlash(./) and resolve DotDot (..) components
-------------- next part --------------
# HG changeset patch
# User uckelman at adsl-208-39.dsl.uva.nl
# Date 1258500243 -3600
# Node ID aa42666df3a0d3c5ba65a165b98b7f1ce63ec5ec
# Parent  f627e54903f3c8e8f78bd454c814a983927e9b73
Replaced StringBuffer with StringBuilder.

diff --git a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
--- a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
+++ b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
@@ -350,7 +350,7 @@
 
         int elements = endIndex - beginIndex;
         String result = null;
-        StringBuffer result1 = new StringBuffer("");
+        StringBuilder result1 = new StringBuilder("");
         int index = beginIndex;
         for (; elements-- != 0;) {
             if (endIndex == offsets.size() && elements == 0) {
@@ -382,7 +382,7 @@
 
         int elements = endIndex - beginIndex;
         String result = null;
-        StringBuffer result1 = new StringBuffer("");
+        StringBuilder result1 = new StringBuilder("");
         int index = beginIndex;
 
         for (; elements-- != 0;) {
diff --git a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipPathParser.java b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipPathParser.java
--- a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipPathParser.java
+++ b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipPathParser.java
@@ -286,10 +286,9 @@
             }
         }
         //Construct final path
-        StringBuffer resolved = (prefix) ? new StringBuffer("/") : new StringBuffer("");
+        final StringBuilder resolved = new StringBuilder(prefix ? "/" : "");
         for (String comp : stack) {
             resolved.append(comp).append("/");
-
         }
 
         String resolvedPath = "";
-------------- next part --------------
# HG changeset patch
# User uckelman at adsl-208-39.dsl.uva.nl
# Date 1258588273 -3600
# Node ID bfd37a1b962cec761051254d4b3737f5132c8086
# Parent  aa42666df3a0d3c5ba65a165b98b7f1ce63ec5ec
ZipFilePath.normalize() now works.
ZipPathParser.normalize() now chops trailing slashes.
ZipPathParser.resolve() now handles leading '..' in relative paths properly.

diff --git a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
--- a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
+++ b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
@@ -1183,21 +1183,24 @@
     }
 
     /*
-     * /foo//                -- >  /foo/
-     * /foo/./               -- > /foo/
+     * /foo//                -- > /foo
+     * /foo/./               -- > /foo
      * /foo/../bar           -- > /bar
      * /foo/../bar/../baz    -- > /baz
      * //foo//./bar          -- > /foo/bar
      * /../                  -- > /
-     * ../foo                -- > foo
+     * ../foo                -- > ../foo
      * foo/bar/..            -- > foo
-     * foo/../../bar         -- > bar
+     * foo/../../bar         -- > ../bar
      * foo/../bar            -- > bar
      **/
     @Override
     public Path normalize() {
-        //pathForZip is normalized path for ZipFileSystem
-        return new ZipFilePath(fileSystem, pathForZip, pathForZip);
+        return new ZipFilePath(
+            fileSystem,
+            ZipPathParser.resolve(new String(path)).getBytes(),
+            pathForZip
+        );
     }
 
     @Override
diff --git a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipPathParser.java b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipPathParser.java
--- a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipPathParser.java
+++ b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipPathParser.java
@@ -32,8 +32,9 @@
 package com.sun.nio.zipfs;
 
 import java.nio.file.*;
+import java.util.Iterator;
 import java.util.LinkedList;
-import java.util.Stack;
+import java.util.ListIterator;
 import java.util.regex.*;
 
 
@@ -132,7 +133,7 @@
             }
             return 0; // path length >1 and No Prefix
         }
-        return 0; //Path lenght=1 and it does not have any prefix
+        return 0; //Path length=1 and it does not have any prefix
     }
 
     static int nextNonSeparator(String path, int off) {
@@ -178,7 +179,6 @@
             }
         }
         return compList;
-
     }
 
     private static int findZipComponent(int prefixLen, String path) {
@@ -241,64 +241,62 @@
 
         }
 
+        // Remove trailing slash unless whole path is '/'
+        if (size > 1 && pathArr[size-1] == '/') size--;
+
         return new String(pathArr, 0, size);
     }
 
     // Remove DotSlash(./) and resolve DotDot (..) components
     static String resolve(String path) {
+        final boolean prefix = path.startsWith("/");
+        final LinkedList<String> parts = getComponents(path);
 
-        int len = path.length();
-        char pathArr[] = new char[len];
-        path.getChars(0, len, pathArr, 0);
-        //Remove ./ component ,Remove this if not necessary
-        char ch = pathArr[0];
-        int prefixPos = (ch == '/') ? 1 : 0;
-        int size = len;
+        int previousDirsAtBeginning = 0;
 
-        for (int i = prefixPos + 1; i < size; i++) {
-            if ((pathArr[i] == '/' && pathArr[i - 1] == '.') &&
-                    (i == prefixPos + 1 || pathArr[i - 2] == '/')) {
-                System.arraycopy(pathArr, i + 1, pathArr, i - 1, size - (i + 1));
-                size -= 2;
-                i--;
+        // Remove redundant parts.
+        for (ListIterator<String> i = parts.listIterator(); i.hasNext(); ) {
+            final String part = i.next();
+
+            if (part.equals(".")) {
+                // ".": Remove.
+                i.remove();
             }
+            else if (part.equals("..")) {  
+                // "..":
+                // Absolute paths: Remove this and the previous name, if any.
+                // Relative paths: Keep if we are part of a leading string of
+                // ".." names; otherwise remove this and the previous name.
+                if (prefix || i.previousIndex() > previousDirsAtBeginning) {
+                    i.remove();
 
-        }
-        //Remove final . if path has one. which is not needed for zip path
-        if ((size >= 2) && pathArr[size - 1] == '.' && pathArr[size - 2] == '/') {
-            System.arraycopy(pathArr, 0, pathArr, 0, size - 1);
-            size -= 1;
+                    // hasPrevious() can be false for the absolute path "/.."
+                    if (i.hasPrevious()) {
+                        i.previous();
+                        i.remove();
+                    }
+                }
+                else {
+                    // We are relative, must keep leading ".."
+                    previousDirsAtBeginning++;
+                }
+            }
+            else {
+                // Otherwise keep this name (for now).
+            }
         }
 
-        //Resolve ../ components
-        String pathStr = new String(pathArr, 0, size);
-        boolean prefix = pathStr.startsWith("/");
-        boolean endsWith = pathStr.endsWith("/");
-        LinkedList<String> compArr = getComponents(pathStr);
-        Stack<String> stack = new Stack<String>();
-        for (int i = 0; i < compArr.size(); i++) {
-            stack.push(compArr.get(i));
-            if (compArr.get(i).equals("..")) {
-                stack.pop();
-                if (i > 0 && stack.size() > 0) {
-                    stack.pop();
-                }
+        // Construct the resulting path string.
+        final StringBuilder res = new StringBuilder(prefix ? "/" : "");
+
+        final Iterator<String> i = parts.iterator();
+        if (i.hasNext()) { 
+            res.append(i.next());
+            while (i.hasNext()) {
+                res.append("/").append(i.next());
             }
         }
-        //Construct final path
-        final StringBuilder resolved = new StringBuilder(prefix ? "/" : "");
-        for (String comp : stack) {
-            resolved.append(comp).append("/");
-        }
 
-        String resolvedPath = "";
-        if (!resolved.toString().equals("")) {
-            if (!endsWith && resolved.length() != 1) {
-                resolvedPath = resolved.substring(0, resolved.length() - 1);
-            } else {
-                resolvedPath = resolved.toString();
-            }
-        }
-        return (resolvedPath);
+        return res.toString();
     }
 }
-------------- next part --------------
# HG changeset patch
# User Joel Uckelman <uckelman at nomic.net>
# Date 1259358283 -3600
# Node ID ed35a667769562b2090f5e2ef509f58b3b3528e6
# Parent  bfd37a1b962cec761051254d4b3737f5132c8086
Introduced checkPath() to throw NPE and ProviderMismatchExceptions before
using Paths passed as arguments.

diff --git a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
--- a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
+++ b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
@@ -103,6 +103,18 @@
         this.pathForPrint = pathInZip; //given path
     }
 
+    private ZipFilePath checkPath(Path path) {
+        if (path == null) {
+            throw new NullPointerException();
+        }
+  
+        if (!(path instanceof ZipFilePath)) {
+            throw new ProviderMismatchException();
+        }
+        
+        return (ZipFilePath) path;
+    }
+
     public boolean isNestedZip() {
         Pattern pattern = Pattern.compile("\\.(?i)(zip|jar)");
         Matcher matcher = null;
@@ -498,26 +510,21 @@
 
     @Override
     public Path relativize(Path other) {
-        if (other == null) {
-            throw new NullPointerException();
-        }
-        if (!(other instanceof ZipFilePath)) {
-            throw new ProviderMismatchException();
-        }
-        ZipFilePath other1 = (ZipFilePath) other;
-        if (other1.equals(this)) {
+        final ZipFilePath otherPath = checkPath(other);
+
+        if (otherPath.equals(this)) {
             return null;
         }
-        if (this.isAbsolute() != other1.isAbsolute()) {
-            return other1;
+        if (this.isAbsolute() != otherPath.isAbsolute()) {
+            return otherPath;
         }
 
         int i = 0;
         int ti = this.getNameCount();
-        int oi = other1.getNameCount();
+        int oi = otherPath.getNameCount();
 
         for (; i < ti && i < oi; i++) {
-            if (!this.getName(i).equals(other1.getName(i))) {
+            if (!this.getName(i).equals(otherPath.getName(i))) {
                 break;
             }
         }
@@ -531,7 +538,7 @@
         ZipFilePath subPath = null;
         int subpathlen = 0;
         if (i < oi) {
-            subPath = other1.subpath(i, oi);
+            subPath = otherPath.subpath(i, oi);
             subpathlen = subPath.path.length;
         }
         byte[] result = new byte[arr.length + subpathlen];
@@ -592,41 +599,28 @@
 
     @Override
     public boolean startsWith(Path other) {
+        final ZipFilePath otherPath = checkPath(other);
 
-        ZipFilePath other1 = null;
-        if (other == null) {
-            throw new NullPointerException();
-        }
-        if (other instanceof ZipFilePath) {
-            other1 = (ZipFilePath) other;
-        }
-
-        int otherCount = other1.getNameCount();
+        int otherCount = otherPath.getNameCount();
         if (getNameCount() < otherCount) {
             return false;
         }
 
         for (int i = 0; i < otherCount; i++) {
-            if (other1.getName(i).equals(getName(i))) {
+            if (otherPath.getName(i).equals(getName(i))) {
                 continue;
             } else {
                 return false;
             }
         }
         return true;
-
     }
 
     @Override
     public boolean endsWith(Path other) {
-        ZipFilePath other1 = null;
-        if (other == null) {
-            throw new NullPointerException();
-        }
-        if (other instanceof ZipFilePath) {
-            other1 = (ZipFilePath) other;
-        }
-        int i = other1.getNameCount();
+        final ZipFilePath otherPath = checkPath(other);
+
+        int i = otherPath.getNameCount();
         int j = getNameCount();
 
         if (j < i) {
@@ -634,14 +628,13 @@
         }
 
         for (--i, --j; i >= 0; i--, j--) {
-            if (other1.getName(i).equals(getName(j))) {
+            if (otherPath.getName(i).equals(getName(j))) {
                 continue;
             } else {
                 return false;
             }
         }
         return true;
-
     }
 
     public FileSystemProvider provider() {
@@ -677,9 +670,7 @@
 
     @Override
     public int compareTo(Path other) {
-
-        ZipFilePath otherPath = (ZipFilePath) other;
-        //int c = zipPath.compareTo(otherPath.zipPath);
+        final ZipFilePath otherPath = checkPath(other);
 
         int len1 = path.length;
         int len2 = otherPath.path.length;
-------------- next part --------------
# HG changeset patch
# User Joel Uckelman <uckelman at nomic.net>
# Date 1259363453 -3600
# Node ID 9f5cf65ff5e02cfdd74144a772e819c0425f273b
# Parent  ed35a667769562b2090f5e2ef509f58b3b3528e6
Fixed endsWith() to handle absolute other path properly (/foo !endsWith /).

diff --git a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
--- a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
+++ b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
@@ -618,22 +618,28 @@
 
     @Override
     public boolean endsWith(Path other) {
-        final ZipFilePath otherPath = checkPath(other);
+        final ZipFilePath that = checkPath(other);
 
-        int i = otherPath.getNameCount();
-        int j = getNameCount();
+        if (that.isAbsolute()) {
+            return this.isAbsolute() ? this.equals(that) : false;
+        }
+
+        int i = that.getNameCount();
+        int j = this.getNameCount();
 
         if (j < i) {
             return false;
         }
-
+  
         for (--i, --j; i >= 0; i--, j--) {
-            if (otherPath.getName(i).equals(getName(j))) {
-                continue;
-            } else {
-                return false;
+            if (that.getName(i).equals(this.getName(j))) {
+              continue;
+            }
+            else {
+              return false;
             }
         }
+
         return true;
     }
 
-------------- next part --------------
# HG changeset patch
# User Joel Uckelman <uckelman at nomic.net>
# Date 1259365111 -3600
# Node ID e05c7be3271e4d74c10a4b2672de3025245bad19
# Parent  9f5cf65ff5e02cfdd74144a772e819c0425f273b
Fixed startsWith(): Absolute paths do not start with relative ones.

diff --git a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
--- a/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
+++ b/src/share/demo/nio/ZipFileSystem/com/sun/nio/zipfs/ZipFilePath.java
@@ -599,20 +599,26 @@
 
     @Override
     public boolean startsWith(Path other) {
-        final ZipFilePath otherPath = checkPath(other);
+        final ZipFilePath that = checkPath(other);
 
-        int otherCount = otherPath.getNameCount();
-        if (getNameCount() < otherCount) {
+        if (that.isAbsolute() != this.isAbsolute()) {
             return false;
         }
 
-        for (int i = 0; i < otherCount; i++) {
-            if (otherPath.getName(i).equals(getName(i))) {
+        final int thatCount = that.getNameCount();
+        if (getNameCount() < thatCount) {
+            return false;
+        }
+      
+        for (int i = 0; i < thatCount; i++) {
+            if (that.getName(i).equals(getName(i))) {
                 continue;
-            } else {
+            }
+            else {
                 return false;
             }
         }
+
         return true;
     }
 


More information about the nio-dev mailing list