Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit07219f7

Browse files
author
duke
committed
Added webrev for jdk/24465
1 parent2fb7923 commit07219f7

File tree

3 files changed

+3
-0
lines changed

3 files changed

+3
-0
lines changed

‎jdk/24465/12/commits.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
[{"commit":{"message":"Address review comments"},"files":[{"filename":"src\/java.base\/share\/classes\/java\/security\/Security.java"}],"sha":"4e7206c082e4b0152d999b32eba0063a506eff67"},{"commit":{"message":"Slightly improve ConfigFileTestDirPermissions\n\nExtract restrictedAcl() AutoCloseable and also use AutoCloseable for the\ntemporary directory cleanup."},"files":[{"filename":"test\/jdk\/java\/security\/Security\/ConfigFileTestDirPermissions.java"}],"sha":"29c88c00aae065ee2a3f13271436892dce269e61"},{"commit":{"message":"Go back to java.nio.file.Path::toRealPath\n\nAfter JDK-8355342 (1f08a3ede2445fb05d9700a1293d681ca89cbf5b)\nFile::getCanonicalFile is no longer resolving symlinks in Windows, plus\nthere is some reluctance from core-libs maintainers about relying on\nspecific java.io.File behaviour with regard to parent directories\npermissions.\n\nOn the other hand, path resolution has been moved to a place where it's\nstrictly needed (when resolving a relative include directive), and this\nshould mitigate the majority of the use-cases, being an improvement with\nrespect to the current status quo.\n\nGiven the previous, I think that Path::toRealPath in its current\nlocation is a good compromise.\n\nThis also reverts commit 7abb62c069ad35f4ec34f6cd9b9f6d05febceecc, since\nwe are no longer supporting that use-case."},"files":[{"filename":"src\/java.base\/share\/classes\/java\/security\/Security.java"},{"filename":"test\/jdk\/java\/security\/Security\/ConfigFileTestDirPermissions.java"}],"sha":"e80a28ea787b74f22f62f24a09bbf37c8cdbce8a"},{"commit":{"message":"Merge openjdk:master into franferrax:JDK-8352728"},"files":[],"sha":"6811268ed1cd9343a36bedd1d479572e36bcb0e0"},{"commit":{"message":"Merge openjdk:master into franferrax:JDK-8352728"},"files":[],"sha":"b5063f0040187aa21cd2d320623e593db611a058"},{"commit":{"message":"Do symlinks resolution in a single place\n\nRemove path normalization from debugging messages, only keep it where\nstrictly necessary.\n\nAdjust test case to the new debugging messages which contain absolute\nbut not normalized paths (can contain some ..\/..\/ elements)."},"files":[{"filename":"src\/java.base\/share\/classes\/java\/security\/Security.java"},{"filename":"test\/jdk\/java\/security\/Security\/ConfigFileTest.java"}],"sha":"7c80874c25bc99783ad24fb22d2c080d33c5503a"},{"commit":{"message":"Use the right OutputAnalyzer method\n\nOutputAnalyzer::stderrMatches returns a boolean while\nOutputAnalyzer::stderrShouldMatch performs the check."},"files":[{"filename":"test\/jdk\/java\/security\/Security\/ConfigFileTest.java"}],"sha":"4483469e453100259aa28d900943c34a4ab994b4"},{"commit":{"message":"Remove path resolution from exception message"},"files":[{"filename":"src\/java.base\/share\/classes\/java\/security\/Security.java"},{"filename":"test\/jdk\/java\/security\/Security\/ConfigFileTest.java"}],"sha":"40bc832ef22670a6ba2481fb53d2ebe4b3bbd434"},{"commit":{"message":"Detect cyclic includes with Files::isSameFile\n\ncheckCyclicInclude() is invoked after we successfully get an InputStream\nfor the path to avoid skipping the same IOException several times inside\ncheckCyclicInclude() if the path doesn't exist.\n\nAlso, perform symlinks resolution only in the following cases:\n • When we need to resolve a relative include\n • For clarity to the user in logging messages\n • For clarity to the user in exception messages\n\nIn the first case, the resolution is a requirement, in the last two\ncases it is a nice-to-have. But given the last two are exceptional\ncases anyway, we let any resolution error bubble up."},"files":[{"filename":"src\/java.base\/share\/classes\/java\/security\/Security.java"}],"sha":"a8d865c4985e6660655b27df28e76882855b2087"},{"commit":{"message":"Merge openjdk:master into franferrax:JDK-8352728"},"files":[],"sha":"4e77fb36d23fd51d529e49e4e5c362133f2beb16"},{"commit":{"message":"Introduce a shell test for anonymous pipes\n\nThis use case has been discussed and analyzed in the pull request, but\nwe didn't have a test case for it. By introducing a test, we make sure\nwe don't have regressions in this area, regardless of the alternative\nwe choose to advance with for this fix."},"files":[{"filename":"test\/jdk\/java\/security\/Security\/ConfigFileTestAnonymousPipes.sh"}],"sha":"71718e522feaf8e6404c526588b400d3073b26c2"},{"commit":{"message":"Merge openjdk:master into franferrax:JDK-8352728"},"files":[],"sha":"4cfce61b8752e6dd25f2d9fca002f9c6e72768fc"},{"commit":{"message":"Fix typo"},"files":[{"filename":"src\/java.base\/share\/classes\/java\/security\/Security.java"}],"sha":"35933475549a36b19ce196410d93450d48471cba"},{"commit":{"message":"Merge openjdk:master into franferrax:JDK-8352728"},"files":[],"sha":"fc25514c9126cff5f0c1e46db2278d6cca8fe442"},{"commit":{"message":"Reintroduce links test using directory junctions\n\nJunctions do not require elevation, so this is a way of testing\nsoft-links are resolved without requiring elevation. This is useful\nbecause we need to avoid elevation in order to reproduce the parent\ndirectories permission issue.\n\nThis is testing directories structure:\n\n 📁 JDK-8352728-tmp-*\/\n ├─🔒 jdk-parent-dir\/ (ACL with REMOVED-PERMISSIONS)\n │ └─📁 jdk\/\n │ ├─📁 conf\/\n │ │ ├─📁 security\/\n │ │ │ ├─📄 java.security\n │ │ │ │ 📝 include link-to-other-dir\/other.properties\n │ │ │ ├─🔗 link-to-other-dir\/ ⟹ 📁 JDK-8352728-tmp-*\/other-dir\n │ │ │ └─... (JUNCTION)\n │ │ └─...\n │ └─...\n ├─📁 other-dir\/\n │ └─📄 other.properties\n │ 📝 include ..\/relatively.included.properties\n └─📄 relatively.included.properties\n 📝 test.property.name=test_property_value"},"files":[{"filename":"test\/jdk\/java\/security\/Security\/ConfigFileTestDirPermissions.java"}],"sha":"7abb62c069ad35f4ec34f6cd9b9f6d05febceecc"},{"commit":{"message":"Do minor adjustments\n\nUpdate copyright year, improve comments and use File::toPath to convert\nback to Path."},"files":[{"filename":"src\/java.base\/share\/classes\/java\/security\/Security.java"}],"sha":"a8c5ca745298a7efdef23bc7416bf2aacd284143"},{"commit":{"message":"Merge openjdk\/master into JDK-8352728"},"files":[],"sha":"7047a994219c569d3ce2163586c4b8d42f941c08"},{"commit":{"message":"8352728: InternalError loading java.security due to Windows parent folder permissions"},"files":[{"filename":"src\/java.base\/share\/classes\/java\/security\/Security.java"},{"filename":"test\/jdk\/java\/security\/Security\/ConfigFileTestDirPermissions.java"}],"sha":"308479dd767e162e0d8e16018cf19563ff9b6203"}]

