RFR: 8302819: Remove JAR Index [v2]

Eirik Bjorsnos duke at openjdk.org
Thu Mar 30 11:17:12 UTC 2023

On Thu, 30 Mar 2023 07:00:47 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Eirik Bjorsnos has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 11 additional commits since the last revision:
>>  - Remove blank line 93 as per suggestion by Mandy
>>  - Merge branch 'master' into remove-jar-index
>>  - Remove JarIndex default constructor
>>  - Make JarIndex.INDEX_NAME package private
>>  - Remove outdated reference to URLClassLoader using JarIndex
>>  - Make JarIndex package private
>>  - The JarIndex.merge method ended up being used only by the JarIndexMergeTest test. Removing this test, the JarIndex.merge methods and a number of other JarIndex methods not used by the 'jar' tool
>>  - Revert the export of sun.security.action to jdk.jartool. JarIndex can use System.property instead since the 'jar' tool does not run with a SecurityManager
>>  - Revert noisy whitespace changes
>>  - Revert noisy whitespace changes
>>  - ... and 1 more: https://git.openjdk.org/jdk/compare/85c16dfb...2b5d8a4a
> src/java.base/share/classes/java/util/jar/JarFile.java line 212:
>> 210:     /**
>> 211:      * The 'JAR index' feature has been removed, but we still need to
>> 212:      * process existing files which have this entry.
> "but we still need ..."   Maybe better to say that that JarInputStream and the verification of signed JARs need to skip this attribute.

I rephrased this to be more specific about which code still needs this. Agree "we" is not the best choice of subject here.

> src/jdk.jartool/share/classes/sun/tools/jar/Main.java line 73:
>> 71: import jdk.internal.opt.CommandLine;
>> 72: 
>> 73: import java.time.LocalDateTime;
> This can move up to the imports of the other java.* classes.


> test/jdk/java/util/jar/JarFile/mrjar/TestVersionedStream.java line 29:
>> 27:  * @summary basic tests for multi-release jar versioned streams
>> 28:  * @library /test/lib
>> 29:  * @modules jdk.jartool/sun.tools.jar
> This test could be changed to use ToolProvider.findFirst("jar") and it would avoid needing to use sun.tools.jar.Main directly.

An improvement a bit unrelated to this PR, but it was a simple change so I have implemented it.


PR Review Comment: https://git.openjdk.org/jdk/pull/13158#discussion_r1153102163
PR Review Comment: https://git.openjdk.org/jdk/pull/13158#discussion_r1153102298
PR Review Comment: https://git.openjdk.org/jdk/pull/13158#discussion_r1153101442

More information about the security-dev mailing list