Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"resource:" URL scheme not supported (needed for GraalVM) #9116

Closed
kohlschuetter opened this issue Jan 1, 2023 · 8 comments
Closed

"resource:" URL scheme not supported (needed for GraalVM) #9116

kohlschuetter opened this issue Jan 1, 2023 · 8 comments
Labels
Bug For general bugs on Jetty side Stale For auto-closed stale issues and pull requests

Comments

@kohlschuetter
Copy link
Contributor

kohlschuetter commented Jan 1, 2023

Jetty version(s)
12.0.0.alpha3

Java version/vendor (use: java -version)
openjdk version "19.0.1" 2022-10-18
OpenJDK Runtime Environment GraalVM CE 22.3.0 (build 19.0.1+10-jvmci-22.3-b08)
OpenJDK 64-Bit Server VM GraalVM CE 22.3.0 (build 19.0.1+10-jvmci-22.3-b08, mixed mode, sharing)

OS type/version
macOS

Description
When using Resources from the resource class path, GraalVM turns them into URLs of the abstract "resource:" scheme (e.g., resource:/com/example/server/webapp/). This resource scheme appears to be currently unsupported by Jetty:

java.lang.IllegalArgumentException: URI scheme not supported: resource:/com/example/server/webapp/
	at org.eclipse.jetty.util.resource.ResourceFactoryInternals$CompositeResourceFactory.newResource(ResourceFactoryInternals.java:145)
	at org.eclipse.jetty.util.resource.ResourceFactory.newResource(ResourceFactory.java:229)

How to reproduce?

Place a file named "test.txt" in the resource class path relative to some class (e.g., foo.bar.SomeClass -> src/main/resources/foo/bar/test.txt). Build GraalVM native image.

Compare the output of

Resource res = ResourceFactory.root().newResource(SomeClass.class.getResource("test.txt"));
System.out.println(res);    

between Hotspot and native-image.

With Hotspot, the URL will be something like jar:file:///path/to/classpath.jar!/foo/bar/test.txt or file:///path/to/resource/foo/bar/test.txt).

With GraalVM native-image, it's resource:/foo/bar/test.txt.

Note that this also means that tricks to enumerate/list the contents of a resource directory (by traversing the file hierarchy or jar archive) will fail.

@kohlschuetter kohlschuetter added the Bug For general bugs on Jetty side label Jan 1, 2023
@joakime
Copy link
Contributor

joakime commented Jan 1, 2023

The Resource layer in Jetty 12 is based on java.nio.file.Path and java.nio.file.FileSystem.
This was desired as the old school URL Protocol Handling is now deprecated in newer versions of Java.
So are all of the constructors on the URL class.

We have a fallback Resource implementation for URL support, but you have to register your scheme.
Example:

ResourceFactory.registerResourceFactory("resource", new URLResourceFactory());

See also https://github.com/eclipse/jetty.project/blob/jetty-12.0.x/jetty-core/jetty-util/src/test/java/org/eclipse/jetty/util/resource/UrlResourceFactoryTest.java

@joakime
Copy link
Contributor

joakime commented Jan 1, 2023

You could, alternatively, also register your own Resource implementation using the same ResourceFactory.registerResourceFactory() method.

@kohlschuetter
Copy link
Contributor Author

kohlschuetter commented Jan 2, 2023

Thanks for the fast response, @joakime!

So is GraalVM support out of scope for now?
Would you accept a GraalVM-compatible ResourceFactory as a submission?

fwiw, I tried the URLResourceFactory approach, but it seems it needs additional work. For example, the exists() check in URLResourceFactory fails for resource: URLs because newConnection always succeeds.

@joakime
Copy link
Contributor

joakime commented Jan 2, 2023

how would your exists() check work on your proposed ResourceFactory?
The behavior of exists() is the same as it is on Jetty 9/10/11 (which uses URL as well).

Perhaps we should add a getInputStream() check to exists as well? (but man is that SLOOOOOW).

@joakime
Copy link
Contributor

joakime commented Jan 2, 2023

GraalVM support is not something we spend time on, far too few people in the community use GraalVM to make the effort worth it.

We accept PRs though, so if you feel this is useful, then lets see it.

kohlschuetter added a commit to kohlschuetter/jetty.project that referenced this issue Jan 7, 2023
GraalVM Native-Image makes its classpath resources accessible through an
opaque resource: URL scheme.

Add support for this scheme in URIUtil and PathResource.

Automatically create the NativeImageResourceFileSystem when necessary.
Also add a workaround where converting such Native-Image Paths back to
URI would yield the wrong URI (potentially a bug in GraalVM).

