RFR: 8302819: Remove JAR Index
Jaikiran Pai
jpai at openjdk.org
Tue Mar 28 17:57:43 UTC 2023
On Thu, 23 Mar 2023 14:33:19 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:
>> This PR removes the JAR index feature from the runtime:
>>
>> - `URLClassPath` is updated to remove the `enableJarIndex` system property and any code which would be called when this property was `true`
>> - The `JarIndex` implementation class is moved into `jdk.jartool` module.
>> - The `InvalidJarIndexError` exception class is removed because it falls out of use
>> - The test `test/jdk/sun/misc/JarIndex/metaInfFileNames/Basic.java` is removed because it depends on the JarIndex feature being present
>> - The test `test/jdk/sun/misc/JarIndex/JarIndexMergeForClassLoaderTest.java` is removed because it depends on the JarIndex feature being present
>> - The test `test/jdk/sun/misc/JarIndex/JarIndexMergeTest.java` is removed because it end up being the only caller of the JarIndex.merge feature
>> - All `JarIndex` methods/constructors which are not used by the `jar -i` implementation are removed.
>> - `JarIndex` is given package-private access.
>>
>> Outstanding code work:
>>
>> - Create tests for `JarFile` and `JarInputStream` accepting dusty INDEX jars.
>>
>> Outstanding work:
>>
>> - CSR for the removal
>> - Release notes for the removal
>> - Coordination of the update of the Jar File Specification
>
> Sorry my previous message wasn't clear.
>
>> Hooking in a simple deprecation warning in this PR would propably not require a lot of extra review cycles, perhaps the opposite. So I'm happy to do that if that is what you meant
>
> Yes, that's what I meant - I think we should add a warning message when that option is used, as part of these changes. I don't think we can completely remove that option in this release, since it wasn't deprecated (for removal) before, so it would be a good idea to start warning. Like you, I too would let Alan provide guidance on whether this should be done as part of this PR.
> @jaikiran Do you agree with the move of `JarIndex` from `java.base` to `jdk.jartool`? Since it ends up only being used there, I thought it would be cleaner if we moved it. But the move is not strictly necessary. What's your thoughts?
With the changes in this PR, the `JarIndex` (an internal) class would only be relevant/useful in the `jar` tool. So I think it makes sense to move it to the `jdk.jartool` module. Additionally you could even make it package-private there, instead of public. It would anyway need some updates to its javadoc and maybe remove any no longer used methods.
On a related note, I see that the `module-info.java` of `java.base` module has been updated to export `sun.security.action` package to the `jdk.jartool`. I think that is because the (now moved) `JarIndex` uses the `sun.security.action.GetPropertyAction.privilegedGetProperty` utility method to read a system property. I don't think we should be exporting that package. Instead, the `JarIndex` class can be updated to do `AccessController.doPrivileged(...)`.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/13158#issuecomment-1481371199
More information about the security-dev
mailing list