Skip to content

Conversation

@daggerok
Copy link

@daggerok daggerok commented May 9, 2017

See more about issue here: spring-projects/spring-boot#9133

@snicoll
Copy link
Member

snicoll commented May 9, 2017

@daggerok as I've asked you on the issue you've opened in Spring Boot, we need a Jira issue in our tracker for this change. Thanks!

@daggerok
Copy link
Author

daggerok commented May 9, 2017

@snicoll I created one here: https://jira.spring.io/browse/SPR-15529
can you please change label?

@daggerok daggerok closed this May 9, 2017
@daggerok daggerok reopened this May 9, 2017
@daggerok daggerok changed the title Encode uri string before it's URI created. Remove redundant slashes in URI-string before it's URI will be resolve. May 9, 2017
* </p>
*/
private static String squashSlashes(String uri) {
return isNull(uri) ? uri : uri.replaceAll("/{2,}", "/");
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't this collapse http://example.com/// to http:/example.com/?

Copy link
Author

@daggerok daggerok May 15, 2017

Choose a reason for hiding this comment

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

@Shredder121, you are talking about URL, but not URI, which actually i'm replacing..
so answering on your question: no, it wouldn't, it should collapse only URI: /// -> /

Copy link

@luyuanwan luyuanwan May 23, 2017

Choose a reason for hiding this comment

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

I think function squashSlashes should move to some tool utility class. :)

@snicoll
Copy link
Member

snicoll commented May 22, 2017

@daggerok is that PR still relevant?

@rstoyanchev
Copy link
Contributor

This evolved from https://jira.spring.io/browse/SPR-15529 to https://jira.spring.io/browse/SPR-15560 and where the actual issue was not the duplicate slash but a bug in how ReactorServerHttpRequest created the URI. So this can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants