Skip to content

Commit cf25efc

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 50a4769)
1 parent 36510cf commit cf25efc

File tree

3 files changed

+96
-55
lines changed

3 files changed

+96
-55
lines changed

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

Lines changed: 63 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2017 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

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

66+
private static class ResourceUrlEncodingRequestWrapper extends HttpServletRequestWrapper {
6367

64-
private static class ResourceUrlEncodingResponseWrapper extends HttpServletResponseWrapper {
65-
66-
private final HttpServletRequest request;
68+
@Nullable
69+
private ResourceUrlProvider resourceUrlProvider;
6770

68-
/* Cache the index and prefix of the path within the DispatcherServlet mapping */
6971
@Nullable
7072
private Integer indexLookupPath;
7173

7274
private String prefixLookupPath = "";
7375

74-
public ResourceUrlEncodingResponseWrapper(HttpServletRequest request, HttpServletResponse wrapped) {
75-
super(wrapped);
76-
this.request = request;
76+
ResourceUrlEncodingRequestWrapper(HttpServletRequest request) {
77+
super(request);
7778
}
7879

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

98-
return super.encodeURL(url);
99-
}
100-
101-
@Nullable
102-
private ResourceUrlProvider getResourceUrlProvider() {
103-
return (ResourceUrlProvider) this.request.getAttribute(
104-
ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR);
10589
}
10690

107-
private int initLookupPath(ResourceUrlProvider urlProvider) {
91+
private void initLookupPath(ResourceUrlProvider urlProvider) {
92+
this.resourceUrlProvider = urlProvider;
10893
if (this.indexLookupPath == null) {
109-
UrlPathHelper pathHelper = urlProvider.getUrlPathHelper();
110-
String requestUri = pathHelper.getRequestUri(this.request);
111-
String lookupPath = pathHelper.getLookupPathForRequest(this.request);
94+
UrlPathHelper pathHelper = this.resourceUrlProvider.getUrlPathHelper();
95+
String requestUri = pathHelper.getRequestUri(this);
96+
String lookupPath = pathHelper.getLookupPathForRequest(this);
11297
this.indexLookupPath = requestUri.lastIndexOf(lookupPath);
11398
this.prefixLookupPath = requestUri.substring(0, this.indexLookupPath);
11499
if ("/".equals(lookupPath) && !"/".equals(requestUri)) {
115-
String contextPath = pathHelper.getContextPath(this.request);
100+
String contextPath = pathHelper.getContextPath(this);
116101
if (requestUri.equals(contextPath)) {
117102
this.indexLookupPath = requestUri.length();
118103
this.prefixLookupPath = requestUri;
119104
}
120105
}
121106
}
122-
return this.indexLookupPath;
107+
}
108+
109+
@Nullable
110+
public String resolveUrlPath(String url) {
111+
if (this.resourceUrlProvider == null) {
112+
logger.debug("Request attribute exposing ResourceUrlProvider not found");
113+
return null;
114+
}
115+
if (url.startsWith(this.prefixLookupPath)) {
116+
int suffixIndex = getQueryParamsIndex(url);
117+
String suffix = url.substring(suffixIndex);
118+
String lookupPath = url.substring(this.indexLookupPath, suffixIndex);
119+
lookupPath = this.resourceUrlProvider.getForLookupPath(lookupPath);
120+
if (lookupPath != null) {
121+
return this.prefixLookupPath + lookupPath + suffix;
122+
}
123+
}
124+
return null;
123125
}
124126

125127
private int getQueryParamsIndex(String url) {
@@ -128,4 +130,24 @@ private int getQueryParamsIndex(String url) {
128130
}
129131
}
130132

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

‎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: 8 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.
@@ -55,8 +55,6 @@ public class ResourceUrlProviderJavaConfigTests {
5555
@Before
5656
@SuppressWarnings("resource")
5757
public void setup() throws Exception {
58-
this.filterChain = new MockFilterChain(this.servlet, new ResourceUrlEncodingFilter());
59-
6058
AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext();
6159
context.setServletContext(new MockServletContext());
6260
context.register(WebConfig.class);
@@ -66,8 +64,13 @@ public void setup() throws Exception {
6664
this.request.setContextPath("/myapp");
6765
this.response = new MockHttpServletResponse();
6866

69-
Object urlProvider = context.getBean(ResourceUrlProvider.class);
70-
this.request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, urlProvider);
67+
this.filterChain = new MockFilterChain(this.servlet,
68+
new ResourceUrlEncodingFilter(),
69+
(request, response, chain) -> {
70+
Object urlProvider = context.getBean(ResourceUrlProvider.class);
71+
request.setAttribute(ResourceUrlProviderExposingInterceptor.RESOURCE_URL_PROVIDER_ATTR, urlProvider);
72+
chain.doFilter(request, response);
73+
});
7174
}
7275

7376
@Test

0 commit comments

Comments
 (0)