8165723: JarFile::isMultiRelease() method returns false when it should return true
authorredestad
Mon, 12 Sep 2016 17:45:55 +0200
changeset 40812 dc3755b63ade
parent 40811 1fb2b94fa1d0
child 40813 dd5aa9c67561
8165723: JarFile::isMultiRelease() method returns false when it should return true Reviewed-by: alanb
jdk/src/java.base/share/classes/java/util/jar/JarFile.java
jdk/test/java/util/jar/JarFile/mrjar/MultiReleaseJarAPI.java
jdk/test/lib/testlibrary/java/util/jar/CreateMultiReleaseTestJars.java
--- a/jdk/src/java.base/share/classes/java/util/jar/JarFile.java	Mon Sep 12 18:27:33 2016 +0530
+++ b/jdk/src/java.base/share/classes/java/util/jar/JarFile.java	Mon Sep 12 17:45:55 2016 +0200
@@ -836,18 +836,25 @@
     private static final byte[] CLASSPATH_CHARS =
             {'C','L','A','S','S','-','P','A','T','H', ':', ' '};
 
-    // The bad character shift for "class-path:"
+    // The bad character shift for "class-path: "
     private static final byte[] CLASSPATH_LASTOCC;
 
+    // The good suffix shift for "class-path: "
+    private static final byte[] CLASSPATH_OPTOSFT;
+
     private static final byte[] MULTIRELEASE_CHARS =
             {'M','U','L','T','I','-','R','E','L','E', 'A', 'S', 'E', ':',
                     ' ', 'T', 'R', 'U', 'E'};
 
-    // The bad character shift for "multi-release: "
+    // The bad character shift for "multi-release: true"
     private static final byte[] MULTIRELEASE_LASTOCC;
 
