From fe3c1b9d07c80b1fd94ce7816893f7eecbee4287 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 14 Oct 2024 22:11:06 -0700 Subject: [PATCH] Fix `PathMatchingResourcePatternResolver` manifest classpath discovery Update `PathMatchingResourcePatternResolver` so that in addition to searching the `java.class.path` system property for classpath enties, it also searches the `MANIFEST.MF` files from within those jars. Prior to this commit, the `addClassPathManifestEntries()` method expected that the JVM had added `Class-Path` manifest entries to the `java.class.path` system property, however, this did not always happen. The updated code now performs a deep search by loading `MANIFEST.MF` files from jars discovered from the system property. To deal with potential performance issue, loaded results are also now cached. The updated code has been tested with Spring Boot 3.3 jars extracted using `java -Djarmode=tools`. See gh-33705 --- .../PathMatchingResourcePatternResolver.java | 143 +++++++++++++----- ...assPathManifestEntriesTestApplication.java | 36 +++++ ...hMatchingResourcePatternResolverTests.java | 118 +++++++++++++++ 3 files changed, 261 insertions(+), 36 deletions(-) create mode 100644 spring-core/src/test/java/org/springframework/core/io/support/ClassPathManifestEntriesTestApplication.java diff --git a/spring-core/src/main/java/org/springframework/core/io/support/PathMatchingResourcePatternResolver.java b/spring-core/src/main/java/org/springframework/core/io/support/PathMatchingResourcePatternResolver.java index b2d9959923b0..3315e5b2df99 100644 --- a/spring-core/src/main/java/org/springframework/core/io/support/PathMatchingResourcePatternResolver.java +++ b/spring-core/src/main/java/org/springframework/core/io/support/PathMatchingResourcePatternResolver.java @@ -39,16 +39,21 @@ import java.nio.file.Path; import java.util.Collections; import java.util.Enumeration; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.Map; import java.util.NavigableSet; import java.util.Objects; import java.util.Set; +import java.util.StringTokenizer; import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Predicate; +import java.util.jar.Attributes; +import java.util.jar.Attributes.Name; import java.util.jar.JarEntry; import java.util.jar.JarFile; +import java.util.jar.Manifest; import java.util.stream.Collectors; import java.util.stream.Stream; import java.util.zip.ZipException; @@ -230,6 +235,9 @@ public class PathMatchingResourcePatternResolver implements ResourcePatternResol private static final Predicate isNotSystemModule = resolvedModule -> !systemModuleNames.contains(resolvedModule.name()); + @Nullable + private static Set classPathManifestEntriesCache; + @Nullable private static Method equinoxResolveMethod; @@ -522,25 +530,30 @@ protected void addAllClassLoaderJarRoots(@Nullable ClassLoader classLoader, Set< * @since 4.3 */ protected void addClassPathManifestEntries(Set result) { + Set entries = classPathManifestEntriesCache; + if (entries == null) { + entries = getClassPathManifestEntries(); + classPathManifestEntriesCache = entries; + } + for (ClassPathManifestEntry entry : entries) { + if (!result.contains(entry.resource()) && + (entry.alternative() != null && !result.contains(entry.alternative()))) { + result.add(entry.resource()); + } + } + } + + private Set getClassPathManifestEntries() { + Set manifestEntries = new HashSet<>(); + Set seen = new HashSet<>(); try { - String javaClassPathProperty = System.getProperty("java.class.path"); - for (String path : StringUtils.delimitedListToStringArray(javaClassPathProperty, File.pathSeparator)) { + String paths = System.getProperty("java.class.path"); + for (String path : StringUtils.delimitedListToStringArray(paths, File.pathSeparator)) { try { - String filePath = new File(path).getAbsolutePath(); - int prefixIndex = filePath.indexOf(':'); - if (prefixIndex == 1) { - // Possibly a drive prefix on Windows (for example, "c:"), so we prepend a slash - // and convert the drive letter to uppercase for consistent duplicate detection. - filePath = "/" + StringUtils.capitalize(filePath); - } - // Since '#' can appear in directories/filenames, java.net.URL should not treat it as a fragment - filePath = StringUtils.replace(filePath, "#", "%23"); - // Build URL that points to the root of the jar file - UrlResource jarResource = new UrlResource(ResourceUtils.JAR_URL_PREFIX + - ResourceUtils.FILE_URL_PREFIX + filePath + ResourceUtils.JAR_URL_SEPARATOR); - // Potentially overlapping with URLClassLoader.getURLs() result in addAllClassLoaderJarRoots(). - if (!result.contains(jarResource) && !hasDuplicate(filePath, result) && jarResource.exists()) { - result.add(jarResource); + File jar = new File(path).getAbsoluteFile(); + if (jar.isFile() && seen.add(jar)) { + manifestEntries.add(ClassPathManifestEntry.of(jar)); + manifestEntries.addAll(getClassPathManifestEntriesFromJar(jar)); } } catch (MalformedURLException ex) { @@ -550,34 +563,45 @@ protected void addClassPathManifestEntries(Set result) { } } } + return Collections.unmodifiableSet(manifestEntries); } catch (Exception ex) { if (logger.isDebugEnabled()) { logger.debug("Failed to evaluate 'java.class.path' manifest entries: " + ex); } + return Collections.emptySet(); } } - /** - * Check whether the given file path has a duplicate but differently structured entry - * in the existing result, i.e. with or without a leading slash. - * @param filePath the file path (with or without a leading slash) - * @param result the current result - * @return {@code true} if there is a duplicate (i.e. to ignore the given file path), - * {@code false} to proceed with adding a corresponding resource to the current result - */ - private boolean hasDuplicate(String filePath, Set result) { - if (result.isEmpty()) { - return false; - } - String duplicatePath = (filePath.startsWith("/") ? filePath.substring(1) : "/" + filePath); - try { - return result.contains(new UrlResource(ResourceUtils.JAR_URL_PREFIX + ResourceUtils.FILE_URL_PREFIX + - duplicatePath + ResourceUtils.JAR_URL_SEPARATOR)); + private Set getClassPathManifestEntriesFromJar(File jar) throws IOException { + URL base = jar.toURI().toURL(); + File parent = jar.getAbsoluteFile().getParentFile(); + try (JarFile jarFile = new JarFile(jar)) { + Manifest manifest = jarFile.getManifest(); + Attributes attributes = (manifest != null) ? manifest.getMainAttributes() : null; + String classPath = (attributes != null) ? attributes.getValue(Name.CLASS_PATH) : null; + Set manifestEntries = new HashSet<>(); + if (StringUtils.hasLength(classPath)) { + StringTokenizer tokenizer = new StringTokenizer(classPath); + while (tokenizer.hasMoreTokens()) { + String path = tokenizer.nextToken(); + if (path.indexOf(':') >= 0 && !"file".equalsIgnoreCase(new URL(base, path).getProtocol())) { + // See jdk.internal.loader.URLClassPath.JarLoader.tryResolveFile(URL, String) + continue; + } + File candidate = new File(parent, path); + if (candidate.isFile() && candidate.getCanonicalPath().contains(parent.getCanonicalPath())) { + manifestEntries.add(ClassPathManifestEntry.of(candidate)); + } + } + } + return Collections.unmodifiableSet(manifestEntries); } - catch (MalformedURLException ex) { - // Ignore: just for testing against duplicate. - return false; + catch (Exception ex) { + if (logger.isDebugEnabled()) { + logger.debug("Failed to load manifest entries from jar file '" + jar + "': " + ex); + } + return Collections.emptySet(); } } @@ -1170,4 +1194,51 @@ public String toString() { } } + + /** + * A single {@code Class-Path} manifest entry. + */ + private record ClassPathManifestEntry(Resource resource, @Nullable Resource alternative) { + + private static final String JARFILE_URL_PREFIX = ResourceUtils.JAR_URL_PREFIX + ResourceUtils.FILE_URL_PREFIX; + + static ClassPathManifestEntry of(File file) throws MalformedURLException { + String path = fixPath(file.getAbsolutePath()); + Resource resource = asJarFileResource(path); + Resource alternative = createAlternative(path); + return new ClassPathManifestEntry(resource, alternative); + } + + private static String fixPath(String path) { + int prefixIndex = path.indexOf(':'); + if (prefixIndex == 1) { + // Possibly a drive prefix on Windows (for example, "c:"), so we prepend a slash + // and convert the drive letter to uppercase for consistent duplicate detection. + path = "/" + StringUtils.capitalize(path); + } + // Since '#' can appear in directories/filenames, java.net.URL should not treat it as a fragment + return StringUtils.replace(path, "#", "%23"); + } + + /** + * Return a alternative form of the resource, i.e. with or without a leading slash. + * @param path the file path (with or without a leading slash) + * @return the alternative form or {@code null} + */ + @Nullable + private static Resource createAlternative(String path) { + try { + String alternativePath = path.startsWith("/") ? path.substring(1) : "/" + path; + return asJarFileResource(alternativePath); + } + catch (MalformedURLException ex) { + return null; + } + } + + private static Resource asJarFileResource(String path) + throws MalformedURLException { + return new UrlResource(JARFILE_URL_PREFIX + path + ResourceUtils.JAR_URL_SEPARATOR); + } + } } diff --git a/spring-core/src/test/java/org/springframework/core/io/support/ClassPathManifestEntriesTestApplication.java b/spring-core/src/test/java/org/springframework/core/io/support/ClassPathManifestEntriesTestApplication.java new file mode 100644 index 000000000000..6e64240a96f7 --- /dev/null +++ b/spring-core/src/test/java/org/springframework/core/io/support/ClassPathManifestEntriesTestApplication.java @@ -0,0 +1,36 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.core.io.support; + +import java.io.IOException; +import java.util.List; + +/** + * Class packaged into a temporary jar to test + * {@link PathMatchingResourcePatternResolver} detection of classpath manifest + * entries. + * + * @author Phillip Webb + */ +public class ClassPathManifestEntriesTestApplication { + + public static void main(String[] args) throws IOException { + PathMatchingResourcePatternResolver resolver = new PathMatchingResourcePatternResolver(); + System.out.println("!!!!" + List.of(resolver.getResources("classpath*:/**/*.txt"))); + } + +} diff --git a/spring-core/src/test/java/org/springframework/core/io/support/PathMatchingResourcePatternResolverTests.java b/spring-core/src/test/java/org/springframework/core/io/support/PathMatchingResourcePatternResolverTests.java index 8e05574a4687..ee8b7352dad8 100644 --- a/spring-core/src/test/java/org/springframework/core/io/support/PathMatchingResourcePatternResolverTests.java +++ b/spring-core/src/test/java/org/springframework/core/io/support/PathMatchingResourcePatternResolverTests.java @@ -16,23 +16,44 @@ package org.springframework.core.io.support; +import java.io.File; import java.io.FileNotFoundException; +import java.io.FileOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.UncheckedIOException; +import java.net.JarURLConnection; +import java.net.URISyntaxException; import java.net.URL; import java.net.URLClassLoader; +import java.net.URLConnection; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Arrays; +import java.util.Enumeration; import java.util.List; +import java.util.jar.Attributes; +import java.util.jar.Attributes.Name; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; +import java.util.jar.JarOutputStream; +import java.util.jar.Manifest; import java.util.stream.Collectors; +import java.util.zip.ZipEntry; +import org.apache.commons.logging.LogFactory; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.springframework.core.io.DefaultResourceLoader; import org.springframework.core.io.FileSystemResource; import org.springframework.core.io.Resource; +import org.springframework.util.ClassUtils; +import org.springframework.util.FileSystemUtils; +import org.springframework.util.StreamUtils; import org.springframework.util.StringUtils; import static org.assertj.core.api.Assertions.assertThat; @@ -278,6 +299,103 @@ void rootPatternRetrievalInJarFiles() throws IOException { } } + @Nested + class ClassPathManifestEntries { + + @TempDir + Path temp; + + @Test + void javaDashJarFindsClassPathManifestEntries() throws Exception { + Path lib = this.temp.resolve("lib"); + Files.createDirectories(lib); + writeAssetJar(lib.resolve("asset.jar")); + writeApplicationJar(this.temp.resolve("app.jar")); + String java = ProcessHandle.current().info().command().get(); + Process process = new ProcessBuilder(java, "-jar", "app.jar") + .directory(this.temp.toFile()) + .start(); + assertThat(process.waitFor()).isZero(); + String result = StreamUtils.copyToString(process.getInputStream(), StandardCharsets.UTF_8); + assertThat(result.replace("\\", "/")).contains("!!!!").contains("/lib/asset.jar!/assets/file.txt"); + } + + private void writeAssetJar(Path path) throws Exception { + try (JarOutputStream jar = new JarOutputStream(new FileOutputStream(path.toFile()))) { + jar.putNextEntry(new ZipEntry("assets/")); + jar.closeEntry(); + jar.putNextEntry(new ZipEntry("assets/file.txt")); + StreamUtils.copy("test", StandardCharsets.UTF_8, jar); + jar.closeEntry(); + } + } + + private void writeApplicationJar(Path path) throws Exception { + Manifest manifest = new Manifest(); + Attributes mainAttributes = manifest.getMainAttributes(); + mainAttributes.put(Name.CLASS_PATH, buildSpringClassPath() + "lib/asset.jar"); + mainAttributes.put(Name.MAIN_CLASS, ClassPathManifestEntriesTestApplication.class.getName()); + mainAttributes.put(Name.MANIFEST_VERSION, "1.0"); + try (JarOutputStream jar = new JarOutputStream(new FileOutputStream(path.toFile()), manifest)) { + String appClassResource = ClassUtils.convertClassNameToResourcePath( + ClassPathManifestEntriesTestApplication.class.getName()) + + ClassUtils.CLASS_FILE_SUFFIX; + String folder = ""; + for (String name : appClassResource.split("/")) { + if (!name.endsWith(ClassUtils.CLASS_FILE_SUFFIX)) { + folder += name + "/"; + jar.putNextEntry(new ZipEntry(folder)); + jar.closeEntry(); + } + else { + jar.putNextEntry(new ZipEntry(folder + name)); + try (InputStream in = getClass().getResourceAsStream(name)) { + in.transferTo(jar); + } + jar.closeEntry(); + } + } + } + } + + private String buildSpringClassPath() throws Exception { + return copyClasses(PathMatchingResourcePatternResolver.class, "spring-core") + + copyClasses(LogFactory.class, "commons-logging"); + } + + private String copyClasses(Class sourceClass, String destinationName) + throws URISyntaxException, IOException { + Path destination = this.temp.resolve(destinationName); + String resourcePath = ClassUtils.convertClassNameToResourcePath(sourceClass.getName()) + + ClassUtils.CLASS_FILE_SUFFIX; + URL resource = getClass().getClassLoader().getResource(resourcePath); + URL url = new URL(resource.toString().replace(resourcePath, "")); + URLConnection connection = url.openConnection(); + if (connection instanceof JarURLConnection jarUrlConnection) { + try (JarFile jarFile = jarUrlConnection.getJarFile()) { + Enumeration entries = jarFile.entries(); + while (entries.hasMoreElements()) { + JarEntry entry = entries.nextElement(); + if (!entry.isDirectory()) { + Path entryPath = destination.resolve(entry.getName()); + try (InputStream in = jarFile.getInputStream(entry)) { + Files.createDirectories(entryPath.getParent()); + Files.copy(in, destination.resolve(entry.getName())); + } + } + } + } + } + else { + File source = new File(url.toURI()); + Files.createDirectories(destination); + FileSystemUtils.copyRecursively(source, destination.toFile()); + } + return destinationName + "/ "; + } + + } + private void assertFilenames(String pattern, String... filenames) { assertFilenames(pattern, false, filenames);