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

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

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@
import java.net.URL;
import java.net.URLClassLoader;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileSystemNotFoundException;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Objects;
Expand Down Expand Up @@ -54,6 +57,7 @@ public final class URIUtil
.with("file:")
.with("jrt:")
.with("jar:")
.with("resource:")
joakime marked this conversation as resolved.
Show resolved Hide resolved
.build();

// From https://www.rfc-editor.org/rfc/rfc3986
Expand Down Expand Up @@ -211,6 +215,8 @@ public final class URIUtil

private static final boolean[] ENCODE_PATH_NEEDS_ENCODING;

private static final String URI_BAD_RESOURCE_PREFIX = "file:///resources!";

private URIUtil()
{
}
Expand Down Expand Up @@ -1735,6 +1741,54 @@ public static String addQueries(String query1, String query2)
return query1 + '&' + query2;
}

/**
* Corrects any bad {@code resource} based URIs, such as those starting with {@code resource:file:///resources!}.
*
* @param uri
* The URI to correct.
* @return the corrected URI, or the original URI.
* @see <a href="https://github.com/oracle/graal/issues/5720">Graal issue 5720</a>
*/
public static URI correctResourceURI(URI uri)
{
if (uri == null || !"resource".equals(uri.getScheme()))
return uri;

String ssp = uri.getSchemeSpecificPart();
if (ssp.startsWith(URI_BAD_RESOURCE_PREFIX))
{
return URI.create("resource:" + ssp.substring(URI_BAD_RESOURCE_PREFIX.length()));
}
else
{
return uri;
}
}

/**
* A wrapper for {@link Paths#get(URI)} and {@link Path#of(URI)}.
*
* It automatically handles custom file systems, such as ZipFileSystem and NativeImageResourceFileSystem.
*
* @param uri
* The URI.
* @return The path.
* @throws IOException
* on error.
*/
public static Path getPath(URI uri) throws IOException
{
try
{
return Path.of(uri);
Fixed Show fixed Hide fixed
}
catch (FileSystemNotFoundException e)
{
FileSystems.newFileSystem(uri, Collections.emptyMap());
return Path.of(uri);
Fixed Show fixed Hide fixed
}
}

/**
* <p>
* Corrects any bad {@code file} based URIs (even within a {@code jar:file:} based URIs) from the bad out-of-spec
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public class PathResource extends Resource
.caseSensitive(false)
.with("file")
.with("jrt")
.with("resource")
joakime marked this conversation as resolved.
Show resolved Hide resolved
.build();

// The path object represented by this instance
Expand Down Expand Up @@ -141,15 +142,16 @@ public static boolean isSameName(Path pathA, Path pathB)
* Must be an absolute URI using the <code>file</code> scheme.
*
* @param uri the URI to build this PathResource from.
* @throws IOException if the path could not be resolved.
*/
PathResource(URI uri)
PathResource(URI uri) throws IOException
joakime marked this conversation as resolved.
Show resolved Hide resolved
{
this(uri, false);
}

PathResource(URI uri, boolean bypassAllowedSchemeCheck)
PathResource(URI uri, boolean bypassAllowedSchemeCheck) throws IOException
{
this(Paths.get(uri), uri, bypassAllowedSchemeCheck);
this(URIUtil.getPath(URIUtil.correctResourceURI(uri)), uri, bypassAllowedSchemeCheck);
}

PathResource(Path path)
Expand All @@ -166,6 +168,7 @@ public static boolean isSameName(Path pathA, Path pathB)
*/
PathResource(Path path, URI uri, boolean bypassAllowedSchemeCheck)
{
uri = URIUtil.correctResourceURI(uri);
if (!uri.isAbsolute())
throw new IllegalArgumentException("not an absolute uri: " + uri);
if (!bypassAllowedSchemeCheck && !SUPPORTED_SCHEMES.contains(uri.getScheme()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,46 @@

package org.eclipse.jetty.util.resource;

import java.io.IOException;
import java.net.URI;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;

import org.eclipse.jetty.util.URIUtil;

public class PathResourceFactory implements ResourceFactory
{
@Override
public Resource newResource(URI uri)
{
Path path = Paths.get(uri.normalize());
Path path = Path.of(uri.normalize());
if (!Files.exists(path))
return null;
return new PathResource(path, uri, false);
}

@Override
public Resource newResource(Path path)
{
if (path == null)
return null;

URI uri = path.toUri();

try
{
// Validate URI conversion, and trigger FileSystem initialization, if necessary
if (URIUtil.getPath(uri) == null)
return null;
}
catch (IOException e)
{
return null;
}

if (!Files.exists(path))
return null;

return new PathResource(path, uri, false);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class ResourceFactoryInternals
PathResourceFactory pathResourceFactory = new PathResourceFactory();
RESOURCE_FACTORIES.put("file", pathResourceFactory);
RESOURCE_FACTORIES.put("jrt", pathResourceFactory);
RESOURCE_FACTORIES.put("resource", pathResourceFactory); // GraalVM native-image resource
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shall not be automatic or defaulted.
The modifications you have made to URIUtil and PathResource should be refactored out into a standalone GraalVMResourceFactory implementation, not by modifying the default behaviors.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've thought about that, but then GraalVM-unaware code that simply tries to serve resources from the classpath will fail.

I've amended my change to transparently support the following real-world use case:

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

The new change only enables "resource:" URLs when a well-known GraalvM Native Image system property is set.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can go one of two ways:

  1. automatic discovery of unknown schemes. We have always done this for URL handlers, but now it looks like we'd also need to do an attempt to to look for a filesystem for a Path based unknown scheme provided by the JVM. That makes me less inclined to go automatic.
  2. A simple registration mechanism, that can easily be put into a module. Potentially fairly generic so it can add either URL handlers or FileSystem mounters for an arbitrary resource. A graalvm module would then do the mapping for "resource" to the FileSystem mounting extension of PathResourceFactory.

}

static ResourceFactory ROOT = new CompositeResourceFactory()
Expand Down