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.baes...@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.w...@oracle.com>; Baesken, Matthias >> <matthias.baes...@sap.com> >> Cc: Sean Mullan <sean.mul...@oracle.com>; security- >> d...@openjdk.java.net; core-libs-...@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 >
