Assert static resource location

rstoyanchev · rstoyanchev · commit 789d7effa9b4 · 2024-10-15T19:23:07.000+01:00
Closes gh-33712
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
@@ -44,12 +44,11 @@ class PathResourceLookupFunction implements Function<ServerRequest, Mono<Resourc
 
 	public PathResourceLookupFunction(String pattern, Resource location) {
 		Assert.hasLength(pattern, "'pattern' must not be empty");
-		Assert.notNull(location, "'location' must not be null");
+		ResourceHandlerUtils.assertResourceLocation(location);
 		this.pattern = PathPatternParser.defaultInstance.parse(pattern);
 		this.location = location;
 	}
 
-
 	@Override
 	public Mono<Resource> apply(ServerRequest request) {
 		PathContainer pathContainer = request.requestPath().pathWithinApplication();
diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceHandlerUtils.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceHandlerUtils.java
@@ -27,6 +27,8 @@
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.UrlResource;
 import org.springframework.core.log.LogFormatUtils;
+import org.springframework.lang.Nullable;
+import org.springframework.util.Assert;
 import org.springframework.util.ResourceUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.web.util.UriUtils;
@@ -42,6 +44,42 @@ public abstract class ResourceHandlerUtils {
 
 	private static final Log logger = LogFactory.getLog(ResourceHandlerUtils.class);
 
+	private static final String FOLDER_SEPARATOR = "/";
+
+	private static final String WINDOWS_FOLDER_SEPARATOR = "\\";
+
+
+	/**
+	 * Assert the given location is not null, and its path ends on slash.
+	 */
+	public static void assertResourceLocation(@Nullable Resource location) {
+		Assert.notNull(location, "Resource location must not be null");
+		try {
+			String path;
+			if (location instanceof UrlResource) {
+				path = location.getURL().toExternalForm();
+			}
+			else if (location instanceof ClassPathResource classPathResource) {
+				path = classPathResource.getPath();
+			}
+			else {
+				path = location.getURL().getPath();
+			}
+			assertLocationPath(path);
+		}
+		catch (IOException ex) {
+			// ignore
+		}
+	}
+
+	/**
+	 * Assert the given location path is a directory and ends on slash.
+	 */
+	public static void assertLocationPath(@Nullable String path) {
+		Assert.notNull(path, "Resource location path must not be null");
+		Assert.isTrue(path.endsWith(FOLDER_SEPARATOR) || path.endsWith(WINDOWS_FOLDER_SEPARATOR),
+				"Resource location does not end with slash: " + path);
+	}
 
 	/**
 	 * Normalize the given resource path replacing the following:
diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/resource/ResourceWebHandler.java
@@ -160,7 +160,10 @@ public List<String> getLocationValues() {
 	public void setLocations(@Nullable List<Resource> locations) {
 		this.locationResources.clear();
 		if (locations != null) {
-			this.locationResources.addAll(locations);
+			for (Resource location : locations) {
+				ResourceHandlerUtils.assertResourceLocation(location);
+				this.locationResources.add(location);
+			}
 		}
 	}
 
@@ -376,6 +379,7 @@ private void resolveResourceLocations() {
 			Assert.isTrue(CollectionUtils.isEmpty(this.locationResources), "Please set " +
 					"either Resource-based \"locations\" or String-based \"locationValues\", but not both.");
 			for (String location : this.locationValues) {
+				ResourceHandlerUtils.assertLocationPath(location);
 				result.add(this.resourceLoader.getResource(location));
 			}
 		}
diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/config/DelegatingWebFluxConfigurationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/config/DelegatingWebFluxConfigurationTests.java
@@ -124,7 +124,7 @@ void resourceHandlerMapping() {
 		delegatingConfig.setConfigurers(Collections.singletonList(webFluxConfigurer));
 		willAnswer(invocation -> {
 			ResourceHandlerRegistry registry = invocation.getArgument(0);
-			registry.addResourceHandler("/static/**").addResourceLocations("classpath:/static");
+			registry.addResourceHandler("/static/**").addResourceLocations("classpath:/static/");
 			return null;
 		}).given(webFluxConfigurer).addResourceHandlers(any(ResourceHandlerRegistry.class));
 
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
@@ -47,7 +47,7 @@ class PathResourceLookupFunction implements Function<ServerRequest, Optional<Res
 
 	public PathResourceLookupFunction(String pattern, Resource location) {
 		Assert.hasLength(pattern, "'pattern' must not be empty");
-		Assert.notNull(location, "'location' must not be null");
+		ResourceHandlerUtils.assertResourceLocation(location);
 		this.pattern = PathPatternParser.defaultInstance.parse(pattern);
 		this.location = location;
 	}
diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHandlerUtils.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/resource/ResourceHandlerUtils.java
@@ -27,6 +27,8 @@
 import org.springframework.core.io.Resource;
 import org.springframework.core.io.UrlResource;
 import org.springframework.core.log.LogFormatUtils;
+import org.springframework.lang.Nullable;
+import org.springframework.util.Assert;
 import org.springframework.util.ResourceUtils;
 import org.springframework.util.StringUtils;
 import org.springframework.web.context.support.ServletContextResource;
@@ -42,6 +44,42 @@ public abstract class ResourceHandlerUtils {
 
 	private static final Log logger = LogFactory.getLog(ResourceHandlerUtils.class);
 
+	private static final String FOLDER_SEPARATOR = "/";
+
+	private static final String WINDOWS_FOLDER_SEPARATOR = "\\";
+
+
+	/**
+	 * Assert the given location is not null, and its path ends on slash.
+	 */
+	public static void assertResourceLocation(@Nullable Resource location) {
+		Assert.notNull(location, "Resource location must not be null");
+		try {
+			String path;
+			if (location instanceof UrlResource) {
+				path = location.getURL().toExternalForm();
+			}
+			else if (location instanceof ClassPathResource classPathResource) {
+				path = classPathResource.getPath();
+			}
+			else {
+				path = location.getURL().getPath();
+			}
+			assertLocationPath(path);
+		}
+		catch (IOException ex) {
+			// ignore
+		}
+	}
+
+	/**
+	 * Assert the given location path is a directory and ends on slash.
+	 */
+	public static void assertLocationPath(@Nullable String path) {
+		Assert.notNull(path, "Resource location path must not be null");
+		Assert.isTrue(path.endsWith(FOLDER_SEPARATOR) || path.endsWith(WINDOWS_FOLDER_SEPARATOR),
+				"Resource location does not end with slash: " + path);
+	}
 
 	/**
 	 * Normalize the given resource path replacing the following:
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
@@ -180,7 +180,10 @@ public void setLocationValues(List<String> locations) {
 	public void setLocations(List<Resource> locations) {
 		Assert.notNull(locations, "Locations list must not be null");
 		this.locationResources.clear();
-		this.locationResources.addAll(locations);
+		for (Resource location : locations) {
+			ResourceHandlerUtils.assertResourceLocation(location);
+			this.locationResources.add(location);
+		}
 	}
 
 	/**
@@ -493,6 +496,7 @@ private void resolveResourceLocations() {
 					charset = Charset.forName(value);
 					location = location.substring(endIndex + 1);
 				}
+				ResourceHandlerUtils.assertLocationPath(location);
 				Resource resource = applicationContext.getResource(location);
 				if (location.equals("/") && !(resource instanceof ServletContextResource)) {
 					throw new IllegalStateException(
diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistryTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/ResourceHandlerRegistryTests.java
@@ -204,7 +204,7 @@ void resourceChainWithOverrides() {
 
 	@Test
 	void urlResourceWithCharset() {
-		this.registration.addResourceLocations("[charset=ISO-8859-1]file:///tmp");
+		this.registration.addResourceLocations("[charset=ISO-8859-1]file:///tmp/");
 		this.registration.resourceChain(true);
 
 		ResourceHttpRequestHandler handler = getHandler("/resources/**");
diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupportExtensionTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/config/annotation/WebMvcConfigurationSupportExtensionTests.java
@@ -457,7 +457,7 @@ public void configureViewResolvers(ViewResolverRegistry registry) {
 
 		@Override
 		public void addResourceHandlers(ResourceHandlerRegistry registry) {
-			registry.addResourceHandler("/resources/**").addResourceLocations("src/test/java");
+			registry.addResourceHandler("/resources/**").addResourceLocations("src/test/java/");
 		}
 
 		@Override
diff --git a/spring-webmvc/src/test/resources/org/springframework/web/servlet/config/mvc-config-resources-chain.xml b/spring-webmvc/src/test/resources/org/springframework/web/servlet/config/mvc-config-resources-chain.xml
@@ -23,7 +23,7 @@
 
 	<bean class="org.springframework.beans.factory.config.PropertyPlaceholderConfigurer">
 		<property name="properties">
-			<value>location=file:///tmp</value>
+			<value>location=file:///tmp/</value>
 		</property>
 	</bean>
 

Original file line number	Diff line number	Diff line change
`@@ -160,7 +160,10 @@ public List<String> getLocationValues() {`
`160`	`160`	`public void setLocations(@Nullable List<Resource> locations) {`
`161`	`161`	`this.locationResources.clear();`
`162`	`162`	`if (locations != null) {`
`163`		`- this.locationResources.addAll(locations);`
	`163`	`+ for (Resource location : locations) {`
	`164`	`+ ResourceHandlerUtils.assertResourceLocation(location);`
	`165`	`+ this.locationResources.add(location);`
	`166`	`+ }`
`164`	`167`	`}`
`165`	`168`	`}`
`166`	`169`
`@@ -376,6 +379,7 @@ private void resolveResourceLocations() {`
`376`	`379`	`Assert.isTrue(CollectionUtils.isEmpty(this.locationResources), "Please set " +`
`377`	`380`	`"either Resource-based \"locations\" or String-based \"locationValues\", but not both.");`
`378`	`381`	`for (String location : this.locationValues) {`
	`382`	`+ ResourceHandlerUtils.assertLocationPath(location);`
`379`	`383`	`result.add(this.resourceLoader.getResource(location));`
`380`	`384`	`}`
`381`	`385`	`}`
Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ class PathResourceLookupFunction implements Function<ServerRequest, Optional<Res`
`47`	`47`
`48`	`48`	`public PathResourceLookupFunction(String pattern, Resource location) {`
`49`	`49`	`Assert.hasLength(pattern, "'pattern' must not be empty");`
`50`		`- Assert.notNull(location, "'location' must not be null");`
	`50`	`+ ResourceHandlerUtils.assertResourceLocation(location);`
`51`	`51`	`this.pattern = PathPatternParser.defaultInstance.parse(pattern);`
`52`	`52`	`this.location = location;`
`53`	`53`	`}`
Original file line number	Diff line number	Diff line change
`@@ -457,7 +457,7 @@ public void configureViewResolvers(ViewResolverRegistry registry) {`
`457`	`457`
`458`	`458`	`@Override`
`459`	`459`	`public void addResourceHandlers(ResourceHandlerRegistry registry) {`
`460`		`- registry.addResourceHandler("/resources/**").addResourceLocations("src/test/java");`
	`460`	`+ registry.addResourceHandler("/resources/**").addResourceLocations("src/test/java/");`
`461`	`461`	`}`
`462`	`462`
`463`	`463`	`@Override`