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

Make multiple slash sanitization optional in web util #34242

Closed
wants to merge 1 commit into from

Conversation

vishnugt
Copy link

Make multiple slash sanitization optional in UriComponentsBuilder and UrlPathHelper

No breaking changes introduced; users can continue using the default behavior without modification.
Optional multi-slash sanitization is added to align with section 3.3 of RFC 3986, which permits empty path segments.

Closes gh-34076

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 11, 2025
* <ul>
* <li>replace all "//" by "/"</li>
* </ul>
*/
private static String getSanitizedPath(final String path) {
private String getSanitizedPath(final String path) {
Copy link
Author

@vishnugt vishnugt Jan 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a private static method that had 2 usages in the class, made it non-static to use the flag.
I can abstract the actual removing slash logic into a static method and have a non-static wrapper if you think that makes sense.

@@ -911,7 +931,7 @@ public void append(String... pathSegments) {
}

@Override
public @Nullable PathComponent build() {
public @Nullable PathComponent build(boolean sanitizePath) {
Copy link
Author

@vishnugt vishnugt Jan 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we don't use the sanitizePath flag, but I couldn't come up with any other approach that requires minimal changes while addressing the sanitizePath flag, especially considering that two of the three implementations make use of it. If you have a better solution, I’d be happy to implement it.

@bclozel
Copy link
Member

bclozel commented Jan 13, 2025

Thanks for contributing to Spring Framework.
Unfortunately, #34076 has been declined so I'll decline this PR as well.

@bclozel bclozel closed this Jan 13, 2025
@bclozel bclozel added in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple slash sanitisation should be optional in UriComponentsBuilder
3 participants