root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

Clean duplicate separators in resource URLs 

Most Servlet containers do this anyway, but not all, and not
consistently for forward and backslashes.

Issue: SPR-16616
This commit is contained in:
Rossen Stoyanchev 2018-03-19 17:11:25 -04:00
parent fdd756ce8a
commit 0e28bee0f1
4 changed files with 231 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java
View File

@ -328,7 +328,15 @@ public class ResourceWebHandler implements WebHandler, InitializingBean {
if (path.contains("%")) {
try {
// Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars
if (isInvalidPath(URLDecoder.decode(path, "UTF-8"))) {
String decodedPath = URLDecoder.decode(path, "UTF-8");
if (isInvalidPath(decodedPath)) {
if (logger.isTraceEnabled()) {
logger.trace("Ignoring invalid resource path with escape sequences [" + path + "].");
}
return Mono.empty();
}
decodedPath = processPath(decodedPath);
if (isInvalidPath(decodedPath)) {
if (logger.isTraceEnabled()) {
logger.trace("Ignoring invalid resource path with escape sequences [" + path + "].");
}
@ -352,12 +360,47 @@ public class ResourceWebHandler implements WebHandler, InitializingBean {
}
/**
* Process the given resource path to be used.
* <p>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.
* <p>The default implementation replaces:
* <ul>
* <li>Backslash with forward slash.
* <li>Duplicate occurrences of slash with a single slash.
* <li>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"}.
* </ul>
* @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) == '/') {

89
spring-webflux/src/test/java/org/springframework/web/reactive/resource/ResourceWebHandlerTests.java
View File

@ -54,13 +54,10 @@ import org.springframework.web.server.MethodNotAllowedException;
import org.springframework.web.server.ResponseStatusException;
import org.springframework.web.server.ServerWebExchange;
import static org.hamcrest.Matchers.instanceOf;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.*;
/**
* Unit tests for {@link ResourceWebHandler}.
@ -240,39 +237,85 @@ public class ResourceWebHandlerTests {
}
@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(Mono.just(resource));
ResourceWebHandler handler = new ResourceWebHandler();
handler.setLocations(Collections.singletonList(new ClassPathResource("test/", getClass())));
handler.setResourceResolvers(Collections.singletonList(resolver));
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, ResourceWebHandler handler) {
ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get(""));
setPathWithinHandlerMapping(exchange, requestPath);
StepVerifier.create(handler.handle(exchange))
.expectErrorSatisfies(err -> {
assertThat(err, instanceOf(ResponseStatusException.class));
assertEquals(HttpStatus.NOT_FOUND, ((ResponseStatusException) err).getStatus());
}).verify(TIMEOUT);
}
@Test
public void testResolvePathWithTraversal() throws Exception {
for (HttpMethod method : HttpMethod.values()) {
testInvalidPath(method);
testResolvePathWithTraversal(method);
}
}
private void testInvalidPath(HttpMethod httpMethod) throws Exception {
private void testResolvePathWithTraversal(HttpMethod httpMethod) throws Exception {
Resource location = new ClassPathResource("test/", getClass());
this.handler.setLocations(Collections.singletonList(location));
testInvalidPath(httpMethod, "../testsecret/secret.txt", location);
testInvalidPath(httpMethod, "test/../../testsecret/secret.txt", location);
testInvalidPath(httpMethod, ":/../../testsecret/secret.txt", location);
testResolvePathWithTraversal(httpMethod, "../testsecret/secret.txt", location);
testResolvePathWithTraversal(httpMethod, "test/../../testsecret/secret.txt", location);
testResolvePathWithTraversal(httpMethod, ":/../../testsecret/secret.txt", location);
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(httpMethod, "file:" + secretPath, location);
testInvalidPath(httpMethod, "/file:" + secretPath, location);
testInvalidPath(httpMethod, "url:" + secretPath, location);
testInvalidPath(httpMethod, "/url:" + secretPath, location);
testInvalidPath(httpMethod, "////../.." + secretPath, location);
testInvalidPath(httpMethod, "/%2E%2E/testsecret/secret.txt", location);
testInvalidPath(httpMethod, "url:" + secretPath, location);
testResolvePathWithTraversal(httpMethod, "file:" + secretPath, location);
testResolvePathWithTraversal(httpMethod, "/file:" + secretPath, location);
testResolvePathWithTraversal(httpMethod, "url:" + secretPath, location);
testResolvePathWithTraversal(httpMethod, "/url:" + secretPath, location);
testResolvePathWithTraversal(httpMethod, "////../.." + secretPath, location);
testResolvePathWithTraversal(httpMethod, "/%2E%2E/testsecret/secret.txt", location);
testResolvePathWithTraversal(httpMethod, "%2F%2F%2E%2E%2F%2Ftestsecret/secret.txt", location);
testResolvePathWithTraversal(httpMethod, "url:" + secretPath, location);
// The following tests fail with a MalformedURLException on Windows
// testInvalidPath(location, "/" + secretPath);
// testInvalidPath(location, "/ " + secretPath);
// testResolvePathWithTraversal(location, "/" + secretPath);
// testResolvePathWithTraversal(location, "/ " + secretPath);
}
private void testInvalidPath(HttpMethod httpMethod, String requestPath, Resource location) throws Exception {
private void testResolvePathWithTraversal(HttpMethod httpMethod, String requestPath, Resource location) throws Exception {
ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.method(httpMethod, ""));
setPathWithinHandlerMapping(exchange, requestPath);
StepVerifier.create(this.handler.handle(exchange))

52
spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandler.java
View File

@ -520,7 +520,15 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
if (path.contains("%")) {
try {
// Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars
if (isInvalidPath(URLDecoder.decode(path, "UTF-8"))) {
String decodedPath = URLDecoder.decode(path, "UTF-8");
if (isInvalidPath(decodedPath)) {
if (logger.isTraceEnabled()) {
logger.trace("Ignoring invalid resource path with escape sequences [" + path + "].");
}
return null;
}
decodedPath = processPath(decodedPath);
if (isInvalidPath(decodedPath)) {
if (logger.isTraceEnabled()) {
logger.trace("Ignoring invalid resource path with escape sequences [" + path + "].");
}
@ -543,13 +551,47 @@ public class ResourceHttpRequestHandler extends WebContentGenerator
}
/**
* Process the given resource path to be used.
* <p>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.
* <p>The default implementation replaces:
* <ul>
* <li>Backslash with forward slash.
* <li>Duplicate occurrences of slash with a single slash.
* <li>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"}.
* </ul>
* @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) == '/') {

88
spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerTests.java
View File

@ -43,6 +43,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}.
@ -303,41 +304,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);
@ -356,7 +400,8 @@ public class ResourceHttpRequestHandlerTests {
}
@Test
public void processPath() throws Exception {
public void processPath() {
// Unchanged
assertSame("/foo/bar", this.handler.processPath("/foo/bar"));
assertSame("foo/bar", this.handler.processPath("foo/bar"));
@ -382,10 +427,17 @@ public class ResourceHttpRequestHandlerTests {
assertEquals("/", this.handler.processPath("/"));
assertEquals("/", this.handler.processPath("///"));
assertEquals("/", this.handler.processPath("/ / / "));
assertEquals("/", this.handler.processPath("\\/ \\/ \\/ "));
// duplicate slash or backslash
assertEquals("/foo/ /bar/baz/", this.handler.processPath("//foo/ /bar//baz//"));
assertEquals("/foo/ /bar/baz/", this.handler.processPath("\\\\foo\\ \\bar\\\\baz\\\\"));
assertEquals("foo/bar", this.handler.processPath("foo\\\\/\\////bar"));
}
@Test
public void initAllowedLocations() throws Exception {
public void initAllowedLocations() {
PathResourceResolver resolver = (PathResourceResolver) this.handler.getResourceResolvers().get(0);
Resource[] locations = resolver.getAllowedLocations();