8216362: Better error message handling when there is an invalid Manifest

Reviewed-by: lancea, rriggs, mullan

src/java.base/share/classes/java/util/jar/Manifest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 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
@@ -58,14 +58,10 @@ public class Manifest implements Cloneable {
     // associated JarVerifier, not null when called by JarFile::getManifest.
     private final JarVerifier jv;
 
-    // name of the corresponding jar archive if available.
-    private final String jarFilename;
-
     /**
      * Constructs a new, empty Manifest.
      */
     public Manifest() {
-        jarFilename = null;
         jv = null;
     }
 
@@ -84,7 +80,7 @@ public class Manifest implements Cloneable {
      *
      * @param is the input stream containing manifest data
      * @param jarFilename the name of the corresponding jar archive if available
-     * @throws IOException if an I/O error has occured
+     * @throws IOException if an I/O error has occurred
      */
     Manifest(InputStream is, String jarFilename) throws IOException {
         this(null, is, jarFilename);
@@ -93,10 +89,14 @@ public class Manifest implements Cloneable {
     /**
      * Constructs a new Manifest from the specified input stream
      * and associates it with a JarVerifier.
+     *
+     * @param jv the JarVerifier to use
+     * @param is the input stream containing manifest data
+     * @param jarFilename the name of the corresponding jar archive if available
+     * @throws IOException if an I/O error has occurred
      */
     Manifest(JarVerifier jv, InputStream is, String jarFilename) throws IOException {
-        read(is);
-        this.jarFilename = jarFilename;
+        read(is, jarFilename);
         this.jv = jv;
     }
 
@@ -108,7 +108,6 @@ public class Manifest implements Cloneable {
     public Manifest(Manifest man) {
         attr.putAll(man.getMainAttributes());
         entries.putAll(man.getEntries());
-        jarFilename = null;
         jv = man.jv;
     }
 
@@ -250,6 +249,10 @@ public class Manifest implements Cloneable {
      * @exception IOException if an I/O error has occurred
      */
     public void read(InputStream is) throws IOException {
+        read(is, null);
+    }
+
+    private void read(InputStream is, String jarFilename) throws IOException {
         // Buffered input stream for reading manifest data
         FastInputStream fis = new FastInputStream(is);
         // Line buffer
@@ -285,7 +288,7 @@ public class Manifest implements Cloneable {
             if (name == null) {
                 name = parseName(lbuf, len);
                 if (name == null) {
-                    throw new IOException("invalid manifest format"
+                    throw new IOException("invalid manifest format ("
                               + getErrorPosition(jarFilename, lineNumber) + ")");
                 }
                 if (fis.peek() == ' ') {

test/jdk/java/util/jar/Manifest/IncludeInExceptionsTest.java

@@ -0,0 +1,101 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.io.File;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
+import java.util.concurrent.Callable;
+import java.util.jar.JarEntry;
+import java.util.jar.JarFile;
+import java.util.jar.JarOutputStream;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * @test
+ * @bug 8216362
+ * @run main/othervm -Djdk.includeInExceptions=jar IncludeInExceptionsTest yes
+ * @run main/othervm IncludeInExceptionsTest
+ * @summary Verify that the property jdk.net.includeInExceptions works as expected
+ * when an error occurs while reading an invalid Manifest file.
+ */
+/*
+ * @see Manifest#Manifest(JarVerifier,InputStream,String)
+ * @see Manifest#getErrorPosition
+ */
+public class IncludeInExceptionsTest {
+
+    static final String FILENAME = "Unique-Filename-Expected-In_Msg.jar";
+
+    static final byte[] INVALID_MANIFEST = (
+            "Manifest-Version: 1.0\r\n" +
+            "\r\n" +
+            "Illegal\r\n" +
+            "\r\n").getBytes(UTF_8);
+
+    static String createJarInvalidManifest(String jar) throws IOException {
+        try (OutputStream out = Files.newOutputStream(Paths.get(jar));
+            JarOutputStream jos = new JarOutputStream(out)) {
+            JarEntry je = new JarEntry(JarFile.MANIFEST_NAME);
+            jos.putNextEntry(je);
+            jos.write(INVALID_MANIFEST);
+            jos.closeEntry();
+        }
+        return jar;
+    }
+
+    static void test(Callable<?> attempt, boolean includeInExceptions) throws Exception {
+        try {
+            attempt.call();
+            throw new AssertionError("Excpected Exception not thrown");
+        } catch (IOException e) {
+            boolean foundFileName = e.getMessage().contains(FILENAME);
+            if(includeInExceptions && !foundFileName) {
+                throw new AssertionError("JAR file name expected but not found in error message");
+            } else if (foundFileName && !includeInExceptions) {
+                throw new AssertionError("JAR file name found but should not be in error message");
+            }
+        }
+    }
+
+    public static void main(String[] args) throws Exception {
+
+        boolean includeInExceptions;
+        if(args.length > 0) {
+            includeInExceptions = true;
+            System.out.println("**** Running test WITH -Djdk.includeInExceptions=jar");
+        } else {
+            includeInExceptions = false;
+            System.out.println("**** Running test WITHOUT -Djdk.includeInExceptions=jar");
+        }
+
+        test(() -> new JarFile(createJarInvalidManifest(FILENAME)).getManifest(),
+                includeInExceptions);
+        test(() -> new JarFile(createJarInvalidManifest("Verifying-" + FILENAME),
+                true).getManifest(), includeInExceptions);
+    }
+
+}
+