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 <[email protected]> > Sent: Mittwoch, 29. August 2018 16:27 > To: Baesken, Matthias <[email protected]> > Cc: Alan Bateman <[email protected]>; Sean Mullan > <[email protected]>; Chris Hegarty <[email protected]>; > [email protected]; [email protected] > 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 > <[email protected]> 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 <[email protected]> > >> Sent: Montag, 27. August 2018 17:35 > >> To: Baesken, Matthias <[email protected]> > >> Cc: Alan Bateman <[email protected]>; Sean Mullan > >> <[email protected]>; Chris Hegarty <[email protected]>; > >> [email protected]; [email protected] > >> Subject: Re: [RFR] 8205525 : Improve exception messages during manifest > >> parsing of jar archives > >> > >> > >> > >>> On Aug 27, 2018, at 10:25 PM, Baesken, Matthias > >> <[email protected]> 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 <[email protected]> > >>>> Sent: Montag, 27. August 2018 15:52 > >>>> To: Baesken, Matthias <[email protected]>; Sean Mullan > >>>> <[email protected]>; Chris Hegarty > <[email protected]> > >>>> Cc: [email protected]; security Dev OpenJDK <security- > >>>> [email protected]> > >>>> 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 > >
