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

[Weijun Wang]

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
>