‎jdk/24465/12/comparison.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"files":[{"patch":"@@ -2,1 +2,1 @@\n- * Copyright (c) 1996, 2024, Oracle and\/or its affiliates. All rights reserved.\n+ * Copyright (c) 1996, 2025, Oracle and\/or its affiliates. All rights reserved.\n@@ -37,0 +37,1 @@\n+import java.util.ArrayList;\n@@ -115,1 +116,1 @@\n- private static final Set<Path> activePaths = new HashSet<>();\n+ private static final List<Path> activePaths = new ArrayList<>();\n@@ -256,1 +257,4 @@\n- path = currentPath.resolveSibling(path);\n+ \/\/ We perform symlinks resolution on currentPath under the\n+ \/\/ rationale that the original file writer is the one who\n+ \/\/ decided where the relative includes should resolve.\n+ path = currentPath.toRealPath().resolveSibling(path);\n@@ -265,0 +269,14 @@\n+ private static void checkCyclicInclude(Path path) {\n+ for (Path activePath : activePaths) {\n+ try {\n+ if (Files.isSameFile(path, activePath)) {\n+ throw new InternalError(\n+ \"Cyclic include of '\" + path + \"'\");\n+ }\n+ } catch (IOException e) {\n+ throw new InternalError(\"Failed to check cyclic \" +\n+ \"inclusion of '\" + path + \"'\", e);\n+ }\n+ }\n+ }\n+\n@@ -267,4 +285,1 @@\n- boolean isRegularFile = Files.isRegularFile(path);\n- if (isRegularFile) {\n- path = path.toRealPath();\n- } else if (Files.isDirectory(path)) {\n+ if (Files.isDirectory(path)) {\n@@ -272,5 +287,0 @@\n- } else {\n- path = path.toAbsolutePath();\n- }\n- if (activePaths.contains(path)) {\n- throw new InternalError(\"Cyclic include of '\" + path + \"'\");\n@@ -279,0 +289,1 @@\n+ checkCyclicInclude(path);\n@@ -281,1 +292,1 @@\n- currentPath = isRegularFile ? path : null;\n+ currentPath = Files.isRegularFile(path) ? path : null;\n@@ -288,1 +299,1 @@\n- activePaths.remove(path);\n+ activePaths.removeLast();\n","filename":"src\/java.base\/share\/classes\/java\/security\/Security.java","additions":25,"deletions":14,"binary":false,"changes":39,"status":"modified"},{"patch":"@@ -397,2 +397,3 @@\n- ex.assertError(\n- \"InternalError: Cyclic include of '\" + masterFile.path + \"'\");\n+ ex.assertError(\"Cyclic include\");\n+ ex.getOutputAnalyzer().stderrShouldMatch(\"\\\\QInternalError: Cyclic \" +\n+ \"include of '\\\\E[^']+\\\\Q\" + masterFile.fileName + \"'\\\\E\");\n@@ -445,0 +446,1 @@\n+ final Path displayPath;\n@@ -446,1 +448,1 @@\n- private Include(PropsFile propsFile, String value) {\n+ private Include(PropsFile propsFile, String value, Path displayPath) {\n@@ -449,0 +451,6 @@\n+ this.displayPath = displayPath;\n+ }\n+\n+ static Include ofAbsolute(PropsFile propsFile) {\n+ return new Include(propsFile, propsFile.path.toString(),\n+ propsFile.path);\n@@ -451,2 +459,3 @@\n- static Include of(PropsFile propsFile) {\n- return new Include(propsFile, propsFile.path.toString());\n+ static Include ofRelative(PropsFile propsFile, Path baseDir) {\n+ Path rel = baseDir.relativize(propsFile.path);\n+ return new Include(propsFile, rel.toString(), baseDir.resolve(rel));\n@@ -455,2 +464,2 @@\n- static Include of(PropsFile propsFile, String value) {\n- return new Include(propsFile, value);\n+ Include withNewValue(String newValue) {\n+ return new Include(propsFile, newValue, displayPath);\n@@ -512,1 +521,1 @@\n- addIncludeDefinition(Include.of(propsFile));\n+ addIncludeDefinition(Include.ofAbsolute(propsFile));\n@@ -516,2 +525,1 @@\n- addIncludeDefinition(Include.of(propsFile,\n- path.getParent().relativize(propsFile.path).toString()));\n+ addIncludeDefinition(Include.ofRelative(propsFile, path.getParent()));\n@@ -526,1 +534,1 @@\n- oa.shouldContain(\"finished processing \" + include.propsFile.path);\n+ oa.shouldContain(\"finished processing \" + include.displayPath);\n@@ -538,1 +546,1 @@\n- oa.shouldContain(\"finished processing \" + include.propsFile.path);\n+ oa.shouldContain(\"finished processing \" + include.displayPath);\n@@ -573,1 +581,1 @@\n- include = Include.of(include.propsFile,\n+ include = include.withNewValue(\n","filename":"test\/jdk\/java\/security\/Security\/ConfigFileTest.java","additions":21,"deletions":13,"binary":false,"changes":34,"status":"modified"},{"patch":"@@ -0,0 +1,61 @@\n+#\n+# Copyright (c) 2025, Red Hat, Inc.\n+#\n+# DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.\n+#\n+# This code is free software; you can redistribute it and\/or modify it\n+# under the terms of the GNU General Public License version 2 only, as\n+# published by the Free Software Foundation.\n+#\n+# This code is distributed in the hope that it will be useful, but WITHOUT\n+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or\n+# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License\n+# version 2 for more details (a copy is included in the LICENSE file that\n+# accompanied this code).\n+#\n+# You should have received a copy of the GNU General Public License version\n+# 2 along with this work; if not, write to the Free Software Foundation,\n+# Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.\n+#\n+# Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA\n+# or visit www.oracle.com if you need additional information or have any\n+# questions.\n+#\n+\n+# @test\n+# @summary Ensures the java executable is able to load extra security\n+# properties files from anonymous pipes (non-regular files).\n+# @bug 8352728\n+# @requires os.family == \"linux\"\n+# @run shell\/timeout=30 ConfigFileTestAnonymousPipes.sh\n+\n+if [ -z \"${TESTJAVA}\" ]; then\n+ JAVA=java\n+else\n+ JAVA=\"${TESTJAVA}\/bin\/java\"\n+fi\n+\n+TEST_PROP=\"ConfigFileTestAnonymousPipes.property.name=PROPERTY_VALUE\"\n+\n+function check_java() {\n+ local java_output java_exit_code\n+ java_output=\"$(\"${JAVA}\" ${TESTVMOPTS} ${TESTJAVAOPTS} -XshowSettings:security:properties \\\n+ -Djava.security.debug=properties \"$@\" -version 2>&1)\"\n+ java_exit_code=$?\n+ if [ ${java_exit_code} -ne 0 ] || ! grep -qF \"${TEST_PROP}\" <<<\"${java_output}\"; then\n+ echo \"TEST FAILED (java exit code: ${java_exit_code})\"\n+ echo \"${java_output}\"\n+ exit 1\n+ fi\n+}\n+\n+# https:\/\/www.gnu.org\/software\/bash\/manual\/bash.html#Pipelines\n+echo \"Extra properties from pipeline\"\n+echo \"${TEST_PROP}\" | check_java -Djava.security.properties=\/dev\/stdin || exit 1\n+\n+# https:\/\/www.gnu.org\/software\/bash\/manual\/bash.html#Process-Substitution\n+echo \"Extra properties from process-substitution, include other from pipeline\"\n+echo \"${TEST_PROP}\" | check_java -Djava.security.properties=<(echo \"include \/dev\/stdin\") || exit 1\n+\n+echo \"TEST PASS - OK\"\n+exit 0\n","filename":"test\/jdk\/java\/security\/Security\/ConfigFileTestAnonymousPipes.sh","additions":61,"deletions":0,"binary":false,"changes":61,"status":"added"},{"patch":"@@ -0,0 +1,84 @@\n+\/*\n+ * Copyright (c) 2025, Red Hat, Inc.\n+ *\n+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.\n+ *\n+ * This code is free software; you can redistribute it and\/or modify it\n+ * under the terms of the GNU General Public License version 2 only, as\n+ * published by the Free Software Foundation.\n+ *\n+ * This code is distributed in the hope that it will be useful, but WITHOUT\n+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or\n+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License\n+ * version 2 for more details (a copy is included in the LICENSE file that\n+ * accompanied this code).\n+ *\n+ * You should have received a copy of the GNU General Public License version\n+ * 2 along with this work; if not, write to the Free Software Foundation,\n+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.\n+ *\n+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA\n+ * or visit www.oracle.com if you need additional information or have any\n+ * questions.\n+ *\/\n+\n+import jdk.test.lib.process.ProcessTools;\n+import jdk.test.lib.util.FileUtils;\n+\n+import java.io.IOException;\n+import java.nio.file.Files;\n+import java.nio.file.Path;\n+import java.nio.file.attribute.AclEntry;\n+import java.nio.file.attribute.AclEntryType;\n+import java.nio.file.attribute.AclFileAttributeView;\n+import java.util.List;\n+\n+\/*\n+ * @test\n+ * @summary Ensures java.security is loadable in Windows, even when the user\n+ * does not have permissions on one of the parent directories.\n+ * @bug 8352728\n+ * @requires os.family == \"windows\"\n+ * @library \/test\/lib\n+ * @run main ConfigFileTestDirPermissions\n+ *\/\n+\n+public class ConfigFileTestDirPermissions {\n+ private static AutoCloseable restrictedAcl(Path path) throws IOException {\n+ AclFileAttributeView view =\n+ Files.getFileAttributeView(path, AclFileAttributeView.class);\n+ List<AclEntry> originalAcl = List.copyOf(view.getAcl());\n+ view.setAcl(List.of(AclEntry.newBuilder().setType(AclEntryType.DENY)\n+ .setPrincipal(Files.getOwner(path)).build()));\n+ return () -> view.setAcl(originalAcl);\n+ }\n+\n+ public static void main(String[] args) throws Exception {\n+ Path temp = Files.createTempDirectory(\"JDK-8352728-tmp-\");\n+ try (AutoCloseable a1 = () -> FileUtils.deleteFileTreeUnchecked(temp)) {\n+ \/\/ Copy the jdk to a different directory\n+ Path originalJdk = Path.of(System.getProperty(\"test.jdk\"));\n+ Path jdk = temp.resolve(\"jdk-parent-dir\", \"jdk\");\n+ Files.createDirectories(jdk);\n+ FileUtils.copyDirectory(originalJdk, jdk);\n+\n+ \/\/ Remove current user permissions from jdk-parent-dir\n+ try (AutoCloseable a2 = restrictedAcl(jdk.getParent())) {\n+ \/\/ Make sure the permissions are affecting the current user\n+ try {\n+ jdk.toRealPath();\n+ throw new jtreg.SkippedException(\"Must run non-elevated!\");\n+ } catch (IOException expected) { }\n+\n+ \/\/ Execute the copied jdk, ensuring java.security.Security is\n+ \/\/ loaded (i.e. use -XshowSettings:security:properties)\n+ ProcessTools.executeProcess(new ProcessBuilder(\n+ List.of(jdk.resolve(\"bin\", \"java.exe\").toString(),\n+ \"-Djava.security.debug=properties\",\n+ \"-XshowSettings:security:properties\",\n+ \"-version\"))).shouldHaveExitValue(0);\n+ }\n+ }\n+ System.out.println(\"TEST PASS - OK\");\n+ }\n+}\n","filename":"test\/jdk\/java\/security\/Security\/ConfigFileTestDirPermissions.java","additions":84,"deletions":0,"binary":false,"changes":84,"status":"added"}]}

‎jdk/24465/12/metadata.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"head":{"repo":{"full_name":"franferrax\/jdk","html_url":"https:\/\/github.com\/franferrax\/jdk"},"sha":"4e7206c082e4b0152d999b32eba0063a506eff67"},"created_at":"2025-11-28T18:35:01.513117032Z","base":{"repo":{"full_name":"openjdk\/jdk","html_url":"https:\/\/git.openjdk.org\/jdk"},"sha":"c028369dcb0a677541b89117b0800125bc7c6c33"}}

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp