RFR for 6443578 and 6202130: UTF-8 in Manifests
Philipp Kunz
philipp.kunz at paratix.ch
Wed May 2 05:21:37 UTC 2018
Hi,
Recently, I tried to fix only bug 6202130 with the intention to fix bug
6443578 later with the intention to get some opportunity for feedback,
but haven't got any, and propose now a fix for both together which in my
opinion makes more sense.
See attached patch.
Some considerations, assumptions, and explanations
* In my opinion, the code for writing manifests was distributed in the
two classes Attributes and Manifest in an elegant way but somewhat
difficult to explain the coherence. I chose to group the code that
writes manifests into a new class ManifestWriter. The main incentive
for that was to prevent or reduce duplicated code I would have had
to change twice otherwise. This also results in a source file of a
suitable size.
* I could not support the assumption that the write and writeMain
methods in Attributes couldn't be referenced anywhere so I
deprecated them rather than having them removed.
* I assumed the patch will not make it into JDK 10 and, hence, the
deprecated annotations are attributed with since = 11.
* I could not figure out any reason for the use of DataOutputStream
and did not use it.
* Performance-wise I assume that the code is approximately comparable
to the previous version. The biggest improvement in this respect I
hope comes from removing the String that contains the byte array
constructed with deprecated String(byte[], int, int, int) and then
copying it over again to a StringBuffer and from there to a String
again and then Characters. On the other hand, keeping whole
characters together when breaking lines might make it slightly
slower. I hope my changes are an overall improvement, but I haven't
measured it.
* For telling first from continuation bytes of utf-8 characters apart
I re-used a method isNotUtfContinuationByte from either StringCoding
or UTF_8.Decoder. Unfortunately I found no way not to duplicate it.
* Where it said before "XXX Need to handle UTF8 values and break up
lines longer than 72 bytes" in Attributes#writeMain I did not dare
to remove the comment completely because it still does not deal
correctly with version headers longer than 72 bytes and the set of
allowed values. I changed it accordingly. Two similar comments are
removed in the patch.
* I added two tests, WriteDeprecated and NullKeysAndValues, to
demonstrate compatibility as good as I could. Might however not be
desired to keep and having to maintain.
* LineBrokenMultiByteCharacter for jarsigner should not be removed or
not so immediately because someone might attempt to sign an older
jarfile created without that patch with a newer jarsigner that
already contains it.
suggested changes or additions to the bug database: (i have no
permissions to edit it myself)
* Re-combine copies of isNotUtfContinuationByte (three by now).
Relates to 6184334. Worth to file another issue?
* Manifest versions have specific specifications, cannot break across
lines and can contain a subset of characters only. Bug 6910466
relates but is not exactly the same. If someone else is convinced
that writing a manifest should issue a warning or any other way to
deal with a version that does not conform to the specification, I'd
suggest to create a separate bug for that.
Now, I would be glad if someone sponsored a review. This is only my
third attempt to submit a patch which is why I chose a lesser important
subject to fix in order to get familiar and now I understand it's not
the most attractive patch to review. Please don't hesitate to suggest
what I could do better or differently.
As a bonus, with these changes, manifest files will always be displayed
correctly with just any utf capable viewer even if they contain
multi-byte utf characters that would have been broken across a line
break with the current/previous implementation and all manifests will
become also valid strings in Java.
Regards,
Philipp
On 20.04.2018 00:58, Philipp Kunz wrote:
> Hi,
>
> I tried to fix bug 6202130 about manifest utf support and come up now
> with a test as suggested in the bug's comments that shows that utf
> charset actually works before removing the comments from the code.
>
> When I wanted to remove the XXX comments about utf it occurred to me
> that version attributes ("Signature-Version" and "Manifest-Version")
> would never be broken across lines and should anyway not support the
> whole utf character set which sounds more like related to bugs 6910466
> or 4935610 but it's not a real fit. Therefore, I could not remove one
> such comment of Attributes#writeMain but I changed it. The first
> comment in bug 6202130 mentions only two comments but there are three
> in Attributes. In the attached patch I removed only two of three and
> changed the remaining third to not mention utf anymore.
>
> At the moment, at least until 6443578 is fixed, multi-byte utf
> characters can be broken across lines. It might be worth a
> consideration to test that explicitly as well but then I guess there
> is not much of a point in testing the current behavior that will
> change with 6443578, hopefully soon. There are in my opinion enough
> characters broken across lines in the attached test that demonstrate
> that this still works like it did before.
>
> I would have preferred also to remove the calls to deprecated
> String(byte[], int, int, int) but then figured it relates more to bug
> 6443578 than 6202130 and now prefer to do that in another separate patch.
>
> Bug 6202130 also states that lines are broken by String.length not by
> byte length. While it looks so at first glance, I could not confirm.
> The combination of getBytes("UTF8"), String(byte[], int, int, int),
> and then DataOutputStream.writeBytes(String) in that combination does
> not drop high-bytes because every byte (whether a whole character or
> only a part of a multi-byte character) becomes a character in
> String(...) containing that byte in its low-byte which will be read
> again from writeBytes(...). Or put in a different way, every utf
> encoded byte becomes a character and multi-byte utf characters are
> converted into multiple string characters containing one byte each in
> their lower bytes. The current solution is not nice, but at least
> works. With that respect I'd like to suggest to deprecate
> DataOutputStream.writeBytes(String) because it does something not
> exactly expected when guessing from its name and that would suit a
> byte[] parameter better very much like it has been done with
> String(byte[], int, int, int). Any advice about the procedure to
> deprecate something?
>
> I was surprised that it was not trivial to list all valid utf
> characters. If someone has a better idea than isValidUtfCharacter in
> the attached test, let me know.
>
> Altogether, I would not consider 6202130 resolved completely, unless
> maybe all remaining points are copied to 6443578 and maybe another bug
> about valid values for "Signature-Version" and "Manifest-Version" if
> at all desired. But still I consider the attached patch an improvement
> and most of the remainder can then be solved in 6443578 and so far I
> am looking forward to any kind of feedback.
>
> Regards,
> Philipp
>
diff -r 2ace90aec488
src/java.base/share/classes/java/util/jar/Attributes.java
--- a/src/java.base/share/classes/java/util/jar/Attributes.java Mon Apr
30 21:56:54 2018 -0400
+++ b/src/java.base/share/classes/java/util/jar/Attributes.java Wed May
02 07:20:46 2018 +0200
@@ -296,27 +296,13 @@
/*
* Writes the current attributes to the specified data output stream.
- * XXX Need to handle UTF8 values and break up lines longer than 72
bytes
+ *
+ * @deprecated moved to
+ * {@link ManifestWriter#writeSection(java.io.OutputStream)}
*/
- @SuppressWarnings("deprecation")
- void write(DataOutputStream os) throws IOException {
- for (Entry<Object, Object> 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());
- }
- os.writeBytes("\r\n");
+ @Deprecated(since = "11")
+ void write(DataOutputStream os) throws IOException {
+ new ManifestWriter(os).writeSection(this);
}
/*
@@ -324,50 +310,16 @@
* make sure to write out the MANIFEST_VERSION or SIGNATURE_VERSION
* attributes first.
*
- * XXX Need to handle UTF8 values and break up lines longer than 72
bytes
+ * @deprecated moved to
+ * {@link ManifestWriter#writeMain(java.io.OutputStream)}
*/
- @SuppressWarnings("deprecation")
- void writeMain(DataOutputStream out) throws IOException
- {
- // write out the *-Version header first, if it exists
- String vername = Name.MANIFEST_VERSION.toString();
- String version = getValue(vername);
- if (version == null) {
- vername = Name.SIGNATURE_VERSION.toString();
- version = getValue(vername);
- }
-
- if (version != null) {
- out.writeBytes(vername+": "+version+"\r\n");
- }
-
- // write out all attributes except for the version
- // we wrote out earlier
- for (Entry<Object, Object> 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());
- }
- }
- out.writeBytes("\r\n");
+ @Deprecated(since = "11")
+ void writeMain(DataOutputStream os) throws IOException {
+ new ManifestWriter(os).writeMain(this);
}
/*
* Reads attributes from the specified input stream.
- * XXX Need to handle UTF8 values.
*/
@SuppressWarnings("deprecation")
void read(Manifest.FastInputStream is, byte[] lbuf) throws
IOException {
diff -r 2ace90aec488 src/java.base/share/classes/java/util/jar/Manifest.java
--- a/src/java.base/share/classes/java/util/jar/Manifest.java Mon Apr
30 21:56:54 2018 -0400
+++ b/src/java.base/share/classes/java/util/jar/Manifest.java Wed May
02 07:20:46 2018 +0200
@@ -26,7 +26,6 @@
package java.util.jar;
import java.io.FilterInputStream;
-import java.io.DataOutputStream;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.IOException;
@@ -143,31 +142,19 @@
* @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
- attr.writeMain(dos);
- // Now write out the per-entry attributes
- for (Map.Entry<String, Attributes> 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());
- e.getValue().write(dos);
- }
- dos.flush();
+ new ManifestWriter(out).write(this);
}
/**
* Adds line breaks to enforce a maximum 72 bytes per line.
+ *
+ * @see ManifestWriter#write72broken(String)
+ * @deprecated replaced by
+ * {@link ManifestWriter#write72broken(String)}
+ * which keeps whole utf character together when breaking lines
*/
+ @Deprecated(since = "11")
static void make72Safe(StringBuffer line) {
int length = line.length();
int index = 72;
diff -r 2ace90aec488
src/java.base/share/classes/java/util/jar/ManifestWriter.java
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/src/java.base/share/classes/java/util/jar/ManifestWriter.java Wed
May 02 07:20:46 2018 +0200
@@ -0,0 +1,224 @@
+/*
+ * 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. Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * 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.
+ */
+
+package java.util.jar;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.jar.Attributes.Name;
+
+/**
+ * ManifestWriter writes manifests and takes care of their structure,
+ * correct encoding regarding character set, and maximum line width.
+ * <p>
+ * For information on the Manifest format, please see the
+ * <a href="{@docRoot}/../specs/jar/jar.html">
+ * Manifest format specification</a>.
+ *
+ * @see Manifest#write(OutputStream)
+ * @since 11
+ */
+class ManifestWriter {
+
+ /**
+ * The utf-8 character set used for {@link Manifest}s.
+ */
+ private static final java.nio.charset.Charset UTF_8 =
+ StandardCharsets.UTF_8;
+
+ private static final String
+ INDIVIDUAL_SECTION_NAME_HEADER_KEY = "Name",
+ KEY_VALUE_SEPARATOR = ": ",
+ LINE_BREAK = "\r\n",
+ CONTINUATION = " ";
+
+ /**
+ * The <a
href="{@docRoot}/../specs/jar/jar.html#Notes_on_Manifest_and_Signature_Files">
+ * "Notes on Manifest and Signature Files"</a> in the "JAR File
+ * Specification" state that <q>no line may be longer than
+ * <strong>72</strong> bytes (not characters), in its utf8-encoded
form.</q>
+ */
+ private static final int MANIFEST_LINE_WIDTH = 72;
+
+ /**
+ * The underlying output stream where to write a {@link Manifest} to by
+ * {@link #write(Manifest)}.
+ */
+ private final OutputStream out;
+
+ /**
+ * Only constructor.
+ *
+ * @param out output stream where to write a {@link Manifest} to by
+ * {@link #write(Manifest)}
+ */
+ ManifestWriter(OutputStream out) {
+ this.out = out;
+ }
+
+ private static byte[] encode(String value) {
+ return value.getBytes(UTF_8);
+ }
+
+ private void write(String value) throws IOException {
+ out.write(encode(value));
+ }
+
+ private void write(byte[] value, int off, int len) throws IOException {
+ out.write(value, off, len);
+ }
+
+ private static final byte[] LINE_BREAK_BUF = encode(LINE_BREAK);
+
+ private void writeLineBreak() throws IOException {
+ out.write(LINE_BREAK_BUF);
+ }
+
+ private static final byte[] LINE_BREAK_CONTINUATION_BUF =
+ encode(LINE_BREAK + CONTINUATION);
+
+ private void writeContinuationLineBreak() throws IOException {
+ out.write(LINE_BREAK_CONTINUATION_BUF);
+ }
+
+ /**
+ * Writes a header line with added line breaks to enforce a maximum
of 72
+ * bytes per line with respect only to breaks whole characters and
writes
+ * the result to the current {@link ManifestWriter}'s underlying output
+ * stream.
+ */
+ private void write72broken(String header) throws IOException {
+ byte[] buf = encode(header);
+ int l = buf.length;
+ int p = 0; // start position in buf of current line to write
+ int n = MANIFEST_LINE_WIDTH; // number of bytes going on
current line
+ while (true) {
+ if (p + n >= l) {
+ n = l - p; // remainder fits on current (last) line
+ } else {
+ /*
+ * break whole utf characters only:
+ * put the current line end position on the next character
+ * start position / utf non-continuation byte left of and
+ * including n initial value / the maximum line width by
+ * decreasing the current line end position n until not
+ * followed by an utf continuation byte in order to not
break
+ * utf characters across line breaks
+ */
+ while (!isNotUtfContinuationByte(buf[p + n])) n--;
+ }
+ write(buf, p, n);
+ p += n;
+ if (p == l) break;
+ n = MANIFEST_LINE_WIDTH - 1;
+ writeContinuationLineBreak();
+ }
+ }
+
+ /**
+ * @see StringCoding#isNotUtfContinuationByte(int)
+ * @see sun.nio.cs.UTF_8.Decoder#isNotUtfContinuationByte(int)
+ */
+ private static boolean isNotUtfContinuationByte(byte b) {
+ return (b & 0xc0) != 0x80;
+ }
+
+ private static String composeHeaderLine(String name, String value) {
+ return name + KEY_VALUE_SEPARATOR + value;
+ }
+
+ private void writeHeader(String name, String value) throws
IOException {
+ write72broken(composeHeaderLine(name, value));
+ writeLineBreak();
+ }
+
+ /**
+ * Writes the specified attributes to the current output stream.
+ */
+ void writeSection(Attributes attributes) throws IOException {
+ for (Entry<Object, Object> e : attributes.entrySet()) {
+ String name = ((Name) e.getKey()).toString();
+ writeHeader(name, (String) e.getValue());
+ }
+ writeLineBreak();
+ }
+
+ /**
+ * Writes the specified attributes to the current output stream,
+ * make sure to write out the MANIFEST_VERSION or SIGNATURE_VERSION
+ * attributes first.
+ */
+ /*
+ * XXX Need to handle version compliant to specification:
+ * - only digits and periods but no period at the beginning or end
+ * - not more than 72-16-1=55 characters in order not to exceed
line width
+ * - no space after colon or rather change specs and only 54
characters max
+ */
+ void writeMain(Attributes attributes) throws IOException {
+ // write out the *-Version header first, if it exists
+ String vername = Name.MANIFEST_VERSION.toString();
+ String version = attributes.getValue(vername);
+ if (version == null) {
+ vername = Name.SIGNATURE_VERSION.toString();
+ version = attributes.getValue(vername);
+ }
+
+ if (version != null) {
+ // version header cannot be continued on next line
+ write(composeHeaderLine(vername, version));
+ writeLineBreak();
+ }
+
+ // write out all attributes except for the version we wrote out
earlier
+ for (Entry<Object, Object> e : attributes.entrySet()) {
+ String name = ((Name) e.getKey()).toString();
+ if ((version != null) && !(name.equalsIgnoreCase(vername))) {
+ writeHeader(name, (String) e.getValue());
+ }
+ }
+ writeLineBreak();
+ }
+
+ /**
+ * Writes the given {@link Manifest} to the output stream of this
+ * {@link ManifestWriter} instance.
+ *
+ * @param mf the {@link Manifest} to write
+ */
+ void write(Manifest mf) throws IOException {
+ // Write out the main attributes for the manifest
+ writeMain(mf.getMainAttributes());
+ // Now write out the per-entry attributes
+ for (Entry<String, Attributes> e : mf.getEntries().entrySet()) {
+ writeHeader(INDIVIDUAL_SECTION_NAME_HEADER_KEY, e.getKey());
+ writeSection(e.getValue());
+ }
+ }
+
+}
diff -r 2ace90aec488 test/jdk/java/util/jar/Attributes/WriteDeprecated.java
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/util/jar/Attributes/WriteDeprecated.java Wed May
02 07:20:46 2018 +0200
@@ -0,0 +1,123 @@
+/*
+ * 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.DataOutputStream;
+import java.io.IOException;
+import java.util.jar.Attributes;
+import java.util.jar.Attributes.Name;
+import java.util.jar.Manifest;
+import java.lang.reflect.Method;
+
+import org.testng.annotations.Test;
+import static org.testng.Assert.*;
+
+/**
+ * @test
+ * @run testng/othervm --illegal-access=warn WriteDeprecated
+ * @summary Tests that the Attribute's write methods still work despite not
+ * being used any longer by Manifest.
+ */
+ at Deprecated
+/*
+ * Note to future maintainer:
+ * In order to actually being able to test Attributes' write methods work
+ * as before normal manifest and attributes manipulation through their
public
+ * api is not sufficient but then these methods were there before 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 to
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 methods are invoked because it shows that
+ * the behavior remains unchanged.
+ */
+public class WriteDeprecated {
+
+ static final Name KEY = new Name("some-key");
+ static final String VALUE = "value";
+
+ static class AccessibleAttributes extends Attributes {
+
+ void invoke(String name, DataOutputStream os) throws IOException {
+ try {
+ Method method = Attributes.class.getDeclaredMethod(
+ name, DataOutputStream.class);
+ method.setAccessible(true);
+ method.invoke(this, os);
+ } catch (ReflectiveOperationException e) {
+ fail(e.getMessage(), e);
+ }
+ }
+
+ /**
+ * @see Attributes#write(DataOutputStream)
+ */
+ void write(DataOutputStream os) throws IOException {
+ invoke("write", os);
+ }
+
+ /**
+ * @see Attributes#writeMain(DataOutputStream)
+ */
+ void writeMain(DataOutputStream os) throws IOException {
+ invoke("writeMain", os);
+ }
+
+ }
+
+ @Test
+ public void testWriteMethods() throws Exception {
+ ByteArrayOutputStream out = new ByteArrayOutputStream();
+ AccessibleAttributes attributes = new AccessibleAttributes();
+ attributes.put(Name.MANIFEST_VERSION, "1.0");
+ attributes.put(KEY, VALUE);
+ DataOutputStream dos = new DataOutputStream(out);
+ attributes.writeMain(dos);
+ attributes.remove(Name.MANIFEST_VERSION);
+ dos.writeBytes("Name: " + VALUE + "\r\n");
+ attributes.write(dos);
+ dos.flush();
+
+ byte[] mfBytes = out.toByteArray();
+ System.out.println("-------------------------------------------"
+ + "-----------------------------");
+ System.out.print(new String(mfBytes, UTF_8));
+ System.out.println("-------------------------------------------"
+ + "-----------------------------");
+ Manifest mf = new Manifest(new ByteArrayInputStream(mfBytes));
+ assertEquals(mf.getMainAttributes().getValue(KEY), VALUE,
+ "main attributes header value");
+ Attributes attributes2 = mf.getAttributes(VALUE);
+ assertNotNull(attributes2, "named section not found");
+ assertEquals(attributes2.getValue(KEY), VALUE,
+ "named section attributes header value");
+ }
+
+}
diff -r 2ace90aec488 test/jdk/java/util/jar/Manifest/LineBreakCharacter.java
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/util/jar/Manifest/LineBreakCharacter.java Wed May
02 07:20:46 2018 +0200
@@ -0,0 +1,405 @@
+/*
+ * 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.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
+ * @run testng/othervm --illegal-access=warn LineBreakCharacter
+ * @summary Tests reading and writing of jar manifests with headers broken
+ * across lines in conjunction with multi-byte utf characters.
+ * <p>
+ * Line breaks may or may not occur within utf encoded characters that are
+ * represented with more than one byte. Even though characters should
not be
+ * broken across lines according to the specification the previous Manifest
+ * implementation did it and this test makes sure that no multi-byte
characters
+ * are broken apart across a line break when writing manifests and
manifests
+ * are read correctly no matter if multi-byte characters are continued
after a
+ * line break.
+ * <p>
+ * Correct support of all utf characters is the concern of
+ * {@link ValueUtfEncoding}.
+ */
+public class LineBreakCharacter {
+
+ static final int MANIFEST_LINE_CONTENT_WIDTH_BYTES = 72;
+
+ static final String FILL = "x";
+
+ /**
+ * By using names of four characters the same values can be used for
+ * testing line breaks at exact positions for both header and section
+ * names, header names in main and named sections, because a named
+ * section name is represented with a header with key "Name" that
occurs
+ * after a blank line and also has four characters.
+ */
+ static final String NAME = FILL + FILL + FILL + FILL;
+
+ /**
+ * Cover main attributes headers, section names, and headers in named
+ * sections because an implementation might make a difference.
+ */
+ enum PositionInManifest {
+ MAIN_ATTRIBUTES, SECTION_NAME, NAMED_SECTION;
+ }
+
+ /**
+ * @see LineBreakLineWidth#numByteUtfCharacter(int, int)
+ */
+ static String numByteUtfCharacter(int numBytes, int seed) {
+ seed = seed < 0 ? -seed : seed;
+ if (numBytes == 1) {
+ seed %= 0x5F;
+ seed += 0x20; // exclude control characters (0..0x19) here
+ } else if (numBytes == 2) {
+ seed %= 0x800 - 0x80;
+ seed += 0x80;
+ } else if (numBytes == 3) {
+ seed %= 0x10000 - 0x800 + 0xFDD0 - 0xFDEF + 0xFFFE - 0xFFFF;
+ seed += 0x800
+ + (seed >= 0xFDD0 ? 0xFDEF - 0xFDD0 : 0) //
non-characters
+ + (seed % 0x10000) * (0xFFFF - 0xFFFE); // byte
order marks
+ } else {
+ seed %= 0x110000 - (0x10000 - 0xFFFE);
+ seed += 0x10000
+ + (seed % 0x10000) * (0xFFFF - 0xFFFE); // byte
order marks
+ }
+
+ String string = new String(Character.toChars(seed));
+ assertEquals(string.getBytes(UTF_8).length, numBytes,
+ "self-test failed: unexpected utf encoded character
length");
+ return string;
+ }
+
+ @DataProvider(name = "lineBreakParameters")
+ public static Object[][] lineBreakParameters() {
+ LinkedList<Object[]> params = new LinkedList<>();
+
+ // b: number of line breaks before character under test
+ for (int b = 0; b <= 3; b++) {
+
+ // c: character utf encoded length in bytes
+ for (int c = 1; c <= 4; c++) {
+
+ // p: potential break position offset in bytes
+ // p = 0 => before character
+ // p = c => after character and
+ // 0 < p < c => character potentially broken across
line break
+ // within the character
+ for (int p = c; p >= 0; p--) {
+
+ // a: no or one character following the one under test
+ // (a = 0 meaning the character under test is
followed by
+ // end of value which is also represented as a line
break)
+ for (int a = 0; a <= 1; a++) {
+
+ // offset: so many characters (actually bytes here
+ // filled with one byte characters) are needed
to place
+ // the next character into a position relative
to the
+ // maximum line width that it may or may not
have to be
+ // broken onto the next line
+ int offset =
+ // number of lines; - 1 is continuation
space
+ b * (MANIFEST_LINE_CONTENT_WIDTH_BYTES - 1)
+ // line length minus "Name: ".length()
+ + MANIFEST_LINE_CONTENT_WIDTH_BYTES - 6
+ // position of maximum line width
relative to
+ // beginning of encoded character
+ - p;
+ int seed = ((a * 2 + p) * c + c) * 4 + b;
+ String value = "";
+ for (int i = 0; i < offset - 1; i++) {
+ value += numByteUtfCharacter(1, seed++);
+ }
+ // character before the one to test the break
+ value += FILL;
+ String character = numByteUtfCharacter(c, seed);
+ value += character;
+ for (int i = 0; i < a; i++) {
+ // character after the one to test the break
+ value += FILL;
+ }
+
+ for (PositionInManifest i :
+ PositionInManifest.values()) {
+
+ params.add(new Object[] {
+ b, c, p, a, i, character, value});
+ }
+ }
+ }
+ }
+ }
+
+ return params.toArray(new Object[0][0]);
+ }
+
+ /**
+ * Checks that multi-byte utf characters work well with line breaks and
+ * continuation lines in jar manifests.
+ *
+ * Produces otherwise arbitrary manifests so that a specific character
+ * will not just fit on the same line and a line break and continuation
+ * line are required with:<ul>
+ * <li>different encoded sizes of characters: one, two, three, and four
+ * bytes per character (including also utf bmp and surrogate
pairs)</li>
+ * <li>different amount of space remaining on the same line or relative
+ * position of the latest possible line break position with respect
to the
+ * character that can be continued on the same line or will have to be
+ * continued on the next line after a line break</li>
+ * <li>different number of preceding line breaks</li>
+ * </ul>
+ * For each of these cases the break position is verified in the binary
+ * manifest.
+ */
+ @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, NAME, value);
+
+ // expect the whole character on the next line unless it fits
+ // completely on the current line
+ boolean breakExpected = p < c;
+ String brokenPart = character;
+ if (breakExpected) {
+ brokenPart = "\r\n " + brokenPart;
+ }
+ // 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 == 1) {
+ if (!breakExpected) {
+ brokenPart += "\r\n ";
+ }
+ brokenPart += FILL;
+ }
+ brokenPart = FILL + brokenPart + "\r\n";
+ assertOccurrence(mfBytes, brokenPart.getBytes(UTF_8));
+ readManifestAndAssertValue(mfBytes, i, NAME, value);
+ }
+
+ 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(NAME, attributes);
+ attributes.put(new Name(name), value);
+ break;
+ }
+
+ 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("-------------------------------------------"
+ + "-----------------------------");
+ return mfBytes;
+ }
+
+ static void assertOccurrence(byte[] mf, byte[] part) {
+ List<Integer> matchPos = new LinkedList<>();
+ for (int i = 0; i < mf.length; i++) {
+ for (int j = 0; j < part.length && i + j <= mf.length; j++) {
+ if (part[j] == 0) {
+ if (i + j != mf.length) {
+ break; // expected eof not found
+ }
+ } else if (i + j == mf.length) {
+ break;
+ } else if (mf[i + j] != part[j]) {
+ break;
+ }
+ if (j == part.length - 1) {
+ matchPos.add(i);
+ }
+ }
+ }
+ assertEquals(matchPos.size(), 1, "not "
+ + (matchPos.size() < 1 ? "found" : "unique") + ": '"
+ + new String(part, UTF_8) + "'");
+ }
+
+ static void readManifestAndAssertValue(
+ byte[] mfBytes, PositionInManifest i, String name, String
value)
+ throws IOException {
+ Manifest mf = new Manifest(new ByteArrayInputStream(mfBytes));
+
+ switch (i) {
+ case MAIN_ATTRIBUTES:
+ assertEquals(mf.getMainAttributes().getValue(name), value,
+ "main attributes header value");
+ break;
+ case SECTION_NAME:
+ Attributes attributes = mf.getAttributes(value);
+ assertNotNull(attributes, "named section not found");
+ break;
+ case NAMED_SECTION:
+ attributes =
+ mf.getAttributes(NAME);
+ assertEquals(attributes.getValue(name), value,
+ "named section attributes header value");
+ break;
+ }
+ }
+
+ @Test(dataProvider = "lineBreakParameters")
+ public void readContinuationLines(int b, int c, int p, int a,
+ PositionInManifest i, String character, String value)
+ throws IOException {
+ byte[] mfBytes = writeManifestWithBrokenCharacters(i, NAME, value);
+
+ ByteArrayOutputStream buf = new ByteArrayOutputStream();
+ buf.write(FILL.getBytes(UTF_8));
+ byte[] characterBytes = character.getBytes(UTF_8);
+ // the portion of the character that fits on the current line
before
+ // a break at 72 bytes, ranges from nothing (p == 0) to the whole
+ // character (p == c)
+ for (int j = 0; j < p; j++) {
+ buf.write(characterBytes, j, 1);
+ }
+ // expect a line break at exactly 72 bytes from the beginning
of the
+ // line unless the whole character fits on that line
+ boolean breakExpected = p < c;
+ if (breakExpected) {
+ buf.write("\r\n ".getBytes(UTF_8));
+ }
+ // the remaining portion of the character, if any
+ for (int j = p; j < c; j++) {
+ buf.write(characterBytes, j, 1);
+ }
+ // expect another linebreak if the whole character fitted on
the same
+ // line and there is another character
+ if (a == 1) {
+ if (c == p) {
+ buf.write("\r\n ".getBytes(UTF_8));
+ }
+ buf.write(FILL.getBytes(UTF_8));
+ }
+ buf.write("\r\n".getBytes(UTF_8));
+ byte[] brokenPart = buf.toByteArray();
+ assertOccurrence(mfBytes, brokenPart);
+ readManifestAndAssertValue(mfBytes, i, NAME, value);
+ }
+
+ /*
+ * From previous Manifest implementation reduced to the minimum
required to
+ * demonstrate compatibility
+ */
+ @SuppressWarnings("deprecation")
+ static byte[] writeManifestWithBrokenCharacters(
+ PositionInManifest i, String name, String value)
+ throws IOException {
+ byte[] vb = value.getBytes("UTF8");
+ value = new String(vb, 0, 0, vb.length);
+ ByteArrayOutputStream out = new ByteArrayOutputStream();
+ DataOutputStream dos = new DataOutputStream(out);
+ String vername = Name.MANIFEST_VERSION.toString();
+ dos.writeBytes(vername + ": 0.1\r\n");
+
+ if (i == PositionInManifest.MAIN_ATTRIBUTES) {
+ StringBuffer buffer = new StringBuffer(name);
+ buffer.append(": ");
+ buffer.append(value);
+ make72Safe(buffer);
+ buffer.append("\r\n");
+ dos.writeBytes(buffer.toString());
+ }
+ dos.writeBytes("\r\n");
+
+ if (i == PositionInManifest.SECTION_NAME ||
+ i == PositionInManifest.NAMED_SECTION) {
+ StringBuffer buffer = new StringBuffer("Name: ");
+ if (i == PositionInManifest.SECTION_NAME) {
+ buffer.append(value);
+ } else {
+ buffer.append(NAME);
+ }
+ make72Safe(buffer);
+ buffer.append("\r\n");
+ dos.writeBytes(buffer.toString());
+
+ if (i == PositionInManifest.NAMED_SECTION) {
+ buffer = new StringBuffer(name);
+ buffer.append(": ");
+ buffer.append(value);
+ make72Safe(buffer);
+ buffer.append("\r\n");
+ dos.writeBytes(buffer.toString());
+ }
+
+ dos.writeBytes("\r\n");
+ }
+
+ dos.flush();
+ byte[] mfBytes = out.toByteArray();
+ System.out.println("-------------------------------------------"
+ + "-----------------------------");
+ System.out.print(new String(mfBytes, UTF_8));
+ System.out.println("-------------------------------------------"
+ + "-----------------------------");
+ return mfBytes;
+ }
+
+ /**
+ * Adds line breaks to enforce a maximum 72 bytes per line.
+ * From previous Manifest implementation.
+ */
+ static void make72Safe(StringBuffer line) {
+ try {
+ Method method = Manifest.class.getDeclaredMethod(
+ "make72Safe", StringBuffer.class);
+ method.setAccessible(true);
+ method.invoke(null, line);
+ } catch (ReflectiveOperationException e) {
+ fail(e.getMessage(), e);
+ }
+ }
+
+}
diff -r 2ace90aec488 test/jdk/java/util/jar/Manifest/LineBreakLineWidth.java
--- a/test/jdk/java/util/jar/Manifest/LineBreakLineWidth.java Mon Apr
30 21:56:54 2018 -0400
+++ b/test/jdk/java/util/jar/Manifest/LineBreakLineWidth.java Wed May
02 07:20:46 2018 +0200
@@ -26,9 +26,13 @@
import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
+import java.util.Set;
+import java.util.TreeSet;
import java.util.jar.Manifest;
import java.util.jar.Attributes;
import java.util.jar.Attributes.Name;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
import org.testng.annotations.Test;
import static org.testng.Assert.*;
@@ -37,8 +41,10 @@
* @test
* @bug 6372077
* @run testng LineBreakLineWidth
- * @summary write valid manifests with respect to line breaks
- * and read any line width
+ * @summary Write valid manifests with respect to line widths breaking
longer
+ * lines, especially for 6372077 that the lines are
sufficiently wide
+ * to contain all possible valid header names, and read any line
+ * width.
*/
public class LineBreakLineWidth {
@@ -49,6 +55,11 @@
final static int MAX_HEADER_NAME_LENGTH = 70;
/**
+ * maximum line width not including the line break
+ */
+ final static int MAX_LINE_CONTENT_LENGTH = 72;
+
+ /**
* 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
@@ -60,20 +71,27 @@
final static int TEST_WIDTH_RANGE = 123;
/**
- * tests if only valid manifest files can written depending on the
header
+ * 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
+ * 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.
+ * <p>
+ * While this test covers header names which can contain only
one-byte utf
+ * encoded characters and the resulting manifest line widths,
multi-byte
+ * utf encoded characters can occur in header values, which is
subject of
+ * {@link #testManifestLineWidthMaximumRange()}.
*/
@Test
public void testWriteValidManifestOrException() throws IOException {
+ assertTrue(TEST_WIDTH_RANGE > MAX_HEADER_NAME_LENGTH);
+
/*
- * multi-byte utf-8 characters cannot occur in header names,
+ * 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
+ * Hence, each character in a header name uses exactly one byte and
* variable length utf-8 character encoding doesn't affect
this test.
*/
@@ -111,18 +129,18 @@
}
/**
- * tests that manifest files can be read even if the line breaks are
+ * 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
+ * The <a
href="{@docRoot}/../specs/jar/jar.html#Notes_on_Manifest_and_Signature_Files">
+ * "Notes on Manifest and Signature Files"</a> in the "JAR File
+ * Specification" state that <q>no line may be longer than 72 bytes
(not
+ * characters), in its utf8-encoded form.</q> but allows for earlier or
* additional line breaks.
* <p>
- * the most important purpose of this test case is probably to make
sure
+ * 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.
*/
@@ -186,7 +204,8 @@
* form.
*/
String lineBrokenSectionName = breakLines(lineWidth, "Name: "
+ name);
- String lineBrokenNameAndValue = breakLines(lineWidth, name + ":
" + value);
+ String lineBrokenNameAndValue = breakLines(lineWidth, name + ": "
+ + value);
ByteArrayOutputStream mfBuf = new ByteArrayOutputStream();
mfBuf.write("Manifest-Version: 1.0".getBytes(UTF_8));
@@ -264,7 +283,7 @@
return mfBytes;
}
- private static void printManifest(byte[] mfBytes) {
+ static void printManifest(byte[] mfBytes) {
final String sepLine =
"----------------------------------------------"
+ "---------------------|-|-|"; // |-positions:
---68-70-72
System.out.println(sepLine);
@@ -272,13 +291,179 @@
System.out.println(sepLine);
}
- private static void assertMainAndSectionValues(Manifest mf, String
name,
+ 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");
+ Attributes attributes = mf.getAttributes(name);
+ assertNotNull(attributes, "named section not found");
+ String sectionValue = attributes.getValue(name);
assertEquals(value, sectionValue, "value different in named
section");
}
+ @Test
+ public void testWriteManifestOnOneLine() throws IOException {
+ testWriteVersionInfoLineWidth(Name.MANIFEST_VERSION);
+ }
+
+ @Test
+ public void testWriteSignatureVersionOnOneLine() throws IOException {
+ testWriteVersionInfoLineWidth(Name.SIGNATURE_VERSION);
+ }
+
+ /**
+ * Tests that manifest versions "Manifest-Version" and
"Signature-Version"
+ * are not continued on a next line.
+ */
+ void testWriteVersionInfoLineWidth(Name version) throws IOException {
+ // e: number of characters of version header exceeding line width
+ for (int e = 0; e <= 1; e++) {
+ Manifest mf = new Manifest();
+ mf.getMainAttributes().put(version,
+ "01234567890123456789012345678901234567890123456789012345"
+ .substring(0, MAX_HEADER_NAME_LENGTH
+ - version.toString().length() + e));
+ ByteArrayOutputStream out = new ByteArrayOutputStream();
+ mf.write(out);
+ byte[] mfBytes = out.toByteArray();
+ printManifest(mfBytes);
+ // maximum line width can only be exceeded if header not broken
+ assertEquals(MAX_LINE_CONTENT_LENGTH + e,
+ findMaximumLineWidth(mfBytes));
+ }
+ }
+
+ static int findMaximumLineWidth(byte[] mfBytes) {
+ int max = 0;
+ int b = 0; // start position of current line
+ for (int i = 0; i < mfBytes.length; i++) {
+ switch (mfBytes[i]) {
+ case 10:
+ case 13:
+ max = Math.max(max, i - b);
+ b = i + 1;
+ }
+ }
+ return max;
+ }
+
+ /**
+ * @see LineBreakCharacter#numByteUtfCharacter(int, int)
+ */
+ static String numByteUtfCharacter(int numBytes, int seed) {
+ seed = seed < 0 ? -seed : seed;
+ if (numBytes == 1) {
+ seed %= 0x5F;
+ seed += 0x20; // exclude control characters (0..0x19) here
+ } else if (numBytes == 2) {
+ seed %= 0x800 - 0x80;
+ seed += 0x80;
+ } else if (numBytes == 3) {
+ seed %= 0x10000 - 0x800 + 0xFDD0 - 0xFDEF + 0xFFFE - 0xFFFF;
+ seed += 0x800
+ + (seed >= 0xFDD0 ? 0xFDEF - 0xFDD0 : 0) //
non-characters
+ + (seed % 0x10000) * (0xFFFF - 0xFFFE); // byte
order marks
+ } else {
+ seed %= 0x110000 - (0x10000 - 0xFFFE);
+ seed += 0x10000
+ + (seed % 0x10000) * (0xFFFF - 0xFFFE); // byte
order marks
+ }
+
+ String string = new String(Character.toChars(seed));
+ assertEquals(string.getBytes(UTF_8).length, numBytes,
+ "self-test failed: unexpected utf encoded character
length");
+ return string;
+ }
+
+ /**
+ * Tests that the manifest line content width is between <em>69 (1 +
+ * maximum line width (without line break) 72 - maximum utf encoded
+ * character length 4)</em> and <em>72</em> before continued on the
next
+ * line, aka continuation line, depending only on the last character
+ * before the line break and not on those before or after in order to
+ * ensure the minimum number of line breaks and continuation lines
+ * possible.
+ * <p>
+ * The <a
href="{@docRoot}/../specs/jar/jar.html#Notes_on_Manifest_and_Signature_Files">
+ * "Notes on Manifest and Signature Files"</a> in the "JAR File
+ * Specification" state that <q>no line may be longer than 72 bytes
(not
+ * characters), in its utf8-encoded form.</q> and allows for earlier or
+ * additional line breaks.
+ * <p>
+ * The number of characters a line may be less than the number of bytes
+ * but this is not explicitly verified because it can be assumed if
some
+ * characters use more than one byte each and the number of bytes is as
+ * expected up to 72 that fewer than 72 characters were placed on
the same
+ * line.
+ * <p>
+ * While {@link #testWriteValidManifestOrException()} tests header
names
+ * lengths, this test covers the values.
+ */
+ @Test
+ public void testManifestLineWidthRange() throws Exception {
+ final int MAX_UTF_CHARACTER_ENCODED_LENGTH = 4;
+ final String TEST_NAME = "test";
+
+ // value will be filled with many possible sequences of utf
+ // characters of different encoded numbers of bytes in the form of
+ // "k times i-bytes size character" + "k times i-bytes size
character"
+ // and by prepending up to 72 "x" characters at every possible
position
+ // relative to a possible line break.
+ String value = "";
+ int seed = 0;
+ for (int i = 1; i <= MAX_UTF_CHARACTER_ENCODED_LENGTH; i++) {
+ for (int j = 1; j <= MAX_UTF_CHARACTER_ENCODED_LENGTH; j++) {
+ String valueI = "", valueJ = "";
+ for (int k = 1; k < MAX_LINE_CONTENT_LENGTH; k++) {
+ valueI += numByteUtfCharacter(i, seed++);
+ valueJ += numByteUtfCharacter(j, seed++);
+ value += valueI + valueJ;
+ }
+ }
+ }
+
+ String offset = "";
+ for (int i = 0; i <= MAX_LINE_CONTENT_LENGTH; i++) {
+ offset += "x";
+ byte[] mfBytes = writeManifest(TEST_NAME, offset + value);
+ Manifest mf = new Manifest(new ByteArrayInputStream(mfBytes));
+ assertMainAndSectionValues(mf, TEST_NAME, offset + value);
+
+ Set<Integer> allowedWidths = IntStream.rangeClosed(
+ MAX_LINE_CONTENT_LENGTH -
MAX_UTF_CHARACTER_ENCODED_LENGTH + 1,
+ MAX_LINE_CONTENT_LENGTH).boxed().collect(Collectors.toSet());
+ assertEquals(allowedWidths, findContinuedLineWidths(mfBytes));
+ }
+ }
+
+ /**
+ * Counts the length of lines (without the line breaks) which are
+ * continued, not the continuation lines but those before that did
not fit
+ * on one line, whereby continuation lines may be continued again on
+ * another line, and returns their length in bytes, not characters.
+ */
+ static Set<Integer> findContinuedLineWidths(byte[] mfBytes) {
+ Set<Integer> widths = new TreeSet<>();
+ int previousLineWidth = -1;
+ int b = 0; // start position of current line
+ for (int i = 0; i < mfBytes.length; i++) {
+ switch (mfBytes[i]) {
+ case '\r':
+ case '\n':
+ previousLineWidth = i - b;
+ if (mfBytes[i] == '\r' &&
+ i + 1 < mfBytes.length && mfBytes[i + 1] == '\n') {
+ i++;
+ }
+ b = i + 1;
+ break;
+ case ' ':
+ if (i == b) {
+ widths.add(previousLineWidth);
+ }
+ }
+ }
+ return widths;
+ }
+
}
diff -r 2ace90aec488 test/jdk/java/util/jar/Manifest/NullKeysAndValues.java
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/util/jar/Manifest/NullKeysAndValues.java Wed May
02 07:20:46 2018 +0200
@@ -0,0 +1,149 @@
+/*
+ * 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
+ * @run testng/othervm --illegal-access=warn NullKeysAndValues
+ * @summary Tests manifests with {@code null} 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 NullKeysAndValues {
+
+ static final String SOME_KEY = "some-key";
+ static final String SOME_VALUE = "some value";
+ static final String STRNULL = "null";
+
+ @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
+ }
+ }
+
+ @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 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 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 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);
+ }
+
+ 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 2ace90aec488 test/jdk/java/util/jar/Manifest/ValueUtfEncoding.java
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/util/jar/Manifest/ValueUtfEncoding.java Wed May
02 07:20:46 2018 +0200
@@ -0,0 +1,226 @@
+/*
+ * 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
+ * @run testng ValueUtfEncoding
+ * @summary Tests complete manifest values utf encoding
+ * <p>
+ * This test writes and reads a manifest that contains every valid utf
+ * character (three times), grouped into manifest header values with about
+ * 65535 bytes each or slightly more, resulting in a single huge
manifest with
+ * 3 * 67 + 1 values and 13703968 bytes in the manifest's encoded form in
+ * total. This way, all possible 1111995 utf characters are covered in one
+ * manifest.
+ * <p>
+ * Every character occurs three times, once in a main attribute value,
once in
+ * a section name, and once in a named section attribute value, because
+ * implementation of writing the main section headers differs from the one
+ * writing named section headers in
+ * {@link Attributes#writeMain(java.io.DataOutputStream)} and
+ * {@link Attributes#write(java.io.DataOutputStream)} due to special
order of
+ * {@link Name#MANIFEST_VERSION} and {@link Name#SIGNATURE_VERSION}.
+ * and also {@link Manifest#read(java.io.InputStream)} treating reading the
+ * main section differently from reading named sections names headers.
+ * <p>
+ * Only header values are tested. Characters for header names are much more
+ * limited and very simple ones are used just to get valid and
different ones.
+ * <p>
+ * Correct support of all utf characters also has some relation to breaking
+ * characters across lines, see {@link LineBreakCharacter}.
+ */
+public class ValueUtfEncoding {
+
+ /**
+ * From the specifications:
+ * <q>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.</q>
+ *
+ * @see <a
href="{@docRoot}/../specs/jar/jar.html#Notes_on_Manifest_and_Signature_Files">Notes
on Manifest and Signature Files</a>
+ */
+ static final int MIN_VALUE_LENGTH_SUPPORTED = 2 << 16 - 1;
+
+ static final int MAX_UTF_CHARACTER_ENCODED_LENGTH = 4;
+
+ static boolean isValidUtfCharacter(int codePoint) {
+ if (0xFDD0 <= codePoint && codePoint <= 0xFDEF) {
+ return false; /* non-characters */
+ }
+ if ((codePoint & 0xFFFE) == 0xFFFE) {
+ return false; /* byte order marks */
+ }
+ return true;
+ }
+
+ /**
+ * returns {@code true} if {@code codePoint} is explicitly forbidden in
+ * manifest values based on a statement from the specs:
+ * <pre>otherchar: any UTF-8 character except NUL, CR and LF<pre>
+ *
+ * @see <a
href="{@docRoot}/../specs/jar/jar.html#Section-Specification">Jar File
Specification</a>
+ */
+ static boolean isInvalidManifestValueCharacter(int codePoint) {
+ return codePoint == 0 /* NUL */
+ || codePoint == '\r' /* CR */
+ || codePoint == '\n' /* LF */;
+ };
+
+ /**
+ * Produces a list of strings with all known utf characters except
those
+ * invalid in manifest header values with at least
+ * {@link #MIN_VALUE_LENGTH_SUPPORTED} utf-8 encoded bytes each
+ * except the last string which contains just the remaining characters.
+ */
+ static List<String> produceValidManifestUtfCharacterValues() {
+ int maxLengthBytes = MIN_VALUE_LENGTH_SUPPORTED +
+ // exceed the specified limit by at least one character
+ MAX_UTF_CHARACTER_ENCODED_LENGTH + 1;
+
+ int numberOfUsedCodePoints = 0;
+ ArrayList<String> values = new ArrayList<>();
+ byte[] valueBuf = new byte[maxLengthBytes];
+ int pos = 0;
+ for (int codePoint = Character.MIN_CODE_POINT;
+ codePoint <= Character.MAX_CODE_POINT; codePoint++) {
+ if (!isValidUtfCharacter(codePoint)) {
+ continue;
+ }
+ if (isInvalidManifestValueCharacter(codePoint)) {
+ continue;
+ }
+ numberOfUsedCodePoints++;
+
+ byte[] charBuf =
+ new
String(Character.toChars(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));
+ }
+
+ if (numberOfUsedCodePoints !=
+ (17 << 16) /* utf space */
+ - 66 /* non-characters */
+ - 3 /* nul, cr, lf */) {
+ fail("self-test: utf character set not covered exactly");
+ }
+
+ return values;
+ }
+
+ /**
+ * returns simple, valid, short, and distinct manifest header names.
+ * The returned name cannot be "{@code Manifest-Version}" because the
+ * returned string does not contain "{@code -}".
+ *
+ * @param seed seed to produce distinct names
+ */
+ static String 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 name.toString();
+ }
+
+ /**
+ * covers writing and reading of manifests with all known utf
characters.
+ *
+ * Because the implementation used different portions of code
depending on
+ * where the value occurs to read or write in earlier versions, each
+ * character is tested in each of the three positions:<ul>
+ * <li>main attribute header,</li>
+ * <li>named section name, which is in fact a header value after a
blank
+ * line, and</li>
+ * <li>named sections header values</li>
+ * <ul>
+ */
+ @Test
+ public void testValueUtfEncoding() throws IOException {
+ Manifest mf = new Manifest();
+ mf.getMainAttributes().put(Name.MANIFEST_VERSION, "1.0");
+
+ List<String> values = produceValidManifestUtfCharacterValues();
+ for (int i = 0; i < values.size(); i++) {
+ String name = azName(i);
+ String value = values.get(i);
+
+ mf.getMainAttributes().put(new Name(name), value);
+ Attributes attributes = new Attributes();
+ mf.getEntries().put(value, attributes);
+ attributes.put(new Name(name), value);
+ }
+
+ mf = writeAndRead(mf);
+
+ for (int i = 0; i < values.size(); i++) {
+ String value = values.get(i);
+ String name = azName(i);
+
+ assertEquals(mf.getMainAttributes().getValue(name), value,
+ "main attributes header value");
+ Attributes attributes = mf.getAttributes(value);
+ assertNotNull(attributes, "named section not found");
+ 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();
+
+ 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 2ace90aec488
test/jdk/sun/security/tools/jarsigner/LineBrokenMultiByteCharacter.java
---
a/test/jdk/sun/security/tools/jarsigner/LineBrokenMultiByteCharacter.java
Mon Apr 30 21:56:54 2018 -0400
+++
b/test/jdk/sun/security/tools/jarsigner/LineBrokenMultiByteCharacter.java
Wed May 02 07:20:46 2018 +0200
@@ -21,35 +21,48 @@
* questions.
*/
-/*
- * @test
- * @bug 6695402
- * @summary verify signatures of jars containing classes with names
- * with multi-byte unicode characters broken across lines
- * @library /test/lib
- */
-
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Paths;
+import java.util.HashMap;
+import java.util.jar.Attributes.Name;
import java.util.jar.JarFile;
-import java.util.jar.Attributes.Name;
import java.util.jar.JarEntry;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
import static java.nio.charset.StandardCharsets.UTF_8;
import jdk.test.lib.SecurityTools;
import jdk.test.lib.util.JarUtils;
+import org.testng.annotations.*;
+import static org.testng.Assert.*;
+
+/**
+ * @test
+ * @bug 6695402
+ * @library /test/lib
+ * @run testng LineBrokenMultiByteCharacter
+ * @summary verify signatures of jars containing classes with names
+ * with multi-byte unicode characters broken across lines
+ *
+ * Before utf characters were kept in one piece by bug 6443578 and
partially
+ * broken across a line break in manifests, bug 6695402 could occur
resulting
+ * in a signature considered invalid. That can still happen after bug
6443578
+ * fixed when signing manifests that already contain entries with
characters
+ * broken across a line break, presumably when an older manifest is present
+ * created with a different jdk.
+ */
public class LineBrokenMultiByteCharacter {
/**
- * this name will break across lines in MANIFEST.MF at the
+ * 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
* at its exact position.
- *
- * because no file with such a name exists {@link JarUtils} will
add the
+ * <p>
+ * Because no file with such a name exists {@link JarUtils} will
add the
* name itself as contents to the jar entry which would have
contained a
* compiled class in the original bug. For this test, the contents
of the
* files contained in the jar file is not important as long as
they get
@@ -58,55 +71,157 @@
* @see #verifyClassNameLineBroken(JarFile, String)
*/
static final String testClassName =
-
"LineBrokenMultiByteCharacterA1234567890B1234567890C123456789D1234\u00E9xyz.class";
+ "LineBrokenMultiByteCharacterA1234567890B1234567890C123456789"
+ + "D1234\u00E9xyz.class";
+ /**
+ * Used for a test case where an entry / class file is added to an
existing
+ * signed jar that already contains an entry with this name which
of course
+ * has to be distinct from the one tested.
+ */
static final String anotherName =
-
"LineBrokenMultiByteCharacterA1234567890B1234567890C123456789D1234567890.class";
+ "LineBrokenMultiByteCharacterA1234567890B1234567890C123456789"
+ + "D1234567890.class";
static final String alias = "a";
static final String keystoreFileName = "test.jks";
- static final String manifestFileName = "MANIFEST.MF";
+ static final String manifestFileName = JarFile.MANIFEST_NAME;
- public static void main(String[] args) throws Exception {
- prepare();
+ byte[] manifestNoDigest = "Manifest-Version:
1.0\r\n\r\n".getBytes(UTF_8);
+ byte[] manifestWithDigest; // with character broken across line break
- testSignJar("test.jar");
- testSignJarNoManifest("test-no-manifest.jar");
- testSignJarUpdate("test-update.jar", "test-updated.jar");
+ /**
+ * Simulate 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 character under test.
+ */
+ static byte[] rebreak(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;
}
- static void prepare() throws Exception {
+ @BeforeClass
+ public void prepare() throws Exception {
SecurityTools.keytool("-keystore", keystoreFileName,
"-genkeypair",
"-storepass", "changeit", "-keypass", "changeit",
"-storetype",
"JKS", "-alias", alias, "-dname", "CN=X", "-validity",
"366")
.shouldHaveExitValue(0);
- Files.write(Paths.get(manifestFileName), (Name.
- MANIFEST_VERSION.toString() + ":
1.0\r\n").getBytes(UTF_8));
+ String jarFileName = "reference-manifest.jar";
+ createJarWithManifest(jarFileName, manifestNoDigest,
+ testClassName, anotherName);
+ SecurityTools.jarsigner("-keystore", keystoreFileName,
"-storetype",
+ "JKS", "-storepass", "changeit", "-debug", jarFileName,
alias)
+ .shouldHaveExitValue(0);
+ try (ZipFile refJar = new ZipFile(jarFileName);) {
+ ZipEntry mfEntry = refJar.getEntry(JarFile.MANIFEST_NAME);
+ manifestWithDigest =
refJar.getInputStream(mfEntry).readAllBytes();
+ }
+ System.out.println("manifestWithDigest before re-break = " +
+ new String(manifestWithDigest, UTF_8));
+ manifestWithDigest = rebreak(manifestWithDigest);
+ System.out.println("manifestWithDigest after re-break = " +
+ new String(manifestWithDigest, UTF_8));
+ // now, manifestWithDigest is accepted as unmodified by
+ // 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
+ // signature verification will check it, which is the test case.
+
+ Files.createDirectory(Paths.get("META-INF/"));
}
- static void testSignJar(String jarFileName) throws Exception {
- JarUtils.createJar(jarFileName, manifestFileName, testClassName);
- verifyJarSignature(jarFileName);
+ @Test
+ public void testSignJarWithNoExistingClassEntry() throws Exception {
+ String jarFileName = "test-eacuteinonepiece.jar";
+ createJarWithManifest(jarFileName, manifestNoDigest,
testClassName);
+ signAndVerifyJarSignature(jarFileName, false);
}
- static void testSignJarNoManifest(String jarFileName) throws
Exception {
- JarUtils.createJar(jarFileName, testClassName);
- verifyJarSignature(jarFileName);
+ @Test
+ public void testSignJarWithBrokenEAcuteClassEntry() throws Exception {
+ String jarFileName = "test-brokeneacute.jar";
+ createJarWithManifest(jarFileName, manifestWithDigest,
testClassName);
+ signAndVerifyJarSignature(jarFileName, true);
}
- static void testSignJarUpdate(
- String initialFileName, String updatedFileName) throws
Exception {
- JarUtils.createJar(initialFileName, manifestFileName, anotherName);
+ @Test
+ public void testSignJarNoManifest() throws Exception {
+ String jarFileName = "test-no-manifest.jar";
+ JarUtils.createJar(jarFileName, testClassName);
+ signAndVerifyJarSignature(jarFileName, false);
+ }
+
+ @Test
+ public void testSignJarUpdateWithEAcuteClassEntryInOnePiece()
+ throws Exception {
+ String initialFileName = "test-eacuteinonepiece-update.jar";
+ String updatedFileName = "test-eacuteinonepiece-updated.jar";
+ createJarWithManifest(initialFileName, manifestNoDigest,
anotherName);
SecurityTools.jarsigner("-keystore", keystoreFileName,
"-storetype",
"JKS", "-storepass", "changeit", "-debug",
initialFileName,
alias).shouldHaveExitValue(0);
JarUtils.updateJar(initialFileName, updatedFileName,
testClassName);
- verifyJarSignature(updatedFileName);
+ signAndVerifyJarSignature(updatedFileName, false);
}
- static void verifyJarSignature(String jarFileName) throws Exception {
- // actually sign the jar
+ @Test
+ public void testSignJarUpdateWithBrokenEAcuteClassEntry()
+ throws Exception {
+ String initialFileName = "test-brokeneacute-update.jar";
+ String updatedFileName = "test-brokeneacute-updated.jar";
+ createJarWithManifest(initialFileName, manifestWithDigest,
anotherName);
+ SecurityTools.jarsigner("-keystore", keystoreFileName,
"-storetype",
+ "JKS", "-storepass", "changeit", "-debug", initialFileName,
+ alias).shouldHaveExitValue(0);
+ JarUtils.updateJar(initialFileName, updatedFileName,
testClassName);
+ signAndVerifyJarSignature(updatedFileName, true);
+ }
+
+ void signAndVerifyJarSignature(String jarFileName,
+ boolean expectBrokenEAcute) throws Exception {
SecurityTools.jarsigner("-keystore", keystoreFileName,
"-storetype",
"JKS", "-storepass", "changeit", "-debug",
jarFileName, alias)
.shouldHaveExitValue(0);
@@ -114,34 +229,34 @@
try (
JarFile jar = new JarFile(jarFileName);
) {
- verifyClassNameLineBroken(jar, testClassName);
+ verifyClassNameLineBroken(jar, testClassName,
expectBrokenEAcute);
verifyCodeSigners(jar, jar.getJarEntry(testClassName));
}
}
/**
- * it would be too easy to miss the actual test case by just
renaming an
+ * It would be too easy to miss the actual test case by just
renaming an
* identifier so that the multi-byte encoded character would not
any longer
* be broken across a line break.
*
- * this check here verifies that the actual test case is tested
based on
+ * This check here verifies that the actual test case is tested
based on
* the manifest and not based on the signature file because at the
moment,
* 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.
*/
- static void verifyClassNameLineBroken(JarFile jar, String className)
- throws IOException {
+ static void verifyClassNameLineBroken(JarFile jar, String className,
+ boolean expectBrokenEAcute) throws IOException {
byte[] eAcute = "\u00E9".getBytes(UTF_8);
byte[] eAcuteBroken =
new byte[] {eAcute[0], '\r', '\n', ' ', eAcute[1]};
- if (jar.getManifest().getAttributes(className) == null) {
- throw new AssertionError(className + " not found in manifest");
- }
+ assertNotNull(jar.getManifest().getAttributes(className),
+ className + " not found in manifest");
JarEntry manifestEntry = jar.getJarEntry(JarFile.MANIFEST_NAME);
+ System.out.println("expectBrokenEAcute = " + expectBrokenEAcute);
+ System.out.println("Manifest = \n" + new String(
+ jar.getInputStream(manifestEntry).readAllBytes(), UTF_8));
+
try (
InputStream manifestIs = jar.getInputStream(manifestEntry);
) {
@@ -156,10 +271,12 @@
bytesMatched = 0;
}
}
- if (bytesMatched < eAcuteBroken.length) {
- throw new AssertionError("self-test failed: multi-byte "
- + "utf-8 character not broken across lines");
- }
+ assertEquals(expectBrokenEAcute,
+ bytesMatched == eAcuteBroken.length,
+ "self-test failed: multi-byte "
+ + "utf-8 character "
+ + (expectBrokenEAcute ? "not " : "")
+ + "broken across lines");
}
}
@@ -175,11 +292,16 @@
// a check for the presence of code signers is sufficient to check
// bug JDK-6695402. no need to also verify the actual code signers
// attributes here.
- if (jarEntry.getCodeSigners() == null
- || jarEntry.getCodeSigners().length == 0) {
- throw new AssertionError(
- "no signing certificate found for " +
jarEntry.getName());
- }
+ assertTrue(jarEntry.getCodeSigners() != null
+ && jarEntry.getCodeSigners().length > 0,
+ "no signing certificate found for " + jarEntry.getName());
+ }
+
+ static void createJarWithManifest(String jarFileName, byte[] manifest,
+ String... files) throws IOException {
+ JarUtils.createJar("yetwithoutmanifest-" + jarFileName, files);
+ JarUtils.updateJar("yetwithoutmanifest-" + jarFileName,
jarFileName,
+ new HashMap<>() {{ put(manifestFileName, manifest); }});
}
}
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20180502/3367c096/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 6372077and6443578.patch
Type: text/x-patch
Size: 79654 bytes
Desc: not available
URL: <https://mail.openjdk.org/pipermail/security-dev/attachments/20180502/3367c096/6372077and6443578.patch>
More information about the security-dev
mailing list