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.
