Issue #12000 - use URIUtil::toURI instead of URI::create in MavenWebAppContext #12001
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ce / ResourceFactory). Also use Path.of() instead of Paths.get() per javadoc recommendations.
gregw
requested changes
Jul 3, 2024
| @@ -1892,24 +1892,27 @@ else if (scheme.equalsIgnoreCase("file")) | |||
| } | |||
|
|
|||
| /** | |||
| * <p>Convert a String into a URI suitable for use as a Resource.</p> | |||
| * <p>Convert a String into a URI in a sane way.</p> | |||
There was a problem hiding this comment.
you need to say it better than "in a sane way". what does that mean?
| * @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> | ||
| * This exits to take an end user provided String and make a usable URI out of it. |
Comment on lines
+1899
to
+1900
| * It is capable of dealing with paths that have spaces, windows slashes, windows UNC references, | ||
| * relative paths, absolute paths, and much more. |
There was a problem hiding this comment.
This still feels like a job of a ResourceFactory. Why is this needed?
| @@ -231,7 +231,7 @@ public void setResourceBases(String[] resourceBases) | |||
| // This is a user provided list of configurations. | |||
| // We have to assume that mounting can happen. | |||
| List<URI> uris = Stream.of(resourceBases) | |||
| .map(URI::create) | |||
| .map(URIUtil::toURI) | |||
There was a problem hiding this comment.
ResourceFactory is the class we should be using to convert user provided strings. Especially as what we want is a CompositeResource at the end.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Restore
URIUtil.toURI(String), properly javadoc it, and use it where end-user provided Strings are given.Include maven-plugin IT test that replicates reported issue.
Fixes #12000