Skip to content

Commit 9efe37e

Browse files
bclozelbclozel
committed
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 cf25efc)
1 parent 8e980d9 commit 9efe37e

File tree

3 files changed

+126
-51
lines changed

3 files changed

+126
-51
lines changed

‎spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilter.java

Lines changed: 60 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -22,6 +22,7 @@
2222
import javax.servlet.ServletRequest;
2323
import javax.servlet.ServletResponse;
2424
import javax.servlet.http.HttpServletRequest;
25+
import javax.servlet.http.HttpServletRequestWrapper;
2526
import javax.servlet.http.HttpServletResponse;
2627
import javax.servlet.http.HttpServletResponseWrapper;
2728

@@ -52,65 +53,50 @@ public class ResourceUrlEncodingFilter extends GenericFilterBean {
5253
public void doFilter(ServletRequest request, ServletResponse response, FilterChain filterChain)
5354
throws IOException, ServletException {
5455
if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) {
55-
throw new ServletException("ResourceUrlEncodingFilter just supports HTTP requests");
56+
throw new ServletException("ResourceUrlEncodingFilter only supports HTTP requests");
5657
}
57-
HttpServletRequest httpRequest = (HttpServletRequest) request;
58-
HttpServletResponse httpResponse = (HttpServletResponse) response;
59-
filterChain.doFilter(httpRequest, new ResourceUrlEncodingResponseWrapper(httpRequest, httpResponse));
58+
ResourceUrlEncodingRequestWrapper wrappedRequest =
59+
new ResourceUrlEncodingRequestWrapper((HttpServletRequest) request);
60+
ResourceUrlEncodingResponseWrapper wrappedResponse =
61+
new ResourceUrlEncodingResponseWrapper(wrappedRequest, (HttpServletResponse) response);
62+
filterChain.doFilter(wrappedRequest, wrappedResponse);
6063
}
6164

