From 1214624265207fcdf5ccee39c0db88a110fb9b6f Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Mon, 24 Nov 2014 14:08:54 +0100 Subject: [PATCH] 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) --- .../resource/ResourceHttpRequestHandler.java | 17 ++++++--- .../ResourceHttpRequestHandlerTests.java | 37 +++++++++---------- 2 files changed, 29 insertions(+), 25 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java index df889899735..cdafd181fd7 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java @@ -21,7 +21,6 @@ import java.io.InputStream; import java.io.UnsupportedEncodingException; import java.net.URLDecoder; import java.util.List; - import javax.activation.FileTypeMap; import javax.activation.MimetypesFileTypeMap; import javax.servlet.ServletException; @@ -30,6 +29,7 @@ import javax.servlet.http.HttpServletResponse; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; + import org.springframework.beans.factory.InitializingBean; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; @@ -43,6 +43,7 @@ import org.springframework.util.StreamUtils; import org.springframework.util.StringUtils; import org.springframework.web.HttpRequestHandler; import org.springframework.web.context.request.ServletWebRequest; +import org.springframework.web.context.support.ServletContextResource; import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.support.WebContentGenerator; @@ -300,13 +301,17 @@ public class ResourceHttpRequestHandler extends WebContentGenerator implements H } String resourcePath; String locationPath; - if (resource instanceof ClassPathResource) { + if (resource instanceof UrlResource) { + resourcePath = resource.getURL().toExternalForm(); + locationPath = location.getURL().toExternalForm(); + } + else if (resource instanceof ClassPathResource) { resourcePath = ((ClassPathResource) resource).getPath(); locationPath = ((ClassPathResource) location).getPath(); } - else if (resource instanceof UrlResource) { - resourcePath = resource.getURL().toExternalForm(); - locationPath = location.getURL().toExternalForm(); + else if (resource instanceof ServletContextResource) { + resourcePath = ((ServletContextResource) resource).getPath(); + locationPath = ((ServletContextResource) location).getPath(); } else { resourcePath = resource.getURL().getPath(); @@ -317,7 +322,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator implements H return false; } if (resourcePath.contains("%")) { - // Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars + // Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars... if (URLDecoder.decode(resourcePath, "UTF-8").contains("../")) { if (logger.isTraceEnabled()) { logger.trace("Resolved resource path contains \"../\" after decoding: " + resourcePath); diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java index ae809141b66..956238f7445 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java @@ -16,18 +16,14 @@ package org.springframework.web.servlet.resource; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; - import java.util.ArrayList; import java.util.Arrays; import java.util.List; - import javax.servlet.http.HttpServletResponse; import org.junit.Before; import org.junit.Test; + import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; import org.springframework.core.io.UrlResource; @@ -37,6 +33,8 @@ import org.springframework.mock.web.test.MockServletContext; import org.springframework.web.HttpRequestMethodNotSupportedException; import org.springframework.web.servlet.HandlerMapping; +import static org.junit.Assert.*; + /** * @author Keith Donald * @author Jeremy Grelle @@ -127,7 +125,6 @@ public class ResourceHttpRequestHandlerTests { assertEquals("function foo() { console.log(\"hello world\"); }", response.getContentAsString()); } - @Test public void invalidPath() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); @@ -159,6 +156,18 @@ public class ResourceHttpRequestHandlerTests { testInvalidPath(location, "url:" + secretPath, request, response); } + private void testInvalidPath(Resource location, String requestPath, + MockHttpServletRequest request, MockHttpServletResponse response) throws Exception { + + request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, requestPath); + response = new MockHttpServletResponse(); + this.handler.handleRequest(request, response); + if (!location.createRelative(requestPath).exists() && !requestPath.contains(":")) { + fail(requestPath + " doesn't actually exist as a relative path"); + } + assertEquals(404, response.getStatus()); + } + @Test public void ignoreInvalidEscapeSequence() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); @@ -192,7 +201,7 @@ public class ResourceHttpRequestHandlerTests { assertEquals("/foo/bar", this.handler.processPath(" // /// //// foo/bar")); assertEquals("/foo/bar", this.handler.processPath((char) 1 + " / " + (char) 127 + " // foo/bar")); - // root ot empty path + // root or empty path assertEquals("", this.handler.processPath(" ")); assertEquals("/", this.handler.processPath("/")); assertEquals("/", this.handler.processPath("///")); @@ -243,7 +252,7 @@ public class ResourceHttpRequestHandlerTests { assertEquals(404, response.getStatus()); } - @Test(expected=IllegalStateException.class) + @Test(expected = IllegalStateException.class) public void noPathWithinHandlerMappingAttribute() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.setMethod("GET"); @@ -251,7 +260,7 @@ public class ResourceHttpRequestHandlerTests { handler.handleRequest(request, response); } - @Test(expected=HttpRequestMethodNotSupportedException.class) + @Test(expected = HttpRequestMethodNotSupportedException.class) public void unsupportedHttpMethod() throws Exception { MockHttpServletRequest request = new MockHttpServletRequest(); request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, "/foo.css"); @@ -270,16 +279,6 @@ public class ResourceHttpRequestHandlerTests { assertEquals(404, response.getStatus()); } - private void testInvalidPath(Resource location, String requestPath, - MockHttpServletRequest request, MockHttpServletResponse response) throws Exception { - - request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, requestPath); - response = new MockHttpServletResponse(); - this.handler.handleRequest(request, response); - assertTrue(location.createRelative(requestPath).exists()); - assertEquals(404, response.getStatus()); - } - private static class TestServletContext extends MockServletContext {