[RFR] 8205525 : Improve exception messages during manifest parsing of jar archives

Weijun Wang weijun.wang at oracle.com
Fri Aug 31 02:32:08 UTC 2018


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.baesken at 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.wang at oracle.com>
>> Sent: Donnerstag, 30. August 2018 16:04
>> To: Baesken, Matthias <matthias.baesken at sap.com>
>> Cc: Alan Bateman <Alan.Bateman at oracle.com>; Sean Mullan
>> <sean.mullan at oracle.com>; Chris Hegarty <chris.hegarty at oracle.com>;
>> security-dev at openjdk.java.net; core-libs-dev at 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.baesken at 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



More information about the core-libs-dev mailing list