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

Weijun Wang weijun.wang at oracle.com
Tue Sep 11 11:03:33 UTC 2018


Attributes.java:

- Line 377: Too long, add a break.

Otherwise fine.

I don't have a strong opinion on making Manifest.jarFilename final or a different property name.

Thanks
Max

> On Sep 11, 2018, at 5:07 PM, Baesken, Matthias <matthias.baesken at sap.com> wrote:
> 
> Hello, please check the new webrev  :
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.10/
> 
> I kept the jarPath   category name .
> 
> Best regards, Matthias
> 
> 
>> -----Original Message-----
>> From: Langer, Christoph
>> Sent: Montag, 10. September 2018 21:30
>> To: Weijun Wang <weijun.wang at oracle.com>; Baesken, Matthias
>> <matthias.baesken at sap.com>
>> Cc: Sean Mullan <sean.mullan 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
>> 
>> Hi,
>> 
>>>> do you think we need property jdk.includeInExceptions=jar<File/Path> at
>>> all, if we don't resolve the absolute path?
>>> 
>>> I think so. File path is still sensitive.
>>> 
>>> In fact, I tend to believe people usually use absolute paths for JAR files (or
>>> maybe made absolute by using a file:// URL somewhere inside JDK). Do
>> you
>>> really see relative jar paths while testing this code change?
>> 
>> I agree.
>> 
>>>> small remark to the code:
>>>> src/java.base/share/classes/sun/security/util/SecurityProperties.java
>>>> 36     public static String privilegeGetOverridable(String propName) {
>>>> 
>>>> Should that method really be public? At the moment it doesn't seem to
>> be
>>> used outside of SecurityProperties.
>>> 
>>> I like it to be public. There are quite some other such system/security
>>> properties (Ex: jdk.serialFilter) that can make use of this method.
>> 
>> Ok, maybe it should be named "priviledgedGetOverridable" then.
>> 
>> @Matthias:
>> Further small cleanups, as you touch the files:
>> src/java.base/share/classes/java/util/jar/Manifest.java: line 35: you can
>> remove "import java.util.Iterator;"
>> 
>> src/java.base/share/classes/sun/net/util/SocketExceptions.java:
>> line 41: indentation is ridiculously long, I think it should be 8 chars
>> 
>> src/java.base/share/classes/sun/security/util/SecurityProperties.java:
>> Indentation of lines 38-45 is too deep, should be 12.
>> The 2 methods could use a brief Javadoc.
>> 
>> Best regards
>> Christoph
> 



More information about the core-libs-dev mailing list