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 >
