[RFR] 8205525 : Improve exception messages during manifest parsing of jar archives (original) (raw)

Sean Mullan sean.mullan at oracle.com
Fri Aug 10 15:39:24 UTC 2018

I need more time to finish my review but here are some initial comments:

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

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

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.hegarty at oracle.com>; Alan Bateman <Alan.Bateman at oracle.com> Cc: core-libs-dev at openjdk.java.net; Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Langer, Christoph <christoph.langer at 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.hegarty at oracle.com> Sent: Donnerstag, 19. Juli 2018 14:54 To: Alan Bateman <Alan.Bateman at oracle.com>; Baesken, Matthias <matthias.baesken at sap.com> Cc: core-libs-dev at openjdk.java.net; Lindenmaier, Goetz <goetz.lindenmaier at 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.Bateman at 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.

More information about the core-libs-dev mailing list