On Thu, 14 Nov 2024 11:21:54 GMT, Eirik Bjørsnøs <eir...@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
