From 3c42363ba49fdaf7f77f2bd135ecdf55167b982b Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Fri, 28 Oct 2022 21:22:58 +0200 Subject: [PATCH] Do not close GraalVM Native image FileSystem after classpath scanning As can be seen in a modified version of the following example project, attempting to access a resource discovered via classpath scanning within a GraalVM native image -- for example via the Files.exists(...) invocation in FileSystemResource -- currently results in a ClosedFileSystemException since PathMatchingResourcePatternResolver explicitly closes the native image FileSystem that backs `resource:` resources. Sample project: https://github.com/joshlong/spring-boot-3-aot-graphql To address this issue, this commit removes the explicit close() invocation for custom FileSystems after classpath scanning in PathMatchingResourcePatternResolver. See https://github.com/spring-projects/spring-graphql/issues/495 Closes gh-29397 --- .../PathMatchingResourcePatternResolver.java | 115 ++++++++---------- ...hMatchingResourcePatternResolverTests.java | 25 +++- 2 files changed, 75 insertions(+), 65 deletions(-) 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 368c4557f3..8870124eb1 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 @@ -31,7 +31,6 @@ import java.net.URISyntaxException; import java.net.URL; import java.net.URLClassLoader; import java.net.URLConnection; -import java.nio.file.FileSystem; import java.nio.file.FileSystemNotFoundException; import java.nio.file.FileSystems; import java.nio.file.Files; @@ -748,76 +747,68 @@ public class PathMatchingResourcePatternResolver implements ResourcePatternResol return Collections.emptySet(); } - FileSystem fileSystem = null; - try { - Path rootPath = null; - if (rootDirUri.isAbsolute() && !rootDirUri.isOpaque()) { - // Prefer Path resolution from URI if possible + Path rootPath = null; + if (rootDirUri.isAbsolute() && !rootDirUri.isOpaque()) { + // Prefer Path resolution from URI if possible + try { try { - try { - rootPath = Path.of(rootDirUri); - } - catch (FileSystemNotFoundException ex) { - // If the file system was not found, assume it's a custom file system that needs to be installed. - fileSystem = FileSystems.newFileSystem(rootDirUri, Map.of(), ClassUtils.getDefaultClassLoader()); - rootPath = Path.of(rootDirUri); - } + rootPath = Path.of(rootDirUri); } - catch (Exception ex) { - if (logger.isDebugEnabled()) { - logger.debug("Failed to resolve %s in file system: %s".formatted(rootDirUri, ex)); - } - // Fallback via Resource.getFile() below + catch (FileSystemNotFoundException ex) { + // If the file system was not found, assume it's a custom file system that needs to be installed. + FileSystems.newFileSystem(rootDirUri, Map.of(), ClassUtils.getDefaultClassLoader()); + rootPath = Path.of(rootDirUri); } } - if (rootPath == null) { - // Resource.getFile() resolution as a fallback - - // for custom URI formats and custom Resource implementations - rootPath = Path.of(rootDirResource.getFile().getAbsolutePath()); - } - - String rootDir = StringUtils.cleanPath(rootPath.toString()); - if (!rootDir.endsWith("/")) { - rootDir += "/"; - } - - Path rootPathForPattern = rootPath; - String resourcePattern = rootDir + StringUtils.cleanPath(subPattern); - Predicate isMatchingFile = path -> (!path.equals(rootPathForPattern) && - getPathMatcher().match(resourcePattern, StringUtils.cleanPath(path.toString()))); - - if (logger.isTraceEnabled()) { - logger.trace("Searching directory [%s] for files matching pattern [%s]" - .formatted(rootPath.toAbsolutePath(), subPattern)); - } - - Set result = new LinkedHashSet<>(); - try (Stream files = Files.walk(rootPath)) { - files.filter(isMatchingFile).sorted().forEach(file -> { - try { - result.add(new FileSystemResource(file)); - } - catch (Exception ex) { - if (logger.isDebugEnabled()) { - logger.debug("Failed to convert file %s to an org.springframework.core.io.Resource: %s" - .formatted(file, ex)); - } - } - }); - } catch (Exception ex) { if (logger.isDebugEnabled()) { - logger.debug("Failed to complete search in directory [%s] for files matching pattern [%s]: %s" - .formatted(rootPath.toAbsolutePath(), subPattern, ex)); + logger.debug("Failed to resolve %s in file system: %s".formatted(rootDirUri, ex)); } - } - return result; - } - finally { - if (fileSystem != null) { - fileSystem.close(); + // Fallback via Resource.getFile() below } } + if (rootPath == null) { + // Resource.getFile() resolution as a fallback - + // for custom URI formats and custom Resource implementations + rootPath = Path.of(rootDirResource.getFile().getAbsolutePath()); + } + + String rootDir = StringUtils.cleanPath(rootPath.toString()); + if (!rootDir.endsWith("/")) { + rootDir += "/"; + } + + Path rootPathForPattern = rootPath; + String resourcePattern = rootDir + StringUtils.cleanPath(subPattern); + Predicate isMatchingFile = path -> (!path.equals(rootPathForPattern) && + getPathMatcher().match(resourcePattern, StringUtils.cleanPath(path.toString()))); + + if (logger.isTraceEnabled()) { + logger.trace("Searching directory [%s] for files matching pattern [%s]" + .formatted(rootPath.toAbsolutePath(), subPattern)); + } + + Set result = new LinkedHashSet<>(); + try (Stream files = Files.walk(rootPath)) { + files.filter(isMatchingFile).sorted().forEach(file -> { + try { + result.add(new FileSystemResource(file)); + } + catch (Exception ex) { + if (logger.isDebugEnabled()) { + logger.debug("Failed to convert file %s to an org.springframework.core.io.Resource: %s" + .formatted(file, ex)); + } + } + }); + } + catch (Exception ex) { + if (logger.isDebugEnabled()) { + logger.debug("Failed to complete search in directory [%s] for files matching pattern [%s]: %s" + .formatted(rootPath.toAbsolutePath(), subPattern, ex)); + } + } + return result; } /** 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 c9de0064a2..a7fb84910b 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 @@ -19,6 +19,7 @@ package org.springframework.core.io.support; import java.io.FileNotFoundException; import java.io.IOException; import java.io.UncheckedIOException; +import java.net.URL; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Arrays; @@ -129,8 +130,16 @@ class PathMatchingResourcePatternResolverTests { List actualSubPaths = getSubPathsIgnoringClassFilesEtc(pattern, pathPrefix); - // We do NOT find "support" if the pattern ENDS with a slash. - assertThat(actualSubPaths).isEmpty(); + URL url = getClass().getClassLoader().getResource("org/springframework/core/io/support/EncodedResource.class"); + if (!url.getProtocol().equals("jar")) { + // We do NOT find "support" if the pattern ENDS with a slash if org/springframework/core/io/support + // is in the local file system. + assertThat(actualSubPaths).isEmpty(); + } + else { + // But we do find "support/" if org/springframework/core/io/support is found in a JAR on the classpath. + assertThat(actualSubPaths).containsExactly("support/"); + } } @Test @@ -258,6 +267,7 @@ class PathMatchingResourcePatternResolverTests { try { Resource[] resources = resolver.getResources(pattern); List actualNames = Arrays.stream(resources) + .peek(resource -> assertThat(resource.exists()).as(resource + " exists").isTrue()) .map(Resource::getFilename) .sorted() .toList(); @@ -304,7 +314,16 @@ class PathMatchingResourcePatternResolverTests { // GraalVM native image which cannot support Path#toFile. // // See: https://github.com/spring-projects/spring-framework/issues/29243 - return ((FileSystemResource) resource).getPath(); + if (resource instanceof FileSystemResource fileSystemResource) { + return fileSystemResource.getPath(); + } + try { + // Fall back to URL in case the resource came from a JAR + return resource.getURL().getPath(); + } + catch (IOException ex) { + throw new UncheckedIOException(ex); + } } }