-
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 all 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 |
|---|---|---|
|
|
@@ -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: |
||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If on windows I pass in a
referenceoffile:///c:/some/thing.txtthen is theschemereported after conversion toURIfileorc?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The scheme would be
fileThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See the testcases in ResourceFactoryTest.testSplitIsAbsolute for some windows examples.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The 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)