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
This commit is contained in:
Brian Clozel 2016-11-16 19:02:20 +01:00
parent 9bcc7c3b06
commit b10045dc0e
2 changed files with 32 additions and 47 deletions

View File

@ -19,6 +19,8 @@ package org.springframework.web.servlet.resource;
import java.io.IOException; import java.io.IOException;
import javax.servlet.FilterChain; import javax.servlet.FilterChain;
import javax.servlet.ServletException; import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse; import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpServletResponseWrapper; 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.Log;
import org.apache.commons.logging.LogFactory; import org.apache.commons.logging.LogFactory;
import org.springframework.web.filter.OncePerRequestFilter; import org.springframework.web.filter.GenericFilterBean;
import org.springframework.web.util.UrlPathHelper; import org.springframework.web.util.UrlPathHelper;
/** /**
@ -41,16 +43,20 @@ import org.springframework.web.util.UrlPathHelper;
* @author Brian Clozel * @author Brian Clozel
* @since 4.1 * @since 4.1
*/ */
public class ResourceUrlEncodingFilter extends OncePerRequestFilter { public class ResourceUrlEncodingFilter extends GenericFilterBean {
private static final Log logger = LogFactory.getLog(ResourceUrlEncodingFilter.class); private static final Log logger = LogFactory.getLog(ResourceUrlEncodingFilter.class);
@Override @Override
protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) public void doFilter(ServletRequest request, ServletResponse response, FilterChain filterChain)
throws ServletException, IOException { throws IOException, ServletException {
if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) {
filterChain.doFilter(request, new ResourceUrlEncodingResponseWrapper(request, response)); throw new ServletException("ResourceUrlEncodingFilter just supports HTTP requests");
}
HttpServletRequest httpRequest = (HttpServletRequest) request;
HttpServletResponse httpResponse = (HttpServletResponse) response;
filterChain.doFilter(httpRequest, new ResourceUrlEncodingResponseWrapper(httpRequest, httpResponse));
} }

View File

@ -15,15 +15,9 @@
*/ */
package org.springframework.web.servlet.resource; package org.springframework.web.servlet.resource;
import java.io.IOException;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.List; 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 javax.servlet.http.HttpServletResponse;
import org.junit.Before; 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.MockHttpServletRequest;
import org.springframework.mock.web.test.MockHttpServletResponse; import org.springframework.mock.web.test.MockHttpServletResponse;
import static org.junit.Assert.*; import static org.junit.Assert.assertEquals;
/** /**
* Unit tests for {@code ResourceUrlEncodingFilter}. * Unit tests for {@code ResourceUrlEncodingFilter}.
@ -64,12 +58,9 @@ public class ResourceUrlEncodingFilterTests {
request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider);
MockHttpServletResponse response = new MockHttpServletResponse(); MockHttpServletResponse response = new MockHttpServletResponse();
this.filter.doFilterInternal(request, response, new FilterChain() { this.filter.doFilter(request, response, (req, res) -> {
@Override String result = ((HttpServletResponse) res).encodeURL("/resources/bar.css");
public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { assertEquals("/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result);
String result = ((HttpServletResponse)response).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); request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider);
MockHttpServletResponse response = new MockHttpServletResponse(); MockHttpServletResponse response = new MockHttpServletResponse();
this.filter.doFilterInternal(request, response, new FilterChain() { this.filter.doFilter(request, response, (req, res) -> {
@Override String result = ((HttpServletResponse) res).encodeURL("/context/resources/bar.css");
public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result);
String result = ((HttpServletResponse)response).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); request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider);
MockHttpServletResponse response = new MockHttpServletResponse(); MockHttpServletResponse response = new MockHttpServletResponse();
this.filter.doFilterInternal(request, response, (request1, response1) -> { this.filter.doFilter(request, response, (req, res) -> {
String result = ((HttpServletResponse) response1).encodeURL("/context/resources/bar.css"); String result = ((HttpServletResponse) res).encodeURL("/context/resources/bar.css");
assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result); assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result);
}); });
} }
@ -110,8 +98,8 @@ public class ResourceUrlEncodingFilterTests {
request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider);
MockHttpServletResponse response = new MockHttpServletResponse(); MockHttpServletResponse response = new MockHttpServletResponse();
this.filter.doFilterInternal(request, response, (request1, response1) -> { this.filter.doFilter(request, response, (req, res) -> {
String result = ((HttpServletResponse) response1).encodeURL("/context/resources/bar.css"); String result = ((HttpServletResponse) res).encodeURL("/context/resources/bar.css");
assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result); assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result);
}); });
} }
@ -124,12 +112,9 @@ public class ResourceUrlEncodingFilterTests {
request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider);
MockHttpServletResponse response = new MockHttpServletResponse(); MockHttpServletResponse response = new MockHttpServletResponse();
this.filter.doFilterInternal(request, response, new FilterChain() { this.filter.doFilter(request, response, (req, res) -> {
@Override String result = ((HttpServletResponse) res).encodeURL("?foo=1");
public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { assertEquals("?foo=1", result);
String result = ((HttpServletResponse)response).encodeURL("?foo=1");
assertEquals("?foo=1", result);
}
}); });
} }
@ -141,12 +126,9 @@ public class ResourceUrlEncodingFilterTests {
request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider);
MockHttpServletResponse response = new MockHttpServletResponse(); MockHttpServletResponse response = new MockHttpServletResponse();
this.filter.doFilterInternal(request, response, new FilterChain() { this.filter.doFilter(request, response, (req, res) -> {
@Override String result = ((HttpServletResponse) res).encodeURL("/resources/bar.css?foo=bar&url=http://example.org");
public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { assertEquals("/resources/bar-11e16cf79faee7ac698c805cf28248d2.css?foo=bar&url=http://example.org", result);
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);
}
}); });
} }
@ -159,12 +141,9 @@ public class ResourceUrlEncodingFilterTests {
request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider); request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider);
MockHttpServletResponse response = new MockHttpServletResponse(); MockHttpServletResponse response = new MockHttpServletResponse();
this.filter.doFilterInternal(request, response, new FilterChain() { this.filter.doFilter(request, response, (req, res) -> {
@Override String result = ((HttpServletResponse) res).encodeURL("index?key=value");
public void doFilter(ServletRequest request, ServletResponse response) throws IOException, ServletException { assertEquals("index?key=value", result);
String result = ((HttpServletResponse)response).encodeURL("index?key=value");
assertEquals("index?key=value", result);
}
}); });
} }