8222440: (zipfs) JarFileSystem does not correctly handle versioned entries if no root entry is present
Reviewed-by: lancea
--- 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.