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