-
Notifications
You must be signed in to change notification settings - Fork 2k
Issue #13588 - Improve ResourceFactory.split() and URIUtil.toURI to work properly on Windows. #13652
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
Issue #13588 - Improve ResourceFactory.split() and URIUtil.toURI to work properly on Windows. #13652
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,6 @@ | |
| import java.io.File; | ||
| import java.io.IOException; | ||
| import java.net.URI; | ||
| import java.net.URISyntaxException; | ||
| import java.net.URL; | ||
| import java.net.URLClassLoader; | ||
| import java.nio.charset.StandardCharsets; | ||
|
|
@@ -1901,72 +1900,75 @@ else if (scheme.equalsIgnoreCase("file")) | |
| } | ||
|
|
||
| /** | ||
| * <p>Convert a String into a URI suitable for use as a Resource.</p> | ||
| * <p>Convert a String reference into an absolute URI.</p> | ||
| * | ||
| * @param resource If the string starts with one of the ALLOWED_SCHEMES, then it is assumed to be a | ||
| * representation of a {@link URI}, otherwise it is treated as a {@link Path}. | ||
| * @return The {@link URI} form of the resource. | ||
| * @deprecated This method is currently resolving relative paths against the current directory, which is a mechanism | ||
| * that should be implemented by a {@link ResourceFactory}. All calls to this method need to be reviewed. | ||
| * <p>If given a relative reference string to a path, it will convert it to an absolute path | ||
| * using the same techniques present in the JVM itself (eg: {@code Path.of(reference).toAbsolutePath()})</p> | ||
| * | ||
| * <p>Relative URI reference strings (eg: {@code "file:path/dir/"} | ||
| * and {@code "file:path/foo.jar!/"}) are not supported.</p> | ||
| * | ||
| * @param reference the raw String input. | ||
| * @return The absolute {@link URI} form of the reference. | ||
| * @throws IllegalArgumentException if unable to convert the input string. | ||
| */ | ||
| @Deprecated(since = "12.0.8") | ||
| public static URI toURI(String resource) | ||
| public static URI toURI(String reference) | ||
| { | ||
| Objects.requireNonNull(resource); | ||
| Objects.requireNonNull(reference); | ||
|
|
||
| if (URIUtil.hasScheme(resource)) | ||
| try | ||
| { | ||
| try | ||
| /* Perform URI test first. | ||
| * We don't want to perform Path.of(String) first. | ||
| * | ||
| * Example: reference parameter is the String "file:///path/to/dir" | ||
| * | ||
| * On Unix, the Path.of(reference) will result in a relative directory reference | ||
| * that includes the "file:" portion in the path after the current working directory. | ||
| * You'll wind up with something like "file:///home/user/code/jetty/12.1.x/jetty-core/jetty-util/file:///path/to/dir" in this case | ||
| * | ||
| * On Windows, the Path.of(reference) will not allow a Path.of("file:///path/to/dir") to work. | ||
| * This is because there cannot be multi-character drive letters (yes, Windows is limited to only 26 drive letters max) | ||
| */ | ||
| URI uri = URI.create(reference); | ||
| if (uri.isAbsolute()) | ||
| { | ||
| URI uri = new URI(resource); | ||
|
|
||
| if (ResourceFactory.isSupported(uri)) | ||
| return correctURI(uri); | ||
|
|
||
| // We don't have a supported URI scheme | ||
| if (uri.getScheme().length() == 1) | ||
| // At this point we have a string detected as a URI. | ||
| // But that could also include Windows paths like "C:\path\to\foo.jar" or "C:/path/to/foo.jar" | ||
| String scheme = uri.getScheme(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If on windows I pass in a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The scheme would be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See the testcases in ResourceFactoryTest.testSplitIsAbsolute for some windows examples. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also the testcases in URIUtilTest.testToURI (and it's parameters at URIUtilTest.toURICases) |
||
| if (scheme.length() == 1 && Character.isLetter(scheme.charAt(0))) | ||
| { | ||
| // Input is a possible Windows path disguised as a URI "D:/path/to/resource.txt". | ||
| try | ||
| { | ||
| return toURI(Paths.get(resource).toUri().toASCIIString()); | ||
| } | ||
| catch (InvalidPathException x) | ||
| { | ||
| LOG.trace("ignored", x); | ||
| } | ||
| // Single character schemes are assumed to be windows. | ||
| // Make it a file: URI and process it separately. | ||
| return toURI("file:///" + uri.toASCIIString()); | ||
| } | ||
| { | ||
| // Anything else, scheme wise, is acceptable. | ||
| return correctURI(uri); | ||
| } | ||
|
|
||
| // If we reached this point, that means the input String has a scheme, | ||
| // and is not recognized as supported by the registered schemes in ResourceFactory. | ||
| if (LOG.isDebugEnabled()) | ||
| LOG.debug("URI scheme is not registered: {}", uri.toASCIIString()); | ||
| throw new IllegalArgumentException("URI scheme not registered: " + uri.getScheme()); | ||
| } | ||
| catch (URISyntaxException x) | ||
| { | ||
| // We have an input string that has what looks like a scheme, but isn't a URI. | ||
| // Eg: "C:\path\to\resource.txt" | ||
| LOG.trace("ignored", x); | ||
| } | ||
| } | ||
| catch (IllegalArgumentException e) | ||
| { | ||
| LOG.trace("IGNORED: Invalid as URI Reference: {}", reference, e); | ||
| } | ||
|
|
||
| // If we reached this point, we have a String with no valid scheme. | ||
| // Treat it as a Path, as that's all we have left to investigate. | ||
| try | ||
| { | ||
| return toURI(Paths.get(resource).toUri().toASCIIString()); | ||
| Path path = Path.of(reference).toAbsolutePath(); | ||
| return path.toUri(); | ||
| } | ||
| catch (InvalidPathException x) | ||
| catch (InvalidPathException e) | ||
| { | ||
| LOG.trace("ignored", x); | ||
| // Not a path reference. | ||
| LOG.trace("IGNORED: Invalid as Path Reference: {}", reference, e); | ||
| } | ||
|
|
||
| // If we reached this here, that means the input string cannot be used as | ||
| // a URI or a File Path. The cause is usually due to bad input (eg: | ||
| // characters that are not supported by file system) | ||
| if (LOG.isDebugEnabled()) | ||
| LOG.debug("Input string cannot be converted to URI \"{}\"", resource); | ||
| LOG.debug("Input string cannot be converted to URI \"{}\"", reference); | ||
| throw new IllegalArgumentException("Cannot be converted to URI"); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -358,7 +358,7 @@ default Resource newResource(String resource) | |
| // Treat it as a Path, as that's all we have left to investigate. | ||
| try | ||
| { | ||
| Path path = Paths.get(resource); | ||
| Path path = Paths.get(resource).toAbsolutePath(); | ||
| URI uri = new URI(path.toUri().toASCIIString()); | ||
| return new PathResource(path, uri, true); | ||
| } | ||
|
|
@@ -500,11 +500,13 @@ default List<Resource> split(String str, String delim) | |
| /** | ||
| * Split a string of references by provided delims into a List of {@link Resource}. | ||
| * <p> | ||
| * Each part of the input string could be path references (unix or windows style), string URI references, or even glob references (eg: {@code /path/to/libs/*}). | ||
| * Each part of the input string could be path references (unix or windows style), | ||
| * string URI references, or even glob references (eg: {@code /path/to/libs/*}). | ||
| * Note: that if you use the {@code :} character in your delims, then URI references will be impossible. | ||
| * </p> | ||
| * <p> | ||
| * If the result of processing the input segment is a java archive it will not be automatically mounted, the caller must mount if necessary | ||
| * If the result of processing the input segment is a java archive it will not be automatically mounted, | ||
| * the caller must mount if necessary | ||
| * </p> | ||
| * | ||
| * @param str the input string of references | ||
|
|
@@ -523,40 +525,40 @@ default List<Resource> split(String str, String delims, boolean unwrap) | |
| try | ||
| { | ||
| // Is this a glob reference? | ||
| // Note: a glob reference can be a java.net.URI | ||
| if (reference.endsWith("/*") || reference.endsWith("\\*")) | ||
| { | ||
| Resource dir = newResource(reference.substring(0, reference.length() - 2)); | ||
| // Get the raw directory reference (without the trailing glob). | ||
| // This can be "/path/to/dir/*" (on unix an absolute path, on windows a relative path) | ||
| // or "C:/path/to/dir/*" (windows java Path syntax) | ||
| // or "C:\path\to\dir\*" (windows native syntax) | ||
| // or even a URI like "file:///path/to/dir/*" (always an absolute URI) | ||
| // and "file:///C:/path/to/dir/*" | ||
| // and "jar:file:///C:/path/to/foo.jar!/deep/*" | ||
| String rawDir = reference.substring(0, reference.length() - 2); | ||
| // Convert to a URI without loading it as a Resource (yet). | ||
| URI uri = URIUtil.toURI(rawDir); | ||
| // Load rawDir as a Resource | ||
| Resource dir = newResource(uri); | ||
| if (dir.isDirectory()) | ||
| { | ||
| // Loop through resource entries for content that will match glob. | ||
| List<Resource> expanded = dir.list(); | ||
| expanded.sort(ResourceCollators.byName(true)); | ||
| expanded.stream().filter(r -> FileID.isLibArchive(r.getName())).forEach(list::add); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| // Simple reference. Could have a scheme like jar:file:, or just file:. | ||
| // Or it could be a relative reference, in which case we need to | ||
| // ensure it is absolute. Otherwise, comparisons between Resources | ||
| // that point to the same resource but one is relative and one is absolute | ||
| // will fail. | ||
| if (URIUtil.hasScheme(reference)) | ||
| // Simple reference, could be relative, could be windows syntax, could be a URI. | ||
| URI uri = URIUtil.toURI(reference); | ||
| if ("jar".equals(uri.getScheme()) && unwrap) | ||
| { | ||
| // Could be a jar:file url, ensure it is unwrapped back to the file | ||
| // otherwise it will be a MountedPathResource and consume a mount point | ||
| // that might be unnecessary - the caller should always decide whether to mount | ||
| URI uri = new URI(reference); | ||
| if (unwrap) | ||
| list.add(newResource(URIUtil.unwrapContainer(uri))); | ||
| else | ||
| list.add(newResource(uri.toASCIIString())); | ||
| list.add(newResource(URIUtil.unwrapContainer(uri))); | ||
| } | ||
| else | ||
| { | ||
| Path p = Paths.get(reference); | ||
| if (!p.isAbsolute() && LOG.isDebugEnabled()) | ||
| LOG.warn("Non-absolute path: {}", reference); | ||
| list.add(newResource(p.toAbsolutePath())); | ||
| list.add(newResource(uri)); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a bit cautious that this is a behaviour change - previously relative references were ignored, but now they will be used. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Relative references are 100% necessary to support here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prior to your change, relative references were not ignored. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Example: |
||
| } | ||
| } | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.