+    // The good suffix shift for "multi-release: true"
+    private static final byte[] MULTIRELEASE_OPTOSFT;
+
     static {
         CLASSPATH_LASTOCC = new byte[64];
+        CLASSPATH_OPTOSFT = new byte[12];
         CLASSPATH_LASTOCC[(int)'C' - 32] = 1;
         CLASSPATH_LASTOCC[(int)'L' - 32] = 2;
         CLASSPATH_LASTOCC[(int)'S' - 32] = 5;
@@ -858,8 +865,13 @@
         CLASSPATH_LASTOCC[(int)'H' - 32] = 10;
         CLASSPATH_LASTOCC[(int)':' - 32] = 11;
         CLASSPATH_LASTOCC[(int)' ' - 32] = 12;
+        for (int i = 0; i < 11; i++) {
+            CLASSPATH_OPTOSFT[i] = 12;
+        }
+        CLASSPATH_OPTOSFT[11] = 1;
 
         MULTIRELEASE_LASTOCC = new byte[64];
+        MULTIRELEASE_OPTOSFT = new byte[19];
         MULTIRELEASE_LASTOCC[(int)'M' - 32] = 1;
         MULTIRELEASE_LASTOCC[(int)'I' - 32] = 5;
         MULTIRELEASE_LASTOCC[(int)'-' - 32] = 6;
@@ -872,6 +884,11 @@
         MULTIRELEASE_LASTOCC[(int)'R' - 32] = 17;
         MULTIRELEASE_LASTOCC[(int)'U' - 32] = 18;
         MULTIRELEASE_LASTOCC[(int)'E' - 32] = 19;
+        for (int i = 0; i < 17; i++) {
+            MULTIRELEASE_OPTOSFT[i] = 19;
+        }
+        MULTIRELEASE_OPTOSFT[17] = 6;
+        MULTIRELEASE_OPTOSFT[18] = 1;
     }
 
     private JarEntry getManEntry() {
@@ -913,7 +930,7 @@
      * Since there are no repeated substring in our search strings,
      * the good suffix shifts can be replaced with a comparison.
      */
-    private int match(byte[] src, byte[] b, byte[] lastOcc) {
+    private int match(byte[] src, byte[] b, byte[] lastOcc, byte[] optoSft) {
         int len = src.length;
         int last = b.length - len;
         int i = 0;
@@ -926,9 +943,8 @@
 
                     if (c != src[j]) {
                         // no match
-                        int goodShift = (j < len - 1) ? len : 1;
                         int badShift = lastOcc[c - 32];
-                        i += Math.max(j + 1 - badShift, goodShift);
+                        i += Math.max(j + 1 - badShift, optoSft[j]);
                         continue next;
                     }
                 } else {
@@ -958,10 +974,11 @@
             if (manEntry != null) {
                 byte[] b = getBytes(manEntry);
                 hasClassPathAttribute = match(CLASSPATH_CHARS, b,
-                        CLASSPATH_LASTOCC) != -1;
+                        CLASSPATH_LASTOCC, CLASSPATH_OPTOSFT) != -1;
                 // is this a multi-release jar file
                 if (MULTI_RELEASE_ENABLED) {
-                    int i = match(MULTIRELEASE_CHARS, b, MULTIRELEASE_LASTOCC);
+                    int i = match(MULTIRELEASE_CHARS, b, MULTIRELEASE_LASTOCC,
+                            MULTIRELEASE_OPTOSFT);
                     if (i != -1) {
                         i += MULTIRELEASE_CHARS.length;
                         if (i < b.length) {
--- a/jdk/test/java/util/jar/JarFile/mrjar/MultiReleaseJarAPI.java	Mon Sep 12 18:27:33 2016 +0530
+++ b/jdk/test/java/util/jar/JarFile/mrjar/MultiReleaseJarAPI.java	Mon Sep 12 17:45:55 2016 +0200
@@ -23,23 +23,29 @@
 
 /*
  * @test
- * @bug 8132734 8144062
+ * @bug 8132734 8144062 8165723
  * @summary Test the extended API and the aliasing additions in JarFile that
  *          support multi-release jar files
- * @library /lib/testlibrary/java/util/jar
+ * @library /lib/testlibrary/java/util/jar /lib/testlibrary/
  * @build Compiler JarBuilder CreateMultiReleaseTestJars
+ * @build jdk.testlibrary.RandomFactory
  * @run testng MultiReleaseJarAPI
  */
 
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.util.Arrays;
+import java.util.Map;
+import java.util.Random;
 import java.util.jar.JarFile;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipFile;
 
+import jdk.testlibrary.RandomFactory;
+
 import org.testng.Assert;
 import org.testng.annotations.AfterClass;
 import org.testng.annotations.BeforeClass;
@@ -48,12 +54,15 @@
 
 public class MultiReleaseJarAPI {
 
+    private static final Random RANDOM = RandomFactory.getRandom();
+
     String userdir = System.getProperty("user.dir",".");
     CreateMultiReleaseTestJars creator =  new CreateMultiReleaseTestJars();
     File unversioned = new File(userdir, "unversioned.jar");
     File multirelease = new File(userdir, "multi-release.jar");
     File signedmultirelease = new File(userdir, "signed-multi-release.jar");
 
+
     @BeforeClass
     public void initialize() throws Exception {
         creator.compileEntries();
@@ -99,10 +108,35 @@
         testCustomMultiReleaseValue("true\r ", false);
         testCustomMultiReleaseValue("true\n true", false);
         testCustomMultiReleaseValue("true\r\n true", false);
+
+        // generate "random" Strings to use as extra attributes, and
+        // verify that Multi-Release: true is always properly matched
+        for (int i = 0; i < 100; i++) {
+            byte[] keyBytes = new byte[RANDOM.nextInt(70) + 1];
+            Arrays.fill(keyBytes, (byte)('a' + RANDOM.nextInt(24)));
+            byte[] valueBytes = new byte[RANDOM.nextInt(70) + 1];
+            Arrays.fill(valueBytes, (byte)('a' + RANDOM.nextInt(24)));
+
+            String key = new String(keyBytes, StandardCharsets.UTF_8);
+            String value = new String(valueBytes, StandardCharsets.UTF_8);
+            // test that Multi-Release: true anywhere in the manifest always
+            // return true
+            testCustomMultiReleaseValue("true", Map.of(key, value), true);
+
+            // test that we don't get any false positives
+            testCustomMultiReleaseValue("false", Map.of(key, value), false);
+        }
     }
 
-    private void testCustomMultiReleaseValue(String value, boolean expected) throws Exception {
-        creator.buildCustomMultiReleaseJar("custom-mr.jar", value);
+    private void testCustomMultiReleaseValue(String value, boolean expected)
+            throws Exception {
+        testCustomMultiReleaseValue(value, Map.of(), expected);
+    }
+
+    private void testCustomMultiReleaseValue(String value,
+            Map<String, String> extraAttributes, boolean expected)
+            throws Exception {
+        creator.buildCustomMultiReleaseJar("custom-mr.jar", value, extraAttributes);
         File custom = new File(userdir, "custom-mr.jar");
         try (JarFile jf = new JarFile(custom, true, ZipFile.OPEN_READ, Runtime.version())) {
             Assert.assertEquals(jf.isMultiRelease(), expected);
--- a/jdk/test/lib/testlibrary/java/util/jar/CreateMultiReleaseTestJars.java	Mon Sep 12 18:27:33 2016 +0530
+++ b/jdk/test/lib/testlibrary/java/util/jar/CreateMultiReleaseTestJars.java	Mon Sep 12 17:45:55 2016 +0200
@@ -88,12 +88,28 @@
     }
 
     public void buildMultiReleaseJar() throws IOException {
-        buildCustomMultiReleaseJar("multi-release.jar", "true");
+        JarBuilder jb = customMultiReleaseJar("multi-release.jar", "true");
+        addEntries(jb);
+        jb.build();
     }
 
-    public void buildCustomMultiReleaseJar(String filename, String multiReleaseValue) throws IOException {
+    private JarBuilder customMultiReleaseJar(String filename, String multiReleaseValue)
+            throws IOException {
         JarBuilder jb = new JarBuilder(filename);
         jb.addAttribute("Multi-Release", multiReleaseValue);
+        return jb;
+    }
+
+    public void buildCustomMultiReleaseJar(String filename, String multiReleaseValue,
+            Map<String, String> extraAttributes) throws IOException {
+        JarBuilder jb = new JarBuilder(filename);
+        extraAttributes.entrySet()
+                .forEach(entry -> jb.addAttribute(entry.getKey(), entry.getValue()));
+        jb.addAttribute("Multi-Release", multiReleaseValue);
+        jb.build();
+    }
+
+    private void addEntries(JarBuilder jb) {
         jb.addEntry("README", readme8.getBytes());
         jb.addEntry("version/Main.java", main.getBytes());
         jb.addEntry("version/Main.class", rootClasses.get("version.Main"));
@@ -107,26 +123,6 @@
         jb.addEntry("META-INF/versions/10/README", readme10.getBytes());
         jb.addEntry("META-INF/versions/10/version/Version.java", java10.getBytes());
         jb.addEntry("META-INF/versions/10/version/Version.class", version10Classes.get("version.Version"));
-        jb.build();
-    }
-
-    public void buildShortMultiReleaseJar() throws IOException {
-        JarBuilder jb = new JarBuilder("short-multi-release.jar");
-        jb.addAttribute("Multi-Release", "true");
-        jb.addEntry("README", readme8.getBytes());
-        jb.addEntry("version/Main.java", main.getBytes());
-        jb.addEntry("version/Main.class", rootClasses.get("version.Main"));
-        jb.addEntry("version/Version.java", java8.getBytes());
-        jb.addEntry("version/Version.class", rootClasses.get("version.Version"));
-        jb.addEntry("META-INF/versions/9/README", readme9.getBytes());
-        jb.addEntry("META-INF/versions/9/version/Version.java", java9.getBytes());
-        jb.addEntry("META-INF/versions/9/version/PackagePrivate.java", ppjava9.getBytes());
-        // no entry for META-INF/versions/9/version/Version.class
-        jb.addEntry("META-INF/versions/9/version/PackagePrivate.class", version9Classes.get("version.PackagePrivate"));
-        jb.addEntry("META-INF/versions/10/README", readme10.getBytes());
-        jb.addEntry("META-INF/versions/10/version/Version.java", java10.getBytes());
-        jb.addEntry("META-INF/versions/10/version/Version.class", version10Classes.get("version.Version"));
-        jb.build();
     }
 
     public void buildSignedMultiReleaseJar() throws Exception {