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.w...@oracle.com> > Sent: Mittwoch, 29. August 2018 16:27 > 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 > > 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.baes...@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.w...@oracle.com> > >> Sent: Montag, 27. August 2018 17:35 > >> 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 27, 2018, at 10:25 PM, Baesken, Matthias > >> <matthias.baes...@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.bate...@oracle.com> > >>>> Sent: Montag, 27. August 2018 15:52 > >>>> To: Baesken, Matthias <matthias.baes...@sap.com>; Sean Mullan > >>>> <sean.mul...@oracle.com>; Chris Hegarty > <chris.hega...@oracle.com> > >>>> Cc: core-libs-dev@openjdk.java.net; security Dev OpenJDK <security- > >>>> d...@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 > >
