Skip to content

Commit 1214624

Browse files
bclozeljhoeller
authored andcommittedNov 24, 2014
Fix location checks for servlet 3 resources
SPR-12354 applied new checks to make sure that served static resources are under authorized locations. Prior to this change, serving static resources from Servlet 3 locations such as "/webjars/" would not work since those locations can be within one of the JARs on path. In that case, the checkLocation method would return false and disallow serving that static resource. This change fixes this issue by making sure to call the `ServletContextResource.getPath()` method for servlet context resources. Note that there's a known workaround for this issue, which is using a classpath scheme as location, such as: "classpath:/META-INF/resources/webjars/" instead of "/webjars". Issue: SPR-12432 (cherry picked from commit 161d3e3)
1 parent 493e846 commit 1214624

File tree

2 files changed

+29
-25
lines changed

2 files changed

+29
-25
lines changed
 

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

+11-6
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import java.io.UnsupportedEncodingException;
2222
import java.net.URLDecoder;
2323
import java.util.List;
24-
2524
import javax.activation.FileTypeMap;
2625
import javax.activation.MimetypesFileTypeMap;
2726
import javax.servlet.ServletException;
@@ -30,6 +29,7 @@
3029

3130
import org.apache.commons.logging.Log;
3231
import org.apache.commons.logging.LogFactory;
32+
3333
import org.springframework.beans.factory.InitializingBean;
3434
import org.springframework.core.io.ClassPathResource;
3535
import org.springframework.core.io.Resource;
@@ -43,6 +43,7 @@
4343
import org.springframework.util.StringUtils;
4444
import org.springframework.web.HttpRequestHandler;
4545
import org.springframework.web.context.request.ServletWebRequest;
46+
import org.springframework.web.context.support.ServletContextResource;
4647
import org.springframework.web.servlet.HandlerMapping;
4748
import org.springframework.web.servlet.support.WebContentGenerator;
4849

@@ -300,13 +301,17 @@ private boolean isResourceUnderLocation(Resource resource, Resource location) th
300301
}
301302
String resourcePath;
302303
String locationPath;
303-
if (resource instanceof ClassPathResource) {
304+
if (resource instanceof UrlResource) {
305+
resourcePath = resource.getURL().toExternalForm();
306+
locationPath = location.getURL().toExternalForm();
307+
}
308+
else if (resource instanceof ClassPathResource) {
304309
resourcePath = ((ClassPathResource) resource).getPath();
305310
locationPath = ((ClassPathResource) location).getPath();
306311
}
307-
else if (resource instanceof UrlResource) {
308-
resourcePath = resource.getURL().toExternalForm();
309-
locationPath = location.getURL().toExternalForm();
312+
else if (resource instanceof ServletContextResource) {
313+
resourcePath = ((ServletContextResource) resource).getPath();
314+
locationPath = ((ServletContextResource) location).getPath();
310315
}
311316
else {
312317
resourcePath = resource.getURL().getPath();
@@ -317,7 +322,7 @@ else if (resource instanceof UrlResource) {
317322
return false;
318323
}
319324
if (resourcePath.contains("%")) {
320-
// Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars
325+
// Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars...
321326
if (URLDecoder.decode(resourcePath, "UTF-8").contains("../")) {
322327
if (logger.isTraceEnabled()) {
323328
logger.trace("Resolved resource path contains \"../\" after decoding: " + resourcePath);

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

+18-19
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,14 @@
1616

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

19-
import static org.junit.Assert.assertEquals;
20-
import static org.junit.Assert.assertSame;
21-
import static org.junit.Assert.assertTrue;
22-
2319
import java.util.ArrayList;
2420
import java.util.Arrays;
2521
import java.util.List;
26-
2722
import javax.servlet.http.HttpServletResponse;
2823

2924
import org.junit.Before;
3025
import org.junit.Test;
26+
3127
import org.springframework.core.io.ClassPathResource;
3228
import org.springframework.core.io.Resource;
3329
import org.springframework.core.io.UrlResource;
@@ -37,6 +33,8 @@
3733
import org.springframework.web.HttpRequestMethodNotSupportedException;
3834
import org.springframework.web.servlet.HandlerMapping;
3935

36+
import static org.junit.Assert.*;
37+
4038
/**
4139
* @author Keith Donald
4240
* @author Jeremy Grelle
@@ -127,7 +125,6 @@ public void getResourceFromSubDirectoryOfAlternatePath() throws Exception {
127125
assertEquals("function foo() { console.log(\"hello world\"); }", response.getContentAsString());
128126
}
129127

130-
131128
@Test
132129
public void invalidPath() throws Exception {
133130
MockHttpServletRequest request = new MockHttpServletRequest();
@@ -159,6 +156,18 @@ public void invalidPath() throws Exception {
159156
testInvalidPath(location, "url:" + secretPath, request, response);
160157
}
161158

159+
private void testInvalidPath(Resource location, String requestPath,
160+
MockHttpServletRequest request, MockHttpServletResponse response) throws Exception {
161+
162+
request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, requestPath);
163+
response = new MockHttpServletResponse();
164+
this.handler.handleRequest(request, response);
165+
if (!location.createRelative(requestPath).exists() && !requestPath.contains(":")) {
166+
fail(requestPath + " doesn't actually exist as a relative path");
167+
}
168+
assertEquals(404, response.getStatus());
169+
}
170+
162171
@Test
163172
public void ignoreInvalidEscapeSequence() throws Exception {
164173
MockHttpServletRequest request = new MockHttpServletRequest();
@@ -192,7 +201,7 @@ public void processPath() throws Exception {
192201
assertEquals("/foo/bar", this.handler.processPath(" // /// //// foo/bar"));
193202
assertEquals("/foo/bar", this.handler.processPath((char) 1 + " / " + (char) 127 + " // foo/bar"));
194203

195-
// root ot empty path
204+
// root or empty path
196205
assertEquals("", this.handler.processPath(" "));
197206
assertEquals("/", this.handler.processPath("/"));
198207
assertEquals("/", this.handler.processPath("///"));
@@ -243,15 +252,15 @@ public void missingResourcePath() throws Exception {
243252
assertEquals(404, response.getStatus());
244253
}
245254

246-
@Test(expected=IllegalStateException.class)
255+
@Test(expected = IllegalStateException.class)
247256
public void noPathWithinHandlerMappingAttribute() throws Exception {
248257
MockHttpServletRequest request = new MockHttpServletRequest();
249258
request.setMethod("GET");
250259
MockHttpServletResponse response = new MockHttpServletResponse();
251260
handler.handleRequest(request, response);
252261
}
253262

254-
@Test(expected=HttpRequestMethodNotSupportedException.class)
263+
@Test(expected = HttpRequestMethodNotSupportedException.class)
255264
public void unsupportedHttpMethod() throws Exception {
256265
MockHttpServletRequest request = new MockHttpServletRequest();
257266
request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "/foo.css");
@@ -270,16 +279,6 @@ public void resourceNotFound() throws Exception {
270279
assertEquals(404, response.getStatus());
271280
}
272281

273-
private void testInvalidPath(Resource location, String requestPath,
274-
MockHttpServletRequest request, MockHttpServletResponse response) throws Exception {
275-
276-
request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, requestPath);
277-
response = new MockHttpServletResponse();
278-
this.handler.handleRequest(request, response);
279-
assertTrue(location.createRelative(requestPath).exists());
280-
assertEquals(404, response.getStatus());
281-
}
282-
283282

284283
private static class TestServletContext extends MockServletContext {
285284

0 commit comments

Comments
 (0)