Avoid use of java.net.URL constructors (for JDK 20 compatibility)

jhoeller · jhoeller · commit 934231729123 · 2023-04-30T23:03:39.000+02:00
Explicit path computation also leads to consistent relative path semantics for resource URLs. Closes gh-29481 Closes gh-28522
diff --git a/spring-core/src/main/java/org/springframework/core/io/UrlResource.java b/spring-core/src/main/java/org/springframework/core/io/UrlResource.java
@@ -89,33 +89,31 @@ public UrlResource(URI uri) throws MalformedURLException {
 	}
 
 	/**
-	 * Create a new {@code UrlResource} based on a URL path.
+	 * Create a new {@code UrlResource} based on a URI path.
 	 * <p>Note: The given path needs to be pre-encoded if necessary.
-	 * @param path a URL path
-	 * @throws MalformedURLException if the given URL path is not valid
-	 * @see java.net.URL#URL(String)
+	 * @param path a URI path
+	 * @throws MalformedURLException if the given URI path is not valid
+	 * @see ResourceUtils#toURI(String)
 	 */
 	public UrlResource(String path) throws MalformedURLException {
 		Assert.notNull(path, "Path must not be null");
+		String cleanedPath = StringUtils.cleanPath(path);
+		URI uri;
+		URL url;
 
-		// Equivalent without java.net.URL constructor - for building on JDK 20+
-		/*
 		try {
-			String cleanedPath = StringUtils.cleanPath(path);
-			this.uri = ResourceUtils.toURI(cleanedPath);
-			this.url = this.uri.toURL();
-			this.cleanedUrl = cleanedPath;
+			// Prefer URI construction with toURL conversion (as of 6.1)
+			uri = ResourceUtils.toURI(cleanedPath);
+			url = uri.toURL();
 		}
 		catch (URISyntaxException | IllegalArgumentException ex) {
-			MalformedURLException exToThrow = new MalformedURLException(ex.getMessage());
-			exToThrow.initCause(ex);
-			throw exToThrow;
+			uri = null;
+			url = ResourceUtils.toURL(path);
 		}
-		*/
 
-		this.uri = null;
-		this.url = ResourceUtils.toURL(path);
-		this.cleanedUrl = StringUtils.cleanPath(path);
+		this.uri = uri;
+		this.url = url;
+		this.cleanedUrl = cleanedPath;
 	}
 
 	/**
diff --git a/spring-core/src/main/java/org/springframework/util/ResourceUtils.java b/spring-core/src/main/java/org/springframework/util/ResourceUtils.java
@@ -390,20 +390,17 @@ public static URI toURI(String location) throws URISyntaxException {
 	 * @throws MalformedURLException if the location wasn't a valid URL
 	 * @since 6.0
 	 */
