RFR: 8341551: Revisit jdk.internal.loader.URLClassPath.JarLoader after JEP 486 [v2]

Eirik Bjørsnøs eirbjo at openjdk.org
Wed Dec 4 16:28:59 UTC 2024


On Wed, 4 Dec 2024 15:46:28 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Can I please get a review of this change which proposes to address https://bugs.openjdk.org/browse/JDK-8341551? 
>> 
>> The primary work in this PR is the specification of the previously existing `sun.misc.URLClassPath.disableJarChecking` system property and how the internal implementation of `java.net.URLClassLoader` treats it. The complete details about this property is available in the CSR for this change here https://bugs.openjdk.org/browse/JDK-8345394.
>> 
>> A new jtreg test has been introduced to exercise the usage of this system property.
>
> Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision:
> 
>   improve code comment

src/java.base/share/classes/jdk/internal/loader/URLClassPath.java line 89:

> 87:         // JAR check is disabled by default and will be enabled only if the "disable JAR check"
> 88:         // system property has been set to "false".
> 89:         JAR_CHECKING_ENABLED = p != null && p.equals("false");

I thow `BooleanArithmeticOverflowException` :-)

I think this would be easier to follow if the static field name was aligned with the property name itself. It's easier if they have the same interpretation. 

So you could keep  `DISABLE_JAR_CHECKING` and just do 

`DISABLE_JAR_CHECKING = !"false".equals(p)`

and then just `if (DISABLE_JAR_CHECKING)` below.

If you don't like that, at least consider that the above can be simplified to `"false".equals(p)`

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22545#discussion_r1869893481


More information about the core-libs-dev mailing list