Hi,
Manifest parsing an input stream given to the constructor or the read
method ignores the contents of the last line unless terminated with a
line break.
In course of my attempt to fix 8217375, I found that Manifest ignores
the last line when not terminated with a line break whereas
ManifestDigester throws an exception.
Without ManifestDigester's different behavior, I'd have opted for
parsing the last line also without a line break (or the last line being
empty).
But ManifestDigester cannot easily be changed that way, too, because
the line breaks are part of the digested manifest portions.
I don't think Manifest should accept any manifests that cannot later as
well be signed and should therefore raise an error when there is no
line break at the end of a manifest.
Adding a new error condition adds potential for compatibility issues.
On the other hand, I don't think the last 'ill'-terminated line should
really be ignored silently.
A good example how things can go wrong this way is
JavaClassPathTest.java. It uses a one-line manifest without a trailing
line break and m1.jar does not contain the expected Class-Path entry.
In the long term the best option would probably be to re-unite
Manifest's and ManifestDigester's parsing-related code the biggest
challenge of which is not to change digests and thereby break existing
jar signatures.
Attached is a patch with a proposed fix.
The bug number in
test/jdk/java/util/jar/Manifest/NoLineBreakAtEndOfManifestFile.java is
from another bug that lead me to it. If someone creates another bug it
can be replaced. I haven't found an existing issue that matches.
In test/jdk/tools/launcher/modules/classpath/JavaClassPathTest.java is
a test case testJARManifestClassPathAttribute/testClassPathAttribute
which I have no clue how this should really work. As far as I can tell
the test is and was wrong but nevertheless passes.
Is there a chance to find a sponsor for fixing this?
Regards,
Philipp
diff -r 7c17199fa37d src/java.base/share/classes/java/util/jar/Attributes.java
--- a/src/java.base/share/classes/java/util/jar/Attributes.java Fri Feb 15 08:21:08 2019 -0500
+++ b/src/java.base/share/classes/java/util/jar/Attributes.java Fri Feb 15 21:57:03 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2019, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -367,7 +367,7 @@
byte[] lastline = null;
int len;
- while ((len = is.readLine(lbuf)) != -1) {
+ while ((len = is.readLine(lbuf)) > 0) {
boolean lineContinued = false;
byte c = lbuf[--len];
lineNumber++;
diff -r 7c17199fa37d src/java.base/share/classes/java/util/jar/Manifest.java
--- a/src/java.base/share/classes/java/util/jar/Manifest.java Fri Feb 15 08:21:08 2019 -0500
+++ b/src/java.base/share/classes/java/util/jar/Manifest.java Fri Feb 15 21:57:03 2019 +0100
@@ -299,12 +299,13 @@
boolean skipEmptyLines = true;
byte[] lastline = null;
- while ((len = fis.readLine(lbuf)) != -1) {
+ while ((len = fis.readLine(lbuf)) > 0) {
byte c = lbuf[--len];
lineNumber++;
if (c != '\n' && c != '\r') {
- throw new IOException("manifest line too long ("
+ throw new IOException("manifest line too long"
+ + " or no line break on last line ("
+ getErrorPosition(jarFilename, lineNumber) + ")");
}
if (len > 0 && lbuf[len-1] == '\r') {
@@ -461,7 +462,10 @@
/*
* Reads 'len' bytes from the input stream, or until an end-of-line
- * is reached. Returns the number of bytes read.
+ * or the end of the input stream is reached.
+ * Returns the number of bytes read. If that returned number of bytes
+ * read is zero, the end of the stream has been reached
+ * and zero is returned as the number of read bytes.
*/
public int readLine(byte[] b, int off, int len) throws IOException {
byte[] tbuf = this.buf;
@@ -472,7 +476,7 @@
fill();
avail = count - pos;
if (avail <= 0) {
- return -1;
+ break;
}
}
int n = len - total;
diff -r 7c17199fa37d test/jdk/java/util/jar/Manifest/IncludeInExceptionsTest.java
--- a/test/jdk/java/util/jar/Manifest/IncludeInExceptionsTest.java Fri Feb 15 08:21:08 2019 -0500
+++ b/test/jdk/java/util/jar/Manifest/IncludeInExceptionsTest.java Fri Feb 15 21:57:03 2019 +0100
@@ -21,7 +21,6 @@
* questions.
*/
-import java.io.File;
import java.io.IOException;
import java.io.OutputStream;
import java.nio.file.Files;
diff -r 7c17199fa37d test/jdk/java/util/jar/Manifest/NoLineBreakAtEndOfManifestFile.java
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/util/jar/Manifest/NoLineBreakAtEndOfManifestFile.java Fri Feb 15 21:57:03 2019 +0100
@@ -0,0 +1,71 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.util.jar.Manifest;
+import java.util.jar.Attributes.Name;
+
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.testng.Assert.*;
+
+/**
+ * @test
+ * @bug 8217375
+ * @run testng NoLineBreakAtEndOfManifestFile
+ * @summary Check that manifests don't just ignore the last line if not
+ * properly terminated with a line break.
+ */
+public class NoLineBreakAtEndOfManifestFile {
+
+ @DataProvider(name = "invalidManifests")
+ public static Object[][] invalidManifests() {
+ return new Object[][] {
+ { Name.MANIFEST_VERSION + ": 1.0" },
+ { Name.MANIFEST_VERSION + ": 1.0\r\n" +
+ "\r\n" +
+ "Name: Section Name\r\n" +
+ "X: Y" }
+ };
+ }
+
+ @Test(dataProvider = "invalidManifests")
+ public void testConstructor(String manifest) {
+ assertThrows(IOException.class, () -> {
+ new Manifest(
+ new ByteArrayInputStream(manifest.getBytes(UTF_8)));
+ });
+ }
+
+ @Test(dataProvider = "invalidManifests")
+ public void testRead(String manifest) {
+ assertThrows(IOException.class, () -> {
+ new Manifest().read(
+ new ByteArrayInputStream(manifest.getBytes(UTF_8)));
+ });
+ }
+
+}
diff -r 7c17199fa37d test/jdk/tools/launcher/modules/classpath/JavaClassPathTest.java
--- a/test/jdk/tools/launcher/modules/classpath/JavaClassPathTest.java Fri Feb 15 08:21:08 2019 -0500
+++ b/test/jdk/tools/launcher/modules/classpath/JavaClassPathTest.java Fri Feb 15 21:57:03 2019 +0100
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -21,13 +21,14 @@
* questions.
*/
-import java.io.BufferedWriter;
+import java.io.PrintWriter;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
+import java.util.jar.Attributes;
import java.util.spi.ToolProvider;
import java.util.stream.Stream;
@@ -86,13 +87,13 @@
file.toString()) == 0);
Path manifest = LIB_DIR.resolve("manifest");
- try (BufferedWriter writer = Files.newBufferedWriter(manifest)) {
- writer.write("CLASS-PATH: lib/m.jar");
+ try (PrintWriter writer = new PrintWriter(manifest.toFile())) {
+ writer.println(Attributes.Name.CLASS_PATH + ": lib/m.jar");
}
- jarfile = LIB_DIR.resolve("m1.jar");
+ Path cpJarfile = LIB_DIR.resolve("m-cp.jar");
assertTrue(jartool.run(System.out, System.err, "cfme",
- jarfile.toString(), manifest.toString(), TEST_MAIN,
- file.toString()) == 0);
+ cpJarfile.toString(), manifest.toString(),
+ TEST_MAIN, file.toString()) == 0);
}
@DataProvider(name = "classpath")
@@ -144,7 +145,6 @@
args.add(Boolean.toString(false));
args.add(expected);
-
assertTrue(execute(args).getExitValue() == 0);
}
@@ -175,17 +175,17 @@
}
/*
- * Test CLASS-PATH attribute in manifest
+ * Test "Class-Path" attribute in manifest
*/
@Test
- public void testClassPathAttribute() throws Throwable {
- String jarfile = LIB_DIR.resolve("m1.jar").toString();
+ public void testJARManifestClassPathAttribute() throws Throwable {
+ String jarfile = LIB_DIR.resolve("m-cp.jar").toString();
List<String> args = new ArrayList<>();
args.add("-jar");
args.add(jarfile);
args.add(Boolean.toString(false));
- args.add(jarfile);
+ args.add(jarfile); // not the class path specified in the manifest
assertTrue(execute(args).getExitValue() == 0);
@@ -197,6 +197,10 @@
args.add(jarfile);
assertTrue(execute(args).getExitValue() == 0);
+
+ // TODO: How exactly does this test show that the JAR manifest
+ // Class-Path makes a difference or has any effect?
+ // I haven't understood that...
}
private OutputAnalyzer execute(List<String> options) throws Throwable {
diff -r 7c17199fa37d test/jdk/tools/launcher/modules/classpath/src/m/jdk/test/Main.java
--- a/test/jdk/tools/launcher/modules/classpath/src/m/jdk/test/Main.java Fri Feb 15 08:21:08 2019 -0500
+++ b/test/jdk/tools/launcher/modules/classpath/src/m/jdk/test/Main.java Fri Feb 15 21:57:03 2019 +0100
@@ -1,5 +1,5 @@
-/**
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+/*
+ * Copyright (c) 2016, 2019, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -29,9 +29,10 @@
static final String JAVA_CLASS_PATH = "java.class.path";
public static void main(String[] args) throws Exception {
- boolean expected = args[0].equals("true");
+ boolean expected = Boolean.parseBoolean(args[0]);
String cpath = args.length > 1 ? args[1] : "";
String value = System.getProperty(JAVA_CLASS_PATH);
+ System.out.println(JAVA_CLASS_PATH + " = " + value);
if (!value.equals(cpath)) {
throw new RuntimeException(JAVA_CLASS_PATH + "=" + value +
" expected=" + cpath);
@@ -39,8 +40,11 @@
ClassLoader loader = ClassLoader.getSystemClassLoader();
URL url = loader.getResource("jdk/test/res.properties");
+ System.out.println("jdk/test/res.properties " +
+ (url != null ? "present" : "not present") + " in class path");
if ((expected && url == null) || (!expected && url != null)) {
- throw new RuntimeException("URL: " + url + " expected non-null: " + expected);
+ throw new RuntimeException("URL: " + url +
+ " expected non-null: " + expected);
}
}
}