jetty#9116
oracle/graal#5720
kohlschuetter added a commit to kohlschuetter/jetty.project that referenced this issue Jan 7, 2023
GraalVM Native-Image makes its classpath resources accessible through an
opaque resource: URL scheme.

Add support for this scheme in URIUtil and PathResource.

Automatically create the NativeImageResourceFileSystem when necessary.
Also add a workaround where converting such Native-Image Paths back to
URI would yield the wrong URI (potentially a bug in GraalVM).

jetty#9116
oracle/graal#5720
kohlschuetter added a commit to kohlschuetter/jetty.project that referenced this issue Jan 7, 2023
GraalVM Native-Image makes its classpath resources accessible through an
opaque resource: URL scheme.

Add support for this scheme in URIUtil and PathResource.

Automatically create the NativeImageResourceFileSystem when necessary.
Also add a workaround where converting such Native-Image Paths back to
URI would yield the wrong URI (potentially a bug in GraalVM).

jetty#9116
oracle/graal#5720

Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
@kohlschuetter
Copy link
Contributor Author

I think I've chased this bug long enough. Java Paths are fun, until they're not.

kohlschuetter added a commit to kohlschuetter/jetty.project that referenced this issue Jan 7, 2023
GraalVM Native-Image makes its classpath resources accessible through an
opaque resource: URL scheme.

Add support for this scheme in URIUtil and PathResource.

Automatically create the NativeImageResourceFileSystem when necessary.
Also add a workaround where converting such Native-Image Paths back to
URI would yield the wrong URI (potentially a bug in GraalVM).

jetty#9116
oracle/graal#5720

Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
joakime added a commit that referenced this issue Jan 17, 2023
…rcePath2

* util: Add support for GraalVM Native-Image resource:-URIs and Paths

GraalVM Native-Image makes its classpath resources accessible through an
opaque resource: URL scheme.

Add support for this scheme in URIUtil and PathResource.

Automatically create the NativeImageResourceFileSystem when necessary.
Also add a workaround where converting such Native-Image Paths back to
URI would yield the wrong URI (potentially a bug in GraalVM).

#9116
oracle/graal#5720

* URIUtil: Suppress CodeQL false positive error about path injection

Github CodeQL code scanning reports a high-severity error "Uncontrolled
data used in path expression", because a path depends on a user-provided
value.

This is a false positive.

Suppress the error by annotating a corresponding @SuppressWarnings tag.

Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>

* URIUtil: Removed unused code

KNOWN_SCHEMES isn't used anywhere.

* PathResource: Selectively enable resource: for GraalVM Native Image

In regular HotSpot VMs, the resource: scheme may be registered by other
clients. However, in GraalVM Native Image, this is used for classpath
resources.

This resource scheme needs to be enabled by default for Native Image
environments so we can support code that is unaware of GraalVM
internals, such as:

URL resourceURL = MyClass.class.getResource("some/resource");
Resource resource = ResourceFactory.root().newResource(resourceURL);

Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>

* PathResourceFactory: Ignore certain RuntimeExceptions upon init

Ignore FileSystemAlreadyExistsException, ProviderNotFoundException, etc.
that may be thrown upon calling FileSystems.newFileSystem.

Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>

* PathResource: Work-around false positive CodeQL warning

CodeQL prevents amending the call to Paths.get.

Work-around this by using a separate constructor. The additional benefit
is that URIUtil.correctResourceURI won't need to correct the URI twice.

Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>

* PathResource: Fix "isAlias" issue with resource: URIs

Fix two more places where Native Image resource: URIs need to be
changed.

Without this change, Resource#isAlias() would return true for such URIs,
and a warning like "BaseResource resource:/some/package/ is aliased to
resource:file:///resources!/some/package/" would be logged.

* Move GraalVM Native-Image code to NativeImagePathResource/-Factory

Keep the GraalVM Native-Image code contained in custom subclasses.

We still enable the "resource:" URL scheme by default if a GraalVM
Native-Image environment is detected via System property. This allows us
to transparently support calls like
ResourceFactory.root().newResource(MyClass.class.getResorce("webapp"))

Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>

* Refactor Graal native-image resource: handling

Detect the resource: scheme by checking the scheme of a well-known
resource instead of looking at a system property.

Rename NativeImagePathResource* to GraalIssue5720PathResource* and make
these classes package-private.

Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>

* 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.

Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
Copy link

github-actions bot commented Jan 8, 2024

This issue has been automatically marked as stale because it has been a
full year without activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@github-actions github-actions bot added the Stale For auto-closed stale issues and pull requests label Jan 8, 2024
@joakime
Copy link
Contributor

joakime commented Jan 8, 2024

Jetty 12 support was added in PR #9136

@joakime joakime closed this as completed Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side Stale For auto-closed stale issues and pull requests
Projects
None yet
Development

No branches or pull requests

2 participants