spring-projects / spring-framework

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

Add trailing slash to static resource location if missing #33817

Closed cagliostro92 closed 3 weeks ago

cagliostro92 commented 1 month ago

As already stressed by @philwebb in #33815 and by my colleague @milazzo-g, the brand new assertion about the required trailing slash can lead to important breaking changes in both custom project and libraries. As mentioned by the author in #33712:

A location that does not end on "/" does not actually work as intended, which is probably why it hasn't been reported as an issue. We should append the slash where we can, e.g. String location values, and assert Resource locations.

It seems to be clear that this constraint was intended to avoid unexpected behaviour, but the framework can still be accountable for adding the trailing slash without breaking other projects.

closes: #33815

pivotal-cla commented 1 month ago

@cagliostro92 Please sign the Contributor License Agreement!

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

See the FAQ for frequently asked questions.

pivotal-cla commented 1 month ago

@cagliostro92 Thank you for signing the Contributor License Agreement!

cagliostro92 commented 4 weeks ago

Hi @bclozel, I would like to bring to your attention that the offending assertion is still present and invoked in ResourceHttpRequestHandler and PathResourceLookupFunction. Let me know how you prefer handling such cases. PS: I've rebased my branch onto your main a few minutes ago, so if you have already downloaded it, please drop it.

bclozel commented 4 weeks ago

Thanks for the proposal, but I'm declining this one in favor of https://github.com/spring-projects/spring-framework/issues/33815#issuecomment-2449473711

rstoyanchev commented 4 weeks ago

Re-opening as per discussion under #33826.

rstoyanchev commented 3 weeks ago

Thanks for the PR, and I will apply equivalent changes, but I need to start in 6.1.x where the code is different, and then forward merge to main.