RFR: 8344183: (zipfs) SecurityManager cleanup in the ZipFS area [v2]

[Sean Mullan]

On Thu, 14 Nov 2024 14:43:50 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:

>> Please review this PR which cleans up SecurityManager-related code following JEP-486 integraion.
>> 
>> * `ZipFileSystem` is updated to not perform `AccessController::doPrivileged`
>> * `ZipFileSystemProvider` is updated to not perform `AccessController::doPrivileged`
>> *  The test `TestPosix` is updated to perform `AccessController.doPrivileged`
>> 
>> This change should be relatively straight-forward to review. Reviewers may want to look extra close at the exception-unwrapping code in `ZipFileSystem::initOwner` and `ZipFileSystem::initGroup`.
>> 
>> Testing: Tests in `test/jdk/jdk/nio/zipfs` run green locally.
>
> Eirik Bjørsnøs has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update debug logging to not reference privileged operations

src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 252:

> 250:         if (o == null) {
> 251:             try {
> 252:                 PrivilegedExceptionAction<UserPrincipal> pa = ()->Files.getOwner(zfpath);

Can you also remove the `@SuppressWarnings("removal")` on line 239?

src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 294:

> 292:                     return defaultOwner::getName;
> 293:                 }
> 294:                 PrivilegedExceptionAction<GroupPrincipal> pa = ()->zfpv.readAttributes().group();

Can you also remove the @SuppressWarnings("removal") on line 269?

src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java line 483:

> 481:         beginWrite();                // lock and sync
> 482:         try {
> 483:             AccessController.doPrivileged((PrivilegedExceptionAction<Void>)() -> {

Can you also remove the @SuppressWarnings("removal") on line 442?

src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystemProvider.java line 317:

> 315: 
> 316:     //////////////////////////////////////////////////////////////
> 317:     @SuppressWarnings("removal")

You can remove the annotation now.

test/jdk/jdk/nio/zipfs/TestPosix.java line 236:

> 234:             }
> 235:             return zfpv.readAttributes().group().getName();
> 236:         } catch (UnsupportedOperationException | NoSuchFileException e) {

In the prior code, I think `NoSuchFileException` would have been the cause of the `SecurityException`, so the code would have returned `null`. Just wondering why you changed it here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22101#discussion_r1842463056
PR Review Comment: https://git.openjdk.org/jdk/pull/22101#discussion_r1842467210
PR Review Comment: https://git.openjdk.org/jdk/pull/22101#discussion_r1842466546
PR Review Comment: https://git.openjdk.org/jdk/pull/22101#discussion_r1842445637
PR Review Comment: https://git.openjdk.org/jdk/pull/22101#discussion_r1842456357