8222440: (zipfs) JarFileSystem does not correctly handle versioned entries if no root entry is present
authorclanger
Fri, 26 Apr 2019 08:53:29 +0100
changeset 54630 04b17e84c87d
parent 54629 9ebb614d293d
child 54631 3a3e4e473622
8222440: (zipfs) JarFileSystem does not correctly handle versioned entries if no root entry is present Reviewed-by: lancea
src/jdk.zipfs/share/classes/jdk/nio/zipfs/JarFileSystem.java
src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java
test/jdk/jdk/nio/zipfs/jarfs/JFSTester.java
test/jdk/jdk/nio/zipfs/jarfs/root/dir1/leaf1.txt
test/jdk/jdk/nio/zipfs/jarfs/root/dir1/leaf2.txt
test/jdk/jdk/nio/zipfs/jarfs/root/dir2/leaf3.txt
test/jdk/jdk/nio/zipfs/jarfs/root/dir2/leaf4.txt
test/jdk/jdk/nio/zipfs/jarfs/v9/root/dir1/leaf1.txt
test/jdk/jdk/nio/zipfs/jarfs/v9/root/dir1/leaf2.txt
test/jdk/jdk/nio/zipfs/jarfs/v9/root/dir2/leaf3.txt
test/jdk/jdk/nio/zipfs/jarfs/v9/root/dir2/leaf4.txt
--- a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/JarFileSystem.java	Fri Apr 26 00:57:03 2019 -0400
+++ b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/JarFileSystem.java	Fri Apr 26 08:53:29 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -49,20 +49,17 @@
  * @author Steve Drach
  */
 class JarFileSystem extends ZipFileSystem {
-    private Function<byte[],byte[]> lookup;
+    // lookup needs to be initialized because isMultiReleaseJar is called before createVersionedLinks
+    private Function<byte[], byte[]> lookup = path -> path;
 
     @Override
     IndexNode getInode(byte[] path) {
         // check for an alias to a versioned entry
-        byte[] versionedPath = lookup.apply(path);
-        return versionedPath == null ? super.getInode(path) : super.getInode(versionedPath);
+        return super.getInode(lookup.apply(path));
     }
 
-    JarFileSystem(ZipFileSystemProvider provider, Path zfpath, Map<String,?> env)
-            throws IOException {
+    JarFileSystem(ZipFileSystemProvider provider, Path zfpath, Map<String,?> env) throws IOException {
         super(provider, zfpath, env);
-        lookup = path -> path;  // lookup needs to be set before isMultiReleaseJar is called
-                                // because it eventually calls getEntry
         if (isMultiReleaseJar()) {
             int version;
             Object o = env.get("multi-release");
@@ -81,7 +78,7 @@
                 throw new IllegalArgumentException("env parameter must be String, Integer, "
                         + "or Version");
             }
-            lookup = createVersionedLinks(version < 0 ? 0 : version);
+            createVersionedLinks(version < 0 ? 0 : version);
             setReadOnly();
         }
     }
@@ -89,7 +86,7 @@
     private boolean isMultiReleaseJar() throws IOException {
         try (InputStream is = newInputStream(getBytes("/META-INF/MANIFEST.MF"))) {
             String multiRelease = new Manifest(is).getMainAttributes()
-                    .getValue(Attributes.Name.MULTI_RELEASE);
+                .getValue(Attributes.Name.MULTI_RELEASE);
             return "true".equalsIgnoreCase(multiRelease);
         } catch (NoSuchFileException x) {
             return false;
@@ -106,27 +103,71 @@
      * then wrap the map in a function that getEntry can use to override root
      * entry lookup for entries that have corresponding versioned entries
      */
-    private Function<byte[],byte[]> createVersionedLinks(int version) {
-        HashMap<IndexNode,byte[]> aliasMap = new HashMap<>();
+    private void createVersionedLinks(int version) {
         IndexNode verdir = getInode(getBytes("/META-INF/versions"));
-        if (verdir != null) {
-            getVersionMap(version, verdir).values()
-                .forEach(versionNode -> {   // for each META-INF/versions/{n} directory
-                    // put all the leaf inodes, i.e. entries, into the alias map
-                    // possibly shadowing lower versioned entries
-                    walk(versionNode, entryNode -> {
-                        byte[] rootName = getRootName(versionNode, entryNode);
-                        if (rootName != null) {
-                            IndexNode rootNode = getInode(rootName);
-                            if (rootNode == null) { // no matching root node, make a virtual one
-                                rootNode = IndexNode.keyOf(rootName);
-                            }
-                            aliasMap.put(rootNode, entryNode.name);
-                        }
-                    });
-                });
+        // nothing to do, if no /META-INF/versions
+        if (verdir == null) {
+            return;
+        }
+        // otherwise, create a map and for each META-INF/versions/{n} directory
+        // put all the leaf inodes, i.e. entries, into the alias map
+        // possibly shadowing lower versioned entries
+        HashMap<IndexNode, byte[]> aliasMap = new HashMap<>();
+        getVersionMap(version, verdir).values().forEach(versionNode ->
+            walk(versionNode.child, entryNode ->
+                aliasMap.put(
+                    getNodeInRootTree(getRootName(entryNode, versionNode), entryNode.isdir),
+                    entryNode.name))
+        );
+        lookup = path -> {
+            byte[] entry = aliasMap.get(IndexNode.keyOf(path));
+            return entry == null ? path : entry;
+        };
+    }
+
+    /**
+     * Return the node from the root tree. Create it, if it doesn't exist.
+     */
+    private IndexNode getNodeInRootTree(byte[] path, boolean isdir) {
+        IndexNode node = getInode(path);
+        if (node != null) {
+            return node;
         }
-        return path -> aliasMap.get(IndexNode.keyOf(path));
+        IndexNode parent = getParentDir(path);
+        beginWrite();
+        try {
+            node = new IndexNode(path, isdir);
+            node.sibling = parent.child;
+            parent.child = node;
+            inodes.put(node, node);
+            return node;
+        } finally {
+            endWrite();
+        }
+    }
+
+    /**
+     * Return the parent directory node of a path. If the node doesn't exist,
+     * it will be created. Parent directories will be created recursively.
+     * Recursion fuse: We assume at latest the root path can be resolved to a node.
+     */
+    private IndexNode getParentDir(byte[] path) {
+        byte[] parentPath = getParent(path);
+        IndexNode node = inodes.get(IndexNode.keyOf(parentPath));
+        if (node != null) {
+            return node;
+        }
+        IndexNode parent = getParentDir(parentPath);
+        beginWrite();
+        try {
+            node = new IndexNode(parentPath, true);
+            node.sibling = parent.child;
+            parent.child = node;
+            inodes.put(node, node);
+            return node;
+        } finally {
+            endWrite();
+        }
     }
 
     /**
@@ -138,7 +179,7 @@
         TreeMap<Integer,IndexNode> map = new TreeMap<>();
         IndexNode child = metaInfVersions.child;
         while (child != null) {
-            Integer key = getVersion(child.name, metaInfVersions.name.length + 1);
+            Integer key = getVersion(child, metaInfVersions);
             if (key != null && key <= version) {
                 map.put(key, child);
             }
@@ -150,9 +191,11 @@
     /**
      * extract the integer version number -- META-INF/versions/9 returns 9
      */
-    private Integer getVersion(byte[] name, int offset) {
+    private Integer getVersion(IndexNode inode, IndexNode metaInfVersions) {
         try {
-            return Integer.parseInt(getString(Arrays.copyOfRange(name, offset, name.length)));
+            byte[] fullName = inode.name;
+            return Integer.parseInt(getString(Arrays
+                .copyOfRange(fullName, metaInfVersions.name.length + 1, fullName.length)));
         } catch (NumberFormatException x) {
             // ignore this even though it might indicate issues with the JAR structure
             return null;
@@ -162,14 +205,14 @@
     /**
      * walk the IndexNode tree processing all leaf nodes
      */
-    private void walk(IndexNode inode, Consumer<IndexNode> process) {
+    private void walk(IndexNode inode, Consumer<IndexNode> consumer) {
         if (inode == null) return;
         if (inode.isDir()) {
-            walk(inode.child, process);
+            walk(inode.child, consumer);
         } else {
-            process.accept(inode);
+            consumer.accept(inode);
         }
-        walk(inode.sibling, process);
+        walk(inode.sibling, consumer);
     }
 
     /**
@@ -178,9 +221,8 @@
      *   and prefix META-INF/versions/9/
      *   returns foo/bar.class
      */
-    private byte[] getRootName(IndexNode prefix, IndexNode inode) {
-        int offset = prefix.name.length;
+    private byte[] getRootName(IndexNode inode, IndexNode prefix) {
         byte[] fullName = inode.name;
-        return Arrays.copyOfRange(fullName, offset, fullName.length);
+        return Arrays.copyOfRange(fullName, prefix.name.length, fullName.length);
     }
 }
--- a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java	Fri Apr 26 00:57:03 2019 -0400
+++ b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java	Fri Apr 26 08:53:29 2019 +0100
@@ -884,7 +884,7 @@
     }
 
     private static byte[] ROOTPATH = new byte[] { '/' };
-    private static byte[] getParent(byte[] path) {
+    static byte[] getParent(byte[] path) {
         int off = getParentOff(path);
         if (off <= 1)
             return ROOTPATH;
@@ -899,11 +899,11 @@
         return off;
     }
 
-    private final void beginWrite() {
+    final void beginWrite() {
         rwlock.writeLock().lock();
     }
 
-    private final void endWrite() {
+    final void endWrite() {
         rwlock.writeLock().unlock();
     }
 
@@ -926,7 +926,7 @@
     private final ReadWriteLock rwlock = new ReentrantReadWriteLock();
 
     // name -> pos (in cen), IndexNode itself can be used as a "key"
-    private LinkedHashMap<IndexNode, IndexNode> inodes;
+    LinkedHashMap<IndexNode, IndexNode> inodes;
 
     final byte[] getBytes(String name) {
         return zc.getBytes(name);
--- a/test/jdk/jdk/nio/zipfs/jarfs/JFSTester.java	Fri Apr 26 00:57:03 2019 -0400
+++ b/test/jdk/jdk/nio/zipfs/jarfs/JFSTester.java	Fri Apr 26 08:53:29 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -23,15 +23,16 @@
 
 /*
  * @test
- * @bug 8164389
+ * @bug 8164389 8222440
  * @summary walk entries in a jdk.nio.zipfs.JarFileSystem
- * @modules jdk.jartool/sun.tools.jar
+ * @library /lib/testlibrary/java/util/jar
+ * @modules jdk.jartool
  *          jdk.zipfs
+ * @build Compiler JarBuilder
  * @run testng JFSTester
  */
 
 import org.testng.Assert;
-import org.testng.annotations.AfterClass;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
@@ -50,57 +51,76 @@
 
 public class JFSTester {
     private URI jarURI;
-    private Path jarfile;
+
+    final private String root_dir1_leaf1_txt = "This is leaf 1." + System.lineSeparator();
+    final private String root_dir1_leaf2_txt = "This is leaf 2." + System.lineSeparator();
+    final private String root_dir2_leaf3_txt = "This is leaf 3." + System.lineSeparator();
+    final private String root_dir2_leaf4_txt = "This is leaf 4." + System.lineSeparator();
+    final private String v9_root_dir2_leaf3_txt = "This is version 9 leaf 3." + System.lineSeparator();
+    final private String v9_root_dir2_leaf4_txt = "This is version 9 leaf 4." + System.lineSeparator();
+    final private String v9_root_dir3_leaf5_txt = "This is version 9 leaf 5." + System.lineSeparator();
+    final private String v9_root_dir3_leaf6_txt = "This is version 9 leaf 6." + System.lineSeparator();
+    final private String v10_root_dir3_leaf5_txt = "This is version 10 leaf 5." + System.lineSeparator();
+    final private String v10_root_dir3_leaf6_txt = "This is version 10 leaf 6." + System.lineSeparator();
 
     @BeforeClass
     public void initialize() throws Exception {
-        String userdir = System.getProperty("user.dir",".");
-        jarfile = Paths.get(userdir, "test.jar");
-        String srcdir = System.getProperty("test.src");
-        String[] args = (
-                        "-cf "
-                        + jarfile.toString()
-                        + " -C "
-                        + srcdir
-                        + " root --release 9 -C "
-                        + srcdir
-                        + System.getProperty("file.separator")
-                        + "v9 root"
-        ).split(" +");
-        new sun.tools.jar.Main(System.out, System.err, "jar").run(args);
-        String ssp = jarfile.toUri().toString();
-        jarURI = new URI("jar", ssp, null);
-    }
-
-    @AfterClass
-    public void close() throws IOException {
-        Files.deleteIfExists(jarfile);
+        Path jarfile = Paths.get("test.jar");
+        JarBuilder jb = new JarBuilder(jarfile.toString());
+        jb.addAttribute("Multi-Release", "true");
+        jb.addEntry("root/dir1/leaf1.txt", root_dir1_leaf1_txt.getBytes());
+        jb.addEntry("root/dir1/leaf2.txt", root_dir1_leaf2_txt.getBytes());
+        jb.addEntry("root/dir2/leaf3.txt", root_dir2_leaf3_txt.getBytes());
+        jb.addEntry("root/dir2/leaf4.txt", root_dir2_leaf4_txt.getBytes());
+        jb.addEntry("META-INF/versions/9/root/dir2/leaf3.txt", v9_root_dir2_leaf3_txt.getBytes());
+        jb.addEntry("META-INF/versions/9/root/dir2/leaf4.txt", v9_root_dir2_leaf4_txt.getBytes());
+        jb.addEntry("META-INF/versions/9/root/dir3/leaf5.txt", v9_root_dir3_leaf5_txt.getBytes());
+        jb.addEntry("META-INF/versions/9/root/dir3/leaf6.txt", v9_root_dir3_leaf6_txt.getBytes());
+        jb.addEntry("META-INF/versions/10/root/dir3/leaf5.txt", v10_root_dir3_leaf5_txt.getBytes());
+        jb.addEntry("META-INF/versions/10/root/dir3/leaf6.txt", v10_root_dir3_leaf6_txt.getBytes());
+        jb.build();
+        System.out.println("Created " + jarfile + ": " + Files.exists(jarfile));
+        jarURI = new URI("jar", jarfile.toUri().toString(), null);
     }
 
     @Test
     public void testWalk() throws IOException {
-
-        // no configuration, treat multi-release jar as unversioned
-        Map<String,String> env = new HashMap<>();
+        // treat multi-release jar as unversioned
+        Map<String, String> env = new HashMap<>();
         Set<String> contents = doTest(env);
-        Set<String> baseContents = Set.of(
-                "This is leaf 1.\n",
-                "This is leaf 2.\n",
-                "This is leaf 3.\n",
-                "This is leaf 4.\n"
+        Set<String> expectedContents = Set.of(
+            root_dir1_leaf1_txt,
+            root_dir1_leaf2_txt,
+            root_dir2_leaf3_txt,
+            root_dir2_leaf4_txt
         );
-        Assert.assertEquals(contents, baseContents);
+        Assert.assertEquals(contents, expectedContents);
 
-        // a configuration and jar file is multi-release
+        // open file as multi-release for version 9
         env.put("multi-release", "9");
         contents = doTest(env);
-        Set<String> versionedContents = Set.of(
-                "This is versioned leaf 1.\n",
-                "This is versioned leaf 2.\n",
-                "This is versioned leaf 3.\n",
-                "This is versioned leaf 4.\n"
+        expectedContents = Set.of(
+            root_dir1_leaf1_txt,
+            root_dir1_leaf2_txt,
+            v9_root_dir2_leaf3_txt,
+            v9_root_dir2_leaf4_txt,
+            v9_root_dir3_leaf5_txt,
+            v9_root_dir3_leaf6_txt
         );
-        Assert.assertEquals(contents, versionedContents);
+        Assert.assertEquals(contents, expectedContents);
+
+        // open file as multi-release for version 10
+        env.put("multi-release", "10");
+        contents = doTest(env);
+        expectedContents = Set.of(
+            root_dir1_leaf1_txt,
+            root_dir1_leaf2_txt,
+            v9_root_dir2_leaf3_txt,
+            v9_root_dir2_leaf4_txt,
+            v10_root_dir3_leaf5_txt,
+            v10_root_dir3_leaf6_txt
+        );
+        Assert.assertEquals(contents, expectedContents);
     }
 
     private Set<String> doTest(Map<String,String> env) throws IOException {
@@ -108,9 +128,10 @@
         try (FileSystem fs = FileSystems.newFileSystem(jarURI, env)) {
             Path root = fs.getPath("root");
             contents = Files.walk(root)
-                    .filter(p -> !Files.isDirectory(p))
-                    .map(this::pathToContents)
-                    .collect(Collectors.toSet());
+                .filter(p -> !Files.isDirectory(p))
+                .map(this::pathToContents)
+                .sorted()
+                .collect(Collectors.toSet());
         }
         return contents;
     }
--- a/test/jdk/jdk/nio/zipfs/jarfs/root/dir1/leaf1.txt	Fri Apr 26 00:57:03 2019 -0400
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,1 +0,0 @@
-This is leaf 1.
--- a/test/jdk/jdk/nio/zipfs/jarfs/root/dir1/leaf2.txt	Fri Apr 26 00:57:03 2019 -0400
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,1 +0,0 @@
-This is leaf 2.
--- a/test/jdk/jdk/nio/zipfs/jarfs/root/dir2/leaf3.txt	Fri Apr 26 00:57:03 2019 -0400
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,1 +0,0 @@
-This is leaf 3.
--- a/test/jdk/jdk/nio/zipfs/jarfs/root/dir2/leaf4.txt	Fri Apr 26 00:57:03 2019 -0400
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,1 +0,0 @@
-This is leaf 4.
--- a/test/jdk/jdk/nio/zipfs/jarfs/v9/root/dir1/leaf1.txt	Fri Apr 26 00:57:03 2019 -0400
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,1 +0,0 @@
-This is versioned leaf 1.
--- a/test/jdk/jdk/nio/zipfs/jarfs/v9/root/dir1/leaf2.txt	Fri Apr 26 00:57:03 2019 -0400
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,1 +0,0 @@
-This is versioned leaf 2.
--- a/test/jdk/jdk/nio/zipfs/jarfs/v9/root/dir2/leaf3.txt	Fri Apr 26 00:57:03 2019 -0400
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,1 +0,0 @@
-This is versioned leaf 3.
--- a/test/jdk/jdk/nio/zipfs/jarfs/v9/root/dir2/leaf4.txt	Fri Apr 26 00:57:03 2019 -0400
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,1 +0,0 @@
-This is versioned leaf 4.