MockMvcRequestBuilder does not decode pathInfo [SPR-16453]

[spring-projects-issues]

Rob Winch opened SPR-16453 and commented

Description

The HttpServletRequest.getPathInfo() javadoc states:

Returns:
a String, decoded by the web container, specifying extra path information that comes after the servlet path but before the query string in the request URL; or null if the URL does not have any extra path information

However, MockMvcRequestBuilder does not decode pathInfo when calculating it from the provided URL. For example, the following test fails, but should pass

this.builder = new MockHttpServletRequestBuilder(HttpMethod.GET, "/travel/hotels 42");
MockHttpServletRequest request = this.builder.buildRequest(this.servletContext);

assertEquals("/travel/hotels 42", request.getPathInfo());

Impact

This bug has a fairly significant impact now that Spring Security rejects request that appear to be performing double encoding. For example, running the test with Spring Security would result in the following exception:

org.springframework.security.web.firewall.RequestRejectedException: The request was rejected because the URL contained a potentially malicious String "%"

	at org.springframework.security.web.firewall.StrictHttpFirewall.rejectedBlacklistedUrls(StrictHttpFirewall.java:145)
	at org.springframework.security.web.firewall.StrictHttpFirewall.getFirewalledRequest(StrictHttpFirewall.java:120)
	at org.springframework.security.web.FilterChainProxy.doFilterInternal(FilterChainProxy.java:194)
	at org.springframework.security.web.FilterChainProxy.doFilter(FilterChainProxy.java:178)
	at org.springframework.mock.web.MockFilterChain.doFilter(MockFilterChain.java:133)
	at org.springframework.test.web.servlet.MockMvc.perform(MockMvc.java:166)

Work Around

A workaround is by using a RequestPostProcessor to decode the pathInfo. For example:

public class DecodePathInfoPostProcessor implements RequestPostProcessor {
    private final UrlPathHelper urlPathHelper = new UrlPathHelper();
    @Override
    public MockHttpServletRequest postProcessRequest(MockHttpServletRequest request) {
        request.setPathInfo(this.urlPathHelper.decodeRequestString(request, request.getPathInfo()));
        return request;
    }
}

Then using it like this:

MockMvc mvc = MockMvcBuilders.standaloneSetup(new Controller())
	.defaultRequest(get("/").with(new DecodePathInfoPostProcessor()))
	.addFilter(new FilterChainProxy(Collections.emptyList()))
	.build();

mvc.perform(get("/path/hello world/"))
	.andExpect(status().isNotFound());

---

Affects: 4.3.14, 5.0.3

Referenced from: pull request #1659, and commits fe4472d, 0cd427b

Backported to: 4.3.15

0 votes, 5 watchers

[spring-projects-issues]

Rob Winch commented

I added a pull request #1659 I think it would be valuable if this could be patched in 4.3.x as well since cve-2018-1199 was also patched in 4.3.x and this issue will impact users there as well.

[spring-projects-issues]

Rossen Stoyanchev commented

By default MockHttpServletRequestBuilder initializes the requestUri and pathInfo but leaves contextPath and servletPath empty because there is no Servlet container and those shouldn't matter for request mapping purposes.

As a result, as far as I can tell, the pathInfo plays no role in request mapping with MockMvc. The UrlPathHelper ends up using the requestUri and never even needs to look at the pathInfo, and we don't access that directly anywhere else.

So it should be okay to backport to 4.3. When using MockMvc with Spring Security there could be some regression but it looks like that's currently broken as it is.

Juergen Hoeller your thoughts on a backport?

[spring-projects-issues]

Juergen Hoeller commented

Sounds ok for a backport to me. 4.3.15 will only go along with 5.0.5 anyway, so 5.0.4 is a fine testing ground there.