+	@SuppressWarnings("deprecation")  // on JDK 20
 	public static URL toURL(String location) throws MalformedURLException {
-		// Equivalent without java.net.URL constructor - for building on JDK 20+
-		/*
 		try {
+			// Prefer URI construction with toURL conversion (as of 6.1)
 			return toURI(StringUtils.cleanPath(location)).toURL();
 		}
 		catch (URISyntaxException | IllegalArgumentException ex) {
-			MalformedURLException exToThrow = new MalformedURLException(ex.getMessage());
-			exToThrow.initCause(ex);
-			throw exToThrow;
+			// Lenient fallback to deprecated (on JDK 20) URL constructor,
+			// e.g. for decoded location Strings with percent characters.
+			return new URL(location);
 		}
-		*/
-
-		return new URL(location);
 	}
 
 	/**
@@ -419,12 +416,7 @@ public static URL toRelativeURL(URL root, String relativePath) throws MalformedU
 		// # can appear in filenames, java.net.URL should not treat it as a fragment
 		relativePath = StringUtils.replace(relativePath, "#", "%23");
 
-		// Equivalent without java.net.URL constructor - for building on JDK 20+
-		/*
 		return toURL(StringUtils.applyRelativePath(root.toString(), relativePath));
-		*/
-
-		return new URL(root, relativePath);
 	}
 
 	/**
diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/resource/PathResourceResolverTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/resource/PathResourceResolverTests.java
@@ -40,7 +40,6 @@ public class PathResourceResolverTests {
 
 	private static final Duration TIMEOUT = Duration.ofSeconds(5);
 
-
 	private final PathResourceResolver resolver = new PathResourceResolver();
 
 
@@ -64,7 +63,7 @@ public void resolveFromClasspathRoot() {
 		assertThat(actual).isNotNull();
 	}
 
-	@Test // gh-22272
+	@Test  // gh-22272
 	public void resolveWithEncodedPath() throws IOException {
 		Resource classpathLocation = new ClassPathResource("test/", PathResourceResolver.class);
 		testWithEncodedPath(classpathLocation);
@@ -108,10 +107,14 @@ private void testCheckResource(Resource location, String requestPath) throws IOE
 		assertThat(actual).isNull();
 	}
 
-	@Test // gh-23463
+	@Test  // gh-23463
 	public void ignoreInvalidEscapeSequence() throws IOException {
 		UrlResource location = new UrlResource(getClass().getResource("./test/"));
-		Resource resource = location.createRelative("test%file.txt");
+
+		Resource resource = new UrlResource(location.getURL() + "test%file.txt");
+		assertThat(this.resolver.checkResource(resource, location)).isTrue();
+
+		resource = location.createRelative("test%file.txt");
 		assertThat(this.resolver.checkResource(resource, location)).isTrue();
 	}
 
@@ -129,7 +132,7 @@ public void checkResourceWithAllowedLocations() {
 		assertThat(actual).isEqualTo("../testalternatepath/bar.css");
 	}
 
-	@Test // SPR-12624
+	@Test  // SPR-12624
 	public void checkRelativeLocation() throws Exception {
 		String location= new UrlResource(getClass().getResource("./test/")).getURL().toExternalForm();
 		location = location.replace("/test/org/springframework","/test/org/../org/springframework");
@@ -140,13 +143,13 @@ public void checkRelativeLocation() throws Exception {
 		assertThat(resourceMono.block(TIMEOUT)).isNotNull();
 	}
 
-	@Test // SPR-12747
+	@Test  // SPR-12747
 	public void checkFileLocation() throws Exception {
 		Resource resource = getResource("main.css");
 		assertThat(this.resolver.checkResource(resource, resource)).isTrue();
 	}
 
-	@Test // SPR-13241
+	@Test  // SPR-13241
 	public void resolvePathRootResource() {
 		Resource webjarsLocation = new ClassPathResource("/META-INF/resources/webjars/", PathResourceResolver.class);
 		String path = this.resolver.resolveUrlPathInternal(
diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/PathResourceResolverTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/resource/PathResourceResolverTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 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.
@@ -92,10 +92,14 @@ private void testCheckResource(Resource location, String requestPath) throws IOE
 		assertThat(actual).isNull();
 	}
 
-	@Test // gh-23463
+	@Test  // gh-23463
 	public void ignoreInvalidEscapeSequence() throws IOException {
 		UrlResource location = new UrlResource(getClass().getResource("./test/"));
-		Resource resource = location.createRelative("test%file.txt");
+
+		Resource resource = new UrlResource(location.getURL() + "test%file.txt");
+		assertThat(this.resolver.checkResource(resource, location)).isTrue();
+
+		resource = location.createRelative("test%file.txt");
 		assertThat(this.resolver.checkResource(resource, location)).isTrue();
 	}
 
@@ -112,7 +116,7 @@ public void checkResourceWithAllowedLocations() {
 		assertThat(actual).isEqualTo("../testalternatepath/bar.css");
 	}
 
-	@Test // SPR-12432
+	@Test  // SPR-12432
 	public void checkServletContextResource() throws Exception {
 		Resource classpathLocation = new ClassPathResource("test/", PathResourceResolver.class);
 		MockServletContext context = new MockServletContext();
@@ -124,7 +128,7 @@ public void checkServletContextResource() throws Exception {
 		assertThat(this.resolver.checkResource(resource, servletContextLocation)).isTrue();
 	}
 
-	@Test // SPR-12624
+	@Test  // SPR-12624
 	public void checkRelativeLocation() throws Exception {
 		String location= new UrlResource(getClass().getResource("./test/")).getURL().toExternalForm();
 		location = location.replace("/test/org/springframework","/test/org/../org/springframework");
@@ -135,13 +139,13 @@ public void checkRelativeLocation() throws Exception {
 		assertThat(actual).isNotNull();
 	}
 
-	@Test // SPR-12747
+	@Test  // SPR-12747
 	public void checkFileLocation() throws Exception {
 		Resource resource = getResource("main.css");
 		assertThat(this.resolver.checkResource(resource, resource)).isTrue();
 	}
 
-	@Test // SPR-13241
+	@Test  // SPR-13241
 	public void resolvePathRootResource() {
 		Resource webjarsLocation = new ClassPathResource("/META-INF/resources/webjars/", PathResourceResolver.class);
 		String path = this.resolver.resolveUrlPathInternal("", Collections.singletonList(webjarsLocation), null);
