SED-4920 Fragment path with white spaces causes undescriptive error upon AP upload - #703
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces URL decoding using URLDecoder.decode to handle URL-encoded paths in ClassLoaderResourceFilesystem and ResourcePathMatchingResolver. However, using URLDecoder.decode on URL paths is problematic because it incorrectly decodes + characters to spaces. It is recommended to use URI.create(...).getPath() instead to safely decode %20 while preserving literal + characters. Additionally, the jarFile path should also be decoded to prevent potential failures when the JAR file path contains spaces.
| String urlFile = url.getFile(); | ||
| int bangIndex = urlFile.indexOf('!'); | ||
| pathInJar = urlFile.substring(bangIndex + 2); | ||
| pathInJar = URLDecoder.decode(urlFile.substring(bangIndex + 2), StandardCharsets.UTF_8); |
There was a problem hiding this comment.
Using URLDecoder.decode on URL paths is problematic because it decodes + characters to spaces (which is only correct for application/x-www-form-urlencoded query parameters, not URL paths). If a file or directory name contains a literal + character, this will cause a FileNotFoundException or other errors.
A more robust way to decode URL paths in Java is to use URI.create(...).getPath(), which correctly decodes %20 to spaces while preserving + characters.
Additionally, note that jarFile on line 176 has a similar issue: if the path to the JAR file itself contains spaces, new URL(...).getFile() will return a URL-encoded path (with %20), causing new ZipFile(jarFile) to fail. You should decode jarFile as well, for example:
jarFile = URI.create(urlFile.substring(0, bangIndex)).getPath();
| pathInJar = URLDecoder.decode(urlFile.substring(bangIndex + 2), StandardCharsets.UTF_8); | |
| pathInJar = URI.create("file:///" + urlFile.substring(bangIndex + 2)).getPath().substring(1); |
| } | ||
| int lastIndexOf = file.lastIndexOf(getPathSeparator()); | ||
| String lastPath = file.substring(lastIndexOf + 1); | ||
| String lastPath = URLDecoder.decode(file.substring(lastIndexOf + 1), StandardCharsets.UTF_8); |
There was a problem hiding this comment.
Using URLDecoder.decode on URL paths is problematic because it decodes + characters to spaces (which is only correct for application/x-www-form-urlencoded query parameters, not URL paths). If a file or directory name contains a literal + character, this will cause a FileNotFoundException or other errors.
A more robust way to decode URL paths in Java is to use URI.create(...).getPath(), which correctly decodes %20 to spaces while preserving + characters.
| String lastPath = URLDecoder.decode(file.substring(lastIndexOf + 1), StandardCharsets.UTF_8); | |
| String lastPath = java.net.URI.create("file:///" + file.substring(lastIndexOf + 1)).getPath().substring(1); |
@david-stephan I propose the following fix, will still implement some unit tests to be sure