Performance impact of ResourceUrlEncodingFilter on HttpServletResponse#encodeURL

[schosin]

Affects: Spring Framework 5.3.10

---

Prior discussion: https://gitter.im/spring-projects/spring-boot?at=615ffdb17db1e3753e2ba6f9
Repository showcasing the issue: https://github.com/schosin/ResourceUrlEncodingFilterPerformance

When using ResourceUrlEncodingFilter with its default configuration, invocations of HttpServletResponse#encodeURL will be wrapped by ResourceUrlEncodingResponseWrapper, which resolves URLs according to a ResourceUrlProvider.

With WebJarAssetLocator present, this will catch all URLs (/**) and test against five different locations whether the URL passed is a static resource:

[class path resource [META-INF/resources/], class path resource [resources/], class path resource [static/], class path resource [public/], ServletContext resource [/]]

For class path resources, this is done by resolving the path against the resource folder and checking calling Resource#isReadable, which ultimately throws a FileNotFoundException if the resource is not present. See PathResourceResolver#getResource(String, HttpServletRequest, List<? extends Resource>).

Since simple URLs to any handler mappings cause these checks, pages with many links will have poor performance, especialy on machines with weaker CPUs.

A workaround is to disable Spring's default resource handling (spring.web.resources.add-mappings) and registering custom resource handlers mapping handlers directly (e.g. "/css/** -> classpath:/static/css/").

[rstoyanchev]

This is sub-optimal indeed, but by default Spring Boot maps static resources on "/**", which means there is no way to rule out a given link as a non-static resource. All we can do is to pass it through the ResourceResolver chain which tries to match it to a resources under the configured locations.

This isn't too much of an issue when handling incoming requests since controllers are checked before static resources, but for ResourceUrlEncodingFilter it means it has to try to resolve every link. Ideally it should be used with static resource URLs that have some prefix that differentiates them. So one possible action would be to document this in Boot and recommend to switch to prefix based static resource URL paths.

Perhaps, PathResourceResolver could check if all of the locations given to it exist and if not remove them from the list. That could significantly reduce the checks, if you don't have static/, public/, or resources/. Not sure what else we could do on the Spring Framework side.

[schosin]

Would it be an option to reimplement AbstractFileResolvingResource#exists and AbstractFileResolvingResource#isReadable based on AbstractResource#resolveURL?
That would have to be done in sub classes of AbstractFileResolvingResource as not to break compatibility with existing user code, but would help with ClassPathResource, ServletContextResource and UrlResource.

[jhoeller]

@schosin It looks like this is related to #27541 - which I currently have in the works for 5.3.11 - and originally #21372 which led to a reimplementation of AbstractFileResolvingResource.isReadable() in 5.1, implying an existence check in isReadable() itself.

While ServletContextResource overrides both exists() and isReadable() for a more efficient check, ClassPathResource only overrides exists() - whereas as of 5.1, PathResourceResolver only calls isReadable(). It's worth noting that PathResourceResolver used to call resource.exists() && resource.isReadable() but as of 5.1, that's not semantically necessary anymore. Unfortunately it misses out on the efficient existence check that way.

As far as I see right now, overriding isReadable() in ClassPathResource to start with a resolveURL() check (just like its exists() implementation) would address the efficiency problem that you're pointing out, bringing it to the same performance level that ServletContextResource is at. I'm going to try that along with my AbstractFileResolvingResource revision in #27541, making it available in a 5.3.11 snapshot which you could give an early try later today.

[rstoyanchev]

I've made a change to filter out non-existing locations. In the given scenario, with default Boot settings and use of WebJars, it filters out 4 and leaves 1 location (ServletContextResource '/'). Using the sample with 10K and 100K links, I see a significant reduction of processing time, by more than a third. That should complement the other change to improve isReadable.