On Tue, 7 Sep 2021 03:34:27 GMT, wxiang 
<github.com+53162078+shiyu...@openjdk.org> wrote:

> There is a bug for URLClassPath.findResources with JarIndex.
> With some discussions about the bug, the current priority is to remove the 
> JAR index support in URLClassPath, 
> and don’t need to do anything to the jar tool in the short term, except just 
> to move JarIndex to the jdk.jartool module. 
> 
> The PR includes:
> 1. remove the JarIndex support in URLClassPath
> 2. move JarIndex into  jdk.jartool module.

Thanks for taking this on. I've done a pass over the changes and it looks 
complete, just a few issues. Next step will be to create a CSR for this change.

src/java.base/share/classes/java/util/jar/JarFile.java line 220:

> 218:      * The index file name.
> 219:      */
> 220:     public static final String INDEX_NAME = "META-INF/INDEX.LIST";

Adding this as a public field means it becomes part of the API, so it shouldn't 
be public here.

src/java.base/share/classes/java/util/jar/JarVerifier.java line 147:

> 145: 
> 146:                 if (uname.equals(JarFile.MANIFEST_NAME) ||
> 147:                         uname.equals(JarFile.INDEX_NAME)) {

It would be useful if someone from security-libs could comment on this. The 
interaction between signed JAR and JAR index isn't very clear. The change you 
have is safe but it might be that we can drop the checking for INDEX.LIST here.

test/jdk/sun/misc/JarIndex/JarIndexMergeTest.java line 44:

> 42: import sun.tools.jar.JarIndex;
> 43: 
> 44: public class JarIndexMergeTest {

Is this the one remaining test in sun/misc/JarIndex? I think it can be moved to 
test/jdk/java/util/jar. I think we should change the package name in the 
summary comment too.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5383

Reply via email to