spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.63k stars 38.14k forks source link

`org.springframework.util.ResourceUtils#toRelativeURL` drops custom `URLStreamHandler` #33561

Closed ljcvok closed 1 month ago

ljcvok commented 1 month ago

This relates to https://github.com/spring-projects/spring-framework/issues/33199

Due to the custom encryption of JARs we have a custom java.net.URLStreamHandler on URL instance. In recent versions (from 6.1.x) the reimplemented method org.springframework.util.ResourceUtils#toRelativeURL fails to retain the original URLStreamHandler on the root URL instance. Instead, it uses only the string part of the URL and creates a new one.

6.1.x: toURL(StringUtils.applyRelativePath(root.toString(), relativePath));

vs 6.0.x: new URL(root, relativePath);

I propose similar approach as was the fix for https://github.com/spring-projects/spring-framework/issues/33199

if (ResourceUtils.toURL(root.toString()).equals(root)) {
  // Plain URL with default URLStreamHandler -> replace with cleaned path.
  return toURL(StringUtils.applyRelativePath(root.toString(), relativePath));;
} else {
  // Retain original URL instance, potentially including custom URLStreamHandler.
  return new URL(root, relativePath);
}
fendo8888 commented 1 month ago

I used xjar here to upgrade to spring 3.x and encountered this problem:https://github.com/core-lib/xjar/issues/133

fendo8888 commented 1 month ago

Seemingly not in xjar,ResourceUtils.toURL(root.tostring ()).equals(root) is always true,How to make ResourceUtils.toURL(root.tostring ()).equals(root) return false and call return new URL(root, relativePath) image

jhoeller commented 1 month ago

It looks like depending on the URLStreamHandler implementation underneath, the ResourceUtils.toURL(root.toString ()).equals(root) check may indeed mis-identify the URL as equal even if it does not have the same handler associated. This may also apply to the fix in #33199, as far as I can see.

@ljcvok what specific condition causes that equals check to return false in your scenario? Is it within your custom handler?

ljcvok commented 1 month ago

@jhoeller @fendo8888 Indeed, you are correct. The equals method of the URL class calls the handler's equals(URL u1, URL u2) method, but it does not verify whether the underlying handlers are equal. In my opinion, this is a poor design choice.

We have resolved the "clean path" issue, which caused the fix for issue https://github.com/spring-projects/spring-framework/issues/33199 to be effectively overlooked.

One potential workaround would be to compare url.equals(ResourceUtils.toURL(urlString)) instead of ResourceUtils.toURL(root.tostring ()).equals(root) This approach would invoke equals(URL u1, URL u2) on our handler, allowing us to override boolean sameFile(URL u1, URL u2) to return false, which would affect the equality check.

If the Spring team truly intends to clean up and create new URL instances and drop the custom URL handler, we would need to redesign our solution completely.

jhoeller commented 1 month ago

It turns out there is a way to retain our own relative path building: namely, to concatenate and clean the overall path ourselves and then pass it to new URL(root, cleanedPath) where cleanedPath is our resulting absolute path. This preserves our handling of cases such as #28522 where that URL constructor does not lead to the desired result for relative path specifications, while also retaining a custom URLStreamHandler. The only downside is that we keep using a now-deprecated URL constructor (on JDK 20+) but that seems to be unavoidable here.

I'm going to use that approach for ResourceUtils.toRelativeURL and also for PathMatchingResourcePatternResolver.convertClassLoaderURL (revising #33199 accordingly, not relying on any URL.equals comparisons there anymore).