8339460: CDS error when module is located in a directory with space in the name

Reviewed-by: ccheung, iklam

Author: Maxim Kartashev

src/hotspot/share/cds/classListParser.cpp

@@ -508,7 +508,9 @@ InstanceKlass* ClassListParser::load_class_from_source(Symbol* class_name, TRAPS
     THROW_NULL(vmSymbols::java_lang_ClassNotFoundException());
   }
 
-  InstanceKlass* k = UnregisteredClasses::load_class(class_name, _source, CHECK_NULL);
+  ResourceMark rm;
+  char * source_path = os::strdup_check_oom(ClassLoader::uri_to_path(_source));
+  InstanceKlass* k = UnregisteredClasses::load_class(class_name, source_path, CHECK_NULL);
   if (k->local_interfaces()->length() != _interfaces->length()) {
     print_specified_interfaces();
     print_actual_interfaces(k);

src/hotspot/share/cds/classListWriter.cpp

@@ -174,6 +174,8 @@ void ClassListWriter::write_to_stream(const InstanceKlass* k, outputStream* stre
       }
     }
 
+    // NB: the string following "source: " is not really a proper file name, but rather
+    // a truncated URI referring to a file. It must be decoded after reading.
 #ifdef _WINDOWS
     // "file:/C:/dir/foo.jar" -> "C:/dir/foo.jar"
     stream->print(" source: %s", cfs->source() + 6);

src/hotspot/share/cds/filemap.cpp

