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

Lance Andersen lancea at openjdk.org
Thu Nov 14 14:07:31 UTC 2024


On Thu, 14 Nov 2024 11:21:54 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.

Thank you for the PR Eirik, overall it looks OK.  See comments below WRT to exception messages in the test

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

> 222:         } catch (IOException e) {
> 223:             System.out.println("Caught " + e.getClass().getName() + "(" + e.getMessage() +
> 224:                     ") when running a privileged operation to get the default owner.");

Given we are not running a privileged operation, we should probably tweak the exception message

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

> 237:             return defaultOwner;
> 238:         } catch (IOException e) {
> 239:             System.out.println("Caught an exception when running a privileged operation to get the default group.");

Same comment WRT "privileged operation"

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

PR Review: https://git.openjdk.org/jdk/pull/22101#pullrequestreview-2436170504
PR Review Comment: https://git.openjdk.org/jdk/pull/22101#discussion_r1842273651
PR Review Comment: https://git.openjdk.org/jdk/pull/22101#discussion_r1842274986


More information about the core-libs-dev mailing list