65+
private static class ResourceUrlEncodingRequestWrapper extends HttpServletRequestWrapper {
6266

63-
private static class ResourceUrlEncodingResponseWrapper extends HttpServletResponseWrapper {
64-
65-
private final HttpServletRequest request;
67+
private ResourceUrlProvider resourceUrlProvider;
6668

6769
/* Cache the index and prefix of the path within the DispatcherServlet mapping */
6870
private Integer indexLookupPath;
6971

7072
private String prefixLookupPath;
7173

72-
public ResourceUrlEncodingResponseWrapper(HttpServletRequest request, HttpServletResponse wrapped) {
73-
super(wrapped);
74-
this.request = request;
74+
ResourceUrlEncodingRequestWrapper(HttpServletRequest request) {
75+
super(request);
7576
}
7677

7778
@Override
78-
public String encodeURL(String url) {
79-
ResourceUrlProvider resourceUrlProvider = getResourceUrlProvider();
80-
if (resourceUrlProvider == null) {
81-
logger.debug("Request attribute exposing ResourceUrlProvider not found");
82-
return super.encodeURL(url);
83-
}
84-
85-
initLookupPath(resourceUrlProvider);
86-
if (url.startsWith(this.prefixLookupPath)) {
87-
int suffixIndex = getQueryParamsIndex(url);
88-
String suffix = url.substring(suffixIndex);
89-
String lookupPath = url.substring(this.indexLookupPath, suffixIndex);
90-
lookupPath = resourceUrlProvider.getForLookupPath(lookupPath);
91-
if (lookupPath != null) {
92-
return super.encodeURL(this.prefixLookupPath + lookupPath + suffix);
79+
public void setAttribute(String name, Object o) {
80+
super.setAttribute(name, o);
81+
if (ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR.equals(name)) {
82+
if(o instanceof ResourceUrlProvider) {
83+
initLookupPath((ResourceUrlProvider) o);
9384
}
9485
}
9586

96-
return super.encodeURL(url);
97-
}
98-
99-
private ResourceUrlProvider getResourceUrlProvider() {
100-
return (ResourceUrlProvider) this.request.getAttribute(
101-
ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR);
10287
}
10388

10489
private void initLookupPath(ResourceUrlProvider urlProvider) {
90+
this.resourceUrlProvider = urlProvider;
10591
if (this.indexLookupPath == null) {
106-
UrlPathHelper pathHelper = urlProvider.getUrlPathHelper();
107-
String requestUri = pathHelper.getRequestUri(this.request);
108-
String lookupPath = pathHelper.getLookupPathForRequest(this.request);
92+
UrlPathHelper pathHelper = this.resourceUrlProvider.getUrlPathHelper();
93+
String requestUri = pathHelper.getRequestUri(this);
94+
String lookupPath = pathHelper.getLookupPathForRequest(this);
10995
this.indexLookupPath = requestUri.lastIndexOf(lookupPath);
11096
this.prefixLookupPath = requestUri.substring(0, this.indexLookupPath);
11197

11298
if ("/".equals(lookupPath) && !"/".equals(requestUri)) {
113-
String contextPath = pathHelper.getContextPath(this.request);
99+
String contextPath = pathHelper.getContextPath(this);
114100
if (requestUri.equals(contextPath)) {
115101
this.indexLookupPath = requestUri.length();
116102
this.prefixLookupPath = requestUri;
@@ -119,10 +105,47 @@ private void initLookupPath(ResourceUrlProvider urlProvider) {
119105
}
120106
}
121107

108+
public String resolveUrlPath(String url) {
109+
if (this.resourceUrlProvider == null) {
110+
logger.debug("Request attribute exposing ResourceUrlProvider not found");
111+
return null;
112+
}
113+
if (url.startsWith(this.prefixLookupPath)) {
114+
int suffixIndex = getQueryParamsIndex(url);
115+
String suffix = url.substring(suffixIndex);
116+
String lookupPath = url.substring(this.indexLookupPath, suffixIndex);
117+
lookupPath = this.resourceUrlProvider.getForLookupPath(lookupPath);
118+
if (lookupPath != null) {
119+
return this.prefixLookupPath + lookupPath + suffix;
120+
}
121+
}
122+
return null;
123+
}
124+
122125
private int getQueryParamsIndex(String url) {
123126
int index = url.indexOf('?');
124127
return (index > 0 ? index : url.length());
125128
}
126129
}
127130

128-
}
131+
132+
private static class ResourceUrlEncodingResponseWrapper extends HttpServletResponseWrapper {
133+
134+
private final ResourceUrlEncodingRequestWrapper request;
135+
136+
ResourceUrlEncodingResponseWrapper(ResourceUrlEncodingRequestWrapper request, HttpServletResponse wrapped) {
137+
super(wrapped);
138+
this.request = request;
139+
}
140+
141+
@Override
142+
public String encodeURL(String url) {
143+
String urlPath = this.request.resolveUrlPath(url);
144+
if (urlPath != null) {
145+
return super.encodeURL(urlPath);
146+
}
147+
return super.encodeURL(url);
148+
}
149+
}
150+
151+
}

‎spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlEncodingFilterTests.java

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public class ResourceUrlEncodingFilterTests {
3838

3939
private ResourceUrlEncodingFilter filter;
4040

41-
private ResourceUrlProvider resourceUrlProvider;
41+
private ResourceUrlProvider urlProvider;
4242

4343
@Before
4444
public void createFilter() throws Exception {
@@ -49,16 +49,16 @@ public void createFilter() throws Exception {
4949
List<ResourceResolver> resolvers = Arrays.asList(versionResolver, pathResolver);
5050

5151
this.filter = new ResourceUrlEncodingFilter();
52-
this.resourceUrlProvider = createResourceUrlProvider(resolvers);
52+
this.urlProvider = createResourceUrlProvider(resolvers);
5353
}
5454

5555
@Test
5656
public void encodeURL() throws Exception {
5757
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/");
58-
request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider);
5958
MockHttpServletResponse response = new MockHttpServletResponse();
6059

6160
this.filter.doFilter(request, response, (req, res) -> {
61+
req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider);
6262
String result = ((HttpServletResponse) res).encodeURL("/resources/bar.css");
6363
assertEquals("/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result);
6464
});
@@ -68,10 +68,26 @@ public void encodeURL() throws Exception {
6868
public void encodeURLWithContext() throws Exception {
6969
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context/foo");
7070
request.setContextPath("/context");
71-
request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider);
7271
MockHttpServletResponse response = new MockHttpServletResponse();
7372

7473
this.filter.doFilter(request, response, (req, res) -> {
74+
req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider);
75+
String result = ((HttpServletResponse) res).encodeURL("/context/resources/bar.css");
76+
assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result);
77+
});
78+
}
79+
80+
81+
@Test
82+
public void encodeUrlWithContextAndForwardedRequest() throws Exception {
83+
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context/foo");
84+
request.setContextPath("/context");
85+
MockHttpServletResponse response = new MockHttpServletResponse();
86+
87+
this.filter.doFilter(request, response, (req, res) -> {
88+
req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider);
89+
request.setRequestURI("/forwarded");
90+
request.setContextPath("/");
7591
String result = ((HttpServletResponse) res).encodeURL("/context/resources/bar.css");
7692
assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result);
7793
});
@@ -82,10 +98,10 @@ public void encodeURLWithContext() throws Exception {
8298
public void encodeContextPathUrlWithoutSuffix() throws Exception {
8399
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context");
84100
request.setContextPath("/context");
85-
request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider);
86101
MockHttpServletResponse response = new MockHttpServletResponse();
87102

88103
this.filter.doFilter(request, response, (req, res) -> {
104+
req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider);
89105
String result = ((HttpServletResponse) res).encodeURL("/context/resources/bar.css");
90106
assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result);
91107
});
@@ -95,10 +111,10 @@ public void encodeContextPathUrlWithoutSuffix() throws Exception {
95111
public void encodeContextPathUrlWithSuffix() throws Exception {
96112
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context/");
97113
request.setContextPath("/context");
98-
request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider);
99114
MockHttpServletResponse response = new MockHttpServletResponse();
100115

101116
this.filter.doFilter(request, response, (req, res) -> {
117+
req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider);
102118
String result = ((HttpServletResponse) res).encodeURL("/context/resources/bar.css");
103119
assertEquals("/context/resources/bar-11e16cf79faee7ac698c805cf28248d2.css", result);
104120
});
@@ -109,10 +125,10 @@ public void encodeContextPathUrlWithSuffix() throws Exception {
109125
public void encodeEmptyURLWithContext() throws Exception {
110126
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context/foo");
111127
request.setContextPath("/context");
112-
request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider);
113128
MockHttpServletResponse response = new MockHttpServletResponse();
114129

115130
this.filter.doFilter(request, response, (req, res) -> {
131+
req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider);
116132
String result = ((HttpServletResponse) res).encodeURL("?foo=1");
117133
assertEquals("?foo=1", result);
118134
});
@@ -123,10 +139,10 @@ public void encodeEmptyURLWithContext() throws Exception {
123139
public void encodeURLWithRequestParams() throws Exception {
124140
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo");
125141
request.setContextPath("/");
126-
request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider);
127142
MockHttpServletResponse response = new MockHttpServletResponse();
128143

129144
this.filter.doFilter(request, response, (req, res) -> {
145+
req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider);
130146
String result = ((HttpServletResponse) res).encodeURL("/resources/bar.css?foo=bar&url=http://example.org");
131147
assertEquals("/resources/bar-11e16cf79faee7ac698c805cf28248d2.css?foo=bar&url=http://example.org", result);
132148
});
@@ -138,10 +154,10 @@ public void encodeUrlPreventStringOutOfBounds() throws Exception {
138154
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/context-path/index");
139155
request.setContextPath("/context-path");
140156
request.setServletPath("");
141-
request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.resourceUrlProvider);
142157
MockHttpServletResponse response = new MockHttpServletResponse();
143158

144159
this.filter.doFilter(request, response, (req, res) -> {
160+
req.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, this.urlProvider);
145161
String result = ((HttpServletResponse) res).encodeURL("index?key=value");
146162
assertEquals("index?key=value", result);
147163
});

‎spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceUrlProviderJavaConfigTests.java

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2014 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,13 +16,22 @@
1616

1717
package org.springframework.web.servlet.resource;
1818

19+
import java.io.IOException;
20+
import java.util.Arrays;
21+
import javax.servlet.Filter;
22+
import javax.servlet.FilterChain;
23+
import javax.servlet.FilterConfig;
24+
import javax.servlet.ServletException;
25+
import javax.servlet.ServletRequest;
26+
import javax.servlet.ServletResponse;
1927
import javax.servlet.http.HttpServlet;
2028
import javax.servlet.http.HttpServletRequest;
2129
import javax.servlet.http.HttpServletResponse;
2230

2331
import org.junit.Before;
2432
import org.junit.Test;
2533

34+
import org.springframework.context.ApplicationContext;
2635
import org.springframework.context.annotation.Configuration;
2736
import org.springframework.mock.web.test.MockFilterChain;
2837
import org.springframework.mock.web.test.MockHttpServletRequest;
@@ -55,8 +64,6 @@ public class ResourceUrlProviderJavaConfigTests {
5564
@Before
5665
@SuppressWarnings("resource")
5766
public void setup() throws Exception {
58-
this.filterChain = new MockFilterChain(this.servlet, new ResourceUrlEncodingFilter());
59-
6067
AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext();
6168
context.setServletContext(new MockServletContext());
6269
context.register(WebConfig.class);
@@ -66,8 +73,9 @@ public void setup() throws Exception {
6673
this.request.setContextPath("/myapp");
6774
this.response = new MockHttpServletResponse();
6875

69-
Object urlProvider = context.getBean(ResourceUrlProvider.class);
70-
this.request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, urlProvider);
76+
this.filterChain = new MockFilterChain(this.servlet,
77+
new ResourceUrlEncodingFilter(),
78+
new ResourceUrlProviderExposingFilter(context));
7179
}
7280

7381
@Test
@@ -116,6 +124,34 @@ public void addResourceHandlers(ResourceHandlerRegistry registry) {
116124
}
117125
}
118126

127+
@SuppressWarnings("serial")
128+
private static class ResourceUrlProviderExposingFilter implements Filter {
129+
130+
private final ApplicationContext context;
131+
132+
public ResourceUrlProviderExposingFilter(ApplicationContext context) {
133+
this.context = context;
134+
}
135+
136+
@Override
137+
public void init(FilterConfig filterConfig) throws ServletException {
138+
139+
}
140+
141+
@Override
142+
public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
143+
throws IOException, ServletException {
144+
Object urlProvider = context.getBean(ResourceUrlProvider.class);
145+
request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, urlProvider);
146+
chain.doFilter(request, response);
147+
}
148+
149+
@Override
150+
public void destroy() {
151+
152+
}
153+
}
154+
119155
@SuppressWarnings("serial")
120156
private static class TestServlet extends HttpServlet {
121157

0 commit comments

Comments
 (0)