Correct path encoding/decoding in PathResourceResolver

Decoding is required for non-UrlResource when the HandlerMapping is
not expected to decode the path. Encoding is the opposite.

This commit ensures correct determination of whether the HandlerMapping
is expected to have decoded the path or not that in turn depends on
whether PathPattern or PathMatcher is in use.

Closes gh-27791

spring-webmvc/src/main/java/org/springframework/web/servlet/resource/PathResourceResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2021 the original author or authors.
+ * Copyright 2002-2022 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.
@@ -260,36 +260,47 @@ else if (resource instanceof ServletContextResource) {
 	}
 
 	private String encodeOrDecodeIfNecessary(String path, @Nullable HttpServletRequest request, Resource location) {
-		if (shouldDecodeRelativePath(location, request)) {
-			return UriUtils.decode(path, StandardCharsets.UTF_8);
-		}
-		else if (shouldEncodeRelativePath(location) && request != null) {
-			Charset charset = this.locationCharsets.getOrDefault(location, StandardCharsets.UTF_8);
-			StringBuilder sb = new StringBuilder();
-			StringTokenizer tokenizer = new StringTokenizer(path, "/");
-			while (tokenizer.hasMoreTokens()) {
-				String value = UriUtils.encode(tokenizer.nextToken(), charset);
-				sb.append(value);
-				sb.append('/');
+		if (request != null) {
+			boolean usesPathPattern = (
+					ServletRequestPathUtils.hasCachedPath(request) &&
+					ServletRequestPathUtils.getCachedPath(request) instanceof PathContainer);
+
+			if (shouldDecodeRelativePath(location, usesPathPattern)) {
+				return UriUtils.decode(path, StandardCharsets.UTF_8);
 			}
-			if (!path.endsWith("/")) {
-				sb.setLength(sb.length() - 1);
+			else if (shouldEncodeRelativePath(location, usesPathPattern)) {
+				Charset charset = this.locationCharsets.getOrDefault(location, StandardCharsets.UTF_8);
+				StringBuilder sb = new StringBuilder();
+				StringTokenizer tokenizer = new StringTokenizer(path, "/");
+				while (tokenizer.hasMoreTokens()) {
+					String value = UriUtils.encode(tokenizer.nextToken(), charset);
+					sb.append(value);
+					sb.append('/');
+				}
+				if (!path.endsWith("/")) {
+					sb.setLength(sb.length() - 1);
+				}
+				return sb.toString();
 			}
-			return sb.toString();
-		}
-		else {
-			return path;
 		}
+		return path;
 	}
 
-	private boolean shouldDecodeRelativePath(Resource location, @Nullable HttpServletRequest request) {
-		return  (!(location instanceof UrlResource) && request != null &&
-				ServletRequestPathUtils.hasCachedPath(request) &&
-				ServletRequestPathUtils.getCachedPath(request) instanceof PathContainer);
+	/**
+	 * When the {@code HandlerMapping} is set to not decode the URL path, the
+	 * path needs to be decoded for non-{@code UrlResource} locations.
+	 */
+	private boolean shouldDecodeRelativePath(Resource location, boolean usesPathPattern) {
+		return (!(location instanceof UrlResource) &&
+				(usesPathPattern || (this.urlPathHelper != null && !this.urlPathHelper.isUrlDecode())));
 	}
 
-	private boolean shouldEncodeRelativePath(Resource location) {
-		return (location instanceof UrlResource &&
+	/**
+	 * When the {@code HandlerMapping} is set to decode the URL path, the path
+	 * needs to be encoded for {@code UrlResource} locations.
+	 */
+	private boolean shouldEncodeRelativePath(Resource location, boolean usesPathPattern) {
+		return (location instanceof UrlResource && !usesPathPattern &&
 				this.urlPathHelper != null && this.urlPathHelper.isUrlDecode());
 	}
 

spring-webmvc/src/test/java/org/springframework/web/servlet/resource/ResourceHttpRequestHandlerIntegrationTests.java

@@ -41,6 +41,7 @@
 import org.springframework.web.testfixture.servlet.MockServletConfig;
 import org.springframework.web.testfixture.servlet.MockServletContext;
 import org.springframework.web.util.UriUtils;
+import org.springframework.web.util.UrlPathHelper;
 import org.springframework.web.util.pattern.PathPatternParser;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -60,23 +61,35 @@ public class ResourceHttpRequestHandlerIntegrationTests {
 
 	public static Stream<Arguments> argumentSource() {
 		return Stream.of(
-				arguments(true, "/cp"),
-				arguments(true, "/fs"),
-				arguments(true, "/url"),
-				arguments(false, "/cp"),
-				arguments(false, "/fs"),
-				arguments(false, "/url")
+				// PathPattern
+				arguments(true, true, "/cp"),
+				arguments(true, true, "/fs"),
+				arguments(true, true, "/url"),
+
+				arguments(true, false, "/cp"),
+				arguments(true, false, "/fs"),
+				arguments(true, false, "/url"),
+
+				// PathMatcher
+				arguments(false, true, "/cp"),
+				arguments(false, true, "/fs"),
+				arguments(false, true, "/url"),
+
+				arguments(false, false, "/cp"),
+				arguments(false, false, "/fs"),
+				arguments(false, false, "/url")
 		);
 	}
 
 
 	@ParameterizedTest
 	@MethodSource("argumentSource")
-	void cssFile(boolean usePathPatterns, String pathPrefix) throws Exception {
+	void cssFile(boolean usePathPatterns, boolean decodingUrlPathHelper, String pathPrefix) throws Exception {
+
 		MockHttpServletRequest request = initRequest(pathPrefix + "/test/foo.css");
 		MockHttpServletResponse response = new MockHttpServletResponse();
 
-		DispatcherServlet servlet = initDispatcherServlet(usePathPatterns, WebConfig.class);
+		DispatcherServlet servlet = initDispatcherServlet(usePathPatterns, decodingUrlPathHelper, WebConfig.class);
 		servlet.service(request, response);
 
 		String description = "usePathPattern=" + usePathPatterns + ", prefix=" + pathPrefix;
@@ -87,11 +100,13 @@ void cssFile(boolean usePathPatterns, String pathPrefix) throws Exception {
 
 	@ParameterizedTest
 	@MethodSource("argumentSource") // gh-26775
-	void classpathLocationWithEncodedPath(boolean usePathPatterns, String pathPrefix) throws Exception {
+	void classpathLocationWithEncodedPath(
+			boolean usePathPatterns, boolean decodingUrlPathHelper, String pathPrefix) throws Exception {
+
 		MockHttpServletRequest request = initRequest(pathPrefix + "/test/foo with spaces.css");
 		MockHttpServletResponse response = new MockHttpServletResponse();
 
-		DispatcherServlet servlet = initDispatcherServlet(usePathPatterns, WebConfig.class);
+		DispatcherServlet servlet = initDispatcherServlet(usePathPatterns, decodingUrlPathHelper, WebConfig.class);
 		servlet.service(request, response);
 
 		String description = "usePathPattern=" + usePathPatterns + ", prefix=" + pathPrefix;
@@ -100,14 +115,16 @@ void classpathLocationWithEncodedPath(boolean usePathPatterns, String pathPrefix
 		assertThat(response.getContentAsString()).as(description).isEqualTo("h1 { color:red; }");
 	}
 
-	private DispatcherServlet initDispatcherServlet(boolean usePathPatterns, Class<?>... configClasses)
-			throws ServletException {
+	private DispatcherServlet initDispatcherServlet(
+			boolean usePathPatterns, boolean decodingUrlPathHelper, Class<?>... configClasses) throws ServletException {
 
 		AnnotationConfigWebApplicationContext context = new AnnotationConfigWebApplicationContext();
 		context.register(configClasses);
 		if (usePathPatterns) {
 			context.register(PathPatternParserConfig.class);
 		}
+		context.register(decodingUrlPathHelper ?
+				DecodingUrlPathHelperConfig.class : NonDecodingUrlPathHelperConfig.class);
 		context.setServletConfig(this.servletConfig);
 		context.refresh();
 
@@ -171,4 +188,26 @@ public void configurePathMatch(PathMatchConfigurer configurer) {
 		}
 	}
 
+
+	static class DecodingUrlPathHelperConfig implements WebMvcConfigurer {
+
+		@Override
+		public void configurePathMatch(PathMatchConfigurer configurer) {
+			UrlPathHelper helper = new UrlPathHelper();
+			helper.setUrlDecode(true);
+			configurer.setUrlPathHelper(helper);
+		}
+	}
+
+
+	static class NonDecodingUrlPathHelperConfig implements WebMvcConfigurer {
+
+		@Override
+		public void configurePathMatch(PathMatchConfigurer configurer) {
+			UrlPathHelper helper = new UrlPathHelper();
+			helper.setUrlDecode(false);
+			configurer.setUrlPathHelper(helper);
+		}
+	}
+
 }