From b9ebdaaf3710db473a2e1fec8641c316483a22aa Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 21 Mar 2018 23:18:36 -0400 Subject: [PATCH] Backport clean duplicate separators in resource URLs Issue: SPR-16616 --- .../resource/PathResourceResolver.java | 44 ++++--- .../resource/ResourceHttpRequestHandler.java | 112 +++++++++++++----- .../ResourceHttpRequestHandlerTests.java | 78 +++++++++--- 3 files changed, 168 insertions(+), 66 deletions(-) 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: + *

* @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: *