diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java
index 8732c39ecd6..e8cbd8ee50b 100644
--- a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java
+++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -245,21 +245,7 @@ public class PathResourceResolver extends AbstractResourceResolver {
return true;
}
locationPath = (locationPath.endsWith("/") || locationPath.isEmpty() ? locationPath : locationPath + "/");
- if (!resourcePath.startsWith(locationPath)) {
- return false;
- }
-
- if (resourcePath.contains("%")) {
- // 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);
- }
- return false;
- }
- }
-
- return true;
+ return (resourcePath.startsWith(locationPath) && !isInvalidEncodedPath(resourcePath));
}
private String encodeIfNecessary(String path, HttpServletRequest request, Resource location) {
@@ -291,8 +277,30 @@ public class PathResourceResolver extends AbstractResourceResolver {
}
private boolean shouldEncodeRelativePath(Resource location) {
- return location instanceof UrlResource &&
- this.urlPathHelper != null && this.urlPathHelper.isUrlDecode();
+ return (location instanceof UrlResource && this.urlPathHelper != null && this.urlPathHelper.isUrlDecode());
+ }
+
+ private boolean isInvalidEncodedPath(String resourcePath) {
+ if (resourcePath.contains("%")) {
+ // Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars...
+ try {
+ String decodedPath = URLDecoder.decode(resourcePath, "UTF-8");
+ int separatorIndex = decodedPath.indexOf("..") + 2;
+ if (separatorIndex > 1 && separatorIndex < decodedPath.length()) {
+ char separator = decodedPath.charAt(separatorIndex);
+ if (separator == '/' || separator == '\\') {
+ if (logger.isTraceEnabled()) {
+ logger.trace("Resolved resource path contains \"../\" after decoding: " + resourcePath);
+ }
+ }
+ return true;
+ }
+ }
+ catch (UnsupportedEncodingException ex) {
+ // Should never happen...
+ }
+ }
+ return false;
}
}
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 2b31cb5f96c..9bd4a5f92cf 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
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@
package org.springframework.web.servlet.resource;
import java.io.IOException;
+import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import java.nio.charset.Charset;
import java.util.ArrayList;
@@ -507,6 +508,7 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
throw new IllegalStateException("Required request attribute '" +
HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE + "' is not set");
}
+
path = processPath(path);
if (!StringUtils.hasText(path) || isInvalidPath(path)) {
if (logger.isTraceEnabled()) {
@@ -514,25 +516,19 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
}
return null;
}
- if (path.contains("%")) {
- try {
- // Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars
- if (isInvalidPath(URLDecoder.decode(path, "UTF-8"))) {
- if (logger.isTraceEnabled()) {
- logger.trace("Ignoring invalid resource path with escape sequences [" + path + "].");
- }
- return null;
- }
- }
- catch (IllegalArgumentException ex) {
- // ignore
+ if (isInvalidEncodedPath(path)) {
+ if (logger.isTraceEnabled()) {
+ logger.trace("Ignoring invalid resource path with escape sequences [" + path + "]");
}
+ return null;
}
+
ResourceResolverChain resolveChain = new DefaultResourceResolverChain(getResourceResolvers());
Resource resource = resolveChain.resolveResource(request, path, getLocations());
if (resource == null || getResourceTransformers().isEmpty()) {
return resource;
}
+
ResourceTransformerChain transformChain =
new DefaultResourceTransformerChain(resolveChain, getResourceTransformers());
resource = transformChain.transform(request, resource);
@@ -540,13 +536,47 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
}
/**
- * Process the given resource path to be used.
- *
The default implementation replaces any combination of leading '/' and
- * control characters (00-1F and 7F) with a single "/" or "". For example
- * {@code " // /// //// foo/bar"} becomes {@code "/foo/bar"}.
+ * Process the given resource path.
+ *
The default implementation replaces:
+ *
+ * - Backslash with forward slash.
+ *
- Duplicate occurrences of slash with a single slash.
+ *
- Any combination of leading slash and control characters (00-1F and 7F)
+ * with a single "/" or "". For example {@code " / // foo/bar"}
+ * becomes {@code "/foo/bar"}.
+ *
* @since 3.2.12
*/
protected String processPath(String path) {
+ path = StringUtils.replace(path, "\\", "/");
+ path = cleanDuplicateSlashes(path);
+ return cleanLeadingSlash(path);
+ }
+
+ private String cleanDuplicateSlashes(String path) {
+ StringBuilder sb = null;
+ char prev = 0;
+ for (int i = 0; i < path.length(); i++) {
+ char curr = path.charAt(i);
+ try {
+ if ((curr == '/') && (prev == '/')) {
+ if (sb == null) {
+ sb = new StringBuilder(path.substring(0, i));
+ }
+ continue;
+ }
+ if (sb != null) {
+ sb.append(path.charAt(i));
+ }
+ }
+ finally {
+ prev = curr;
+ }
+ }
+ return sb != null ? sb.toString() : path;
+ }
+
+ private String cleanLeadingSlash(String path) {
boolean slash = false;
for (int i = 0; i < path.length(); i++) {
if (path.charAt(i) == '/') {
@@ -556,9 +586,9 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
if (i == 0 || (i == 1 && slash)) {
return path;
}
- path = slash ? "/" + path.substring(i) : path.substring(i);
+ path = (slash ? "/" + path.substring(i) : path.substring(i));
if (logger.isTraceEnabled()) {
- logger.trace("Path after trimming leading '/' and control characters: " + path);
+ logger.trace("Path after trimming leading '/' and control characters: [" + path + "]");
}
return path;
}
@@ -566,6 +596,34 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
return (slash ? "/" : "");
}
+ /**
+ * Check whether the given path contains invalid escape sequences.
+ * @param path the path to validate
+ * @return {@code true} if the path is invalid, {@code false} otherwise
+ */
+ private boolean isInvalidEncodedPath(String path) {
+ if (path.contains("%")) {
+ try {
+ // Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars
+ String decodedPath = URLDecoder.decode(path, "UTF-8");
+ if (isInvalidPath(decodedPath)) {
+ return true;
+ }
+ decodedPath = processPath(decodedPath);
+ if (isInvalidPath(decodedPath)) {
+ return true;
+ }
+ }
+ catch (IllegalArgumentException ex) {
+ // Should never happen...
+ }
+ catch (UnsupportedEncodingException ex) {
+ // Should never happen...
+ }
+ }
+ return false;
+ }
+
/**
* Identifies invalid resource paths. By default rejects:
*
@@ -580,32 +638,24 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
* path starts predictably with a single '/' or does not have one.
* @param path the path to validate
* @return {@code true} if the path is invalid, {@code false} otherwise
+ * @since 3.0.6
*/
protected boolean isInvalidPath(String path) {
- if (logger.isTraceEnabled()) {
- logger.trace("Applying \"invalid path\" checks to path: " + path);
- }
if (path.contains("WEB-INF") || path.contains("META-INF")) {
- if (logger.isTraceEnabled()) {
- logger.trace("Path contains \"WEB-INF\" or \"META-INF\".");
- }
+ logger.trace("Path contains \"WEB-INF\" or \"META-INF\".");
return true;
}
if (path.contains(":/")) {
String relativePath = (path.charAt(0) == '/' ? path.substring(1) : path);
if (ResourceUtils.isUrl(relativePath) || relativePath.startsWith("url:")) {
- if (logger.isTraceEnabled()) {
- logger.trace("Path represents URL or has \"url:\" prefix.");
- }
+ logger.trace("Path represents URL or has \"url:\" prefix.");
return true;
}
}
if (path.contains("..")) {
path = StringUtils.cleanPath(path);
if (path.contains("../")) {
- if (logger.isTraceEnabled()) {
- logger.trace("Path contains \"../\" after call to StringUtils#cleanPath.");
- }
+ logger.trace("Path contains \"../\" after call to StringUtils#cleanPath.");
return true;
}
}
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 2b03c9def91..c7bb0dd77dc 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
@@ -1,5 +1,5 @@
/*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -46,6 +46,7 @@ import org.springframework.web.accept.ContentNegotiationManagerFactoryBean;
import org.springframework.web.servlet.HandlerMapping;
import static org.junit.Assert.*;
+import static org.mockito.Mockito.*;
/**
* Unit tests for {@link ResourceHttpRequestHandler}.
@@ -310,41 +311,84 @@ public class ResourceHttpRequestHandlerTests {
}
@Test
- public void invalidPath() throws Exception {
+ public void testInvalidPath() throws Exception {
+
+ // Use mock ResourceResolver: i.e. we're only testing upfront validations...
+
+ Resource resource = mock(Resource.class);
+ when(resource.getFilename()).thenThrow(new AssertionError("Resource should not be resolved"));
+ when(resource.getInputStream()).thenThrow(new AssertionError("Resource should not be resolved"));
+ ResourceResolver resolver = mock(ResourceResolver.class);
+ when(resolver.resolveResource(any(), any(), any(), any())).thenReturn(resource);
+
+ ResourceHttpRequestHandler handler = new ResourceHttpRequestHandler();
+ handler.setLocations(Collections.singletonList(new ClassPathResource("test/", getClass())));
+ handler.setResourceResolvers(Collections.singletonList(resolver));
+ handler.setServletContext(new TestServletContext());
+ handler.afterPropertiesSet();
+
+ testInvalidPath("../testsecret/secret.txt", handler);
+ testInvalidPath("test/../../testsecret/secret.txt", handler);
+ testInvalidPath(":/../../testsecret/secret.txt", handler);
+
+ Resource location = new UrlResource(getClass().getResource("./test/"));
+ this.handler.setLocations(Collections.singletonList(location));
+ Resource secretResource = new UrlResource(getClass().getResource("testsecret/secret.txt"));
+ String secretPath = secretResource.getURL().getPath();
+
+ testInvalidPath("file:" + secretPath, handler);
+ testInvalidPath("/file:" + secretPath, handler);
+ testInvalidPath("url:" + secretPath, handler);
+ testInvalidPath("/url:" + secretPath, handler);
+ testInvalidPath("/../.." + secretPath, handler);
+ testInvalidPath("/%2E%2E/testsecret/secret.txt", handler);
+ testInvalidPath("/%2E%2E/testsecret/secret.txt", handler);
+ testInvalidPath("%2F%2F%2E%2E%2F%2F%2E%2E" + secretPath, handler);
+ }
+
+ private void testInvalidPath(String requestPath, ResourceHttpRequestHandler handler) throws Exception {
+ this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, requestPath);
+ this.response = new MockHttpServletResponse();
+ handler.handleRequest(this.request, this.response);
+ assertEquals(HttpStatus.NOT_FOUND.value(), this.response.getStatus());
+ }
+
+ @Test
+ public void resolvePathWithTraversal() throws Exception {
for (HttpMethod method : HttpMethod.values()) {
this.request = new MockHttpServletRequest("GET", "");
this.response = new MockHttpServletResponse();
- testInvalidPath(method);
+ testResolvePathWithTraversal(method);
}
}
- private void testInvalidPath(HttpMethod httpMethod) throws Exception {
+ private void testResolvePathWithTraversal(HttpMethod httpMethod) throws Exception {
this.request.setMethod(httpMethod.name());
Resource location = new ClassPathResource("test/", getClass());
this.handler.setLocations(Collections.singletonList(location));
- testInvalidPath(location, "../testsecret/secret.txt");
- testInvalidPath(location, "test/../../testsecret/secret.txt");
- testInvalidPath(location, ":/../../testsecret/secret.txt");
+ testResolvePathWithTraversal(location, "../testsecret/secret.txt");
+ testResolvePathWithTraversal(location, "test/../../testsecret/secret.txt");
+ testResolvePathWithTraversal(location, ":/../../testsecret/secret.txt");
location = new UrlResource(getClass().getResource("./test/"));
this.handler.setLocations(Collections.singletonList(location));
Resource secretResource = new UrlResource(getClass().getResource("testsecret/secret.txt"));
String secretPath = secretResource.getURL().getPath();
- testInvalidPath(location, "file:" + secretPath);
- testInvalidPath(location, "/file:" + secretPath);
- testInvalidPath(location, "url:" + secretPath);
- testInvalidPath(location, "/url:" + secretPath);
- testInvalidPath(location, "/" + secretPath);
- testInvalidPath(location, "////../.." + secretPath);
- testInvalidPath(location, "/%2E%2E/testsecret/secret.txt");
- testInvalidPath(location, "/ " + secretPath);
- testInvalidPath(location, "url:" + secretPath);
+ testResolvePathWithTraversal(location, "file:" + secretPath);
+ testResolvePathWithTraversal(location, "/file:" + secretPath);
+ testResolvePathWithTraversal(location, "url:" + secretPath);
+ testResolvePathWithTraversal(location, "/url:" + secretPath);
+ testResolvePathWithTraversal(location, "/" + secretPath);
+ testResolvePathWithTraversal(location, "////../.." + secretPath);
+ testResolvePathWithTraversal(location, "/%2E%2E/testsecret/secret.txt");
+ testResolvePathWithTraversal(location, "%2F%2F%2E%2E%2F%2Ftestsecret/secret.txt");
+ testResolvePathWithTraversal(location, "/ " + secretPath);
}
- private void testInvalidPath(Resource location, String requestPath) throws Exception {
+ private void testResolvePathWithTraversal(Resource location, String requestPath) throws Exception {
this.request.setAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE, requestPath);
this.response = new MockHttpServletResponse();
this.handler.handleRequest(this.request, this.response);