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

[Baesken, Matthias]

Hi Max :

> 
> - No need to "import java.security.Security".

Sure  I can remove this, it is a leftover.

> - 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.
>

Currently we need it in Manifest.java  public void read(InputStream is) throws IOException { ... }

> 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.

Then lets just say   <jarfile>:<line-number>   (e.g.  test.jar:10 )

 Or ( to mention that it is about a manifest  from the jar )

<jarfile>:manifest-line <line-number>   (e.g.  test.jar:manifest-line 10 )


Best regards, Matthias



> -----Original Message-----
> From: Weijun Wang <weijun.wang at oracle.com>
> Sent: Freitag, 31. August 2018 04:32
> 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
> 
> 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/s
> hare/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