>As Chris and Alan mentioned, you should move the parsing of the property >to a more general location so it can be used by other code that uses >this property.
Hi Sean, Thanks for the input and comments . Could we do the moving of the property parsing do in a followup CR, I would prefer this . Best regards, Matthias > -----Original Message----- > From: Sean Mullan <sean.mul...@oracle.com> > Sent: Freitag, 10. August 2018 17:39 > To: Baesken, Matthias <matthias.baes...@sap.com>; Chris Hegarty > <chris.hega...@oracle.com>; Alan Bateman <alan.bate...@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 > > I need more time to finish my review but here are some initial comments: > > - java.security > > 1079 # jarpath - enables more detailed information in the IOExceptions > thrown > > Use "jarPath" to be consistent with "hostInfo". > > 1080 # by java.util.jar.Attributes and java.util.jar.Manifest > > and java.util.jar.JarFile > > - Manifest.java > > 57 private String jarFilename = null; > > Not necessary to set it to null, as that is the default. > > 82 Manifest(InputStream is, String jarFilename) throws IOException { > 83 this.jarFilename = jarFilename; > 84 read(is); > 85 } > > Read from the InputStream and check for error conditions *before* > setting jarFilename (switch lines 83 & 84). This is a general best > practice but can also protect against finalizer attacks. See JSCG 7-3 > for more info: > https://www.oracle.com/technetwork/java/seccodeguide-139067.html#7 > > - Attributes.java > > As Chris and Alan mentioned, you should move the parsing of the property > to a more general location so it can be used by other code that uses > this property. > > --Sean > > On 8/8/18 11:00 AM, Sean Mullan wrote: > > Cross-posting to security-dev since this fix is security related and > > should also be reviewed there. > > > > --Sean > > > > On 8/7/18 11:00 AM, Baesken, Matthias wrote: > >> Ping .... 😊 , any reviews / comments ? > >> > >> Thanks , Matthias > >> > >>> -----Original Message----- > >>> From: Baesken, Matthias > >>> Sent: Dienstag, 31. Juli 2018 12:28 > >>> To: 'Chris Hegarty' <chris.hega...@oracle.com>; Alan Bateman > >>> <alan.bate...@oracle.com> > >>> Cc: core-libs-dev@openjdk.java.net; Lindenmaier, Goetz > >>> <goetz.lindenma...@sap.com>; Langer, Christoph > >>> <christoph.lan...@sap.com> > >>> Subject: RE: [RFR] 8205525 : Improve exception messages during > manifest > >>> parsing of jar archives > >>> > >>> Hello , > >>> looks like the generalization of the `includeInExceptions` > >>> security property > >>> is now in jdk/jdk after > >>> > >>> "8207846: Generalize the jdk.net.includeInExceptions security property" > >>> > >>> was added, great news and thanks to Chris for driving this ! > >>> > >>> I use this security property now as well , and updated the change : > >>> > >>> http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.3/ > >>> > >>> I updated the CSR as well : > >>> > >>> https://bugs.openjdk.java.net/browse/JDK-8207768 > >>> > >>> > >>> Best regards, Matthias > >>> > >>> > >>> > >>>> -----Original Message----- > >>>> From: Chris Hegarty <chris.hega...@oracle.com> > >>>> Sent: Donnerstag, 19. Juli 2018 14:54 > >>>> To: Alan Bateman <alan.bate...@oracle.com>; Baesken, Matthias > >>>> <matthias.baes...@sap.com> > >>>> Cc: core-libs-dev@openjdk.java.net; Lindenmaier, Goetz > >>>> <goetz.lindenma...@sap.com> > >>>> Subject: Re: [RFR] 8205525 : Improve exception messages during > manifest > >>>> parsing of jar archives > >>>> > >>>> > >>>>> On 19 Jul 2018, at 11:46, Alan Bateman <alan.bate...@oracle.com> > >>>> wrote: > >>>>> > >>>>> On 19/07/2018 09:07, Baesken, Matthias wrote: > >>>>>> Hello, in the meantime I prepared a CSR : > >>>>>> > >>>>>> https://bugs.openjdk.java.net/browse/JDK-8207768 > >>>>>> > >>>>>> > >>>>>>> jdk.includeInExceptions expands the scope. That might be okay but > we > >>>>>>> will need to re-visit jdk.net.includeInExceptions and also move the > >>>>>>> support to somewhere in jdk.internal so that it can be used by > other > >>>>>>> code in java.base. > >>>>>> Is there some support code for " jdk.net.includeInExceptions " > >>>>>> or do > >>>> you just mean the name of the property ? > >>>>>> > >>>>> Chris is right that it's a bit premature to submit a CSR while the > >>>>> question > >>> on > >>>> whether to rename the existing security property is on the table. I > >>>> have no > >>>> objection to doing that. > >>>> > >>>> I filed the following issue to generalize the `includeInExceptions` > >>>> security > >>>> property: > >>>> https://bugs.openjdk.java.net/browse/JDK-8207846 > >>>> > >>>> I would like to resolve 8207846 first, then 8205525 can propose to > >>>> add the > >>>> new argument value, `jarPath`. > >>>> > >>>> -Chris.