From 0f83dc14783a14e38f8bd525892caed0346e7755 Mon Sep 17 00:00:00 2001 From: Yanming Zhou Date: Tue, 12 Nov 2024 16:54:38 +0800 Subject: [PATCH] Align RouterFunctions resource handling Closes: gh-33434 --- .../server/PathResourceLookupFunction.java | 115 +++++++++++++++--- .../function/PathResourceLookupFunction.java | 113 +++++++++++++++-- 2 files changed, 200 insertions(+), 28 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..06cdb1e7e1f7 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2024 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. @@ -18,6 +18,8 @@ import java.io.IOException; import java.io.UncheckedIOException; +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; import java.nio.charset.StandardCharsets; import java.util.function.Function; @@ -30,6 +32,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 +66,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 +91,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 +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 ? "/" : ""); @@ -111,8 +157,33 @@ private boolean isInvalidPath(String path) { return true; } } - if (path.contains("..") && StringUtils.cleanPath(path).contains("../")) { - return true; + return path.contains("..") && StringUtils.cleanPath(path).contains("../"); + } + + /** + * 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, "UTF-8"); + if (isInvalidPath(decodedPath)) { + return true; + } + decodedPath = processPath(decodedPath); + if (isInvalidPath(decodedPath)) { + return true; + } + } + catch (IllegalArgumentException ex) { + // May not be possible to decode... + } + catch (UnsupportedEncodingException ex) { + throw new RuntimeException(ex); + } } return false; } @@ -142,15 +213,27 @@ 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, "UTF-8"); + if (decodedPath.contains("../") || decodedPath.contains("..\\")) { + return true; + } + } + catch (IllegalArgumentException ex) { + // May not be possible to decode... + } + catch (UnsupportedEncodingException ex) { + throw new RuntimeException(ex); + } + } + 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..3d184a34ba44 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2023 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. @@ -18,6 +18,8 @@ import java.io.IOException; import java.io.UncheckedIOException; +import java.io.UnsupportedEncodingException; +import java.net.URLDecoder; import java.nio.charset.StandardCharsets; import java.util.Optional; import java.util.function.Function; @@ -29,6 +31,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 +40,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 +67,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 +92,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 +142,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 +161,29 @@ 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, "UTF-8"); + if (isInvalidPath(decodedPath)) { + return true; + } + decodedPath = processPath(decodedPath); + if (isInvalidPath(decodedPath)) { + return true; + } + } + catch (IllegalArgumentException ex) { + // May not be possible to decode... + } + catch (UnsupportedEncodingException ex) { + throw new RuntimeException(ex); + } + } + return false; + } + private boolean isResourceUnderLocation(Resource resource) throws IOException { if (resource.getClass() != this.location.getClass()) { return false; @@ -129,6 +200,10 @@ else if (resource instanceof ClassPathResource) { resourcePath = ((ClassPathResource) resource).getPath(); locationPath = StringUtils.cleanPath(((ClassPathResource) this.location).getPath()); } + else if (resource instanceof ServletContextResource) { + resourcePath = ((ServletContextResource) resource).getPath(); + locationPath = StringUtils.cleanPath(((ServletContextResource) this.location).getPath()); + } else { resourcePath = resource.getURL().getPath(); locationPath = StringUtils.cleanPath(this.location.getURL().getPath()); @@ -138,13 +213,27 @@ 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, "UTF-8"); + if (decodedPath.contains("../") || decodedPath.contains("..\\")) { + return true; + } + } + catch (IllegalArgumentException ex) { + // May not be possible to decode... + } + catch (UnsupportedEncodingException ex) { + throw new RuntimeException(ex); + } + } + return false; + } @Override public String toString() {