ResourceUrlEncodingFilter causes LookupPathIndexException resulting in HTTP 400 when processing URLs indirectly referencing Welcome Files

[tknall]

Affects: 5.2.6

When the ResourceUrlEncodingFilter receives a parcial request like /somecontext/app/ - actually referencing Welcome Files (like /somecontext/app/index.html) - the initialization of the filter fails raising a LookupPathIndexException which finally leads to a HTTP 400 response status code.

When requesting URIs like /somecontext/app/, the servlet container (e.g. Apache Tomcat) implicitely adds the path to the respective Welcome File to the servlet path within the HttpServletRequest. This is done according to the Servlet Spec:

Servlet 3.1, section 10.10 - "Welcome Files"

[...] If a Web container receives a valid partial request, the Web container must examine
the welcome file list defined in the deployment descriptor. The welcome file list is an
ordered list of partial URLs with no trailing or leading /. The Web server must
append each welcome file in the order specified in the deployment descriptor to the
partial request and check whether a static resource in the WAR is mapped to that
request URI. If no match is found, the Web server MUST again append each
welcome file in the order specified in the deployment descriptor to the partial
request and check if a servlet is mapped to that request URI. The Web container
must send the request to the first resource in the WAR that matches. The container
may send the request to the welcome resource with a forward, a redirect, or a
container specific mechanism that is indistinguishable from a direct request. [...]

The ResourceUrlEncodingFilter fails when trying to match lookupPath against requestUri:

...
String requestUri = pathHelper.getRequestUri(this); // e.g. "/spring-webmvc-demo/app/"
String lookupPath = pathHelper.getLookupPathForRequest(this); // e.g. "/app/index.html"
this.indexLookupPath = requestUri.lastIndexOf(lookupPath);
if (this.indexLookupPath == -1) {
   throw new LookupPathIndexException(lookupPath, requestUri);
}
...

The issue was introduced with Spring 4.3.21/5.1.2 probably with commit 50a47691.

I've prepared a minimum demo project to show the issue:
https://github.com/tknall/spring-mvc-demo

Build the maven project, deploy the resulting war file with a Servlet container (e.g. Tomcat 8.5) and run http://localhost:8080/spring-webmvc-demo/app/ actually referencing an underlying welcome file spring-webmvc-demo/app/index.html with the browser of your choice.

The request fails with

Failed to find lookupPath '/app/index.html' within requestUri '/spring-webmvc-demo/app/'. This could be because the path has invalid encoded characters or isn't normalized.; nested exception is org.springframework.web.servlet.resource.ResourceUrlEncodingFilter$LookupPathIndexException: Failed to find lookupPath '/app/index.html' within requestUri '/spring-webmvc-demo/app/'. This could be because the path has invalid encoded characters or isn't normalized.

[bclozel]

Thanks for your repro project @tknall, this is really useful to understand such complex issues.

I'll try first to summarize the problem.

Previously , the ResourceUrlEncodingFilter would extract the relevant information from the request (request URI, lookup path) when the encoding of an URL is requested to the response and we need to rewrite URLs to resources. This approach was flawed, because by the time encoding is requested the request information might have changed; this is especially true for forwarded requests and JSPs. This was fixed in 5.1.2 with #21954 with the side effect that the extraction phase happens earlier, even if no URL encoding is requested.

Now the error message displayed comes from #22851, because without that fix, we'd get an out of bounds exceptions instead. This happens because of what you've pointed out: if the request URI and the lookup path are out of sync, we can't derive the required information.

So this issue really happens because of a combination between #21954 (extracting earlier) and #22851 (checking that the request URI and lookup path are related).

In this example, the request URI and lookup path don't share a common prefix, since the Servlet container rewrites the servlet path from /some/path/ to /some/path/index.html because the resource exists on disk and this is configured as a welcome file.

I'm wondering about two solutions:

1. we could make the information extraction lazy, avoiding such cases and improving performance as a side effect
2. we could consider that when request URI and lookup path don't share the same prefix is a valid use case and if that happens we should ignore skip this filter somehow. We should do that in a way that we don't break ResourceUrlEncodingFilter throws StringIndexOutOfBoundsException when %ED%B6 is in the URL path #22851 again.

@rstoyanchev what do you think?

[rstoyanchev]

This is based on the default Servlet mapping "/" which means the entire path after the contextPath is the lookup path to use for application mapping. That makes it unnecessary for UrlPathHelper to perform any servletPath checks but you have to tell it that because otherwise it is not aware of how the Servlet is mapped:

@Configuration
@EnableWebMvc
public class MyWebApplicationConfig implements WebMvcConfigurer {

	@Override
	public void configurePathMatch(PathMatchConfigurer configurer) {
		UrlPathHelper pathHelper = new UrlPathHelper();
		pathHelper.setAlwaysUseFullPath(true);
		configurer.setUrlPathHelper(pathHelper);
	}

	// ...

}

The app should now work just the same but more efficiently because getLookupPathForRequest has to do less work and is typically used multiple times per request. The one exception is a Welcome file where the Servlet path does not reflect the requestURI. However it is easy to map this in Spring MVC instead:

@Configuration
@EnableWebMvc
public class MyWebApplicationConfig implements WebMvcConfigurer {

	@Override
	public void configurePathMatch(PathMatchConfigurer configurer) {
		UrlPathHelper pathHelper = new UrlPathHelper();
		pathHelper.setAlwaysUseFullPath(true);
		configurer.setUrlPathHelper(pathHelper);
	}

	@Override
	public void addViewControllers(ViewControllerRegistry registry) {
		registry.addRedirectViewController("app", "app/");
		registry.addViewController("app/").setViewName("forward:index.html");
	}

	// ...

}

This is really worth doing in any case in order to avoid any sort of reliance on the Servlet path which is bad in all sorts of ways. Some of that is explained under #25100 where you can also see that in 5.3 we are going to automatically set alwaysUseFullPath=true where we can determine how the Servlet is mapped and likewise in Spring Boot spring-projects/spring-boot#21499.

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[tknall]

@rstoyanchev Your approach (making UrlPathHelper always perform full patch lookup and adding a dedicated view for the respective welcome file) indeed does the trick but imho its more or less still a kind of workaround (imho) which does not appear very intuitive. The problem still remains part of the code...

[bclozel]

Given what's been done in #25100 and spring-projects/spring-boot#21499 (where this configuration option is used whenever possible), I don't think we can really consider this as a workaround, but rather a sensible configuration option to align with your Servlet container configuration.

As explained in #25100, this is not really about Spring Framework optimizing for a specific case, but rather choosing the safest approach to avoid mapping mismatches or security issues in applications.

I'm closing this issue since this option fixes the underlying problem and the two linked issues have improved the discoverability of this option since it's mostly done for you now.

Thanks!