From 721fba818b97002deaf530e01c393b63ac4c4485 Mon Sep 17 00:00:00 2001 From: Roger Riggs Date: Wed, 9 Apr 2025 14:25:06 -0400 Subject: [PATCH 1/2] 8354053: Remove unused JavaIOFilePermissionAccess The interface is removed from SharedSecrets and its implementation moved to the java.io package where cross package access is not needed. The test is updated to access the internal implementation. Moderized the initialization of jdk.io.permissionsUseCanonicalPath. The remaining support can be removed when FilePermission is removed. --- .../share/classes/java/io/FilePermission.java | 115 ++++++++++-------- .../access/JavaIOFilePermissionAccess.java | 50 -------- .../jdk/internal/access/SharedSecrets.java | 15 --- .../sun/security/util/FilePermCompat.java | 80 ------------ .../FilePermissionCollectionMerge.java | 29 +++-- 5 files changed, 86 insertions(+), 203 deletions(-) delete mode 100644 src/java.base/share/classes/jdk/internal/access/JavaIOFilePermissionAccess.java delete mode 100644 src/java.base/share/classes/sun/security/util/FilePermCompat.java diff --git a/src/java.base/share/classes/java/io/FilePermission.java b/src/java.base/share/classes/java/io/FilePermission.java index aa6f11e00ee87..7310752d2e6b3 100644 --- a/src/java.base/share/classes/java/io/FilePermission.java +++ b/src/java.base/share/classes/java/io/FilePermission.java @@ -34,11 +34,9 @@ import java.util.Vector; import java.util.concurrent.ConcurrentHashMap; -import jdk.internal.access.JavaIOFilePermissionAccess; -import jdk.internal.access.SharedSecrets; import sun.nio.fs.DefaultFileSystemProvider; -import sun.security.util.FilePermCompat; import sun.security.util.SecurityConstants; +import sun.security.util.SecurityProperties; /** * This class represents access to a file or directory. A FilePermission consists @@ -155,6 +153,26 @@ public final class FilePermission extends Permission implements Serializable { private static final char RECURSIVE_CHAR = '-'; private static final char WILD_CHAR = '*'; + /** + * New behavior? Keep compatibility? + * The new behavior does not use the canonical path normalization + */ + private static final boolean nb = initNb(); + + // Initialize the nb flag from the System property jdk.io.permissionsUseCanonicalPath. + private static boolean initNb() { + String flag = SecurityProperties.getOverridableProperty( + "jdk.io.permissionsUseCanonicalPath"); + return switch (flag) { + case "true" -> false; // compatibility mode to canonicalize paths + case "false" -> true; // do not canonicalize + case null -> true; // default, do not canonicalize + default -> + throw new RuntimeException( + "Invalid jdk.io.permissionsUseCanonicalPath: " + flag); + }; + } + // public String toString() { // StringBuilder sb = new StringBuilder(); // sb.append("*** FilePermission on " + getName() + " ***"); @@ -232,51 +250,50 @@ private static Path altPath(Path in) { } } - static { - SharedSecrets.setJavaIOFilePermissionAccess( - /** - * Creates FilePermission objects with special internals. - * See {@link FilePermCompat#newPermPlusAltPath(Permission)} and - * {@link FilePermCompat#newPermUsingAltPath(Permission)}. - */ - new JavaIOFilePermissionAccess() { - public FilePermission newPermPlusAltPath(FilePermission input) { - if (!input.invalid && input.npath2 == null && !input.allFiles) { - Path npath2 = altPath(input.npath); - if (npath2 != null) { - // Please note the name of the new permission is - // different than the original so that when one is - // added to a FilePermissionCollection it will not - // be merged with the original one. - return new FilePermission(input.getName() + "#plus", - input, - input.npath, - npath2, - input.mask, - input.actions); - } - } - return input; - } - public FilePermission newPermUsingAltPath(FilePermission input) { - if (!input.invalid && !input.allFiles) { - Path npath2 = altPath(input.npath); - if (npath2 != null) { - // New name, see above. - return new FilePermission(input.getName() + "#using", - input, - npath2, - null, - input.mask, - input.actions); - } - } - return null; - } + // Construct a new Permission with altPath + // Package private for use by test FilePermissionCollectionMerge + static FilePermission newPermPlusAltPath(FilePermission input) { + if (!input.invalid && input.npath2 == null && !input.allFiles) { + Path npath2 = altPath(input.npath); + if (npath2 != null) { + // Please note the name of the new permission is + // different than the original so that when one is + // added to a FilePermissionCollection it will not + // be merged with the original one. + return new FilePermission(input.getName() + "#plus", + input, + input.npath, + npath2, + input.mask, + input.actions); } - ); + } + return input; } + // Construct a new Permission adding altPath + // Package private for use by test FilePermissionCollectionMerge + static FilePermission newPermUsingAltPath(FilePermission input) { + if (!nb) { + return input; + } + if (!input.invalid && !input.allFiles) { + Path npath2 = altPath(input.npath); + if (npath2 != null) { + // New name, see above. + return new FilePermission(input.getName() + "#using", + input, + npath2, + null, + input.mask, + input.actions); + } + } + return null; +} + + + /** * initialize a FilePermission object. Common to all constructors. * Also called during de-serialization. @@ -291,7 +308,7 @@ private void init(int mask) { if (mask == NONE) throw new IllegalArgumentException("invalid actions mask"); - if (FilePermCompat.nb) { + if (nb) { String name = getName(); if (name == null) @@ -567,7 +584,7 @@ boolean impliesIgnoreMask(FilePermission that) { if (that.allFiles) { return false; } - if (FilePermCompat.nb) { + if (nb) { // Left at least same level of wildness as right if ((this.recursive && that.recursive) != that.recursive || (this.directory && that.directory) != that.directory) { @@ -766,7 +783,7 @@ public boolean equals(Object obj) { if (this.invalid || that.invalid) { return false; } - if (FilePermCompat.nb) { + if (nb) { return (this.mask == that.mask) && (this.allFiles == that.allFiles) && this.npath.equals(that.npath) && @@ -789,7 +806,7 @@ public boolean equals(Object obj) { */ @Override public int hashCode() { - if (FilePermCompat.nb) { + if (nb) { return Objects.hash( mask, allFiles, directory, recursive, npath, npath2, invalid); } else { diff --git a/src/java.base/share/classes/jdk/internal/access/JavaIOFilePermissionAccess.java b/src/java.base/share/classes/jdk/internal/access/JavaIOFilePermissionAccess.java deleted file mode 100644 index ed9f88685d682..0000000000000 --- a/src/java.base/share/classes/jdk/internal/access/JavaIOFilePermissionAccess.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * Copyright (c) 2016, 2025, 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 - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. Oracle designates this - * particular file as subject to the "Classpath" exception as provided - * by Oracle in the LICENSE file that accompanied this code. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ -package jdk.internal.access; - -import java.io.FilePermission; - -public interface JavaIOFilePermissionAccess { - - /** - * Returns a new FilePermission plus an alternative path. - * - * @param input the input - * @return the new FilePermission plus the alt path (as npath2) - * or the input itself if no alt path is available. - */ - @SuppressWarnings("removal") - FilePermission newPermPlusAltPath(FilePermission input); - - /** - * Returns a new FilePermission using an alternative path. - * - * @param input the input - * @return the new FilePermission using the alt path (as npath) - * or null if no alt path is available - */ - @SuppressWarnings("removal") - FilePermission newPermUsingAltPath(FilePermission input); -} diff --git a/src/java.base/share/classes/jdk/internal/access/SharedSecrets.java b/src/java.base/share/classes/jdk/internal/access/SharedSecrets.java index 3ec9d6d4bae8c..bc955a76abb8e 100644 --- a/src/java.base/share/classes/jdk/internal/access/SharedSecrets.java +++ b/src/java.base/share/classes/jdk/internal/access/SharedSecrets.java @@ -69,7 +69,6 @@ public class SharedSecrets { private static JavaLangReflectAccess javaLangReflectAccess; private static JavaIOAccess javaIOAccess; private static JavaIOFileDescriptorAccess javaIOFileDescriptorAccess; - private static JavaIOFilePermissionAccess javaIOFilePermissionAccess; private static JavaIORandomAccessFileAccess javaIORandomAccessFileAccess; private static JavaObjectInputStreamReadString javaObjectInputStreamReadString; private static JavaObjectInputStreamAccess javaObjectInputStreamAccess; @@ -287,20 +286,6 @@ public static void setJavaIOFileDescriptorAccess(JavaIOFileDescriptorAccess jiof javaIOFileDescriptorAccess = jiofda; } - @SuppressWarnings("removal") - public static JavaIOFilePermissionAccess getJavaIOFilePermissionAccess() { - var access = javaIOFilePermissionAccess; - if (access == null) { - ensureClassInitialized(FilePermission.class); - access = javaIOFilePermissionAccess; - } - return access; - } - - public static void setJavaIOFilePermissionAccess(JavaIOFilePermissionAccess jiofpa) { - javaIOFilePermissionAccess = jiofpa; - } - public static JavaIOFileDescriptorAccess getJavaIOFileDescriptorAccess() { var access = javaIOFileDescriptorAccess; if (access == null) { diff --git a/src/java.base/share/classes/sun/security/util/FilePermCompat.java b/src/java.base/share/classes/sun/security/util/FilePermCompat.java deleted file mode 100644 index 651c84a462bef..0000000000000 --- a/src/java.base/share/classes/sun/security/util/FilePermCompat.java +++ /dev/null @@ -1,80 +0,0 @@ -/* - * Copyright (c) 2016, 2025, 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 - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. Oracle designates this - * particular file as subject to the "Classpath" exception as provided - * by Oracle in the LICENSE file that accompanied this code. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ - -package sun.security.util; - -import java.io.FilePermission; -import java.security.Permission; -import jdk.internal.access.SharedSecrets; - -/** - * Take care of FilePermission compatibility after JDK-8164705. - */ -public class FilePermCompat { - /** - * New behavior? Keep compatibility? Both default true. - */ - public static final boolean nb; - public static final boolean compat; - - static { - String flag = SecurityProperties.getOverridableProperty( - "jdk.io.permissionsUseCanonicalPath"); - if (flag == null) { - flag = "false"; - } - switch (flag) { - case "true": - nb = false; - compat = false; - break; - case "false": - nb = true; - compat = true; - break; - default: - throw new RuntimeException( - "Invalid jdk.io.permissionsUseCanonicalPath: " + flag); - } - } - - @SuppressWarnings("removal") - public static Permission newPermPlusAltPath(Permission input) { - if (compat && input instanceof FilePermission) { - return SharedSecrets.getJavaIOFilePermissionAccess() - .newPermPlusAltPath((FilePermission) input); - } - return input; - } - - @SuppressWarnings("removal") - public static Permission newPermUsingAltPath(Permission input) { - if (input instanceof FilePermission) { - return SharedSecrets.getJavaIOFilePermissionAccess() - .newPermUsingAltPath((FilePermission) input); - } - return null; - } -} diff --git a/test/jdk/java/io/FilePermission/FilePermissionCollectionMerge.java b/test/jdk/java/io/FilePermission/FilePermissionCollectionMerge.java index b8964931f3077..7bbf0acc3414e 100644 --- a/test/jdk/java/io/FilePermission/FilePermissionCollectionMerge.java +++ b/test/jdk/java/io/FilePermission/FilePermissionCollectionMerge.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2025, 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 @@ -24,19 +24,21 @@ /** * * @test - * @bug 8168127 + * @bug 8168127 8354053 * @summary FilePermissionCollection merges incorrectly - * @modules java.base/sun.security.util + * @modules java.base/java.io:open * @library /test/lib * @build jdk.test.lib.Asserts * @run main FilePermissionCollectionMerge */ -import sun.security.util.FilePermCompat; import java.io.FilePermission; +import java.lang.reflect.Method; import java.security.Permissions; + import jdk.test.lib.Asserts; +@SuppressWarnings("removal") public class FilePermissionCollectionMerge { public static void main(String[] args) throws Exception { @@ -50,13 +52,22 @@ public static void main(String[] args) throws Exception { test("/x/-"); } - static void test(String arg) { + static void test(String arg) throws Exception { + Method altPathMethod; + Method plusAltPathMethod; + try { + altPathMethod = FilePermission.class.getDeclaredMethod("newPermUsingAltPath", FilePermission.class); + altPathMethod.setAccessible(true); + plusAltPathMethod = FilePermission.class.getDeclaredMethod("newPermPlusAltPath", FilePermission.class); + plusAltPathMethod.setAccessible(true); + } catch (Exception ex) { + System.err.println("File permission compatibility initialization failed"); + throw ex; + } FilePermission fp1 = new FilePermission(arg, "read"); - FilePermission fp2 = (FilePermission) - FilePermCompat.newPermUsingAltPath(fp1); - FilePermission fp3 = (FilePermission) - FilePermCompat.newPermPlusAltPath(fp1); + FilePermission fp2 = (FilePermission) altPathMethod.invoke(null, fp1); + FilePermission fp3 = (FilePermission) plusAltPathMethod.invoke(null, fp1); // All 3 are different Asserts.assertNE(fp1, fp2); From 56680c47c5994212bbdcb564481ae601b1564ac8 Mon Sep 17 00:00:00 2001 From: Roger Riggs Date: Mon, 14 Apr 2025 12:09:16 -0400 Subject: [PATCH 2/2] Simplify as suggested in review comments Convert static methods to instance methods and invoke on the existing FilePermission. --- .../share/classes/java/io/FilePermission.java | 43 +++++++++---------- .../FilePermissionCollectionMerge.java | 8 ++-- 2 files changed, 25 insertions(+), 26 deletions(-) diff --git a/src/java.base/share/classes/java/io/FilePermission.java b/src/java.base/share/classes/java/io/FilePermission.java index 7310752d2e6b3..883f0410b6dc8 100644 --- a/src/java.base/share/classes/java/io/FilePermission.java +++ b/src/java.base/share/classes/java/io/FilePermission.java @@ -251,45 +251,44 @@ private static Path altPath(Path in) { } // Construct a new Permission with altPath - // Package private for use by test FilePermissionCollectionMerge - static FilePermission newPermPlusAltPath(FilePermission input) { - if (!input.invalid && input.npath2 == null && !input.allFiles) { - Path npath2 = altPath(input.npath); + // Used by test FilePermissionCollectionMerge + private FilePermission newPermPlusAltPath() { + System.err.println("PlusAlt path: " + this + ", npath: " + npath); + if (nb && !invalid && npath2 == null && !allFiles) { + Path npath2 = altPath(npath); if (npath2 != null) { // Please note the name of the new permission is // different than the original so that when one is // added to a FilePermissionCollection it will not // be merged with the original one. - return new FilePermission(input.getName() + "#plus", - input, - input.npath, + return new FilePermission(getName() + "#plus", + this, + npath, npath2, - input.mask, - input.actions); + mask, + actions); } } - return input; + return this; } // Construct a new Permission adding altPath - // Package private for use by test FilePermissionCollectionMerge - static FilePermission newPermUsingAltPath(FilePermission input) { - if (!nb) { - return input; - } - if (!input.invalid && !input.allFiles) { - Path npath2 = altPath(input.npath); + // Used by test FilePermissionCollectionMerge + private FilePermission newPermUsingAltPath() { + System.err.println("Alt path: " + this + ", npath: " + npath); + if (!invalid && !allFiles) { + Path npath2 = altPath(npath); if (npath2 != null) { // New name, see above. - return new FilePermission(input.getName() + "#using", - input, + return new FilePermission(getName() + "#using", + this, npath2, null, - input.mask, - input.actions); + mask, + actions); } } - return null; + return this; } diff --git a/test/jdk/java/io/FilePermission/FilePermissionCollectionMerge.java b/test/jdk/java/io/FilePermission/FilePermissionCollectionMerge.java index 7bbf0acc3414e..dde2131e78764 100644 --- a/test/jdk/java/io/FilePermission/FilePermissionCollectionMerge.java +++ b/test/jdk/java/io/FilePermission/FilePermissionCollectionMerge.java @@ -57,17 +57,17 @@ static void test(String arg) throws Exception { Method altPathMethod; Method plusAltPathMethod; try { - altPathMethod = FilePermission.class.getDeclaredMethod("newPermUsingAltPath", FilePermission.class); + altPathMethod = FilePermission.class.getDeclaredMethod("newPermUsingAltPath"); altPathMethod.setAccessible(true); - plusAltPathMethod = FilePermission.class.getDeclaredMethod("newPermPlusAltPath", FilePermission.class); + plusAltPathMethod = FilePermission.class.getDeclaredMethod("newPermPlusAltPath"); plusAltPathMethod.setAccessible(true); } catch (Exception ex) { System.err.println("File permission compatibility initialization failed"); throw ex; } FilePermission fp1 = new FilePermission(arg, "read"); - FilePermission fp2 = (FilePermission) altPathMethod.invoke(null, fp1); - FilePermission fp3 = (FilePermission) plusAltPathMethod.invoke(null, fp1); + FilePermission fp2 = (FilePermission) altPathMethod.invoke(fp1); + FilePermission fp3 = (FilePermission) plusAltPathMethod.invoke(fp1); // All 3 are different Asserts.assertNE(fp1, fp2);