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

Avoid duplicate JAR resources in PathMatchingResourcePatternResolver on MS Windows #31598

Closed
wants to merge 1 commit into from

Conversation

huyachigege
Copy link
Contributor

@huyachigege huyachigege commented Nov 13, 2023

To avoid duplicate JAR resources on MS Windows.

If you use mybatis-plus and your mapper-locations starts with classpath*:, Spring will find duplicate UrlResources for XML files which results in a crash on application startup.

Since their getCleanedUrl values are different mybatis will throw an exception, even though they represent the same resource.

@pivotal-cla
Copy link

@huyachigege Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 13, 2023
@pivotal-cla
Copy link

@huyachigege Thank you for signing the Contributor License Agreement!

@huyachigege
Copy link
Contributor Author

huyachigege commented Nov 13, 2023

ClassLoader.getResources("") will generate a path like jar:file:/F:xyz.xml; whereas, addClassPathManifestEntries() will generate a path like jar:file:F:xyz.xml.

They reference the same underlying resource, but UrlResource.getCleanedUrl() is different for each of them, so you get duplicate UrlResources returned from PathMatchingResourcePatternResolver.getResources(location).

My English is not good, I hope you can understand.

@sbrannen sbrannen changed the title Update PathMatchingResourcePatternResolver.java Avoid duplicate JAR resources in PathMatchingResourcePatternResolver Nov 13, 2023
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Nov 13, 2023
@sbrannen
Copy link
Member

Hi @huyachigege,

Congratulations on creating your first PR ever! 👍

We'll look into it.

My English is not good, I hope you can understand.

Yes, your explanation is good enough to follow.

I edited the text to make it slightly clearer, and hopefully I didn't change the meaning you wished to convey.

@huyachigege
Copy link
Contributor Author

Hi @huyachigege,

Congratulations on creating your first PR ever! 👍

We'll look into it.

My English is not good, I hope you can understand.

Yes, your explanation is good enough to follow.

I edited the text to make it slightly clearer, and hopefully I didn't change the meaning you wished to convey.

Thanks, that is what I mean.

@jhoeller
Copy link
Contributor

addClassPathManifestEntries delegates to hasDuplicate before adding a new entry, and that method is meant to detect exactly such a difference in leading slashes. Could you try to find out why this is not correctly identifying the duplicate in your scenario?

@sbrannen sbrannen added the status: waiting-for-feedback We need additional information before we can continue label Nov 13, 2023
@huyachigege
Copy link
Contributor Author

huyachigege commented Nov 13, 2023

addClassPathManifestEntries delegates to hasDuplicate before adding a new entry, and that method is meant to detect exactly such a difference in leading slashes. Could you try to find out why this is not correctly identifying the duplicate in your scenario?

line 497 will get root resources, it will contain jar:file:/F:xxx/app.jar!/boot-info/classes and jar:file:F:xxx/app.jar!. They are different now. But child of them will be jar:file:/F:xxx/app.jar!/.../xxx.xml and jar:file:F:xxx/app.jar!/.../xxx.xml. They are duplicate on windows. If on Linux, they are both jar:file:/..., so it works on Linux.

You can try on windows and use properties I just told.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 13, 2023
@sbrannen
Copy link
Member

Hi @huyachigege,

line 497 will get root resources,

That doesn't map to the current code on the main branch.

With which version of Spring Framework are you experiencing the duplicates?

@sbrannen sbrannen added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Nov 13, 2023
@huyachigege
Copy link
Contributor Author

huyachigege commented Nov 13, 2023

With which version of Spring Framework are you experiencing the duplicates?

5.3.x,sorry for that

Now is 23 clock in china,I need to go to bed. If there are any questions,I will reply tomorrow

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 13, 2023
@sbrannen
Copy link
Member

If you use mybatis-plus and your mapper-locations starts with classpath*:

Can you please provide us the exact location pattern you are using?

Specifically, what do you have for <???> in classpath*:<???>?

@huyachigege
Copy link
Contributor Author

huyachigege commented Nov 13, 2023

If you use mybatis-plus and your mapper-locations starts with classpath*:

Can you please provide us the exact location pattern you are using?

Specifically, what do you have for <???> in classpath*:<???>?

classpath*:/**/*_MYSQL.xml

@sbrannen
Copy link
Member

classpath*:/**/*_MYSQL.xml

Thanks. That helps.

Can you please tell us which version of Java and Spring Boot you're using as well?

@sbrannen sbrannen added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Nov 13, 2023
@huyachigege
Copy link
Contributor Author

classpath*:/**/*_MYSQL.xml

Thanks. That helps.

Can you please tell us which version of Java and Spring Boot you're using as well?

OpenJDK-11,SpringBoot 2.6.12.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 13, 2023
@sbrannen
Copy link
Member

line 497 will get root resources, it will contain jar:file:/F:xxx/app.jar!/boot-info/classes and jar:file:F:xxx/app.jar!. They are different now. But child of them will be jar:file:/F:xxx/app.jar!/.../xxx.xml and jar:file:F:xxx/app.jar!/.../xxx.xml. They are duplicate on windows. If on Linux, they are both jar:file:/..., so it works on Linux.

Based on that feedback, it appears that we are getting two different roots (rootDirUrl values) for the same underlying JAR:

  • jar:file:F:xxx/app.jar! -- for the actual Spring Boot application JAR
  • jar:file:/F:xxx/app.jar!/boot-info/classes -- from the MANIFEST of the Spring Boot application JAR

Then, in findPathMatchingResources() we add the results of doFindPathMatchingJarResources(rootDirResource, rootDirUrl, subPattern) without checking for duplicates.

The hasDuplicate() method Juergen mentioned is used at a lower level and therefore does not detect duplicates at this higher level.

@sbrannen sbrannen added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Nov 13, 2023
@sbrannen sbrannen self-assigned this Nov 13, 2023
@sbrannen sbrannen added this to the 6.0.14 milestone Nov 13, 2023
@sbrannen sbrannen changed the title Avoid duplicate JAR resources in PathMatchingResourcePatternResolver Avoid duplicate JAR resources in PathMatchingResourcePatternResolver on MS Windows Nov 13, 2023
sbrannen added a commit that referenced this pull request Nov 14, 2023
@sbrannen sbrannen closed this in d5874ab Nov 14, 2023
sbrannen pushed a commit that referenced this pull request Nov 14, 2023
…indows

This commit updates PathMatchingResourcePatternResolver to avoid
returning duplicate resources on MS Windows when searching using the
`classpath*:` prefix and a wildcard pattern that matches resources
which are directly present in a JAR as well as present via classpath
manifest entries.

See gh-31598
Closes gh-31603

(cherry picked from commit d5874ab)
sbrannen added a commit that referenced this pull request Nov 14, 2023
See gh-31598
See gh-31603

(cherry picked from commit e71117d)
@sbrannen
Copy link
Member

This has been merged into main in d5874ab, revised in e71117d, and backported to 5.3.x.

Thanks

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: bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants