On Wed, 4 Dec 2024 15:46:28 GMT, Jaikiran Pai <j...@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