From bfee766f035fb1b122cd3f3703b9e2a2d85abfe6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eirik=20Bj=C3=B8rsn=C3=B8s?= Date: Fri, 15 Nov 2024 07:48:15 +0000 Subject: [PATCH] 8344183: (zipfs) SecurityManager cleanup in the ZipFS area Reviewed-by: mullan, lancea --- .../classes/jdk/nio/zipfs/ZipFileSystem.java | 58 +++++-------------- .../jdk/nio/zipfs/ZipFileSystemProvider.java | 15 +---- test/jdk/jdk/nio/zipfs/TestPosix.java | 37 +++++------- 3 files changed, 28 insertions(+), 82 deletions(-) diff --git a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java index 20ed86eae4f..9ea935551b8 100644 --- a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java +++ b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystem.java @@ -44,10 +44,6 @@ import java.nio.channels.WritableByteChannel; import java.nio.file.*; import java.nio.file.attribute.*; import java.nio.file.spi.FileSystemProvider; -import java.security.AccessController; -import java.security.PrivilegedAction; -import java.security.PrivilegedActionException; -import java.security.PrivilegedExceptionAction; import java.util.*; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; @@ -82,10 +78,8 @@ import static jdk.nio.zipfs.ZipUtils.*; */ class ZipFileSystem extends FileSystem { // statics - @SuppressWarnings("removal") - private static final boolean isWindows = AccessController.doPrivileged( - (PrivilegedAction)()->System.getProperty("os.name") - .startsWith("Windows")); + private static final boolean isWindows = System.getProperty("os.name") + .startsWith("Windows"); private static final byte[] ROOTPATH = new byte[] { '/' }; private static final String PROPERTY_POSIX = "enablePosixFileAttributes"; private static final String PROPERTY_DEFAULT_OWNER = "defaultOwner"; @@ -168,9 +162,7 @@ class ZipFileSystem extends FileSystem { } // sm and existence check zfpath.getFileSystem().provider().checkAccess(zfpath, AccessMode.READ); - @SuppressWarnings("removal") - boolean writeable = AccessController.doPrivileged( - (PrivilegedAction)()->Files.isWritable(zfpath)); + boolean writeable = Files.isWritable(zfpath); this.readOnly = !writeable; this.zc = ZipCoder.get(nameEncoding); this.rootdir = new ZipPath(this, new byte[]{'/'}); @@ -244,23 +236,14 @@ class ZipFileSystem extends FileSystem { // If not specified in env, it is the owner of the archive. If no owner can // be determined, we try to go with system property "user.name". If that's not // accessible, we return "". - @SuppressWarnings("removal") private UserPrincipal initOwner(Path zfpath, Map env) throws IOException { Object o = env.get(PROPERTY_DEFAULT_OWNER); if (o == null) { try { - PrivilegedExceptionAction pa = ()->Files.getOwner(zfpath); - return AccessController.doPrivileged(pa); - } catch (UnsupportedOperationException | PrivilegedActionException e) { - if (e instanceof UnsupportedOperationException || - e.getCause() instanceof NoSuchFileException) - { - PrivilegedAction pa = ()->System.getProperty("user.name"); - String userName = AccessController.doPrivileged(pa); - return ()->userName; - } else { - throw new IOException(e); - } + return Files.getOwner(zfpath); + } catch (UnsupportedOperationException | NoSuchFileException e) { + String userName = System.getProperty("user.name"); + return ()->userName; } } if (o instanceof String) { @@ -282,7 +265,6 @@ class ZipFileSystem extends FileSystem { // If not specified in env, we try to determine the group of the zip archive itself. // If this is not possible/unsupported, we will return a group principal going by // the same name as the default owner. - @SuppressWarnings("removal") private GroupPrincipal initGroup(Path zfpath, Map env) throws IOException { Object o = env.get(PROPERTY_DEFAULT_GROUP); if (o == null) { @@ -291,16 +273,9 @@ class ZipFileSystem extends FileSystem { if (zfpv == null) { return defaultOwner::getName; } - PrivilegedExceptionAction pa = ()->zfpv.readAttributes().group(); - return AccessController.doPrivileged(pa); - } catch (UnsupportedOperationException | PrivilegedActionException e) { - if (e instanceof UnsupportedOperationException || - e.getCause() instanceof NoSuchFileException) - { - return defaultOwner::getName; - } else { - throw new IOException(e); - } + return zfpv.readAttributes().group(); + } catch (UnsupportedOperationException | NoSuchFileException e) { + return defaultOwner::getName; } } if (o instanceof String) { @@ -462,7 +437,6 @@ class ZipFileSystem extends FileSystem { return (path)->pattern.matcher(path.toString()).matches(); } - @SuppressWarnings("removal") @Override public void close() throws IOException { beginWrite(); @@ -480,13 +454,9 @@ class ZipFileSystem extends FileSystem { } beginWrite(); // lock and sync try { - AccessController.doPrivileged((PrivilegedExceptionAction)() -> { - sync(); return null; - }); + sync(); ch.close(); // close the ch just in case no update // and sync didn't close the ch - } catch (PrivilegedActionException e) { - throw (IOException)e.getException(); } finally { endWrite(); } @@ -512,10 +482,8 @@ class ZipFileSystem extends FileSystem { synchronized (tmppaths) { for (Path p : tmppaths) { try { - AccessController.doPrivileged( - (PrivilegedExceptionAction)() -> Files.deleteIfExists(p)); - } catch (PrivilegedActionException e) { - IOException x = (IOException)e.getException(); + Files.deleteIfExists(p); + } catch (IOException x) { if (ioe == null) ioe = x; else diff --git a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystemProvider.java b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystemProvider.java index 615d2a42e20..a8336b0f8f6 100644 --- a/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystemProvider.java +++ b/src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipFileSystemProvider.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2022, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2009, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -39,9 +39,6 @@ import java.nio.file.attribute.BasicFileAttributes; import java.nio.file.attribute.FileAttribute; import java.nio.file.attribute.FileAttributeView; import java.nio.file.spi.FileSystemProvider; -import java.security.AccessController; -import java.security.PrivilegedActionException; -import java.security.PrivilegedExceptionAction; import java.util.HashMap; import java.util.Map; import java.util.Set; @@ -317,17 +314,9 @@ public class ZipFileSystemProvider extends FileSystemProvider { } ////////////////////////////////////////////////////////////// - @SuppressWarnings("removal") void removeFileSystem(Path zfpath, ZipFileSystem zfs) throws IOException { synchronized (filesystems) { - Path tempPath = zfpath; - PrivilegedExceptionAction action = tempPath::toRealPath; - try { - zfpath = AccessController.doPrivileged(action); - } catch (PrivilegedActionException e) { - throw (IOException) e.getException(); - } - filesystems.remove(zfpath, zfs); + filesystems.remove(zfpath.toRealPath(), zfs); } } } diff --git a/test/jdk/jdk/nio/zipfs/TestPosix.java b/test/jdk/jdk/nio/zipfs/TestPosix.java index f63f091c32e..420c1618e12 100644 --- a/test/jdk/jdk/nio/zipfs/TestPosix.java +++ b/test/jdk/jdk/nio/zipfs/TestPosix.java @@ -29,10 +29,6 @@ import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.nio.file.*; import java.nio.file.attribute.*; -import java.security.AccessController; -import java.security.PrivilegedAction; -import java.security.PrivilegedActionException; -import java.security.PrivilegedExceptionAction; import java.time.Instant; import java.util.*; import java.util.concurrent.atomic.AtomicInteger; @@ -219,35 +215,28 @@ public class TestPosix { private static String expectedDefaultOwner(Path zf) { try { - try { - PrivilegedExceptionAction pa = ()->Files.getOwner(zf).getName(); - return AccessController.doPrivileged(pa); - } catch (UnsupportedOperationException e) { - // if we can't get the owner of the file, we fall back to system property user.name - PrivilegedAction pa = ()->System.getProperty("user.name"); - return AccessController.doPrivileged(pa); - } - } catch (PrivilegedActionException | SecurityException e) { + return Files.getOwner(zf).getName(); + } catch (UnsupportedOperationException e) { + // if we can't get the owner of the file, we fall back to system property user.name + return System.getProperty("user.name"); + } catch (IOException e) { System.out.println("Caught " + e.getClass().getName() + "(" + e.getMessage() + - ") when running a privileged operation to get the default owner."); + ") when getting the default owner."); return null; } } private static String expectedDefaultGroup(Path zf, String defaultOwner) { try { - try { - PosixFileAttributeView zfpv = Files.getFileAttributeView(zf, PosixFileAttributeView.class); - if (zfpv == null) { - return defaultOwner; - } - PrivilegedExceptionAction pa = ()->zfpv.readAttributes().group().getName(); - return AccessController.doPrivileged(pa); - } catch (UnsupportedOperationException e) { + PosixFileAttributeView zfpv = Files.getFileAttributeView(zf, PosixFileAttributeView.class); + if (zfpv == null) { return defaultOwner; } - } catch (PrivilegedActionException | SecurityException e) { - System.out.println("Caught an exception when running a privileged operation to get the default group."); + return zfpv.readAttributes().group().getName(); + } catch (UnsupportedOperationException e) { + return defaultOwner; + } catch (IOException e) { + System.out.println("Caught an exception when getting the default group."); e.printStackTrace(); return null; }