@@ -581,7 +581,7 @@ int FileMapInfo::get_module_shared_path_index(Symbol* location) {
 
   // skip_uri_protocol was also called during dump time -- see ClassLoaderExt::process_module_table()
   ResourceMark rm;
-  const char* file = ClassLoader::skip_uri_protocol(location->as_C_string());
+  const char* file = ClassLoader::uri_to_path(location->as_C_string());
   for (int i = ClassLoaderExt::app_module_paths_start_index(); i < get_number_of_shared_paths(); i++) {
     SharedClassPathEntry* ent = shared_path(i);
     if (!ent->is_non_existent()) {

src/hotspot/share/classfile/classLoader.cpp

@@ -81,6 +81,9 @@
 #include "utilities/ostream.hpp"
 #include "utilities/utf8.hpp"
 
+#include <stdlib.h>
+#include <ctype.h>
+
 // Entry point in java.dll for path canonicalization
 
 typedef int (*canonicalize_fn_t)(const char *orig, char *out, int len);
@@ -1209,7 +1212,7 @@ InstanceKlass* ClassLoader::load_class(Symbol* name, PackageEntry* pkg_entry, bo
 }
 
 #if INCLUDE_CDS
-char* ClassLoader::skip_uri_protocol(char* source) {
+static const char* skip_uri_protocol(const char* source) {
   if (strncmp(source, "file:", 5) == 0) {
     // file: protocol path could start with file:/ or file:///
     // locate the char after all the forward slashes
@@ -1228,6 +1231,47 @@ char* ClassLoader::skip_uri_protocol(char* source) {
   return source;
 }
 
+static char decode_percent_encoded(const char *str, size_t& index) {
+  if (str[index] == '%'
+      && isxdigit(str[index + 1])
+      && isxdigit(str[index + 2])) {
+    char hex[3];
+    hex[0] = str[index + 1];
+    hex[1] = str[index + 2];
+    hex[2] = '\0';
+    index += 2;
+    return (char) strtol(hex, NULL, 16);
+  }
+  return str[index];
+}
+
+char* ClassLoader::uri_to_path(const char* uri) {
+  const size_t len = strlen(uri) + 1;
+  char* path = NEW_RESOURCE_ARRAY(char, len);
+
+  uri = skip_uri_protocol(uri);
+
+  if (strncmp(uri, "//", 2) == 0) {
+    // Skip the empty "authority" part
+    uri += 2;
+  }
+
+#ifdef _WINDOWS
+  if (uri[0] == '/') {
+    // Absolute path name on Windows does not begin with a slash
+    uri += 1;
+  }
+#endif
+
+  size_t path_index = 0;
+  for (size_t i = 0; i < strlen(uri); ++i) {
+    char decoded = decode_percent_encoded(uri, i);
+    path[path_index++] = decoded;
+  }
+  path[path_index] = '\0';
+  return path;
+}
+
 // Record the shared classpath index and loader type for classes loaded
 // by the builtin loaders at dump time.
 void ClassLoader::record_result(JavaThread* current, InstanceKlass* ik,
@@ -1261,7 +1305,7 @@ void ClassLoader::record_result(JavaThread* current, InstanceKlass* ik,
     // Save the path from the file: protocol or the module name from the jrt: protocol
     // if no protocol prefix is found, path is the same as stream->source(). This path
     // must be valid since the class has been successfully parsed.
-    char* path = skip_uri_protocol(src);
+    const char* path = ClassLoader::uri_to_path(src);
     assert(path != nullptr, "sanity");
     for (int i = 0; i < FileMapInfo::get_number_of_shared_paths(); i++) {
       SharedClassPathEntry* ent = FileMapInfo::shared_path(i);

src/hotspot/share/classfile/classLoader.hpp

@@ -382,7 +382,7 @@ class ClassLoader: AllStatic {
   // entries during shared classpath setup time.
   static int num_module_path_entries();
   static void  exit_with_path_failure(const char* error, const char* message);
-  static char* skip_uri_protocol(char* source);
+  static char* uri_to_path(const char* uri);
   static void  record_result(JavaThread* current, InstanceKlass* ik,
                              const ClassFileStream* stream, bool redefined);
 #endif

src/hotspot/share/classfile/classLoaderExt.cpp

@@ -100,12 +100,10 @@ void ClassLoaderExt::process_module_table(JavaThread* current, ModuleEntryTable*
     ModulePathsGatherer(JavaThread* current, GrowableArray<char*>* module_paths) :
       _current(current), _module_paths(module_paths) {}
     void do_module(ModuleEntry* m) {
-      char* path = m->location()->as_C_string();
-      if (strncmp(path, "file:", 5) == 0) {
-        path = ClassLoader::skip_uri_protocol(path);
-        char* path_copy = NEW_RESOURCE_ARRAY(char, strlen(path) + 1);
-        strcpy(path_copy, path);
-        _module_paths->append(path_copy);
+      char* uri = m->location()->as_C_string();
+      if (strncmp(uri, "file:", 5) == 0) {
+        char* path = ClassLoader::uri_to_path(uri);
+        _module_paths->append(path);
       }
     }
   };

test/hotspot/jtreg/TEST.groups

@@ -435,6 +435,7 @@ hotspot_cds_only = \
 hotspot_appcds_dynamic = \
   runtime/cds/appcds/ \
  -runtime/cds/appcds/cacheObject \
+ -runtime/cds/appcds/complexURI \
  -runtime/cds/appcds/customLoader \
  -runtime/cds/appcds/dynamicArchive \
  -runtime/cds/appcds/loaderConstraints/DynamicLoaderConstraintsTest.java \

test/hotspot/jtreg/runtime/cds/appcds/complexURI/ComplexURITest.java

@@ -0,0 +1,167 @@
+/* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2024 JetBrains s.r.o.
+ * 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.
+ *
+ * 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.
+ */
+
+/*
+ * @test
+ * @summary Verifies that CDS works with jar located in directories
+ *          with names that need escaping
+ * @bug 8339460
+ * @requires vm.cds
+ * @requires vm.cds.custom.loaders
+ * @requires vm.flagless
+ * @library /test/lib /test/hotspot/jtreg/runtime/cds/appcds
+ * @compile mypackage/Main.java mypackage/Another.java
+ * @run main/othervm ComplexURITest
+ */
+
+import jdk.test.lib.process.OutputAnalyzer;
+import jdk.test.lib.process.ProcessTools;
+import jdk.test.lib.Platform;
+
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+
+public class ComplexURITest {
+    final static String moduleName = "mymodule";
+
+    public static void main(String[] args) throws Exception {
+        System.setProperty("test.noclasspath", "true");
+        String jarFile = JarBuilder.build(moduleName, "mypackage/Main", "mypackage/Another");
+
+        Path subDir = Path.of(".", "dir with space");
+        Files.createDirectory(subDir);
+        Path newJarFilePath = subDir.resolve(moduleName + ".jar");
+        Files.move(Path.of(jarFile), newJarFilePath);
+        jarFile = newJarFilePath.toString();
+
+        final String listFileName = "test-classlist.txt";
+        final String staticArchiveName = "test-static.jsa";
+        final String dynamicArchiveName = "test-dynamic.jsa";
+
+        // Verify static archive creation and use
+        File fileList = new File(listFileName);
+        delete(fileList.toPath());
+        File staticArchive = new File(staticArchiveName);
+        delete(staticArchive.toPath());
+
+        createClassList(jarFile, listFileName);
+        if (!fileList.exists()) {
+            throw new RuntimeException("No class list created at " + fileList);
+        }
+
+        createArchive(jarFile, listFileName, staticArchiveName);
+        if (!staticArchive.exists()) {
+            throw new RuntimeException("No shared classes archive created at " + staticArchive);
+        }
+
+        useArchive(jarFile, staticArchiveName);
+
+        // Verify dynamic archive creation and use
+        File dynamicArchive = new File(dynamicArchiveName);
+        delete(dynamicArchive.toPath());
+
+        createDynamicArchive(jarFile, dynamicArchiveName);
+        if (!dynamicArchive.exists()) {
+            throw new RuntimeException("No dynamic archive created at " + dynamicArchive);
+        }
+
+        testDynamicArchive(jarFile, dynamicArchiveName);
+    }
+
+    private static void delete(Path path) throws Exception {
+        if (Files.exists(path)) {
+            if (Platform.isWindows()) {
+                Files.setAttribute(path, "dos:readonly", false);
+            }
+            Files.delete(path);
+        }
+    }
+
+    private static void createClassList(String jarFile, String list) throws Exception {
+        String[] launchArgs  = {
+                "-XX:DumpLoadedClassList=" + list,
+                "--module-path",
+                jarFile,
+                "--module",
+                moduleName + "/mypackage.Main"};
+        ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder(launchArgs);
+        OutputAnalyzer output = TestCommon.executeAndLog(pb, "create-list");
+        output.shouldHaveExitValue(0);
+    }
+
+    private static void createArchive(String jarFile, String list, String archive) throws Exception {
+        String[] launchArgs  = {
+                "-Xshare:dump",
+                "-XX:SharedClassListFile=" + list,
+                "-XX:SharedArchiveFile=" + archive,
+                "--module-path",
+                jarFile,
+                "--module",
+                moduleName + "/mypackage.Main"};
+        ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder(launchArgs);
+        OutputAnalyzer output = TestCommon.executeAndLog(pb, "dump-archive");
+        output.shouldHaveExitValue(0);
+    }
+
+    private static void useArchive(String jarFile, String archive) throws Exception {
+        String[] launchArgs  = {
+                "-Xshare:on",
+                "-XX:SharedArchiveFile=" + archive,
+                "--module-path",
+                jarFile,
+                "--module",
+                moduleName + "/mypackage.Main"};
+        ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder(launchArgs);
+        OutputAnalyzer output = TestCommon.executeAndLog(pb, "use-archive");
+        output.shouldHaveExitValue(0);
+    }
+
+    private static void createDynamicArchive(String jarFile, String archive) throws Exception {
+        String[] launchArgs  = {
+                "-XX:ArchiveClassesAtExit=" + archive,
+                "--module-path",
+                jarFile,
+                "--module",
+                moduleName + "/mypackage.Main"};
+        ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder(launchArgs);
+        OutputAnalyzer output = TestCommon.executeAndLog(pb, "dynamic-archive");
+        output.shouldHaveExitValue(0);
+    }
+
+    private static void testDynamicArchive(String jarFile, String archive) throws Exception {
+        String[] launchArgs  = {
+                "-XX:SharedArchiveFile=" + archive,
+                "-XX:+PrintSharedArchiveAndExit",
+                "--module-path",
+                jarFile,
+                "--module",
+                moduleName + "/mypackage.Main"};
+        ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder(launchArgs);
+        OutputAnalyzer output = TestCommon.executeAndLog(pb, "dynamic-archive");
+        output.shouldHaveExitValue(0);
+        output.shouldContain("archive is valid");
+        output.shouldContain(": mypackage.Main app_loader");
+        output.shouldContain(": mypackage.Another unregistered_loader");
+    }
+}

test/hotspot/jtreg/runtime/cds/appcds/complexURI/mypackage/Another.java

@@ -0,0 +1,27 @@
+/* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2024 JetBrains s.r.o.
+ * 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.
+ *
+ * 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 mypackage;
+
+public class Another {
+}

test/hotspot/jtreg/runtime/cds/appcds/complexURI/mypackage/Main.java

@@ -0,0 +1,37 @@
+/* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2024 JetBrains s.r.o.
+ * 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.
+ *
+ * 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 mypackage;
+
+import java.net.URL;
+import java.net.URLClassLoader;
+
+public class Main {
+    public static void main(String[] args) throws Exception {
+        URL url1 = Main.class.getProtectionDomain().getCodeSource().getLocation();
+        System.out.println("Will load Another from " + url1);
+        ClassLoader cl = URLClassLoader.newInstance(new URL[] { url1 }, null);
+        var anotherClass = cl.loadClass("mypackage.Another");
+        System.out.println("Class " + anotherClass + " loaded successfully");
+    }
+}