Bug ID: JDK-8214827 Incorrect call ClassLoaders.toFileURL("jrt:/java.compiler")

Type: Bug
Component: hotspot
Sub-Component: runtime
Affected Version: 10,11,12

Priority: P3
Status: Resolved
Resolution: Fixed

Submitted: 2018-12-05
Updated: 2019-11-22
Resolved: 2019-01-10

JDK 11	JDK 12	JDK 13
11.0.3-oracleFixed	12 b28Fixed	13Fixed

ClassLoaders.toFileURL is called only from hotspot, apparently from appcds.
It should only  be called with a String that is an actual filename, but it gets called with the String "jrt:/java.compiler" which seems wrong.  toFileURL canonicalizes the input String, causing undesired risky I/O.

Recipe:
Apply a patch to get toFileURL to throw:
--- a/src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java
+++ b/src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java
@@ -219,6 +219,7 @@
      */
     @Deprecated
     private static URL toFileURL(String s) {
+        if (s.contains(":")) throw new Error("toFileURL(" + s + ")");
         try {
             // Use an intermediate File object to construct a URI/URL without
             // authority component as URLClassPath can't handle URLs with a UNC

and then
cd test/hotspot/jtreg
and run the :hotspot_appcds tests, resulting in 8 failures.
Here's one:
TEST: runtime/appcds/jigsaw/overridetests/OverrideTests.java
...
 stdout: [];
 stderr: [openjdk version "12-internal" 2019-03-19
OpenJDK Runtime Environment (build 12-internal+0-adhoc.martinrb.toFileURL)
OpenJDK 64-Bit Server VM (build 12-internal+0-adhoc.martinrb.toFileURL, mixed mode, sharing)
Exception in thread "main" java.lang.Error: toFileURL(jrt:/jdk.compiler)
	at java.base/jdk.internal.loader.ClassLoaders.toFileURL(ClassLoaders.java:223)
	at java.base/java.lang.ClassLoader.findLoadedClass0(Native Method)
	at java.base/java.lang.ClassLoader.findLoadedClass(ClassLoader.java:1280)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:595)
	at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:581)
	at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:408)
	at test/jdk.test.Main.main(Main.java:73)
]
 exitValue = 1

Fix Request This patch improves AppCDS reliability. Patch applies to 11u with conflicts in tests. See 11u RFR for details about details of the merge and testing: https://mail.openjdk.java.net/pipermail/jdk-updates-dev/2019-February/000504.html
19-02-2019
I updated the 'Fix Version' to 12. Please let me know if anyone sees any issue, thanks.
09-01-2019
Here is the new webrev that detects jrt URL in CDS/vm code and calls the URL constructor instead: http://cr.openjdk.java.net/~jiangli/8214827/webrev.01/
09-01-2019
[~martin]Thanks for the suggestion! I was just discussing the change with Alan via email and proposed to name the method to 'toURL()' as well. Then the method can handle both file URL and jrt URL. Alan also suggested the alternative to do the check for "jrt:/" in hotspot code and have it call the URL constructor when it has a jrt URL.
09-01-2019
Having the method take either a file string or a jrt URL string as input is a bit strange. Better to have two methods or maybe the CDS code could just invoke the URL constructor then the string starts with "jrt:/".
09-01-2019
I took a look at the change to ClassLoaders.java. ClassLoaders.toFileURL is a hotspot-only API, but ... With a name like toFileURL, it should only ever return URLs with a "file" protocol, but that is not the case. Perhaps it should be renamed, or perhaps the test for "jrt:" should be made in hotspot before calling toFileURL? Hotspot should know whether it is looking in the jrt filesystem before converting to a String. There's nothing to stop a regular file from having a name that happens to start with "jrt:"
09-01-2019
I opened the above comments as this is not a security issue. Minimum permission is used when the CodeSource URL is null.
09-01-2019
I finally had time to look into this issue this week. When "jrt:/java.compiler" is given, a null URL is used due to the error in toFileURL(), which causes a wrong ProtectionDomain object being used for archived app classes from the runtime image at runtime. - ProtectionDomain used for archived com.sun.tools.javac.Main (an app class in the runtime image): ProtectionDomain (null <no signer certificates>) jdk.internal.loader.ClassLoaders$AppClassLoader@8bcc55f <no principals> java.security.Permissions@9807454 ( ("java.lang.RuntimePermission" "exitVM") ) - ProtectionDomain for non-shared classes from the runtime image: ProtectionDomain (jrt:/jdk.compiler <no signer certificates>) jdk.internal.loader.ClassLoaders$AppClassLoader@8bcc55f <no principals> java.security.Permissions@3d494fbf ( ("java.lang.RuntimePermission" "exitVM") ("java.lang.RuntimePermission" "accessSystemModules") ) I went back and checked JDK 10 and 11, the ProtectionDomain support for archived app classes in runtime images were broken as well in those binaries. I checked JDK 9 binary and confirmed JDK 9 had the same issue. I dug around and found appcds/test-classes/JimageClassProtDomain.java. However the test didn't perform intended testing due to unnoticed ClassNotFoundException ("sun.tools.javac.Main" should be "com.sun.tools.javac.Main") and masked the issue. Here is the webrev that fixed the toFileURL() and JimageClassProtDomain test issues: http://cr.openjdk.java.net/~jiangli/8214827/webrev.00/ Since JimageClassProtDomain.java uses appcds/test-classes/ProtectionDomain.java as the test driver, I've also updated ProtDomain.java and ProtDomainB.java as they also use ProtectionDomain.java.
09-01-2019
This is a runtime[appcds] bug. I'll update it. Thanks!
05-12-2018
Not sure where this bug belongs - appcds, hotspot runtime and classloader folks may be interested
05-12-2018