> I don't have a strong opinion on making Manifest.jarFilename final Hi, just making it final leads to compile errors anyway.
Best regards, Matthias > -----Original Message----- > From: Weijun Wang <weijun.w...@oracle.com> > Sent: Dienstag, 11. September 2018 13:04 > To: Baesken, Matthias <matthias.baes...@sap.com> > Cc: Langer, Christoph <christoph.lan...@sap.com>; Sean Mullan > <sean.mul...@oracle.com>; security-dev@openjdk.java.net; core-libs- > d...@openjdk.java.net > Subject: Re: [RFR] 8205525 : Improve exception messages during manifest > parsing of jar archives > > 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 > >