On Thu, 14 Nov 2024 13:21:33 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
>> Please review this PR which cleans up security manager related code in
>> `java.util.zip` and `java.util.jar`:
>>
>> * `JarFile` and `ZipFile` are updated to use `System::getProperty` instead
>> of `GetPropertyAction::privilegedGetProperty`
>> * `ZipFile` is updated to not call SM::checkRead, SM::checkDelete when
>> opening files
>> * `ZipOutputStream` is updated to use `Boolean::getBoolean` instead of
>> `GetBooleanAction::privilegedGetProperty`
>>
>> The field `ZipFile.startsWithLoc` is deliberately left alone, that should be
>> handled separately. I found no SM-dependent code in the ZIP or JAR tests.
>>
>> Testing: This is a cleanup PR, no tests are changed or updated. ZIP and JAR
>> tests run green locally. GHA results pending.
>
> Eirik Bjørsnøs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fold lines for System::getProperty when reading enableMultiRelease and
> inhibitZip64
Looks good, just 2 small comments.
src/java.base/share/classes/java/util/jar/JarFile.java line 182:
> 180: }
> 181: RUNTIME_VERSION =
> Runtime.Version.parse(Integer.toString(runtimeVersion));
> 182: String enableMultiRelease =
> System.getProperty("jdk.util.jar.enableMultiRelease", "true");
The line is a little long, can you break it in 2 lines?
src/java.base/share/classes/java/util/zip/ZipOutputStream.java line 59:
> 57: * some in jdk7.
> 58: */
> 59: private static final boolean inhibitZip64 =
> Boolean.getBoolean("jdk.util.zip.inhibitZip64");
The line is a little long, can you break it in 2 lines?
-------------
PR Review: https://git.openjdk.org/jdk/pull/22099#pullrequestreview-2437022553
PR Review Comment: https://git.openjdk.org/jdk/pull/22099#discussion_r1842786359
PR Review Comment: https://git.openjdk.org/jdk/pull/22099#discussion_r1842782569
