Prevent Tomcat URL "reflective access" warnings

Update the jar `Handler` class to support a non-reflective fallback
mechanism when possible. The updated code attempts to capture a regular
jar URL before our handler is installed. It can then use that URL as
context when creating the a fallback URL. The JDK jar `Handler` will
be copied from the context URL to the fallback URL.

Without this commit, resolving new Tomcat URLs of the form
`jar:war:file:...` would result in an ugly "Illegal reflective access"
warning.

Fixes gh-18631

Author: philwebb

settings.gradle

@@ -71,6 +71,7 @@ include "spring-boot-project:spring-boot-test-autoconfigure"
 include "spring-boot-tests:spring-boot-deployment-tests"
 include "spring-boot-tests:spring-boot-integration-tests:spring-boot-configuration-processor-tests"
 include "spring-boot-tests:spring-boot-integration-tests:spring-boot-launch-script-tests"
+include "spring-boot-tests:spring-boot-integration-tests:spring-boot-loader-tests"
 include "spring-boot-tests:spring-boot-integration-tests:spring-boot-server-tests"
 
 file("${rootDir}/spring-boot-project/spring-boot-starters").eachDirMatch(~/spring-boot-starter.*/) {

spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/Handler.java

@@ -57,8 +57,12 @@ public class Handler extends URLStreamHandler {
 
 	private static final String PARENT_DIR = "/../";
 
+	private static final String PROTOCOL_HANDLER = "java.protocol.handler.pkgs";
+
 	private static final String[] FALLBACK_HANDLERS = { "sun.net.www.protocol.jar.Handler" };
 
+	private static URL jarContextUrl;
+
 	private static SoftReference<Map<File, JarFile>> rootFileCache;
 
 	static {
@@ -98,7 +102,8 @@ private boolean isUrlInJarFile(URL url, JarFile jarFile) throws MalformedURLExce
 
 	private URLConnection openFallbackConnection(URL url, Exception reason) throws IOException {
 		try {
-			return openConnection(getFallbackHandler(), url);
+			URLConnection connection = openFallbackContextConnection(url);
+			return (connection != null) ? connection : openFallbackHandlerConnection(url);
 		}
 		catch (Exception ex) {
 			if (reason instanceof IOException) {
@@ -113,16 +118,35 @@ private URLConnection openFallbackConnection(URL url, Exception reason) throws I
 		}
 	}
 
-	private void log(boolean warning, String message, Exception cause) {
+	/**
+	 * Attempt to open a fallback connection by using a context URL captured before the
+	 * jar handler was replaced with our own version. Since this method doesn't use
+	 * reflection it won't trigger "illegal reflective access operation has occurred"
+	 * warnings on Java 13+.
+	 * @param url the URL to open
+	 * @return a {@link URLConnection} or {@code null}
+	 */
+	private URLConnection openFallbackContextConnection(URL url) {
 		try {
-			Level level = warning ? Level.WARNING : Level.FINEST;
-			Logger.getLogger(getClass().getName()).log(level, message, cause);
+			if (jarContextUrl != null) {
+				return new URL(jarContextUrl, url.toExternalForm()).openConnection();
+			}
 		}
 		catch (Exception ex) {
-			if (warning) {
-				System.err.println("WARNING: " + message);
-			}
 		}
+		return null;
+	}
+
+	/**
+	 * Attempt to open a fallback connection by using reflection to access Java's default
+	 * jar {@link URLStreamHandler}.
+	 * @param url the URL to open
+	 * @return the {@link URLConnection}
+	 * @throws Exception if not connection could be opened
+	 */
+	private URLConnection openFallbackHandlerConnection(URL url) throws Exception {
+		URLStreamHandler fallbackHandler = getFallbackHandler();
+		return new URL(null, url.toExternalForm(), fallbackHandler).openConnection();
 	}
 
 	private URLStreamHandler getFallbackHandler() {
@@ -142,8 +166,16 @@ private URLStreamHandler getFallbackHandler() {
 		throw new IllegalStateException("Unable to find fallback handler");
 	}
 
-	private URLConnection openConnection(URLStreamHandler handler, URL url) throws Exception {
-		return new URL(null, url.toExternalForm(), handler).openConnection();
+	private void log(boolean warning, String message, Exception cause) {
+		try {
+			Level level = warning ? Level.WARNING : Level.FINEST;
+			Logger.getLogger(getClass().getName()).log(level, message, cause);
+		}
+		catch (Exception ex) {
+			if (warning) {
+				System.err.println("WARNING: " + message);
+			}
+		}
 	}
 
 	@Override
@@ -333,6 +365,53 @@ static void addToRootFileCache(File sourceFile, JarFile jarFile) {
 		cache.put(sourceFile, jarFile);
 	}
 
+	/**
+	 * If possible, capture a URL that is configured with the original jar handler so that
+	 * we can use it as a fallback context later. We can only do this if we know that we
+	 * can reset the handlers after.
+	 */
+	static void captureJarContextUrl() {
+		if (canResetCachedUrlHandlers()) {
+			String handlers = System.getProperty(PROTOCOL_HANDLER, "");
+			try {
+				System.clearProperty(PROTOCOL_HANDLER);
+				try {
+					resetCachedUrlHandlers();
+					jarContextUrl = new URL("jar:file:context.jar!/");
+					URLConnection connection = jarContextUrl.openConnection();
+					if (connection instanceof JarURLConnection) {
+						jarContextUrl = null;
+					}
+				}
+				catch (Exception ex) {
+				}
+			}
+			finally {
+				if (handlers == null) {
+					System.clearProperty(PROTOCOL_HANDLER);
+				}
+				else {
+					System.setProperty(PROTOCOL_HANDLER, handlers);
+				}
+			}
+			resetCachedUrlHandlers();
+		}
+	}
+
+	private static boolean canResetCachedUrlHandlers() {
+		try {
+			resetCachedUrlHandlers();
+			return true;
+		}
+		catch (Error ex) {
+			return false;
+		}
+	}
+
+	private static void resetCachedUrlHandlers() {
+		URL.setURLStreamHandlerFactory(null);
+	}
+
 	/**
 	 * Set if a generic static exception can be thrown when a URL cannot be connected.
 	 * This optimization is used during class loading to save creating lots of exceptions

spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarFile.java

@@ -411,6 +411,7 @@ JarFileType getType() {
 	 * {@link URLStreamHandler} will be located to deal with jar URLs.
 	 */
 	public static void registerUrlProtocolHandler() {
+		Handler.captureJarContextUrl();
 		String handlers = System.getProperty(PROTOCOL_HANDLER, "");
 		System.setProperty(PROTOCOL_HANDLER,
 				("".equals(handlers) ? HANDLERS_PACKAGE : handlers + "|" + HANDLERS_PACKAGE));

spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/HandlerTests.java

@@ -163,6 +163,7 @@ void fallbackToJdksJarUrlStreamHandler(@TempDir File tempDir) throws Exception {
 		URLConnection jdkConnection = new URL(null, "jar:file:" + testJar.toURI().toURL() + "!/nested.jar!/",
 				this.handler).openConnection();
 		assertThat(jdkConnection).isNotInstanceOf(JarURLConnection.class);
+		assertThat(jdkConnection.getClass().getName()).endsWith(".JarURLConnection");
 	}
 
 	@Test

spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/app/build.gradle

@@ -0,0 +1,18 @@
+plugins {
+	id "java"
+	id "org.springframework.boot"
+}
+
+apply plugin: "io.spring.dependency-management"
+
+repositories {
+	maven { url "file:${rootDir}/../int-test-maven-repository"}
+	mavenCentral()
+	maven { url "https://repo.spring.io/snapshot" }
+	maven { url "https://repo.spring.io/milestone" }
+}
+
+dependencies {
+	implementation("org.springframework.boot:spring-boot-starter-web")
+	implementation("org.webjars:jquery:3.5.0")
+}

spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/app/settings.gradle

@@ -0,0 +1,15 @@
+pluginManagement {
+	repositories {
+		maven { url "file:${rootDir}/../int-test-maven-repository"}
+		mavenCentral()
+		maven { url "https://repo.spring.io/snapshot" }
+		maven { url "https://repo.spring.io/milestone" }
+	}
+	resolutionStrategy {
+		eachPlugin {
+			if (requested.id.id == "org.springframework.boot") {
+				useModule "org.springframework.boot:spring-boot-gradle-plugin:${requested.version}"
+			}
+		}
+	}
+}

spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/app/src/main/java/org/springframework/boot/loaderapp/LoaderTestApplication.java

@@ -0,0 +1,50 @@
+/*
+ * Copyright 2012-2020 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.boot.loaderapp;
+
+import java.net.URL;
+import java.util.Arrays;
+
+import javax.servlet.ServletContext;
+
+import org.springframework.boot.CommandLineRunner;
+import org.springframework.boot.SpringApplication;
+import org.springframework.boot.autoconfigure.SpringBootApplication;
+import org.springframework.context.annotation.Bean;
+import org.springframework.util.FileCopyUtils;
+
+@SpringBootApplication
+public class LoaderTestApplication {
+
+	@Bean
+	public CommandLineRunner commandLineRunner(ServletContext servletContext) {
+		return (args) -> {
+			URL resourceUrl = servletContext.getResource("webjars/jquery/3.5.0/jquery.js");
+			byte[] resourceContent = FileCopyUtils.copyToByteArray(resourceUrl.openStream());
+			URL directUrl = new URL(resourceUrl.toExternalForm());
+			byte[] directContent = FileCopyUtils.copyToByteArray(directUrl.openStream());
+			String message = (!Arrays.equals(resourceContent, directContent)) ? "NO MATCH"
+					: directContent.length + " BYTES";
+			System.out.println(">>>>> " + message + " from " + resourceUrl);
+		};
+	}
+
+	public static void main(String[] args) {
+		SpringApplication.run(LoaderTestApplication.class, args).stop();
+	}
+
+}

spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/build.gradle

@@ -0,0 +1,47 @@
+plugins {
+	id "java"
+	id "org.springframework.boot.conventions"
+	id "org.springframework.boot.integration-test"
+}
+
+description = "Spring Boot Loader Integration Tests"
+
+configurations {
+	app
+}
+
+dependencies {
+	app project(path: ":spring-boot-project:spring-boot-dependencies", configuration: "mavenRepository")
+	app project(path: ":spring-boot-project:spring-boot-tools:spring-boot-gradle-plugin", configuration: "mavenRepository")
+	app project(path: ":spring-boot-project:spring-boot-starters:spring-boot-starter-web", configuration: "mavenRepository")
+
+	intTestImplementation(enforcedPlatform(project(":spring-boot-project:spring-boot-parent")))
+	intTestImplementation(project(":spring-boot-project:spring-boot-tools:spring-boot-test-support"))
+	intTestImplementation(project(":spring-boot-project:spring-boot-starters:spring-boot-starter-test"))
+	intTestImplementation("org.testcontainers:junit-jupiter")
+	intTestImplementation("org.testcontainers:testcontainers")
+}
+
+task syncMavenRepository(type: Sync) {
+	from configurations.app
+	into "${buildDir}/int-test-maven-repository"
+}
+
+task syncAppSource(type: Sync) {
+	from "app"
+	into "${buildDir}/app"
+	filter { line ->
+		line.replace("id \"org.springframework.boot\"", "id \"org.springframework.boot\" version \"${project.version}\"")
+	}
+}
+
+task buildApp(type: GradleBuild) {
+	dependsOn syncAppSource, syncMavenRepository
+	dir = "${buildDir}/app"
+	startParameter.buildCacheEnabled = false
+	tasks  = ["build"]
+}
+
+intTest {
+	dependsOn buildApp
+}

spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/src/intTest/java/org/springframework/boot/loader/LoaderIntegrationTests.java

@@ -0,0 +1,66 @@
+/*
+ * Copyright 2012-2020 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.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.boot.loader;
+
+import java.io.File;
+import java.time.Duration;
+
+import org.junit.jupiter.api.Test;
+import org.testcontainers.containers.GenericContainer;
+import org.testcontainers.containers.output.ToStringConsumer;
+import org.testcontainers.containers.startupcheck.OneShotStartupCheckStrategy;
+import org.testcontainers.junit.jupiter.Container;
+import org.testcontainers.junit.jupiter.Testcontainers;
+import org.testcontainers.utility.DockerImageName;
+import org.testcontainers.utility.MountableFile;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Integration tests loader that supports fat jars.
+ *
+ * @author Phillip Webb
+ */
+@Testcontainers(disabledWithoutDocker = true)
+class LoaderIntegrationTests {
+
+	private static final DockerImageName JRE = DockerImageName.parse("adoptopenjdk:15-jre-hotspot");
+
+	private static ToStringConsumer output = new ToStringConsumer();
+
+	@Container
+	public static GenericContainer<?> container = new GenericContainer<>(JRE).withLogConsumer(output)
+			.withCopyFileToContainer(MountableFile.forHostPath(findApplication().toPath()), "/app.jar")
+			.withStartupCheckStrategy(new OneShotStartupCheckStrategy().withTimeout(Duration.ofMinutes(5)))
+			.withCommand("java", "-jar", "app.jar");
+
+	private static File findApplication() {
+		File appJar = new File("build/app/build/libs/app.jar");
+		if (appJar.isFile()) {
+			return appJar;
+		}
+		throw new IllegalStateException(
+				"Could not find test application in build/app/build/libs directory. Have you built it?");
+	}
+
+	@Test
+	void readUrlsWithoutWarning() {
+		assertThat(output.toUtf8String()).contains(">>>>> 287649 BYTES from").doesNotContain("WARNING:")
+				.doesNotContain("illegal");
+	}
+
+}