Some more comments: Attributes.java
- No need to "import java.security.Security". - In the updated read() method, I think there is no need to use an "int offset" parameter. "int lineNumber" is enough and you can modify it and return it without a new local variable. - I feel like calling Attributes::getErrorPosition from Manifest a little strange. Maybe it's better to define the method in Manifest and call it from Attributes. First, I think putting the method in a higher level object makes more sense. Second, the position is for the whole Manifest and not an Attributes section (I mean the line number is counted from the first line of the manifest). > On Aug 30, 2018, at 10:50 PM, Baesken, Matthias <matthias.baes...@sap.com> > wrote: > > > > Hi Max, probably we should add the info about the MANIFEST.MF , for > example : > change getErrorPosition to > > http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.7/src/java.base/share/classes/java/util/jar/Attributes.java.udiff.html > > > static String getErrorPosition(String filename, final int lineNumber) { > if (filename == null || !jarPathInExceptionText) { > return "META-INF/MANIFEST.MF line:" + lineNumber; > } > > final File file = new File(filename); > return AccessController.doPrivileged(new PrivilegedAction<String>() { > public String run() { > return file.getAbsolutePath() + "!META-INF/MANIFEST.MF line:" > + lineNumber; I prefer "file:line" which is more compact and more commonly used. In fact, I have a small concern on the hardcoded file name here, because when verifying a signed jar, the META-INF/X.SF file is also parsed into a Manifest object and if it's invalid similar exceptions will be thrown. I don't have a nice solution now. if we want to show the .SF name also, we will need a public API because SignatureFileVerifier.java is inside sun.security.util. Something like Manifest(InputStream,jarName,entryName)? Sorry for making this complicated. Thanks Max > } > ..... > > > Best regards, Matthias > > > >> -----Original Message----- >> From: Weijun Wang <weijun.w...@oracle.com> >> Sent: Donnerstag, 30. August 2018 16:04 >> To: Baesken, Matthias <matthias.baes...@sap.com> >> Cc: Alan Bateman <alan.bate...@oracle.com>; Sean Mullan >> <sean.mul...@oracle.com>; Chris Hegarty <chris.hega...@oracle.com>; >> security-...@openjdk.java.net; core-libs-dev@openjdk.java.net >> Subject: Re: [RFR] 8205525 : Improve exception messages during manifest >> parsing of jar archives >> >> >> >>> On Aug 30, 2018, at 8:26 PM, Baesken, Matthias >> <matthias.baes...@sap.com> wrote: >>> >>>> - What will the output look like? Is it "/tmp/x.jar:100"? >>>> >>> >>> Yes it look like this : >>> >>> line too long (/testdata/jars/file_with_long_line_1.jar:2) >> >> Is this a little misleading? I think you mean >> >> /testdata/jars/file_with_long_line_1.jar!META-INF/MANIFEST.MF:2 >> >> Thanks >> Max
