spring-framework
spring-framework copied to clipboard
spring-projects
Fix `PathMatchingResourcePatternResolver` manifest classpath discovery
Update PathMatchingResourcePatternResolver so that in addition to searching the java.class.path system property for classpath enties, it also searches the MANIFEST.MF files from within those jars.
Prior to this commit, the addClassPathManifestEntries() method expected that the JVM had added Class-Path manifest entries to the java.class.path system property, however, this did not always happen.
The updated code now performs a deep search by loading MANIFEST.MF files from jars discovered from the system property. To deal with potential performance issue, loaded results are also now cached.
The updated code has been tested with Spring Boot 3.3 jars extracted using java -Djarmode=tools.
- See https://github.com/spring-projects/spring-boot/issues/42591 for background
Digesting this PR and reviewing occurrences of the problem elsewhere, the topic seems rather involved. For example, https://github.com/classgraph/classgraph/issues/20#issuecomment-141350828 claims that such manifest entries are actually meant to be URLs, not file paths. The approach taken there seems similar to ours in terms of manifest parsing, so I suppose we can proceed with this PR but we should review our assumptions in terms of file path parsing, accepting URLs as well?
Overall, @philwebb - is there a particular driver for submitting this PR for the 6.1.x branch? Such a signifcant refactoring in such a subtle core area seems better off in 6.2 for me, fixing the problem for Boot 3.4 rather than 3.3.x. If that's acceptable, could you please review the definition of manifest entries (URLs vs file paths) from your perspective and revisit the PR accordingly - and while in the process of this, rebase it onto main? I can take over from there then, bringing it into 6.2 GA for November.
is there a particular driver for submitting this PR for the 6.1.x branch?
Nothing other than I considered it a bug so I thought an earlier branch might be better. I'll rebase it onto main.
manifest entries are actually meant to be URLs, not file paths
I think this code is the current source of truth and it does look a little more involved than my PR. I will try to align our logic with theirs.
@jhoeller Rebased to main and forced pushed. I've aligned with the JDK code as best I can, but it's unfortunately pretty hard to test this stuff.
@juliojgd I don't think there are plans to backport due to the risk. See https://github.com/spring-projects/spring-framework/pull/33705#issuecomment-2427305822