From a8359e1145d7b70bd503a6e61dbdeb8d56b35709 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 2 Sep 2024 06:11:52 -0600 Subject: [PATCH] Align RouterFunctions resource handling Closes: gh-33434 (cherry picked from commit d86bf8b2056429edf5494456cffcb2b243331c49) --- .../server/PathResourceLookupFunction.java | 109 +++++++++++++++--- .../function/PathResourceLookupFunction.java | 104 +++++++++++++++-- 2 files changed, 187 insertions(+), 26 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java index 60caf7c19e38..d725a1e6aab1 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/PathResourceLookupFunction.java @@ -18,6 +18,7 @@ import java.io.IOException; import java.io.UncheckedIOException; +import java.net.URLDecoder; import java.nio.charset.StandardCharsets; import java.util.function.Function; @@ -30,6 +31,7 @@ import org.springframework.util.Assert; import org.springframework.util.ResourceUtils; import org.springframework.util.StringUtils; +import org.springframework.web.util.UriUtils; import org.springframework.web.util.pattern.PathPattern; import org.springframework.web.util.pattern.PathPatternParser; @@ -63,13 +65,17 @@ public Mono apply(ServerRequest request) { pathContainer = this.pattern.extractPathWithinPattern(pathContainer); String path = processPath(pathContainer.value()); - if (path.contains("%")) { - path = StringUtils.uriDecode(path, StandardCharsets.UTF_8); + if (!StringUtils.hasText(path) || isInvalidPath(path)) { + return Mono.empty(); } - if (!StringUtils.hasLength(path) || isInvalidPath(path)) { + if (isInvalidEncodedInputPath(path)) { return Mono.empty(); } + if (!(this.location instanceof UrlResource)) { + path = UriUtils.decode(path, StandardCharsets.UTF_8); + } + try { Resource resource = this.location.createRelative(path); if (resource.isReadable() && isResourceUnderLocation(resource)) { @@ -84,7 +90,47 @@ public Mono apply(ServerRequest request) { } } - private String processPath(String path) { + /** + * 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"}. + *
+ */ + 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) == '/') { @@ -94,8 +140,7 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) { if (i == 0 || (i == 1 && slash)) { return path; } - path = slash ? "/" + path.substring(i) : path.substring(i); - return path; + return (slash ? "/" + path.substring(i) : path.substring(i)); } } return (slash ? "/" : ""); @@ -117,6 +162,31 @@ private boolean isInvalidPath(String path) { return false; } + /** + * 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 isInvalidEncodedInputPath(String path) { + if (path.contains("%")) { + try { + // Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars + String decodedPath = URLDecoder.decode(path, StandardCharsets.UTF_8); + if (isInvalidPath(decodedPath)) { + return true; + } + decodedPath = processPath(decodedPath); + if (isInvalidPath(decodedPath)) { + return true; + } + } + catch (IllegalArgumentException ex) { + // May not be possible to decode... + } + } + return false; + } + private boolean isResourceUnderLocation(Resource resource) throws IOException { if (resource.getClass() != this.location.getClass()) { return false; @@ -129,8 +199,8 @@ private boolean isResourceUnderLocation(Resource resource) throws IOException { resourcePath = resource.getURL().toExternalForm(); locationPath = StringUtils.cleanPath(this.location.getURL().toString()); } - else if (resource instanceof ClassPathResource) { - resourcePath = ((ClassPathResource) resource).getPath(); + else if (resource instanceof ClassPathResource classPathResource) { + resourcePath = classPathResource.getPath(); locationPath = StringUtils.cleanPath(((ClassPathResource) this.location).getPath()); } else { @@ -142,15 +212,24 @@ else if (resource instanceof ClassPathResource) { return true; } locationPath = (locationPath.endsWith("/") || locationPath.isEmpty() ? locationPath : locationPath + "/"); - if (!resourcePath.startsWith(locationPath)) { - return false; - } - if (resourcePath.contains("%") && StringUtils.uriDecode(resourcePath, StandardCharsets.UTF_8).contains("../")) { - return false; - } - return true; + return (resourcePath.startsWith(locationPath) && !isInvalidEncodedInputPath(resourcePath)); } + private boolean isInvalidEncodedResourcePath(String resourcePath) { + if (resourcePath.contains("%")) { + // Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars... + try { + String decodedPath = URLDecoder.decode(resourcePath, StandardCharsets.UTF_8); + if (decodedPath.contains("../") || decodedPath.contains("..\\")) { + return true; + } + } + catch (IllegalArgumentException ex) { + // May not be possible to decode... + } + } + return false; + } @Override public String toString() { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java index c37bbfdce919..82557b1c0fbd 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/function/PathResourceLookupFunction.java @@ -18,6 +18,7 @@ import java.io.IOException; import java.io.UncheckedIOException; +import java.net.URLDecoder; import java.nio.charset.StandardCharsets; import java.util.Optional; import java.util.function.Function; @@ -29,6 +30,8 @@ import org.springframework.util.Assert; import org.springframework.util.ResourceUtils; import org.springframework.util.StringUtils; +import org.springframework.web.context.support.ServletContextResource; +import org.springframework.web.util.UriUtils; import org.springframework.web.util.pattern.PathPattern; import org.springframework.web.util.pattern.PathPatternParser; @@ -36,6 +39,7 @@ * Lookup function used by {@link RouterFunctions#resources(String, Resource)}. * * @author Arjen Poutsma + * @author Rossen Stoyanchev * @since 5.2 */ class PathResourceLookupFunction implements Function> { @@ -62,13 +66,17 @@ public Optional apply(ServerRequest request) { pathContainer = this.pattern.extractPathWithinPattern(pathContainer); String path = processPath(pathContainer.value()); - if (path.contains("%")) { - path = StringUtils.uriDecode(path, StandardCharsets.UTF_8); + if (!StringUtils.hasText(path) || isInvalidPath(path)) { + return Optional.empty(); } - if (!StringUtils.hasLength(path) || isInvalidPath(path)) { + if (isInvalidEncodedInputPath(path)) { return Optional.empty(); } + if (!(this.location instanceof UrlResource)) { + path = UriUtils.decode(path, StandardCharsets.UTF_8); + } + try { Resource resource = this.location.createRelative(path); if (resource.isReadable() && isResourceUnderLocation(resource)) { @@ -83,7 +91,47 @@ public Optional apply(ServerRequest request) { } } - private String processPath(String path) { + /** + * 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"}. + *
+ */ + 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) == '/') { @@ -93,8 +141,7 @@ else if (path.charAt(i) > ' ' && path.charAt(i) != 127) { if (i == 0 || (i == 1 && slash)) { return path; } - path = slash ? "/" + path.substring(i) : path.substring(i); - return path; + return (slash ? "/" + path.substring(i) : path.substring(i)); } } return (slash ? "/" : ""); @@ -113,6 +160,26 @@ private boolean isInvalidPath(String path) { return path.contains("..") && StringUtils.cleanPath(path).contains("../"); } + private boolean isInvalidEncodedInputPath(String path) { + if (path.contains("%")) { + try { + // Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars + String decodedPath = URLDecoder.decode(path, StandardCharsets.UTF_8); + if (isInvalidPath(decodedPath)) { + return true; + } + decodedPath = processPath(decodedPath); + if (isInvalidPath(decodedPath)) { + return true; + } + } + catch (IllegalArgumentException ex) { + // May not be possible to decode... + } + } + return false; + } + private boolean isResourceUnderLocation(Resource resource) throws IOException { if (resource.getClass() != this.location.getClass()) { return false; @@ -129,6 +196,10 @@ else if (resource instanceof ClassPathResource) { resourcePath = ((ClassPathResource) resource).getPath(); locationPath = StringUtils.cleanPath(((ClassPathResource) this.location).getPath()); } + else if (resource instanceof ServletContextResource servletContextResource) { + resourcePath = servletContextResource.getPath(); + locationPath = StringUtils.cleanPath(((ServletContextResource) this.location).getPath()); + } else { resourcePath = resource.getURL().getPath(); locationPath = StringUtils.cleanPath(this.location.getURL().getPath()); @@ -138,13 +209,24 @@ else if (resource instanceof ClassPathResource) { return true; } locationPath = (locationPath.endsWith("/") || locationPath.isEmpty() ? locationPath : locationPath + "/"); - if (!resourcePath.startsWith(locationPath)) { - return false; - } - return !resourcePath.contains("%") || - !StringUtils.uriDecode(resourcePath, StandardCharsets.UTF_8).contains("../"); + return (resourcePath.startsWith(locationPath) && !isInvalidEncodedResourcePath(resourcePath)); } + private boolean isInvalidEncodedResourcePath(String resourcePath) { + if (resourcePath.contains("%")) { + // Use URLDecoder (vs UriUtils) to preserve potentially decoded UTF-8 chars... + try { + String decodedPath = URLDecoder.decode(resourcePath, StandardCharsets.UTF_8); + if (decodedPath.contains("../") || decodedPath.contains("..\\")) { + return true; + } + } + catch (IllegalArgumentException ex) { + // May not be possible to decode... + } + } + return false; + } @Override public String toString() {