From 9efe37e649a74001fdd3c5c2ce0ba4def5d55d4d Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Wed, 24 Oct 2018 21:20:29 +0200 Subject: [PATCH] Fix ResourceUrlEncodingFilter lifecycle Prior to this commit, the `ResourceUrlEncodingFilter` would wrap the response and keep a reference to the request. When `HttpServletResponse.encodeURL` is later called during view rendering, the filter looks at the request and extracts context mapping information in order to resolve resource paths in views. This approach is flawed, when the filter is used with JSPs - if the request is forwarded to the container by the `InternalResourceView`, the request information is overwritten by the container. When the view is being rendered, the information available in the request is outdated and does not allow to correctly compute that context mapping information. This commit ensures that that information is being extracted from the request as soon as the `ResourceUrlProvider` is set as a request attribute. Issue: SPR-17421 (Cherry-picked from cf25efc7d3) --- .../resource/ResourceUrlEncodingFilter.java | 97 ++++++++++++------- .../ResourceUrlEncodingFilterTests.java | 34 +++++-- .../ResourceUrlProviderJavaConfigTests.java | 46 ++++++++- 3 files changed, 126 insertions(+), 51 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilter.java index d6ca1c33ecc..66aa5fefa6c 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilter.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,6 +22,7 @@ import javax.servlet.ServletException; import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletRequestWrapper; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponseWrapper; @@ -52,65 +53,50 @@ public class ResourceUrlEncodingFilter extends GenericFilterBean { public void doFilter(ServletRequest request, ServletResponse response, FilterChain filterChain) throws IOException, ServletException { if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) { - throw new ServletException("ResourceUrlEncodingFilter just supports HTTP requests"); + throw new ServletException("ResourceUrlEncodingFilter only supports HTTP requests"); } - HttpServletRequest httpRequest = (HttpServletRequest) request; - HttpServletResponse httpResponse = (HttpServletResponse) response; - filterChain.doFilter(httpRequest, new ResourceUrlEncodingResponseWrapper(httpRequest, httpResponse)); + ResourceUrlEncodingRequestWrapper wrappedRequest = + new ResourceUrlEncodingRequestWrapper((HttpServletRequest) request); + ResourceUrlEncodingResponseWrapper wrappedResponse = + new ResourceUrlEncodingResponseWrapper(wrappedRequest, (HttpServletResponse) response); + filterChain.doFilter(wrappedRequest, wrappedResponse); } + private static class ResourceUrlEncodingRequestWrapper extends HttpServletRequestWrapper { - private static class ResourceUrlEncodingResponseWrapper extends HttpServletResponseWrapper { - - private final HttpServletRequest request; + private ResourceUrlProvider resourceUrlProvider; /* Cache the index and prefix of the path within the DispatcherServlet mapping */ private Integer indexLookupPath; private String prefixLookupPath; - public ResourceUrlEncodingResponseWrapper(HttpServletRequest request, HttpServletResponse wrapped) { - super(wrapped); - this.request = request; + ResourceUrlEncodingRequestWrapper(HttpServletRequest request) { + super(request); } @Override - public String encodeURL(String url) { - ResourceUrlProvider resourceUrlProvider = getResourceUrlProvider(); - if (resourceUrlProvider == null) { - logger.debug("Request attribute exposing ResourceUrlProvider not found"); - return super.encodeURL(url); - } - - initLookupPath(resourceUrlProvider); - if (url.startsWith(this.prefixLookupPath)) { - int suffixIndex = getQueryParamsIndex(url); - String suffix = url.substring(suffixIndex); - String lookupPath = url.substring(this.indexLookupPath, suffixIndex); - lookupPath = resourceUrlProvider.getForLookupPath(lookupPath); - if (lookupPath != null) { - return super.encodeURL(this.prefixLookupPath + lookupPath + suffix); + public void setAttribute(String name, Object o) { + super.setAttribute(name, o); + if (ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR.equals(name)) { + if(o instanceof ResourceUrlProvider) { + initLookupPath((ResourceUrlProvider) o); } } - return super.encodeURL(url); - } - - private ResourceUrlProvider getResourceUrlProvider() { - return (ResourceUrlProvider) this.request.getAttribute( - ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR); } private void initLookupPath(ResourceUrlProvider urlProvider) { + this.resourceUrlProvider = urlProvider; if (this.indexLookupPath == null) { - UrlPathHelper pathHelper = urlProvider.getUrlPathHelper(); - String requestUri = pathHelper.getRequestUri(this.request); - String lookupPath = pathHelper.getLookupPathForRequest(this.request); + UrlPathHelper pathHelper = this.resourceUrlProvider.getUrlPathHelper(); + String requestUri = pathHelper.getRequestUri(this); + String lookupPath = pathHelper.getLookupPathForRequest(this); this.indexLookupPath = requestUri.lastIndexOf(lookupPath); this.prefixLookupPath = requestUri.substring(0, this.indexLookupPath); if ("/".equals(lookupPath) && !"/".equals(requestUri)) { - String contextPath = pathHelper.getContextPath(this.request); + String contextPath = pathHelper.getContextPath(this); if (requestUri.equals(contextPath)) { this.indexLookupPath = requestUri.length(); this.prefixLookupPath = requestUri; @@ -119,10 +105,47 @@ public class ResourceUrlEncodingFilter extends GenericFilterBean { } } + public String resolveUrlPath(String url) { + if (this.resourceUrlProvider == null) { + logger.debug("Request attribute exposing ResourceUrlProvider not found"); + return null; + } + if (url.startsWith(this.prefixLookupPath)) { + int suffixIndex = getQueryParamsIndex(url); + String suffix = url.substring(suffixIndex); + String lookupPath = url.substring(this.indexLookupPath, suffixIndex); + lookupPath = this.resourceUrlProvider.getForLookupPath(lookupPath); + if (lookupPath != null) { + return this.prefixLookupPath + lookupPath + suffix; + } + } + return null; + } + private int getQueryParamsIndex(String url) { int index = url.indexOf('?'); return (index > 0 ? index : url.length()); } } -} + + private static class ResourceUrlEncodingResponseWrapper extends HttpServletResponseWrapper { + + private final ResourceUrlEncodingRequestWrapper request; + + ResourceUrlEncodingResponseWrapper(ResourceUrlEncodingRequestWrapper request, HttpServletResponse wrapped) { + super(wrapped); + this.request = request; + } + + @Override + public String encodeURL(String url) { + String urlPath = this.request.resolveUrlPath(url); + if (urlPath != null) { + return super.encodeURL(urlPath); + } + return super.encodeURL(url); + } + } + +} \ No newline at end of file diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilterTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilterTests.java index d7716c7d7cf..7722f763198 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilterTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilterTests.java @@ -38,7 +38,7 @@ public class ResourceUrlEncodingFilterTests { private ResourceUrlEncodingFilter filter; - private ResourceUrlProvider resourceUrlProvider; + private ResourceUrlProvider urlProvider; @Before public void createFilter() throws Exception { @@ -49,16 +49,16 @@ public class ResourceUrlEncodingFilterTests { List resolvers = Arrays.asList(versionResolver, pathResolver); this.filter = new ResourceUrlEncodingFilter(); - this.resourceUrlProvider = createResourceUrlProvider(resolvers); + this.urlProvider = createResourceUrlProvider(resolvers); } @Test public void encodeURL() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/"); - request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); MockHttpServletResponse response = new MockHttpServletResponse(); this.filter.doFilter(request, response, (req, res) -> { + req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); String result = ((HttpServletResponse) res).encodeURL("/resources/bar.css"); assertEquals("/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result); }); @@ -68,10 +68,26 @@ public class ResourceUrlEncodingFilterTests { public void encodeURLWithContext() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context/foo"); request.setContextPath("/context"); - request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); MockHttpServletResponse response = new MockHttpServletResponse(); this.filter.doFilter(request, response, (req, res) -> { + req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); + String result = ((HttpServletResponse) res).encodeURL("/context/resources/bar.css"); + assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result); + }); + } + + + @Test + public void encodeUrlWithContextAndForwardedRequest() throws Exception { + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context/foo"); + request.setContextPath("/context"); + MockHttpServletResponse response = new MockHttpServletResponse(); + + this.filter.doFilter(request, response, (req, res) -> { + req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); + request.setRequestURI("/forwarded"); + request.setContextPath("/"); String result = ((HttpServletResponse) res).encodeURL("/context/resources/bar.css"); assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result); }); @@ -82,10 +98,10 @@ public class ResourceUrlEncodingFilterTests { public void encodeContextPathUrlWithoutSuffix() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context"); request.setContextPath("/context"); - request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); MockHttpServletResponse response = new MockHttpServletResponse(); this.filter.doFilter(request, response, (req, res) -> { + req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); String result = ((HttpServletResponse) res).encodeURL("/context/resources/bar.css"); assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result); }); @@ -95,10 +111,10 @@ public class ResourceUrlEncodingFilterTests { public void encodeContextPathUrlWithSuffix() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context/"); request.setContextPath("/context"); - request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); MockHttpServletResponse response = new MockHttpServletResponse(); this.filter.doFilter(request, response, (req, res) -> { + req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); String result = ((HttpServletResponse) res).encodeURL("/context/resources/bar.css"); assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result); }); @@ -109,10 +125,10 @@ public class ResourceUrlEncodingFilterTests { public void encodeEmptyURLWithContext() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context/foo"); request.setContextPath("/context"); - request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); MockHttpServletResponse response = new MockHttpServletResponse(); this.filter.doFilter(request, response, (req, res) -> { + req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); String result = ((HttpServletResponse) res).encodeURL("?foo=1"); assertEquals("?foo=1", result); }); @@ -123,10 +139,10 @@ public class ResourceUrlEncodingFilterTests { public void encodeURLWithRequestParams() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo"); request.setContextPath("/"); - request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); MockHttpServletResponse response = new MockHttpServletResponse(); this.filter.doFilter(request, response, (req, res) -> { + req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); String result = ((HttpServletResponse) res).encodeURL("/resources/bar.css?foo=bar&url=http://example.org"); assertEquals("/resources/bar-11e16cf79faee7ac698c805cf28248d2.css?foo=bar&url=http://example.org", result); }); @@ -138,10 +154,10 @@ public class ResourceUrlEncodingFilterTests { MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context-path/index"); request.setContextPath("/context-path"); request.setServletPath(""); - request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); MockHttpServletResponse response = new MockHttpServletResponse(); this.filter.doFilter(request, response, (req, res) -> { + req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider); String result = ((HttpServletResponse) res).encodeURL("index?key=value"); assertEquals("index?key=value", result); }); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlProviderJavaConfigTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlProviderJavaConfigTests.java index 9ad2f6aa4d0..fbdc0f5ca6b 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlProviderJavaConfigTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlProviderJavaConfigTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,14 @@ package org.springframework.web.servlet.resource; +import java.io.IOException; +import java.util.Arrays; +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -23,6 +31,7 @@ import javax.servlet.http.HttpServletResponse; import org.junit.Before; import org.junit.Test; +import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Configuration; import org.springframework.mock.web.test.MockFilterChain; import org.springframework.mock.web.test.MockHttpServletRequest; @@ -55,8 +64,6 @@ public class ResourceUrlProviderJavaConfigTests { @Before @SuppressWarnings("resource") public void setup() throws Exception { - this.filterChain = new MockFilterChain(this.servlet, new ResourceUrlEncodingFilter()); - AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext(); context.setServletContext(new MockServletContext()); context.register(WebConfig.class); @@ -66,8 +73,9 @@ public class ResourceUrlProviderJavaConfigTests { this.request.setContextPath("/myapp"); this.response = new MockHttpServletResponse(); - Object urlProvider = context.getBean(ResourceUrlProvider.class); - this.request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, urlProvider); + this.filterChain = new MockFilterChain(this.servlet, + new ResourceUrlEncodingFilter(), + new ResourceUrlProviderExposingFilter(context)); } @Test @@ -116,6 +124,34 @@ public class ResourceUrlProviderJavaConfigTests { } } + @SuppressWarnings("serial") + private static class ResourceUrlProviderExposingFilter implements Filter { + + private final ApplicationContext context; + + public ResourceUrlProviderExposingFilter(ApplicationContext context) { + this.context = context; + } + + @Override + public void init(FilterConfig filterConfig) throws ServletException { + + } + + @Override + public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) + throws IOException, ServletException { + Object urlProvider = context.getBean(ResourceUrlProvider.class); + request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, urlProvider); + chain.doFilter(request, response); + } + + @Override + public void destroy() { + + } + } + @SuppressWarnings("serial") private static class TestServlet extends HttpServlet {