Skip to content
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

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

Closed
ljcvok opened this issue Sep 18, 2024 · 5 comments
Closed
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Milestone

Comments

@ljcvok
Copy link

ljcvok commented Sep 18, 2024

This relates to #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 #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);
}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 18, 2024
@jhoeller jhoeller added type: regression A bug that is also a regression in: core Issues in core modules (aop, beans, core, context, expression) and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 18, 2024
@jhoeller jhoeller self-assigned this Sep 18, 2024
@jhoeller jhoeller added this to the 6.1.14 milestone Sep 18, 2024
@fendo8888
Copy link

fendo8888 commented Sep 18, 2024

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

@jhoeller jhoeller changed the title org.springframework.util.ResourceUtils#toRelativeURL drops custom URLHandler org.springframework.util.ResourceUtils#toRelativeURL drops custom URLStreamHandler Sep 18, 2024
@fendo8888
Copy link

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
Copy link
Contributor

jhoeller commented Sep 19, 2024

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
Copy link
Author

ljcvok commented Sep 23, 2024

@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 #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
Copy link
Contributor

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

4 participants