diff -r 54aafb3ba9ab src/java.base/share/classes/java/util/jar/Attributes.java --- a/src/java.base/share/classes/java/util/jar/Attributes.java Mon Sep 24 22:12:07 2018 -0700 +++ b/src/java.base/share/classes/java/util/jar/Attributes.java Tue Dec 04 09:25:23 2018 +0100 @@ -36,6 +36,8 @@ import sun.util.logging.PlatformLogger; +import static java.nio.charset.StandardCharsets.UTF_8; + /** * The Attributes class maps Manifest attribute names to associated string * values. Valid attribute names are case-insensitive, are restricted to @@ -298,25 +300,15 @@ * Writes the current attributes to the specified data output stream. * XXX Need to handle UTF8 values and break up lines longer than 72 bytes */ - @SuppressWarnings("deprecation") - void write(DataOutputStream os) throws IOException { + void write(DataOutputStream out) throws IOException { for (Entry e : entrySet()) { StringBuffer buffer = new StringBuffer( ((Name) e.getKey()).toString()); buffer.append(": "); - - String value = (String) e.getValue(); - if (value != null) { - byte[] vb = value.getBytes("UTF8"); - value = new String(vb, 0, 0, vb.length); - } - buffer.append(value); - - Manifest.make72Safe(buffer); - buffer.append("\r\n"); - os.writeBytes(buffer.toString()); + buffer.append((String) e.getValue()); + Manifest.write72(out, buffer.toString()); } - os.writeBytes("\r\n"); + Manifest.write72(out, ""); } /* @@ -326,7 +318,6 @@ * * XXX Need to handle UTF8 values and break up lines longer than 72 bytes */ - @SuppressWarnings("deprecation") void writeMain(DataOutputStream out) throws IOException { // write out the *-Version header first, if it exists @@ -338,7 +329,7 @@ } if (version != null) { - out.writeBytes(vername+": "+version+"\r\n"); + out.write((vername + ": " + version + "\r\n").getBytes(UTF_8)); } // write out all attributes except for the version @@ -346,30 +337,18 @@ for (Entry e : entrySet()) { String name = ((Name) e.getKey()).toString(); if ((version != null) && !(name.equalsIgnoreCase(vername))) { - StringBuffer buffer = new StringBuffer(name); buffer.append(": "); - - String value = (String) e.getValue(); - if (value != null) { - byte[] vb = value.getBytes("UTF8"); - value = new String(vb, 0, 0, vb.length); - } - buffer.append(value); - - Manifest.make72Safe(buffer); - buffer.append("\r\n"); - out.writeBytes(buffer.toString()); + buffer.append((String) e.getValue()); + Manifest.write72(out, buffer.toString()); } } - out.writeBytes("\r\n"); + Manifest.write72(out, ""); } /* * Reads attributes from the specified input stream. - * XXX Need to handle UTF8 values. */ - @SuppressWarnings("deprecation") void read(Manifest.FastInputStream is, byte[] lbuf) throws IOException { String name = null, value; byte[] lastline = null; @@ -401,7 +380,7 @@ lastline = buf; continue; } - value = new String(buf, 0, buf.length, "UTF8"); + value = new String(buf, 0, buf.length, UTF_8); lastline = null; } else { while (lbuf[i++] != ':') { @@ -412,13 +391,13 @@ if (lbuf[i++] != ' ') { throw new IOException("invalid header field"); } - name = new String(lbuf, 0, 0, i - 2); + name = new String(lbuf, 0, i - 2, UTF_8); if (is.peek() == ' ') { lastline = new byte[len - i]; System.arraycopy(lbuf, i, lastline, 0, len - i); continue; } - value = new String(lbuf, i, len - i, "UTF8"); + value = new String(lbuf, i, len - i, UTF_8); } try { if ((putValue(name, value) != null) && (!lineContinued)) { diff -r 54aafb3ba9ab src/java.base/share/classes/java/util/jar/Manifest.java --- a/src/java.base/share/classes/java/util/jar/Manifest.java Mon Sep 24 22:12:07 2018 -0700 +++ b/src/java.base/share/classes/java/util/jar/Manifest.java Tue Dec 04 09:25:23 2018 +0100 @@ -34,6 +34,8 @@ import java.util.HashMap; import java.util.Iterator; +import static java.nio.charset.StandardCharsets.UTF_8; + /** * The Manifest class is used to maintain Manifest entry names and their * associated Attributes. There are main Manifest Attributes as well as @@ -136,14 +138,14 @@ /** * Writes the Manifest to the specified OutputStream. - * Attributes.Name.MANIFEST_VERSION must be set in + * {@link Attributes.Name.MANIFEST_VERSION} or + * {@link Attributes.Name.SIGNATURE_VERSION} must be set in * MainAttributes prior to invoking this method. * * @param out the output stream * @exception IOException if an I/O error has occurred * @see #getMainAttributes */ - @SuppressWarnings("deprecation") public void write(OutputStream out) throws IOException { DataOutputStream dos = new DataOutputStream(out); // Write out the main attributes for the manifest @@ -151,15 +153,8 @@ // Now write out the per-entry attributes for (Map.Entry e : entries.entrySet()) { StringBuffer buffer = new StringBuffer("Name: "); - String value = e.getKey(); - if (value != null) { - byte[] vb = value.getBytes("UTF8"); - value = new String(vb, 0, 0, vb.length); - } - buffer.append(value); - make72Safe(buffer); - buffer.append("\r\n"); - dos.writeBytes(buffer.toString()); + buffer.append(e.getKey()); + write72(dos, buffer.toString()); e.getValue().write(dos); } dos.flush(); @@ -167,7 +162,10 @@ /** * Adds line breaks to enforce a maximum 72 bytes per line. + * + * @deprecation Replaced with {@link #write72}. */ + @Deprecated(since = "12") static void make72Safe(StringBuffer line) { int length = line.length(); int index = 72; @@ -179,6 +177,35 @@ return; } + private static final String LINE_BREAK = "\r\n"; + private static final String CONTINUATION_SPACE = " "; + private static final byte[] LINE_BREAK_BYTES = LINE_BREAK.getBytes(UTF_8); + private static final byte[] LINE_BREAK_WITH_CONTINUATION_SPACE_BYTES = + (LINE_BREAK + CONTINUATION_SPACE).getBytes(UTF_8); + + /** + * Writes {@code line} to {@code out} with line breaks and continuation + * spaces within the limits of 72 bytes of contents on one line. + */ + static void write72(OutputStream out, String line) throws IOException { + if (line.length() > 0) { + byte[] lineBytes = line.getBytes(UTF_8); + 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 index = 1; + while (length - index > 71) { + out.write(lineBytes, index, 71); + index += 71; + out.write(LINE_BREAK_WITH_CONTINUATION_SPACE_BYTES); + } + out.write(lineBytes, index, length - index); + } + + out.write(LINE_BREAK_BYTES); + } + /** * Reads the Manifest from the specified InputStream. The entry * names and attributes read will be merged in with the current @@ -238,7 +265,7 @@ lastline = buf; continue; } - name = new String(buf, 0, buf.length, "UTF8"); + name = new String(buf, 0, buf.length, UTF_8); lastline = null; } Attributes attr = getAttributes(name); @@ -264,7 +291,7 @@ toLower(lbuf[2]) == 'm' && toLower(lbuf[3]) == 'e' && lbuf[4] == ':' && lbuf[5] == ' ') { try { - return new String(lbuf, 6, len - 6, "UTF8"); + return new String(lbuf, 6, len - 6, UTF_8); } catch (Exception e) { } diff -r 54aafb3ba9ab test/jdk/java/util/jar/Attributes/NullAndEmptyKeysAndValues.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/jdk/java/util/jar/Attributes/NullAndEmptyKeysAndValues.java Tue Dec 04 09:25:23 2018 +0100 @@ -0,0 +1,225 @@ +/* + * 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.Attributes; +import java.util.jar.Manifest; +import java.util.jar.Attributes.Name; +import java.lang.reflect.Field; + +import org.testng.annotations.Test; +import static org.testng.Assert.*; + +/** + * @test + * @bug 8066619 + * @run testng/othervm --illegal-access=warn NullAndEmptyKeysAndValues + * @summary Tests manifests with {@code null} and empty string {@code ""} + * values as section name, header name, or value in both main and named + * attributes sections. + */ +/* + * Note to future maintainer: + * In order to actually being able to test all the cases where key and values + * are null normal manifest and attributes manipulation through their public + * api is not sufficient but then there were these null checks there before + * which may or may not have had their reason and this way it's ensured that + * the behavior does not change with that respect. + * Once module isolation is enforced some test cases will not any longer be + * possible and those now tested situations will be guaranteed not to occur + * any longer at all at which point the corresponding tests can be removed + * safely without replacement unless of course another way is found inject the + * tested null values. + * Another trick to access package private class members could be to use + * deserialization or adding a new class to the same package on the classpath. + * Here is not important how the values are set to null because it shows that + * the behavior remains unchanged. + */ +public class NullAndEmptyKeysAndValues { + + static final String SOME_KEY = "some-key"; + static final String SOME_VALUE = "some value"; + static final String STRNULL = "null"; + static final String EMPTY_STR = ""; + static final Name EMPTY_NAME = new Name("tmp") {{ + try { + Field name = Name.class.getDeclaredField("name"); + name.setAccessible(true); + name.set(this, EMPTY_STR); + } catch (Exception e) { + throw new RuntimeException(e.getMessage(), e); + } + }}; + + @Test + public void testMainAttributesHeaderNameNull() throws Exception { + Manifest mf = new Manifest(); + Field attr = mf.getClass().getDeclaredField("attr"); + attr.setAccessible(true); + Attributes mainAtts = new Attributes() {{ + super.put( /* in: */ null, SOME_VALUE); + }}; + attr.set(mf, mainAtts); + mf.getMainAttributes().put(Name.MANIFEST_VERSION, "1.0"); + try { + mf = writeAndRead(mf); + fail(); + } catch ( /* out: */ NullPointerException e) { + return; // ok, was always like this + } + } + + @Test + public void testMainAttributesHeaderNameEmpty() throws Exception { + Manifest mf = new Manifest(); + mf.getMainAttributes().put(Name.MANIFEST_VERSION, "1.0"); + mf.getMainAttributes().put(EMPTY_NAME, SOME_VALUE); + try { + mf = writeAndRead(mf); + fail(); + } catch ( /* out: */ IOException e) { + return; // ok, was always like this + } + } + + @Test + public void testMainAttributesHeaderValueNull() throws Exception { + Manifest mf = new Manifest(); + Field attr = mf.getClass().getDeclaredField("attr"); + attr.setAccessible(true); + Attributes mainAtts = new Attributes() {{ + map.put(new Name(SOME_KEY), /* in: */ null); + }}; + attr.set(mf, mainAtts); + mf.getMainAttributes().put(Name.MANIFEST_VERSION, "1.0"); + mf = writeAndRead(mf); + assertEquals(mf.getMainAttributes().getValue(SOME_KEY), + /* out: */ STRNULL); + } + + @Test + public void testMainAttributesHeaderValueEmpty() throws Exception { + Manifest mf = new Manifest(); + Field attr = mf.getClass().getDeclaredField("attr"); + attr.setAccessible(true); + Attributes mainAtts = new Attributes() {{ + map.put(new Name(SOME_KEY), /* in: */ EMPTY_STR); + }}; + attr.set(mf, mainAtts); + mf.getMainAttributes().put(Name.MANIFEST_VERSION, "1.0"); + mf = writeAndRead(mf); + assertEquals(mf.getMainAttributes().getValue(SOME_KEY), + /* out: */ EMPTY_STR); + } + + @Test + public void testSectionNameNull() throws IOException { + Manifest mf = new Manifest(); + mf.getMainAttributes().put(Name.MANIFEST_VERSION, "1.0"); + mf.getEntries().put( /* in: */ null, new Attributes()); + mf = writeAndRead(mf); + assertNotNull(mf.getEntries().get( /* out: */ STRNULL)); + } + + @Test + public void testSectionNameEmpty() throws IOException { + Manifest mf = new Manifest(); + mf.getMainAttributes().put(Name.MANIFEST_VERSION, "1.0"); + mf.getEntries().put( /* in: */ EMPTY_STR, new Attributes()); + mf = writeAndRead(mf); + assertNotNull(mf.getEntries().get( /* out: */ EMPTY_STR)); + } + + @Test + public void testNamedSectionHeaderNameNull() throws IOException { + Manifest mf = new Manifest(); + mf.getMainAttributes().put(Name.MANIFEST_VERSION, "1.0"); + mf.getEntries().put(SOME_KEY, new Attributes() {{ + map.put( /* in: */ null, SOME_VALUE); + }}); + try { + mf = writeAndRead(mf); + fail(); + } catch ( /* out: */ NullPointerException e) { + return; // ok + } + } + + @Test + public void testNamedSectionHeaderNameEmpty() throws IOException { + Manifest mf = new Manifest(); + mf.getMainAttributes().put(Name.MANIFEST_VERSION, "1.0"); + mf.getEntries().put(SOME_KEY, new Attributes() {{ + map.put( /* in: */ EMPTY_NAME, SOME_VALUE); + }}); + try { + mf = writeAndRead(mf); + fail(); + } catch ( /* out: */ IOException e) { + return; // ok + } + } + + @Test + public void testNamedSectionHeaderValueNull() throws IOException { + Manifest mf = new Manifest(); + mf.getMainAttributes().put(Name.MANIFEST_VERSION, "1.0"); + mf.getEntries().put(SOME_KEY, new Attributes() {{ + map.put(new Name(SOME_KEY), /* in: */ null); + }}); + mf = writeAndRead(mf); + assertEquals(mf.getEntries().get(SOME_KEY).getValue(SOME_KEY), + /* out: */ STRNULL); + } + + @Test + public void testNamedSectionHeaderValueEmpty() throws IOException { + Manifest mf = new Manifest(); + mf.getMainAttributes().put(Name.MANIFEST_VERSION, "1.0"); + mf.getEntries().put(SOME_KEY, new Attributes() {{ + map.put(new Name(SOME_KEY), /* in: */ EMPTY_STR); + }}); + mf = writeAndRead(mf); + assertEquals(mf.getEntries().get(SOME_KEY).getValue(SOME_KEY), + /* out: */ EMPTY_STR); + } + + static Manifest writeAndRead(Manifest mf) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + mf.write(out); + byte[] mfBytes = out.toByteArray(); + System.out.println("-------------------------------------------" + + "-----------------------------"); + System.out.print(new String(mfBytes, UTF_8)); + System.out.println("-------------------------------------------" + + "-----------------------------"); + + ByteArrayInputStream in = new ByteArrayInputStream(mfBytes); + return new Manifest(in); + } + +} diff -r 54aafb3ba9ab test/jdk/java/util/jar/Manifest/ValueUtf8Coding.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/test/jdk/java/util/jar/Manifest/ValueUtf8Coding.java Tue Dec 04 09:25:23 2018 +0100 @@ -0,0 +1,211 @@ +/* + * 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.Attributes; +import java.util.jar.Attributes.Name; +import java.util.jar.Manifest; +import java.util.List; +import java.util.ArrayList; + +import org.testng.annotations.Test; +import static org.testng.Assert.*; + +/** + * @test + * @bug 6202130 8066619 + * @run testng ValueUtf8Coding + * @summary Tests encoding and decoding manifest header values to and from + * UTF-8 with the complete Unicode character set. + */ /* + * see also "../tools/launcher/UnicodeTest.java" for manifest attributes + * parsed during launch + */ +public class ValueUtf8Coding { + + /** + * Maximum number of bytes of UTF-8 encoded characters in one header value. + *

+ * There are too many different Unicode code points (more than one million) + * to fit all into one manifest value. The specifications state: + * Implementations should support 65535-byte (not character) header + * values, and 65535 headers per file. They might run out of memory, + * but there should not be hard-coded limits below these values. + * + * @see + * Notes on Manifest and Signature Files + */ + static final int SUPPORTED_VALUE_LENGTH = 65535; + + /** + * Returns {@code true} if {@code codePoint} is known not to be a supported + * character in manifest header values. Explicitly forbidden in manifest + * header values are according to a statement from the specifications: + * otherchar: any UTF-8 character except NUL, CR and LF. + * {@code NUL} ({@code 0x0}), however, works just fine and might have been + * used and might still be. + * + * @see + * Jar File Specification + */ + static boolean isUnsupportedManifestValueCharacter(int codePoint) { + return codePoint == '\r' /* CR */ || codePoint == '\n' /* LF */; + }; + + /** + * Produces a list of strings with all Unicode characters except those + * explicitly invalid in manifest header values. + * Each string is filled with as many characters as fit into + * {@link #SUPPORTED_VALUE_LENGTH} bytes with UTF-8 encoding except the + * last string which contains the remaining characters. Each of those + * strings becomes a header value the number of which 65535 should be + * supported per file. + * + * @see + * Notes on Manifest and Signature Files + */ + static List produceValuesWithAllUnicodeCharacters() { + ArrayList values = new ArrayList<>(); + byte[] valueBuf = new byte[SUPPORTED_VALUE_LENGTH]; + int pos = 0; + for (int codePoint = Character.MIN_CODE_POINT; + codePoint <= Character.MAX_CODE_POINT; codePoint++) { + if (isUnsupportedManifestValueCharacter(codePoint)) { + continue; + } + + byte[] charBuf = Character.toString(codePoint).getBytes(UTF_8); + if (pos + charBuf.length > valueBuf.length) { + values.add(new String(valueBuf, 0, pos, UTF_8)); + pos = 0; + } + System.arraycopy(charBuf, 0, valueBuf, pos, charBuf.length); + pos += charBuf.length; + } + if (pos > 0) { + values.add(new String(valueBuf, 0, pos, UTF_8)); + } + return values; + } + + /** + * Returns simple, valid, short, and distinct manifest header names. + * The returned name cannot collide with "{@code Manifest-Version}" because + * the returned string does not contain "{@code -}". + */ + static Name azName(int seed) { + StringBuffer name = new StringBuffer(); + do { + name.insert(0, (char) (seed % 26 + (seed < 26 ? 'A' : 'a'))); + seed = seed / 26 - 1; + } while (seed >= 0); + return new Name(name.toString()); + } + + /** + * Writes and reads a manifest with the complete Unicode character set. + * The characters are grouped into manifest header values with about as + * many bytes as allowed each, utilizing a single big manifest. + *

+ * This test assumes that a manifest is encoded and decoded correctly if + * writing and then reading it again results in a manifest with identical + * values as the original. The test is not about other aspects of writing + * and reading manifests than only that, given the fact and the way it + * works for some characters such as the most widely and often used ones, + * it also works for the complete Unicode character set just the same. + *

+ * Only header values are tested. The set of allowed characters for header + * names are much more limited and are a different topic entirely and most + * simple ones are used here as necessary just to get valid and different + * ones (see {@link #azName}). + *

+ * Because the current implementation under test uses different portions + * of code depending on where the value occurs to read or write, each + * character is tested in each of the three positions:

    + *
  • main attribute header,
  • + *
  • named section name, and
  • + *
  • named sections header values
  • + *
+ * Implementation of writing the main section headers in + * {@link Attributes#writeMain(java.io.DataOutputStream)} differs from the + * one writing named section headers in + * {@link Attributes#write(java.io.DataOutputStream)} regarding the special + * order of {@link Name#MANIFEST_VERSION} and + * {@link Name#SIGNATURE_VERSION} and also + * {@link Manifest#read(java.io.InputStream)} at least potentially reads + * main sections differently than reading named sections names headers in + * {@link Attributes#read(Manifest.FastInputStream, byte[])}. + */ + @Test + public void testCompleteUnicodeCharacterSet() throws IOException { + Manifest mf = new Manifest(); + mf.getMainAttributes().put(Name.MANIFEST_VERSION, "1.0"); + + List values = produceValuesWithAllUnicodeCharacters(); + for (int i = 0; i < values.size(); i++) { + Name name = azName(i); + String value = values.get(i); + + mf.getMainAttributes().put(name, value); + Attributes attributes = new Attributes(); + mf.getEntries().put(value, attributes); + attributes.put(name, value); + } + + mf = writeAndRead(mf); + + for (int i = 0; i < values.size(); i++) { + String value = values.get(i); + Name name = azName(i); + + assertEquals(mf.getMainAttributes().getValue(name), value, + "main attributes header value"); + Attributes attributes = mf.getAttributes(value); + assertNotNull(attributes, "named section"); + assertEquals(attributes.getValue(name), value, + "named section attributes value"); + } + } + + static Manifest writeAndRead(Manifest mf) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + mf.write(out); + byte[] mfBytes = out.toByteArray(); + + String sepLine = "----------------------------------------------" + + "--------------------------"; + System.out.println(sepLine); + System.out.print(new String(mfBytes, UTF_8)); + System.out.println(sepLine); + + ByteArrayInputStream in = new ByteArrayInputStream(mfBytes); + return new Manifest(in); + } + +}