From b10045dc0e91a671afe3a3a3ec93fafe2274f84b Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Wed, 16 Nov 2016 19:02:20 +0100 Subject: [PATCH] Do not execute ResourceUrlEncodingFilter only once per request In case the filter is also registered to the ERROR dispatcher, the following happens: * the filter is executed once for the regular execution * the filter should be executed a second time when dispatched to error Since the filter is a `OncePerRequestFilter`, the filter is only executed once and won't be executed when handling the error. This can lead to situations like spring-projects/spring-boot#7348 This commit makes this filter a simple `GenericFilterBean`. Issue: SPR-14891 --- .../resource/ResourceUrlEncodingFilter.java | 18 ++++-- .../ResourceUrlEncodingFilterTests.java | 61 ++++++------------- 2 files changed, 32 insertions(+), 47 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 901d0c5eac..30f3722982 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 @@ -19,6 +19,8 @@ package org.springframework.web.servlet.resource; import java.io.IOException; import javax.servlet.FilterChain; import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponseWrapper; @@ -26,7 +28,7 @@ import javax.servlet.http.HttpServletResponseWrapper; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import org.springframework.web.filter.OncePerRequestFilter; +import org.springframework.web.filter.GenericFilterBean; import org.springframework.web.util.UrlPathHelper; /** @@ -41,16 +43,20 @@ import org.springframework.web.util.UrlPathHelper; * @author Brian Clozel * @since 4.1 */ -public class ResourceUrlEncodingFilter extends OncePerRequestFilter { +public class ResourceUrlEncodingFilter extends GenericFilterBean { private static final Log logger = LogFactory.getLog(ResourceUrlEncodingFilter.class); @Override - protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) - throws ServletException, IOException { - - filterChain.doFilter(request, new ResourceUrlEncodingResponseWrapper(request, response)); + 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"); + } + HttpServletRequest httpRequest = (HttpServletRequest) request; + HttpServletResponse httpResponse = (HttpServletResponse) response; + filterChain.doFilter(httpRequest, new ResourceUrlEncodingResponseWrapper(httpRequest, httpResponse)); } 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 26dd6ae4ea..d7716c7d7c 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 @@ -15,15 +15,9 @@ */ package org.springframework.web.servlet.resource; -import java.io.IOException; import java.util.Arrays; import java.util.Collections; import java.util.List; - -import javax.servlet.FilterChain; -import javax.servlet.ServletException; -import javax.servlet.ServletRequest; -import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletResponse; import org.junit.Before; @@ -33,7 +27,7 @@ import org.springframework.core.io.ClassPathResource; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; /** * Unit tests for {@code ResourceUrlEncodingFilter}. @@ -64,12 +58,9 @@ public class ResourceUrlEncodingFilterTests { request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); MockHttpServletResponse response = new MockHttpServletResponse(); - this.filter.doFilterInternal(request, response, new FilterChain() { - @Override - public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { - String result = ((HttpServletResponse)response).encodeURL("/resources/bar.css"); - assertEquals("/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result); - } + this.filter.doFilter(request, response, (req, res) -> { + String result = ((HttpServletResponse) res).encodeURL("/resources/bar.css"); + assertEquals("/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result); }); } @@ -80,12 +71,9 @@ public class ResourceUrlEncodingFilterTests { request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); MockHttpServletResponse response = new MockHttpServletResponse(); - this.filter.doFilterInternal(request, response, new FilterChain() { - @Override - public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { - String result = ((HttpServletResponse)response).encodeURL("/context/resources/bar.css"); - assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result); - } + this.filter.doFilter(request, response, (req, res) -> { + String result = ((HttpServletResponse) res).encodeURL("/context/resources/bar.css"); + assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result); }); } @@ -97,8 +85,8 @@ public class ResourceUrlEncodingFilterTests { request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); MockHttpServletResponse response = new MockHttpServletResponse(); - this.filter.doFilterInternal(request, response, (request1, response1) -> { - String result = ((HttpServletResponse) response1).encodeURL("/context/resources/bar.css"); + this.filter.doFilter(request, response, (req, res) -> { + String result = ((HttpServletResponse) res).encodeURL("/context/resources/bar.css"); assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result); }); } @@ -110,8 +98,8 @@ public class ResourceUrlEncodingFilterTests { request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); MockHttpServletResponse response = new MockHttpServletResponse(); - this.filter.doFilterInternal(request, response, (request1, response1) -> { - String result = ((HttpServletResponse) response1).encodeURL("/context/resources/bar.css"); + this.filter.doFilter(request, response, (req, res) -> { + String result = ((HttpServletResponse) res).encodeURL("/context/resources/bar.css"); assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result); }); } @@ -124,12 +112,9 @@ public class ResourceUrlEncodingFilterTests { request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); MockHttpServletResponse response = new MockHttpServletResponse(); - this.filter.doFilterInternal(request, response, new FilterChain() { - @Override - public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { - String result = ((HttpServletResponse)response).encodeURL("?foo=1"); - assertEquals("?foo=1", result); - } + this.filter.doFilter(request, response, (req, res) -> { + String result = ((HttpServletResponse) res).encodeURL("?foo=1"); + assertEquals("?foo=1", result); }); } @@ -141,12 +126,9 @@ public class ResourceUrlEncodingFilterTests { request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); MockHttpServletResponse response = new MockHttpServletResponse(); - this.filter.doFilterInternal(request, response, new FilterChain() { - @Override - public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { - String result = ((HttpServletResponse)response).encodeURL("/resources/bar.css?foo=bar&url=http://example.org"); - assertEquals("/resources/bar-11e16cf79faee7ac698c805cf28248d2.css?foo=bar&url=http://example.org", result); - } + this.filter.doFilter(request, response, (req, res) -> { + 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); }); } @@ -159,12 +141,9 @@ public class ResourceUrlEncodingFilterTests { request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); MockHttpServletResponse response = new MockHttpServletResponse(); - this.filter.doFilterInternal(request, response, new FilterChain() { - @Override - public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { - String result = ((HttpServletResponse)response).encodeURL("index?key=value"); - assertEquals("index?key=value", result); - } + this.filter.doFilter(request, response, (req, res) -> { + String result = ((HttpServletResponse) res).encodeURL("index?key=value"); + assertEquals("index?key=value", result); }); }