+ * Combining diacritical marks may be separated from the associated base + * character or other combining diacritical marks of that base character + * by a continuation line break ("{@code \r\n }") if the whole sequence of + * base character and all the combining diacritical marks belonging to it + * exceed 71 bytes in their binary form encoded with UTF-8. This limit is + * only 71 bytes rather than 72 because continuation lines start with a + * space that uses the first byte of the 72 bytes each line can hold up to + * and the first line provides even less space for the value because it + * starts with the name. */ static void println72(OutputStream out, String line) throws IOException { - if (!line.isEmpty()) { - byte[] lineBytes = line.getBytes(UTF_8.INSTANCE); - int length = lineBytes.length; - // first line can hold one byte more than subsequent lines which - // start with a continuation line break space - out.write(lineBytes[0]); - int pos = 1; - while (length - pos > 71) { - out.write(lineBytes, pos, 71); - pos += 71; + int linePos = 0; // number of bytes already put out on current line + BreakIterator boundary = BreakIterator.getCharacterInstance(); + boundary.setText(line); + int start = boundary.first(), end; + while ((end = boundary.next()) != BreakIterator.DONE) { + String character = line.substring(start, end); + start = end; + byte[] characterBytes = character.getBytes(UTF_8.INSTANCE); + int characterLength = characterBytes.length; + // Put out a break onto a new line if the character does not fit on + // the current line anymore but fits on a whole new line. + // In other words, if the current character does not fit on one + // whole line alone, fill the current line first before breaking + // inside of the character onto a new line. + if (linePos + characterLength > 72 && characterLength < 72) { + println(out); + out.write(' '); + linePos = 1; + } + int characterPos = 0; // number of bytes of current character + // already put out + while (linePos + characterLength - characterPos > 72) { + int nextBreakPos = 72 - linePos; + while (isContinuation( + characterBytes[characterPos + nextBreakPos])) { + nextBreakPos--; + } + out.write(characterBytes, characterPos, nextBreakPos); + characterPos += nextBreakPos; println(out); out.write(' '); + linePos = 1; } - out.write(lineBytes, pos, length - pos); + out.write(characterBytes, + characterPos, characterLength - characterPos); + linePos += characterLength - characterPos; } println(out); } --- old/test/jdk/sun/security/tools/jarsigner/LineBrokenMultiByteCharacter.java 2020-02-10 14:32:14.000000000 -0500 +++ new/test/jdk/sun/security/tools/jarsigner/LineBrokenMultiByteCharacter.java 2020-02-10 14:32:14.000000000 -0500 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 2020, 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 @@ -29,15 +29,25 @@ * @library /test/lib */ +import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStream; import java.nio.file.Files; import java.nio.file.Paths; -import java.util.jar.JarFile; +import java.util.Arrays; +import java.util.stream.Collectors; import java.util.jar.Attributes.Name; +import java.util.jar.Manifest; +import java.util.jar.JarFile; import java.util.jar.JarEntry; +import java.util.zip.ZipFile; +import java.util.zip.ZipEntry; +import static java.util.jar.JarFile.MANIFEST_NAME; import static java.nio.charset.StandardCharsets.UTF_8; +import static java.nio.file.StandardCopyOption.COPY_ATTRIBUTES; +import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; import jdk.test.lib.SecurityTools; import jdk.test.lib.util.JarUtils; @@ -46,7 +56,7 @@ /** * this name will break across lines in MANIFEST.MF at the - * middle of a two-byte utf-8 encoded character due to its e acute letter + * middle of a two-byte UTF-8 encoded character due to its e acute letter * at its exact position. * * because no file with such a name exists {@link JarUtils} will add the @@ -63,53 +73,58 @@ static final String anotherName = "LineBrokenMultiByteCharacterA1234567890B1234567890C123456789D1234567890.class"; - static final String alias = "a"; - static final String keystoreFileName = "test.jks"; - static final String manifestFileName = "MANIFEST.MF"; + static final String ALIAS = "A"; + static final String KEYSTORE_FILENAME = "test.jks"; + static final String SOME_OTHER_SIG_FILE = "META-INF/FAKE_SIG.DSA"; public static void main(String[] args) throws Exception { prepare(); testSignJar("test.jar"); - testSignJarNoManifest("test-no-manifest.jar"); testSignJarUpdate("test-update.jar", "test-updated.jar"); } static void prepare() throws Exception { - SecurityTools.keytool("-keystore", keystoreFileName, "-genkeypair", + SecurityTools.keytool("-keystore", KEYSTORE_FILENAME, "-genkeypair", "-keyalg", "dsa", "-storepass", "changeit", "-keypass", "changeit", "-storetype", - "JKS", "-alias", alias, "-dname", "CN=X", "-validity", "366") + "JKS", "-alias", ALIAS, "-dname", "CN=X", "-validity", "366") .shouldHaveExitValue(0); - Files.write(Paths.get(manifestFileName), (Name. + new File(MANIFEST_NAME).getParentFile().mkdirs(); + Files.write(Paths.get(MANIFEST_NAME), (Name. MANIFEST_VERSION.toString() + ": 1.0\r\n").getBytes(UTF_8)); - } - static void testSignJar(String jarFileName) throws Exception { - JarUtils.createJar(jarFileName, manifestFileName, testClassName); - verifyJarSignature(jarFileName); + // prevent jarsigner from assuming it was safe to rewrite the manifest + // and its line breaks assuming there were no other signatures present + Files.write(Paths.get(SOME_OTHER_SIG_FILE), new byte[] {}); } - static void testSignJarNoManifest(String jarFileName) throws Exception { - JarUtils.createJar(jarFileName, testClassName); + static void testSignJar(String jarFileName) throws Exception { + JarUtils.createJar(jarFileName, testClassName, SOME_OTHER_SIG_FILE); + createManifestEntries(jarFileName); + rebreakManifest72bytes(jarFileName); verifyJarSignature(jarFileName); } static void testSignJarUpdate( String initialFileName, String updatedFileName) throws Exception { - JarUtils.createJar(initialFileName, manifestFileName, anotherName); - SecurityTools.jarsigner("-keystore", keystoreFileName, "-storetype", + JarUtils.createJar(initialFileName, testClassName, anotherName, + SOME_OTHER_SIG_FILE); + createManifestEntries(initialFileName); + rebreakManifest72bytes(initialFileName); + removeJarEntry(initialFileName, testClassName); + SecurityTools.jarsigner("-keystore", KEYSTORE_FILENAME, "-storetype", "JKS", "-storepass", "changeit", "-debug", initialFileName, - alias).shouldHaveExitValue(0); + ALIAS).shouldHaveExitValue(0); JarUtils.updateJar(initialFileName, updatedFileName, testClassName); verifyJarSignature(updatedFileName); } static void verifyJarSignature(String jarFileName) throws Exception { // actually sign the jar - SecurityTools.jarsigner("-keystore", keystoreFileName, "-storetype", - "JKS", "-storepass", "changeit", "-debug", jarFileName, alias) + SecurityTools.jarsigner("-keystore", KEYSTORE_FILENAME, "-storetype", + "JKS", "-storepass", "changeit", "-debug", jarFileName, ALIAS) .shouldHaveExitValue(0); try ( @@ -130,7 +145,7 @@ * the signature file does not even contain the desired entry at all. * * this relies on {@link java.util.jar.Manifest} breaking lines unaware - * of bytes that belong to the same multi-byte utf characters. + * of bytes that belong to the same multi-byte UTF-8 encoded characters. */ static void verifyClassNameLineBroken(JarFile jar, String className) throws IOException { @@ -142,7 +157,7 @@ throw new AssertionError(className + " not found in manifest"); } - JarEntry manifestEntry = jar.getJarEntry(JarFile.MANIFEST_NAME); + JarEntry manifestEntry = jar.getJarEntry(MANIFEST_NAME); try ( InputStream manifestIs = jar.getInputStream(manifestEntry); ) { @@ -159,7 +174,7 @@ } if (bytesMatched < eAcuteBroken.length) { throw new AssertionError("self-test failed: multi-byte " - + "utf-8 character not broken across lines"); + + "UTF-8 encoded character not broken across lines"); } } } @@ -183,4 +198,108 @@ } } + static void createManifestEntries(String jarFileName) throws Exception { + JarUtils.updateJarFile(Paths.get(jarFileName), + Paths.get("."), Paths.get(MANIFEST_NAME)); + SecurityTools.jarsigner("-keystore", KEYSTORE_FILENAME, + "-storepass", "changeit", "-debug", jarFileName, ALIAS) + .shouldHaveExitValue(0); + // remove the signature files, only manifest is used + removeJarEntry(jarFileName, + "META-INF/" + ALIAS + ".SF", + "META-INF/" + ALIAS + ".DSA"); + } + + @SuppressWarnings("deprecation") + static void removeJarEntry(String jarFileName, String... entryNames) + throws IOException { + String aCopy = "swap-" + jarFileName; + JarUtils.updateJar(jarFileName, aCopy, Arrays.asList(entryNames) + .stream().collect(Collectors.toMap(e -> e, e -> false))); + Files.copy(Paths.get(aCopy), Paths.get(jarFileName), + COPY_ATTRIBUTES, REPLACE_EXISTING); + Files.delete(Paths.get(aCopy)); + } + + static void rebreakManifest72bytes(String jarFileName) throws Exception { + byte[] manifest; + try (ZipFile zip = new ZipFile(jarFileName)) { + ZipEntry zipEntry = zip.getEntry(MANIFEST_NAME); + manifest = zip.getInputStream(zipEntry).readAllBytes(); + } + Utils.echoManifest(manifest, MANIFEST_NAME + " before re-break:"); + byte[] manifest72 = rebreak72bytes(manifest); + Utils.echoManifest(manifest72, MANIFEST_NAME + " after re-break:"); + String aCopy = "swap-" + jarFileName; + JarUtils.updateManifest(jarFileName, aCopy, new Manifest() { @Override + public void write(OutputStream out) throws IOException { + out.write(manifest72); + } + }); + Files.copy(Paths.get(aCopy), Paths.get(jarFileName), + COPY_ATTRIBUTES, REPLACE_EXISTING); + Files.delete(Paths.get(aCopy)); + } + + /** + * Simulates a jar manifest as it would have been created by an earlier + * JDK by re-arranging the line break at exactly 72 bytes content thereby + * breaking the multi-byte UTF-8 encoded character under test like before + * resolution of bug 6202130. + *
+ * The returned manifest is accepted as unmodified by + * {@link jdk.security.jarsigner.JarSigner#updateDigests + * (ZipEntry,ZipFile,MessageDigest[],Manifest)} on line 985: + *
+ * if (!mfDigest.equalsIgnoreCase(base64Digests[i])) {
+ *
+ * and therefore left unchanged when the jar is signed and also signature
+ * verification will check it.
+ */
+ static byte[] rebreak72bytes(byte[] mf0) {
+ byte[] mf1 = new byte[mf0.length];
+ int c0 = 0, c1 = 0; // bytes since last line start
+ for (int i0 = 0, i1 = 0; i0 < mf0.length; i0++, i1++) {
+ switch (mf0[i0]) {
+ case '\r':
+ if (i0 + 2 < mf0.length &&
+ mf0[i0 + 1] == '\n' && mf0[i0 + 2] == ' ') {
+ // skip line break
+ i0 += 2;
+ i1 -= 1;
+ } else {
+ mf1[i1] = mf0[i0];
+ c0 = c1 = 0;
+ }
+ break;
+ case '\n':
+ if (i0 + 1 < mf0.length && mf0[i0 + 1] == ' ') {
+ // skip line break
+ i0 += 1;
+ i1 -= 1;
+ } else {
+ mf1[i1] = mf0[i0];
+ c0 = c1 = 0;
+ }
+ break;
+ case ' ':
+ if (c0 == 0) {
+ continue;
+ }
+ default:
+ c0++;
+ if (c1 == 72) {
+ mf1[i1++] = '\r';
+ mf1[i1++] = '\n';
+ mf1[i1++] = ' ';
+ c1 = 1;
+ } else {
+ c1++;
+ }
+ mf1[i1] = mf0[i0];
+ }
+ }
+ return mf1;
+ }
+
}
--- /dev/null 2020-02-10 14:32:15.000000000 -0500
+++ new/test/jdk/java/util/jar/Manifest/LineBreakCharacter.java 2020-02-10 14:32:15.000000000 -0500
@@ -0,0 +1,495 @@
+/*
+ * 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 static java.nio.charset.StandardCharsets.UTF_8;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataOutputStream;
+import java.io.IOException;
+import java.util.jar.Attributes;
+import java.util.jar.Manifest;
+import java.util.jar.Attributes.Name;
+import java.util.List;
+import java.util.LinkedList;
+
+import org.testng.annotations.Test;
+import org.testng.annotations.DataProvider;
+import static org.testng.Assert.*;
+
+/**
+ * @test
+ * @bug 6443578 6202130
+ * @run testng LineBreakCharacter
+ * @summary Tests breaking manifest header values across lines in conjunction
+ * with Unicode characters encoded in UTF-8 with a variable number of bytes
+ * when reading and writing jar manifests results in valid UTF-8.
+ * + * The manifest line length limit (72 bytes) may be reached at a position + * between multiple bytes of a single UTF-8 encoded character. Although + * characters should not be broken across lines according to the specification + * the previous Manifest implementation did. + *
+ * This test makes sure that no character is broken apart across a line break + * when writing manifests and also that manifests are still read correctly + * whether or not characters encoded in UTF-8 with more than one byte are + * interrupted with and continued after a line break for compatibility when + * reading older manifests. + */ +public class LineBreakCharacter { + + static final int MANIFEST_LINE_CONTENT_WIDTH_BYTES = 72; + + /** + * Character string that has one byte size in its UTF-8 encoded form to + * yield one byte of position offset. + */ + static final String FILL1BYTE = "x"; + static final String MARK_BEFORE = "y"; + static final String MARK_AFTER = "z"; + + /** + * Four byte name. + * By using header names of four characters length the same values can be + * used for testing line breaks in both headers (in main attributes as well + * as named sections) as well as section names because a named section name + * is represented basically like any other header but follows an empty line + * and the key is always "Name". + * Relative to the start of the value, this way the same offset to the + * character to test breaking can be used in all cases. + */ + static final String FOUR_BYTE_NAME = "Name"; + + /** + * Distinguishes main attributes headers, section names, and headers in + * named sections because an implementation might make a difference. + */ + enum PositionInManifest { + /** + * @see Attributes#writeMain + */ + MAIN_ATTRIBUTES, + /** + * @see Attributes#write + */ + SECTION_NAME, + /** + * @see Manifest#write + */ + NAMED_SECTION; + } + + static String numByteUnicodeCharacter(int numBytes) { + String string; + switch (numBytes) { + case 1: string = "i"; break; + case 2: string = "\u00EF"; break; // small letter i with diaresis + case 3: string = "\uFB00"; break; // small double f ligature + case 4: string = Character.toString(0x2070E); break; // ? + default: throw new RuntimeException(); + } + assertEquals(string.getBytes(UTF_8).length, numBytes, + "self-test failed: unexpected UTF-8 encoded character length"); + return string; + } + + /** + * Produces test cases with all combinations of circumstances covered in + * which a character could possibly be attempted to be broken across a line + * break onto a continuation line:
+ * For each of the cases provided by {@link #lineBreakParameters()} the + * break position is verified in the written manifest binary form as well + * as verified that it restores to the original values when read again. + *
+ * As an additional check, the binary manifests are decoded from UTF-8
+ * into Strings before re-joining continued lines.
+ */
+ @Test(dataProvider = "lineBreakParameters")
+ public void testWriteLineBreaksKeepCharactersTogether(int b, int c, int p,
+ int a, PositionInManifest i, String character, String value)
+ throws IOException {
+ byte[] mfBytes = writeManifest(i, FOUR_BYTE_NAME, value);
+
+ // in order to unambiguously establish the position of "character" in
+ // brokenPart, brokenPart is prepended and appended with what is
+ // expected before and after it...
+ String brokenPart = MARK_BEFORE;
+
+ // expect the whole character on the next line unless it fits
+ // completely on the current line
+ boolean breakExpected = p < c;
+ if (breakExpected) {
+ brokenPart += "\r\n ";
+ }
+ brokenPart += character;
+ // expect a line break before the next character if there is a next
+ // character and the previous not already broken on next line
+ if (a > 0) {
+ if (!breakExpected) {
+ brokenPart += "\r\n ";
+ }
+ brokenPart += MARK_AFTER;
+ }
+ brokenPart = brokenPart + "\r\n";
+ try {
+ assertOccurrence(mfBytes, brokenPart.getBytes(UTF_8));
+ readManifestAndAssertValue(mfBytes, i, FOUR_BYTE_NAME, value);
+ decodeManifestFromUTF8AndAssertHeaderValue(
+ mfBytes, FOUR_BYTE_NAME, value, true);
+ } catch (AssertionError e) {
+ System.out.println("-".repeat(72));
+ System.out.print(new String(mfBytes, UTF_8));
+ System.out.println("-".repeat(72));
+ throw e;
+ }
+ }
+
+ static byte[] writeManifest(PositionInManifest i, String name,
+ String value) throws IOException {
+ Manifest mf = new Manifest();
+ mf.getMainAttributes().put(Name.MANIFEST_VERSION, "1.0");
+ Attributes attributes = new Attributes();
+
+ switch (i) {
+ case MAIN_ATTRIBUTES:
+ mf.getMainAttributes().put(new Name(name), value);
+ break;
+ case SECTION_NAME:
+ mf.getEntries().put(value, attributes);
+ break;
+ case NAMED_SECTION:
+ mf.getEntries().put(FOUR_BYTE_NAME, attributes);
+ attributes.put(new Name(name), value);
+ break;
+ }
+
+ ByteArrayOutputStream out = new ByteArrayOutputStream();
+ mf.write(out);
+ return out.toByteArray();
+ }
+
+ /**
+ * Asserts one and only one occurrence of a sequence of bytes {@code part}
+ * representing the character and how it is expected to be broken and its
+ * surrounding bytes in a larger sequence that corresponds to the manifest
+ * in binary form {@code mf}.
+ */
+ static void assertOccurrence(byte[] mf, byte[] part) {
+ List
+ * From previous Manifest implementation without respect for UTF-8 encoded
+ * character boundaries breaking also within multi-byte UTF-8 encoded
+ * characters.
+ *
+ * @see {@link Manifest#make72Safe(StringBuffer)}
+ */
+ static void make72Safe(StringBuffer line) {
+ int length = line.length();
+ 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
+ }
+ }
+
+ @DataProvider(name = "positionInManifestValues")
+ public static Object[][] positionInManifestValues() {
+ LinkedList