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

Baesken, Matthias matthias.baesken at sap.com
Thu Aug 30 12:26:39 UTC 2018


Hi Max, 

I changed the following in the new webrev :
- added the  copyright+license header to the new file
- called trim on each token

- adjusted the comment java.security  like you suggested

http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.7/

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


Best regards, Matthias



> -----Original Message-----
> From: Weijun Wang <weijun.wang at oracle.com>
> Sent: Mittwoch, 29. August 2018 16:27
> 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
> 
> SecurityProperties.java:
> 
> - Please add the copyright+license header to the new file.
> 
> - The "jdk.includeInExceptions" definition says there can be "Leading and
> trailing whitespaces". You might need to call trim() on each token.
> 
> java.security:
> 
> - I would suggest saying "classes from the java.util.jar package" or just "when
> parsing a jar file".
> 
> Others:
> 
> - What will the output look like? Is it "/tmp/x.jar:100"? If I read correctly, the
> line number is of the manifest and not the jar file.
> 
> Thanks
> Max
> 
> > On Aug 29, 2018, at 10:01 PM, Baesken, Matthias
> <matthias.baesken at sap.com> wrote:
> >
> > Hi Max, thanks for your input .
> >
> > I created another webrev , this contains now   the suggested
> SecurityProperties   class :
> >
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.6/
> >
> > Best regards, Matthias
> >
> >
> >
> >
> >> -----Original Message-----
> >> From: Weijun Wang <weijun.wang at oracle.com>
> >> Sent: Montag, 27. August 2018 17:35
> >> 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 27, 2018, at 10:25 PM, Baesken, Matthias
> >> <matthias.baesken at sap.com> wrote:
> >>>
> >>> Hi Alan ,
> >>>
> >>>> Will sun.net.util.SocketExceptions be changed to use the supporting
> >>>> class or is that a separate issue?
> >>>>
> >>>
> >>> I think this is a separate issue .
> >>>
> >>>> In terms of approach then I think what you have is okay although I think
> >>>> we should try to find a better name than
> "SecuritySystemPropertyHelper"
> >>>> and also get feedback from security-dev on whether they would prefer
> it
> >>>> in the sun.security tree with the other classes security classes.
> >>>
> >>> Suggestions are welcome , I  was a bit unsure about the name as well 😉 !
> >>
> >> I am OK with creating a sun.security.util.SecurityProperties class, and a
> >> method called privilegeGetOverridable() for the 1st part of
> >> SecuritySystemPropertyHelper::initTextProperty (by overridable I mean a
> >> system property can shadow the security property). I have no idea where
> >> the 2nd part should go. Maybe you can just create an
> >> includedInExceptions(String refName) method for this purpose.
> >>
> >> So it will be something like this
> >>
> >> public class SecurityProperties {
> >>
> >>   public static String privilegeGetOverridable(String propName) {
> >>       return AccessController.doPrivileged((PrivilegedAction<String>)
> >>               () -> {
> >>                   String val = System.getProperty(propName);
> >>                   if (val == null) {
> >>                       return Security.getProperty(propName);
> >>                   } else {
> >>                       return val;
> >>                   }
> >>               });
> >>   }
> >>
> >>   public static boolean includedInExceptions(String refName) {
> >>       String val = privilegeGetOverridable("jdk.includeInExceptions");
> >>       if (val == null){
> >>           return false;
> >>       }
> >>       String[] tokens = val.split(",");
> >>       for (String token : tokens) {
> >>           if (token.equalsIgnoreCase(refName))
> >>               return true;
> >>       }
> >>       return false;
> >>   }
> >> }
> >>
> >> Thanks
> >> Max
> >>
> >>
> >>>
> >>> Thanks, Matthias
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Alan Bateman <Alan.Bateman at oracle.com>
> >>>> Sent: Montag, 27. August 2018 15:52
> >>>> To: Baesken, Matthias <matthias.baesken at sap.com>; Sean Mullan
> >>>> <sean.mullan at oracle.com>; Chris Hegarty
> <chris.hegarty at oracle.com>
> >>>> Cc: core-libs-dev at openjdk.java.net; security Dev OpenJDK <security-
> >>>> dev at openjdk.java.net>
> >>>> Subject: Re: [RFR] 8205525 : Improve exception messages during
> manifest
> >>>> parsing of jar archives
> >>>>
> >>>> On 24/08/2018 15:52, Baesken, Matthias wrote:
> >>>>> Hello, I created another webrev :
> >>>>>
> >>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.5/
> >>>>>
> >>>>> - use now  jarPath  instead of  jarpath  in the java security file
> >>>>> -  moved the parsing of the property  to a more general location
> >> (separate
> >>>> class jdk/internal/util/SecuritySystemPropertyHelper.java  )
> >>>>> -  read now  from the InputStream and check for error conditions
> >> *before*
> >>>> setting jarFilename    in the Manifest  constructor
> >>>>>
> >>>> Will sun.net.util.SocketExceptions be changed to use the supporting
> >>>> class or is that a separate issue?
> >>>>
> >>>> In terms of approach then I think what you have is okay although I think
> >>>> we should try to find a better name than
> "SecuritySystemPropertyHelper"
> >>>> and also get feedback from security-dev on whether they would prefer
> it
> >>>> in the sun.security tree with the other classes security classes.
> >>>>
> >>>> -Alan
> >



More information about the security-dev mailing list