# HG changeset patch # User sherman # Date 1295898426 28800 # Node ID 5bc99c45810a36a071c7861d352ccbeb3f2a2b37 # Parent 11f3cdd5c639f967bc00a85a00bfa5899fa97d84 7006576: (zipfs) Path.exists() always returns false on dirs when zip/JAR file built without dirs 7009092: (zipfs) ZipPath.isSameFile() should always return true if this Path and the given Path are equal. 7009085: (zipfs) ZipPath.normalize("/./.") returns null. 7009102: (zipfs) ZipPath.toRealPath() should always return absolute path. Summary: zip filesystem provider update Reviewed-by: alanb diff -r 11f3cdd5c639 -r 5bc99c45810a jdk/src/share/demo/nio/zipfs/src/com/sun/nio/zipfs/ZipFileSystem.java --- a/jdk/src/share/demo/nio/zipfs/src/com/sun/nio/zipfs/ZipFileSystem.java Sat Jan 22 08:43:25 2011 -0500 +++ b/jdk/src/share/demo/nio/zipfs/src/com/sun/nio/zipfs/ZipFileSystem.java Mon Jan 24 11:47:06 2011 -0800 @@ -78,7 +78,6 @@ // configurable by env map private final String defaultDir; // default dir for the file system private final String nameEncoding; // default encoding for name/comment - private final boolean buildDirTree; // build a dir tree for directoryStream ops private final boolean useTempFile; // use a temp file for newOS, default // is to use BAOS for better performance private final boolean createNew; // create a new zip if not exists @@ -94,7 +93,6 @@ this.createNew = "true".equals(env.get("create")); this.nameEncoding = env.containsKey("encoding") ? (String)env.get("encoding") : "UTF-8"; - this.buildDirTree = TRUE.equals(env.get("buildDirTreea")); this.useTempFile = TRUE.equals(env.get("useTempFile")); this.defaultDir = env.containsKey("default.dir") ? (String)env.get("default.dir") : "/"; @@ -290,28 +288,16 @@ try { ensureOpen(); e = getEntry0(path); - } finally { - endRead(); - } - if (e == null) { - if (path.length == 0) { - e = new Entry(new byte[0]); // root - } else if (buildDirTree) { - IndexNode inode = getDirs().get(IndexNode.keyOf(path)); + if (e == null) { + IndexNode inode = getInode(path); if (inode == null) return null; - e = new Entry(inode.name); - } else { - return null; + e = new Entry(inode.name); // pseudo directory + e.method = METHOD_STORED; // STORED for dir + e.mtime = e.atime = e.ctime = -1;// -1 for all times } - e.method = METHOD_STORED; // STORED for dir - BasicFileAttributes bfas = Attributes.readBasicFileAttributes(zfpath); - if (bfas.lastModifiedTime() != null) - e.mtime = bfas.lastModifiedTime().toMillis(); - if (bfas.lastAccessTime() != null) - e.atime = bfas.lastAccessTime().toMillis(); - if (bfas.creationTime() != null) - e.ctime = bfas.creationTime().toMillis(); + } finally { + endRead(); } return new ZipFileAttributes(e); } @@ -346,7 +332,7 @@ beginRead(); try { ensureOpen(); - return getEntry0(path) != null; + return getInode(path) != null; } finally { endRead(); } @@ -355,13 +341,10 @@ boolean isDirectory(byte[] path) throws IOException { - if (buildDirTree) - return getDirs().containsKey(IndexNode.keyOf(path)); - beginRead(); try { - Entry e = getEntry0(path); - return (e != null && e.isDir()) || path.length == 0; + IndexNode n = getInode(path); + return n != null && n.isDir(); } finally { endRead(); } @@ -383,39 +366,16 @@ beginWrite(); // iteration of inodes needs exclusive lock try { ensureOpen(); - if (buildDirTree) { - IndexNode inode = getDirs().get(IndexNode.keyOf(path)); - if (inode == null) - throw new NotDirectoryException(getString(path)); - List list = new ArrayList<>(); - IndexNode child = inode.child; - while (child != null) { - ZipPath zp = toZipPath(child.name); - if (filter == null || filter.accept(zp)) - list.add(zp); - child = child.sibling; - } - return list.iterator(); - } - - if (!isDirectory(path)) + IndexNode inode = getInode(path); + if (inode == null) throw new NotDirectoryException(getString(path)); List list = new ArrayList<>(); - path = toDirectoryPath(path); - for (IndexNode key : inodes.keySet()) { - if (!isParentOf(path, key.name)) // is "path" the parent of "name" - continue; - int off = path.length; - while (off < key.name.length) { - if (key.name[off] == '/') - break; - off++; - } - if (off < (key.name.length - 1)) - continue; - ZipPath zp = toZipPath(key.name); + IndexNode child = inode.child; + while (child != null) { + ZipPath zp = toZipPath(child.name); if (filter == null || filter.accept(zp)) list.add(zp); + child = child.sibling; } return list.iterator(); } finally { @@ -433,7 +393,6 @@ ensureOpen(); if (dir.length == 0 || exists(dir)) // root dir, or exiting dir throw new FileAlreadyExistsException(getString(dir)); - checkParents(dir); Entry e = new Entry(dir, Entry.NEW); e.method = METHOD_STORED; // STORED for dir @@ -476,7 +435,7 @@ checkParents(dst); } Entry u = new Entry(eSrc, Entry.COPY); // copy eSrc entry - u.name = dst; // change name + u.name(dst); // change name if (eSrc.type == Entry.NEW || eSrc.type == Entry.FILECH) { u.type = eSrc.type; // make it the same type @@ -518,7 +477,7 @@ if (opt == APPEND) hasAppend = true; } - beginRead(); // only need a readlock, the "update()" will + beginRead(); // only need a readlock, the "update()" will try { // try to obtain a writelock when the os is ensureOpen(); // being closed. Entry e = getEntry0(path); @@ -869,43 +828,27 @@ private void checkParents(byte[] path) throws IOException { beginRead(); try { - while ((path = getParent(path)) != null) { - if (!inodes.containsKey(IndexNode.keyOf(path))) + while ((path = getParent(path)) != null && path.length != 0) { + if (!inodes.containsKey(IndexNode.keyOf(path))) { throw new NoSuchFileException(getString(path)); + } } } finally { endRead(); } } + private static byte[] ROOTPATH = new byte[0]; private static byte[] getParent(byte[] path) { int off = path.length - 1; if (off > 0 && path[off] == '/') // isDirectory off--; while (off > 0 && path[off] != '/') { off--; } - if (off == 0) - return null; // top entry + if (off <= 0) + return ROOTPATH; return Arrays.copyOf(path, off + 1); } - // If "starter" is the parent directory of "path" - private static boolean isParentOf(byte[] p, byte[] c) { - final int plen = p.length; - if (plen == 0) // root dir - return true; - if (plen >= c.length) - return false; - int n = 0; - while (n < plen) { - if (p[n] != c[n]) - return false; - n++; - } - if (p[n - 1] != '/' && (c[n] != '/' || n == c.length - 1)) - return false; - return true; - } - private final void beginWrite() { rwlock.writeLock().lock(); } @@ -926,7 +869,7 @@ private volatile boolean isOpen = true; private final SeekableByteChannel ch; // channel to the zipfile - final byte[] cen; // CEN & ENDHDR + final byte[] cen; // CEN & ENDHDR private END end; private long locpos; // position of first LOC header (usually 0) @@ -1058,6 +1001,7 @@ if (end.endpos == 0) { inodes = new LinkedHashMap<>(10); locpos = 0; + buildNodeTree(); return null; // only END header present } if (end.cenlen > end.endpos) @@ -1101,6 +1045,7 @@ if (pos + ENDHDR != cen.length) { zerror("invalid CEN header (bad header size)"); } + buildNodeTree(); return cen; } @@ -1125,12 +1070,15 @@ private boolean hasUpdate = false; + // shared key. consumer guarantees the "writeLock" before use it. + private final IndexNode LOOKUPKEY = IndexNode.keyOf(null); + private void updateDelete(Entry e) { beginWrite(); try { - inodes.remove(IndexNode.keyOf(e.name)); //inodes.remove(e.name); + removeFromTree(e); + inodes.remove(e); hasUpdate = true; - dirs = null; } finally { endWrite(); } @@ -1139,9 +1087,16 @@ private void update(Entry e) { beginWrite(); try { - inodes.put(IndexNode.keyOf(e.name), e); //inodes.put(e, e); + IndexNode old = inodes.put(e, e); + if (old != null) { + removeFromTree(old); + } + if (e.type == Entry.NEW || e.type == Entry.FILECH) { + IndexNode parent = inodes.get(LOOKUPKEY.as(getParent(e.name))); + e.sibling = parent.child; + parent.child = e; + } hasUpdate = true; - dirs = null; } finally { endWrite(); } @@ -1229,7 +1184,7 @@ // ext) without enflating/deflating from the old zip // file LOC entry. written += copyLOCEntry(e, true, os, written, buf); - } else { // NEW or FILECH + } else { // NEW, FILECH or CEN e.locoff = written; written += e.writeLOC(os); // write loc header if (e.bytes != null) { // in-memory, deflated @@ -1265,7 +1220,10 @@ } catch (IOException x) { x.printStackTrace(); // skip any in-accurate entry } - } else { // unchanged inode + } else { // unchanged inode + if (inode.pos == -1) { + continue; // pseudo directory node + } e = Entry.readCEN(this, inode.pos); try { written += copyLOCEntry(e, false, os, written, buf); @@ -1318,34 +1276,28 @@ //System.out.printf("->sync(%s) done!%n", toString()); } - private Entry getEntry0(byte[] path) throws IOException { + private IndexNode getInode(byte[] path) { if (path == null) throw new NullPointerException("path"); - if (path.length == 0) - return null; - IndexNode inode = null; IndexNode key = IndexNode.keyOf(path); - if ((inode = inodes.get(key)) == null) { - if (path[path.length -1] == '/') // already has a slash - return null; + IndexNode inode = inodes.get(key); + if (inode == null && + (path.length == 0 || path[path.length -1] != '/')) { + // if does not ends with a slash path = Arrays.copyOf(path, path.length + 1); path[path.length - 1] = '/'; - if ((inode = inodes.get(key.as(path))) == null) - return null; + inode = inodes.get(key.as(path)); } - if (inode instanceof Entry) - return (Entry)inode; - return Entry.readCEN(this, inode.pos); + return inode; } - // Test if the "name" a parent directory of any entry (dir empty) - boolean isAncestor(byte[] name) { - for (Map.Entry entry : inodes.entrySet()) { - byte[] ename = entry.getKey().name; - if (isParentOf(name, ename)) - return true; - } - return false; + private Entry getEntry0(byte[] path) throws IOException { + IndexNode inode = getInode(path); + if (inode instanceof Entry) + return (Entry)inode; + if (inode == null || inode.pos == -1) + return null; + return Entry.readCEN(this, inode.pos); } public void deleteFile(byte[] path, boolean failIfNotExists) @@ -1359,7 +1311,7 @@ if (failIfNotExists) throw new NoSuchFileException(getString(path)); } else { - if (e.isDir() && isAncestor(path)) + if (e.isDir() && e.child != null) throw new DirectoryNotEmptyException(getString(path)); updateDelete(e); } @@ -1539,7 +1491,6 @@ public int available() { return rem > Integer.MAX_VALUE ? Integer.MAX_VALUE : (int) rem; } - public long size() { return size; } @@ -1762,7 +1713,7 @@ int pos = -1; // postion in cen table, -1 menas the // entry does not exists in zip file IndexNode(byte[] name, int pos) { - as(name); + name(name); this.pos = pos; } @@ -1770,15 +1721,25 @@ return new IndexNode(name, -1); } - final IndexNode as(byte[] name) { // reuse the node, mostly - this.name = name; // as a lookup "key" + final void name(byte[] name) { + this.name = name; this.hashcode = Arrays.hashCode(name); + } + + final IndexNode as(byte[] name) { // reuse the node, mostly + name(name); // as a lookup "key" return this; } + boolean isDir() { + return name != null && + (name.length == 0 || name[name.length - 1] == '/'); + } + public boolean equals(Object other) { - if (!(other instanceof IndexNode)) + if (!(other instanceof IndexNode)) { return false; + } return Arrays.equals(name, ((IndexNode)other).name); } @@ -1798,6 +1759,7 @@ static final int FILECH = 3; // fch update in "file" static final int COPY = 4; // copy of a CEN entry + byte[] bytes; // updated content bytes Path file; // use tmp file to store bytes; int type = CEN; // default is the entry read from cen @@ -1825,7 +1787,7 @@ Entry() {} Entry(byte[] name) { - this.name = name; + name(name); this.mtime = System.currentTimeMillis(); this.crc = 0; this.size = 0; @@ -1839,8 +1801,8 @@ } Entry (Entry e, int type) { + name(e.name); this.version = e.version; - this.name = e.name; this.ctime = e.ctime; this.atime = e.atime; this.mtime = e.mtime; @@ -1855,7 +1817,6 @@ this.attrsEx = e.attrsEx; this.locoff = e.locoff; this.comment = e.comment; - this.type = type; } @@ -1865,12 +1826,6 @@ this.method = METHOD_STORED; } - boolean isDir() { - return name != null && - (name.length == 0 || - name[name.length - 1] == '/'); - } - int version() throws ZipException { if (method == METHOD_DEFLATED) return 20; @@ -1909,7 +1864,7 @@ locoff = CENOFF(cen, pos); pos += CENHDR; - name = Arrays.copyOfRange(cen, pos, pos + nlen); + name(Arrays.copyOfRange(cen, pos, pos + nlen)); pos += nlen; if (elen > 0) { @@ -2132,7 +2087,8 @@ } writeShort(os, flag); // general purpose bit flag writeShort(os, method); // compression method - writeInt(os, mtime); // last modification time + // last modification time + writeInt(os, (int)javaToDosTime(mtime)); writeInt(os, crc); // crc-32 if (elen64 != 0) { writeInt(os, ZIP64_MINVAL); @@ -2318,50 +2274,56 @@ // structure. // A possible solution is to build the node tree ourself as // implemented below. - private HashMap dirs; private IndexNode root; - private IndexNode addToDir(IndexNode child) { - IndexNode cinode = dirs.get(child); - if (cinode != null) - return cinode; - - byte[] cname = child.name; - byte[] pname = getParent(cname); - IndexNode pinode; - if (pname != null) - pinode = addToDir(IndexNode.keyOf(pname)); - else - pinode = root; - cinode = inodes.get(child); - if (cname[cname.length -1] != '/') { // not a dir - cinode.sibling = pinode.child; - pinode.child = cinode; - return null; + private void addToTree(IndexNode inode, HashSet dirs) { + if (dirs.contains(inode)) { + return; } - //cinode = dirs.get(child); - if (cinode == null) // pseudo directry entry - cinode = new IndexNode(cname, -1); - cinode.sibling = pinode.child; - pinode.child = cinode; - - dirs.put(child, cinode); - return cinode; + IndexNode parent; + byte[] name = inode.name; + byte[] pname = getParent(name); + if (inodes.containsKey(LOOKUPKEY.as(pname))) { + parent = inodes.get(LOOKUPKEY); + } else { // pseudo directory entry + parent = new IndexNode(pname, -1); + inodes.put(parent, parent); + } + addToTree(parent, dirs); + inode.sibling = parent.child; + parent.child = inode; + if (name[name.length -1] == '/') + dirs.add(inode); } - private HashMap getDirs() - throws IOException - { + private void removeFromTree(IndexNode inode) { + IndexNode parent = inodes.get(LOOKUPKEY.as(getParent(inode.name))); + IndexNode child = parent.child; + if (child == inode) { + parent.child = child.sibling; + } else { + IndexNode last = child; + while ((child = child.sibling) != null) { + if (child == inode) { + last.sibling = child.sibling; + break; + } else { + last = child; + } + } + } + } + + private void buildNodeTree() throws IOException { beginWrite(); try { - if (dirs != null) - return dirs; - dirs = new HashMap<>(); - root = new IndexNode(new byte[0], -1); - dirs.put(root, root); - for (IndexNode node : inodes.keySet()) - addToDir(node); - return dirs; + HashSet dirs = new HashSet<>(); + IndexNode root = new IndexNode(ROOTPATH, -1); + inodes.put(root, root); + dirs.add(root); + for (IndexNode node : inodes.keySet().toArray(new IndexNode[0])) { + addToTree(node, dirs); + } } finally { endWrite(); } diff -r 11f3cdd5c639 -r 5bc99c45810a jdk/src/share/demo/nio/zipfs/src/com/sun/nio/zipfs/ZipPath.java --- a/jdk/src/share/demo/nio/zipfs/src/com/sun/nio/zipfs/ZipPath.java Sat Jan 22 08:43:25 2011 -0500 +++ b/jdk/src/share/demo/nio/zipfs/src/com/sun/nio/zipfs/ZipPath.java Mon Jan 24 11:47:06 2011 -0800 @@ -157,7 +157,7 @@ @Override public ZipPath toRealPath(boolean resolveLinks) throws IOException { - ZipPath realPath = new ZipPath(zfs, getResolvedPath()); + ZipPath realPath = new ZipPath(zfs, getResolvedPath()).toAbsolutePath(); realPath.checkAccess(); return realPath; } @@ -472,8 +472,11 @@ int n = offsets[i]; int len = (i == offsets.length - 1)? (path.length - n):(offsets[i + 1] - n - 1); - if (len == 1 && path[n] == (byte)'.') + if (len == 1 && path[n] == (byte)'.') { + if (m == 0 && path[0] == '/') // absolute path + to[m++] = '/'; continue; + } if (len == 2 && path[n] == '.' && path[n + 1] == '.') { if (lastMOff >= 0) { m = lastM[lastMOff--]; // retreat @@ -726,6 +729,8 @@ @Override public boolean isSameFile(Path other) throws IOException { + if (this.equals(other)) + return true; if (other == null || this.getFileSystem() != other.getFileSystem()) return false; diff -r 11f3cdd5c639 -r 5bc99c45810a jdk/test/demo/zipfs/PathOps.java --- a/jdk/test/demo/zipfs/PathOps.java Sat Jan 22 08:43:25 2011 -0500 +++ b/jdk/test/demo/zipfs/PathOps.java Mon Jan 24 11:47:06 2011 -0800 @@ -193,6 +193,17 @@ return this; } + PathOps isSameFile(String target) { + try { + out.println("check two paths are same"); + checkPath(); + check(path.isSameFile(test(target).path()), true); + } catch (IOException ioe) { + fail(); + } + return this; + } + PathOps invalid() { if (!(exc instanceof InvalidPathException)) { out.println("InvalidPathException not thrown as expected"); @@ -344,7 +355,12 @@ .normalize("foo"); test("/foo/bar/gus/../..") .normalize("/foo"); - + test("/./.") + .normalize("/"); + test("/.") + .normalize("/"); + test("/./abc") + .normalize("/abc"); // invalid test("foo\u0000bar") .invalid(); @@ -365,6 +381,10 @@ .root("/") .parent("/foo") .name("bar"); + + // isSameFile + test("/fileDoesNotExist") + .isSameFile("/fileDoesNotExist"); } static void npes() { diff -r 11f3cdd5c639 -r 5bc99c45810a jdk/test/demo/zipfs/ZipFSTester.java --- a/jdk/test/demo/zipfs/ZipFSTester.java Sat Jan 22 08:43:25 2011 -0500 +++ b/jdk/test/demo/zipfs/ZipFSTester.java Mon Jan 24 11:47:06 2011 -0800 @@ -28,6 +28,7 @@ import java.nio.file.attribute.*; import java.net.*; import java.util.*; +import java.util.zip.*; import static java.nio.file.StandardOpenOption.*; import static java.nio.file.StandardCopyOption.*; @@ -42,7 +43,8 @@ FileSystem fs = null; try { fs = newZipFileSystem(Paths.get(args[0]), new HashMap()); - test(fs); + test0(fs); + test1(fs); test2(fs); // more tests } finally { if (fs != null) @@ -50,11 +52,31 @@ } } - static void test(FileSystem fs) + static void test0(FileSystem fs) + throws Exception + { + List list = new LinkedList<>(); + try (ZipFile zf = new ZipFile(fs.toString())) { + Enumeration zes = zf.entries(); + while (zes.hasMoreElements()) { + list.add(zes.nextElement().getName()); + } + for (String pname : list) { + Path path = fs.getPath(pname); + if (!path.exists()) + throw new RuntimeException("path existence check failed!"); + while ((path = path.getParent()) != null) { + if (!path.exists()) + throw new RuntimeException("parent existence check failed!"); + } + } + } + } + + static void test1(FileSystem fs) throws Exception { Random rdm = new Random(); - // clone a fs and test on it Path tmpfsPath = getTempPath(); Map env = new HashMap(); diff -r 11f3cdd5c639 -r 5bc99c45810a jdk/test/demo/zipfs/basic.sh --- a/jdk/test/demo/zipfs/basic.sh Sat Jan 22 08:43:25 2011 -0500 +++ b/jdk/test/demo/zipfs/basic.sh Mon Jan 24 11:47:06 2011 -0800 @@ -21,7 +21,7 @@ # questions. # # @test -# @bug 6990846 +# @bug 6990846 7009092 7009085 # @summary Test ZipFileSystem demo # @build Basic PathOps ZipFSTester # @run shell basic.sh