From cdea111d3f731d8ef0c71d71e09c063a08b601bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Kohlschu=CC=88tter?= Date: Mon, 16 Jan 2023 19:49:14 +0100 Subject: [PATCH] Only use GraalIssue5720PathResourceFactory when truly needed Previously, we were always enabling this resource factory for GraalVM native-image environments. We now check if that's actually necessary. We fall back to MountedPathResourceFactory or PathResourceFactory, depending on whether the URL contains "!/" or not. --- .../resource/GraalIssue5720PathResource.java | 62 +++++++++++++++++++ .../GraalIssue5720PathResourceFactory.java | 43 +------------ .../resource/ResourceFactoryInternals.java | 26 +++++--- 3 files changed, 80 insertions(+), 51 deletions(-) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/GraalIssue5720PathResource.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/GraalIssue5720PathResource.java index 292501362b2..690178aeeb7 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/GraalIssue5720PathResource.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/GraalIssue5720PathResource.java @@ -13,9 +13,19 @@ package org.eclipse.jetty.util.resource; +import java.io.IOException; import java.net.URI; +import java.net.URISyntaxException; +import java.net.URL; +import java.nio.file.FileSystemAlreadyExistsException; +import java.nio.file.FileSystemNotFoundException; +import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Collections; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * GraalVM Native-Image {@link Path} Resource. @@ -24,6 +34,7 @@ import java.nio.file.Path; */ final class GraalIssue5720PathResource extends PathResource { + private static final Logger LOG = LoggerFactory.getLogger(GraalIssue5720PathResource.class); private static final String URI_BAD_RESOURCE_PREFIX = "file:///resources!"; GraalIssue5720PathResource(Path path, URI uri, boolean bypassAllowedSchemeCheck) @@ -36,6 +47,57 @@ final class GraalIssue5720PathResource extends PathResource return "resource".equals(uri.getScheme()); } + /** + * Checks if the given resource URL is affected by Graal issue 5720. + * + * @param url + * The URL to check. + * @return {@code true} if affected. + */ + static boolean isAffectedURL(URL url) + { + if (url == null || !"resource".equals(url.getProtocol())) + { + return false; + } + + URI uri; + try + { + uri = url.toURI(); + } + catch (URISyntaxException e) + { + return false; + } + + try + { + try + { + uri = Path.of(uri).toUri(); + } + catch (FileSystemNotFoundException e) + { + try + { + FileSystems.newFileSystem(uri, Collections.emptyMap()); + } + catch (FileSystemAlreadyExistsException e2) + { + LOG.debug("Race condition upon calling FileSystems.newFileSystem for: {}", uri, e); + } + uri = Path.of(uri).toUri(); + } + } + catch (IOException | RuntimeException e) + { + LOG.warn("Could not check URL: {}", url, e); + } + + return uri.getSchemeSpecificPart().startsWith(URI_BAD_RESOURCE_PREFIX); + } + /** * Corrects any bad {@code resource} based URIs, such as those starting with {@code resource:file:///resources!}. * diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/GraalIssue5720PathResourceFactory.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/GraalIssue5720PathResourceFactory.java index 2d8d0d5e0fc..6660cf3831b 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/GraalIssue5720PathResourceFactory.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/GraalIssue5720PathResourceFactory.java @@ -13,59 +13,18 @@ package org.eclipse.jetty.util.resource; -import java.io.IOException; import java.net.URI; -import java.net.URISyntaxException; -import java.net.URL; -import java.nio.file.FileSystemNotFoundException; -import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Collections; /** * GraalVM Native-Image {@link PathResourceFactory}. * * @see Graal issue 5720 + * @see GraalIssue5720PathResource */ final class GraalIssue5720PathResourceFactory extends PathResourceFactory { - static final boolean ENABLE_NATIVE_IMAGE_RESOURCE_SCHEME; - - static - { - URL url = GraalIssue5720PathResourceFactory.class.getResource("/org/eclipse/jetty/version/build.properties"); - ENABLE_NATIVE_IMAGE_RESOURCE_SCHEME = (url != null && "resource".equals(url.getProtocol())); - } - - public GraalIssue5720PathResourceFactory() - { - if (ENABLE_NATIVE_IMAGE_RESOURCE_SCHEME) - { - initNativeImageResourceFileSystem(); - } - } - - private void initNativeImageResourceFileSystem() - { - try - { - URI uri = new URI("resource:/"); - try - { - Path.of(uri); - } - catch (FileSystemNotFoundException e) - { - FileSystems.newFileSystem(uri, Collections.emptyMap()); - } - } - catch (IOException | URISyntaxException | RuntimeException ignore) - { - // ignore - } - } - @Override public Resource newResource(URI uri) { diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java index 7cf0e6b5b17..9d3493b7279 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/resource/ResourceFactoryInternals.java @@ -55,19 +55,27 @@ class ResourceFactoryInternals RESOURCE_FACTORIES.put("file", pathResourceFactory); RESOURCE_FACTORIES.put("jrt", pathResourceFactory); - if (GraalIssue5720PathResourceFactory.ENABLE_NATIVE_IMAGE_RESOURCE_SCHEME) - RESOURCE_FACTORIES.put("resource", new GraalIssue5720PathResourceFactory()); - - /* Best effort attempt to detect that an alternate FileSystem type that is in use. - * We don't attempt to look up a Class, as not all runtimes and environments have the classes anymore - * (e.g., they were compiled into native code) - * The build.properties is present in the jetty-util jar, so it's reasonably safe to look for that - * as a resource + /* Best-effort attempt to support an alternate FileSystem type that is in use for classpath + * resources. + * + * The build.properties is present in the jetty-util jar, and explicitly included for reflection + * with native-image (unlike classes, which are not accessible by default), so we use that + * resource as a reference. */ URL url = ResourceFactoryInternals.class.getResource("/org/eclipse/jetty/version/build.properties"); if ((url != null) && !RESOURCE_FACTORIES.contains(url.getProtocol())) { - RESOURCE_FACTORIES.put(url.getProtocol(), mountedPathResourceFactory); + ResourceFactory resourceFactory; + if (GraalIssue5720PathResource.isAffectedURL(url)) + { + resourceFactory = new GraalIssue5720PathResourceFactory(); + } + else + { + resourceFactory = url.toString().contains("!/") ? mountedPathResourceFactory : pathResourceFactory; + } + + RESOURCE_FACTORIES.put(url.getProtocol(), resourceFactory); } }