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

Add advice on Spring MVC path matching for 5.3 and above to the reference documentation #26750

Closed
ZH3FENG opened this issue Apr 1, 2021 · 3 comments
Assignees
Labels
type: documentation A documentation task
Milestone

Comments

@ZH3FENG
Copy link

ZH3FENG commented Apr 1, 2021

I had noticed that in org.springframework.web.util.UrlPathHelper#decodeAndCleanUriString special url will be processed.
uri = removeSemicolonContent(uri); uri = decodeRequestString(request, uri); uri = getSanitizedPath(uri); return uri;

With this process, uri like /;/a/b/c will be changed to //a/b/c, and /;/a%2fb/c will be changed to //a/b/c.
This can be different in Filter(for example, jetty),which will confuse the developer. Sometime may cause security bug.

I'd like to ask, is the any specification like rfc, servlet specification, or anything else.
If any specification available, we can follow it .

Thanks!

@ZH3FENG ZH3FENG changed the title Is there any specification in url process in org.springframework.web.util.UrlPathHelper#decodeAndCleanUriString Is there any specification in url processing by org.springframework.web.util.UrlPathHelper#decodeAndCleanUriString Apr 1, 2021
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 1, 2021
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Apr 6, 2021

The Servlet API unfortunately does not specify precisely how the servletPath is to be normalized. This is why it's best to avoid reliance on the servletPath.

For example if the Servlet is mapped to "/" then you can set alwaysUseFullPath to true and with Servlet 4.0 present we automatically detect that and bypass use of the servletPath. Another option is to switch to use of the parsed PathPattern as an alternative to AntPathMatcher as well as UrlPathHelper, and with PathPattern you also get support for Servlets mapped by prefix, e.g. "/myServlet" without the need for normalizing, that is assuming the prefix itself does not have any characters that require encoding.

Yet another option is to reject URLs that contain ";" if you don't expect them or duplicate slashes. The Spring Security firewall does that.

@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Apr 6, 2021
@rstoyanchev rstoyanchev self-assigned this Apr 6, 2021
@rstoyanchev rstoyanchev added the type: documentation A documentation task label Apr 6, 2021
@rstoyanchev rstoyanchev added this to the 5.3.6 milestone Apr 6, 2021
@rstoyanchev
Copy link
Contributor

I've set this to 5.3.6 in order to update the documentation with comprehensive advice on the topic. There have been a number of changes in 5.3 that should be summarized.

@ZH3FENG
Copy link
Author

ZH3FENG commented Apr 11, 2021

Thanks

@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 Apr 11, 2021
@rstoyanchev rstoyanchev removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged or decided on labels Apr 12, 2021
@rstoyanchev rstoyanchev modified the milestones: 5.3.6, 5.3.7 Apr 12, 2021
@rstoyanchev rstoyanchev changed the title Is there any specification in url processing by org.springframework.web.util.UrlPathHelper#decodeAndCleanUriString Add advice on Spring MVC path matching for 5.3 and above to the reference documentation Apr 26, 2021
Zoran0104 pushed a commit to Zoran0104/spring-framework that referenced this issue Aug 20, 2021
lxbzmy pushed a commit to lxbzmy/spring-framework that referenced this issue Mar 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation task
Projects
None yet
Development

No branches or pull requests

3 participants