6372077: JarFile.getManifest() should handle manifest attribute name 70 bytes
authorrriggs
Thu, 01 Mar 2018 15:50:26 -0500
changeset 49111 1b33025ae610
parent 49110 a7af40c779d8
child 49114 e28aba8a82df
6372077: JarFile.getManifest() should handle manifest attribute name 70 bytes Reviewed-by: alanb, sherman Contributed-by: philipp.kunz@paratix.ch
src/java.base/share/classes/java/util/jar/Attributes.java
src/java.base/share/classes/java/util/jar/Manifest.java
test/jdk/java/util/jar/Manifest/LineBreakLineWidth.java
test/jdk/sun/security/tools/jarsigner/LineBrokenMultiByteCharacter.java
--- a/src/java.base/share/classes/java/util/jar/Attributes.java	Thu Mar 01 12:31:24 2018 -0800
+++ b/src/java.base/share/classes/java/util/jar/Attributes.java	Thu Mar 01 15:50:26 2018 -0500
@@ -40,7 +40,9 @@
  * The Attributes class maps Manifest attribute names to associated string
  * values. Valid attribute names are case-insensitive, are restricted to
  * the ASCII characters in the set [0-9a-zA-Z_-], and cannot exceed 70
- * characters in length. Attribute values can contain any characters and
+ * characters in length. There must be a colon and a SPACE after the name;
+ * the combined length will not exceed 72 characters.
+ * Attribute values can contain any characters and
  * will be UTF8-encoded when written to the output stream.  See the
  * <a href="{@docRoot}/../specs/jar/jar.html">JAR File Specification</a>
  * for more information about valid attribute names and values.
@@ -310,8 +312,8 @@
              }
              buffer.append(value);
 
+             Manifest.make72Safe(buffer);
              buffer.append("\r\n");
-             Manifest.make72Safe(buffer);
              os.writeBytes(buffer.toString());
          }
         os.writeBytes("\r\n");
@@ -355,8 +357,8 @@
                 }
                 buffer.append(value);
 
+                Manifest.make72Safe(buffer);
                 buffer.append("\r\n");
-                Manifest.make72Safe(buffer);
                 out.writeBytes(buffer.toString());
             }
         }
--- a/src/java.base/share/classes/java/util/jar/Manifest.java	Thu Mar 01 12:31:24 2018 -0800
+++ b/src/java.base/share/classes/java/util/jar/Manifest.java	Thu Mar 01 15:50:26 2018 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2018, 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
@@ -157,8 +157,8 @@
                 value = new String(vb, 0, 0, vb.length);
             }
             buffer.append(value);
+            make72Safe(buffer);
             buffer.append("\r\n");
-            make72Safe(buffer);
             dos.writeBytes(buffer.toString());
             e.getValue().write(dos);
         }
@@ -170,13 +170,11 @@
      */
     static void make72Safe(StringBuffer line) {
         int length = line.length();
-        if (length > 72) {
-            int index = 70;
-            while (index < length - 2) {
-                line.insert(index, "\r\n ");
-                index += 72;
-                length += 3;
-            }
+        int index = 72;
+        while (index < length) {
+            line.insert(index, "\r\n ");
+            index += 74; // + line width + line break ("\r\n")
+            length += 3; // + line break ("\r\n") and space
         }
         return;
     }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/util/jar/Manifest/LineBreakLineWidth.java	Thu Mar 01 15:50:26 2018 -0500
@@ -0,0 +1,284 @@
+/*
+ * Copyright (c) 2018, 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 static java.nio.charset.StandardCharsets.UTF_8;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.util.jar.Manifest;
+import java.util.jar.Attributes;
+import java.util.jar.Attributes.Name;
+
+import org.testng.annotations.Test;
+import static org.testng.Assert.*;
+
+/**
+ * @test
+ * @bug 6372077
+ * @run testng LineBreakLineWidth
+ * @summary write valid manifests with respect to line breaks
+ *          and read any line width
+ */
+public class LineBreakLineWidth {
+
+    /**
+     * maximum header name length from {@link Name#isValid(String)}
+     * not including the name-value delimiter <code>": "</code>
+     */
+    final static int MAX_HEADER_NAME_LENGTH = 70;
+
+    /**
+     * range of one..{@link #TEST_WIDTH_RANGE} covered in this test that
+     * exceeds the range of allowed header name lengths or line widths
+     * in order to also cover invalid cases beyond the valid boundaries
+     * and to keep it somewhat independent from the actual manifest width.
+     * <p>
+     * bigger than 72 (maximum manifest header line with in bytes (not utf-8
+     * encoded characters) but otherwise arbitrarily chosen
+     */
+    final static int TEST_WIDTH_RANGE = 123;
+
+    /**
+     * tests if only valid manifest files can written depending on the header
+     * name length or that an exception occurs already on the attempt to write
+     * an invalid one otherwise and that no invalid manifest can be written.
+     * <p>
+     * due to bug JDK-6372077 it was possible to write a manifest that could
+     * not be read again. independent of the actual manifest line width, such
+     * a situation should never happen, which is the subject of this test.
+     */
+    @Test
+    public void testWriteValidManifestOrException() throws IOException {
+        /*
+         * multi-byte utf-8 characters cannot occur in header names,
+         * only in values which are not subject of this test here.
+         * hence, each character in a header name uses exactly one byte and
+         * variable length utf-8 character encoding doesn't affect this test.
+         */
+
+        String name = "";
+        for (int l = 1; l <= TEST_WIDTH_RANGE; l++) {
+            name += "x";
+            System.out.println("name = " + name + ", "
+                    + "name.length = " + name.length());
+
+            if (l <= MAX_HEADER_NAME_LENGTH) {
+                writeValidManifest(name, "somevalue");
+            } else {
+                writeInvalidManifestThrowsException(name, "somevalue");
+            }
+        }
+    }
+
+    static void writeValidManifest(String name, String value)
+            throws IOException {
+        byte[] mfBytes = writeManifest(name, value);
+        Manifest mf = new Manifest(new ByteArrayInputStream(mfBytes));
+        assertMainAndSectionValues(mf, name, value);
+    }
+
+    static void writeInvalidManifestThrowsException(String name, String value)
+            throws IOException {
+        try {
+            writeManifest(name, value);
+        } catch (IllegalArgumentException e) {
+            // no invalid manifest was produced which is considered acceptable
+            return;
+        }
+
+        fail("no error writing manifest considered invalid");
+    }
+
+    /**
+     * tests that manifest files can be read even if the line breaks are
+     * placed in different positions than where the current JDK's
+     * {@link Manifest#write(java.io.OutputStream)} would have put it provided
+     * the manifest is valid otherwise.
+     * <p>
+     * the <a href="{@docRoot}/../specs/jar/jar.html#Notes_on_Manifest_and_Signature_Files">
+     * "Notes on Manifest and Signature Files" in the "JAR File
+     * Specification"</a> state that "no line may be longer than 72 bytes
+     * (not characters), in its utf8-encoded form." but allows for earlier or
+     * additional line breaks.
+     * <p>
+     * the most important purpose of this test case is probably to make sure
+     * that manifest files broken at 70 bytes line width written with the
+     * previous version of {@link Manifest} before this fix still work well.
+     */
+    @Test
+    public void testReadDifferentLineWidths() throws IOException {
+        /*
+         * uses only one-byte utf-8 encoded characters as values.
+         * correctly breaking multi-byte utf-8 encoded characters
+         * would be subject of another test if there was one such.
+         */
+
+        // w: line width
+        // 6 minimum required for section names starting with "Name: "
+        for (int w = 6; w <= TEST_WIDTH_RANGE; w++) {
+
+            // ln: header name length
+            String name = "";
+            // - 2 due to the delimiter ": " that has to fit on the same
+            // line as the name
+            for (int ln = 1; ln <= w - 2; ln++) {
+                name += "x";
+
+                // lv: value length
+                String value = "";
+                for (int lv = 1; lv <= TEST_WIDTH_RANGE; lv++) {
+                    value += "y";
+                }
+
+                System.out.println("lineWidth = " + w);
+                System.out.println("name = " + name + ""
+                        + ", name.length = " + name.length());
+                System.out.println("value = " + value + ""
+                        + ", value.length = " + value.length());
+
+                readSpecificLineWidthManifest(name, value, w);
+            }
+        }
+    }
+
+    static void readSpecificLineWidthManifest(String name, String value,
+            int lineWidth) throws IOException {
+        /*
+         * breaking header names is not allowed and hence cannot be reasonably
+         * tested. it cannot easily be helped, that invalid manifest files
+         * written by the previous Manifest version implementation are illegal
+         * if the header name is 69 or 70 bytes and in that case the name/value
+         * delimiter ": " was broken on a new line.
+         *
+         * changing the line width in Manifest#make72Safe(StringBuffer),
+         * however, also affects at which positions values are broken across
+         * lines (should always have affected values only and never header
+         * names or the delimiter) which is tested here.
+         *
+         * ideally, any previous Manifest implementation would have been used
+         * here to provide manifest files to test reading but these are no
+         * longer available in this version's sources and there might as well
+         * be other libraries writing manifests. Therefore, in order to be able
+         * to test any manifest file considered valid with respect to line
+         * breaks that could not possibly be produced with the current Manifest
+         * implementation, this test provides its own manifests in serialized
+         * form.
+         */
+        String lineBrokenSectionName = breakLines(lineWidth, "Name: " + name);
+        String lineBrokenNameAndValue = breakLines(lineWidth, name + ": " + value);
+
+        ByteArrayOutputStream mfBuf = new ByteArrayOutputStream();
+        mfBuf.write("Manifest-Version: 1.0".getBytes(UTF_8));
+        mfBuf.write("\r\n".getBytes(UTF_8));
+        mfBuf.write(lineBrokenNameAndValue.getBytes(UTF_8));
+        mfBuf.write("\r\n".getBytes(UTF_8));
+        mfBuf.write("\r\n".getBytes(UTF_8));
+        mfBuf.write(lineBrokenSectionName.getBytes(UTF_8));
+        mfBuf.write("\r\n".getBytes(UTF_8));
+        mfBuf.write(lineBrokenNameAndValue.getBytes(UTF_8));
+        mfBuf.write("\r\n".getBytes(UTF_8));
+        mfBuf.write("\r\n".getBytes(UTF_8));
+        byte[] mfBytes = mfBuf.toByteArray();
+        printManifest(mfBytes);
+
+        boolean nameValid = name.length() <= MAX_HEADER_NAME_LENGTH;
+
+        Manifest mf;
+        try {
+            mf = new Manifest(new ByteArrayInputStream(mfBytes));
+        } catch (IOException e) {
+            if (!nameValid &&
+                    e.getMessage().startsWith("invalid header field")) {
+                // expected because the name is not valid
+                return;
+            }
+
+            throw new AssertionError(e.getMessage(), e);
+        }
+
+        assertTrue(nameValid, "failed to detect invalid manifest");
+
+        assertMainAndSectionValues(mf, name, value);
+    }
+
+    static String breakLines(int lineWidth, String nameAndValue) {
+        String lineBrokenNameAndValue = "";
+        int charsOnLastLine = 0;
+        for (int i = 0; i < nameAndValue.length(); i++) {
+            lineBrokenNameAndValue += nameAndValue.substring(i, i + 1);
+            charsOnLastLine++;
+            if (0 < i && i < nameAndValue.length() - 1
+                    && charsOnLastLine == lineWidth) {
+                lineBrokenNameAndValue += "\r\n ";
+                charsOnLastLine = 1;
+            }
+        }
+        return lineBrokenNameAndValue;
+    }
+
+    static byte[] writeManifest(String name, String value) throws IOException {
+        /*
+         * writing manifest main headers is implemented separately from
+         * writing named sections manifest headers:
+         * - java.util.jar.Attributes.writeMain(DataOutputStream)
+         * - java.util.jar.Attributes.write(DataOutputStream)
+         * which is why this is also covered separately in this test by
+         * always adding the same value twice, in the main attributes as
+         * well as in a named section (using the header name also as the
+         * section name).
+         */
+
+        Manifest mf = new Manifest();
+        mf.getMainAttributes().put(Name.MANIFEST_VERSION, "1.0");
+        mf.getMainAttributes().putValue(name, value);
+
+        Attributes section = new Attributes();
+        section.putValue(name, value);
+        mf.getEntries().put(name, section);
+
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        mf.write(out);
+        byte[] mfBytes = out.toByteArray();
+        printManifest(mfBytes);
+        return mfBytes;
+    }
+
+    private static void printManifest(byte[] mfBytes) {
+        final String sepLine = "----------------------------------------------"
+                + "---------------------|-|-|"; // |-positions: ---68-70-72
+        System.out.println(sepLine);
+        System.out.print(new String(mfBytes, UTF_8));
+        System.out.println(sepLine);
+    }
+
+    private static void assertMainAndSectionValues(Manifest mf, String name,
+            String value) {
+        String mainValue = mf.getMainAttributes().getValue(name);
+        String sectionValue = mf.getAttributes(name).getValue(name);
+
+        assertEquals(value, mainValue, "value different in main section");
+        assertEquals(value, sectionValue, "value different in named section");
+    }
+
+}
--- a/test/jdk/sun/security/tools/jarsigner/LineBrokenMultiByteCharacter.java	Thu Mar 01 12:31:24 2018 -0800
+++ b/test/jdk/sun/security/tools/jarsigner/LineBrokenMultiByteCharacter.java	Thu Mar 01 15:50:26 2018 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2018, 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,7 +58,7 @@
      * @see #verifyClassNameLineBroken(JarFile, String)
      */
     static final String testClassName =
-            "LineBrokenMultiByteCharacterA1234567890B1234567890C123456789D12\u00E9xyz.class";
+            "LineBrokenMultiByteCharacterA1234567890B1234567890C123456789D1234\u00E9xyz.class";
 
     static final String anotherName =
             "LineBrokenMultiByteCharacterA1234567890B1234567890C123456789D1234567890.class";