Create new JarFile instance for URL connections

Update `JarURLConnection` to ensure that when connections are opened
a new copy of the JarFile is provided.

Prior to this commit, a single `JarFile` instance was shared which meant
that it could be accidental closed if accessed via
`JarURLConnection.getJarFile()`. If the underlying jar file is closed
then it's possible for a `NoClassDefFoundError` to be thrown if running
on JDK 11 with an active `SecurityManager`.

Closes gh-17796

Author: philwebb

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2019 the original author or authors.
+ * 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.
@@ -60,6 +60,8 @@ public class JarFile extends java.util.jar.JarFile {
 
 	private static final AsciiBytes SIGNATURE_FILE_EXTENSION = new AsciiBytes(".SF");
 
+	private final JarFile parent;
+
 	private final RandomAccessDataFile rootFile;
 
 	private final String pathFromRoot;
@@ -100,6 +102,27 @@ public JarFile(File file) throws IOException {
 		this(file, "", file, JarFileType.DIRECT);
 	}
 
+	/**
+	 * Create a new JarFile copy based on a given parent.
+	 * @param parent the parent jar
+	 * @throws IOException if the file cannot be read
+	 */
+	JarFile(JarFile parent) throws IOException {
+		super(parent.rootFile.getFile());
+		this.parent = parent;
+		this.rootFile = parent.rootFile;
+		this.pathFromRoot = parent.pathFromRoot;
+		this.data = parent.data;
+		this.type = parent.type;
+		this.url = parent.url;
+		this.urlString = parent.urlString;
+		this.entries = parent.entries;
+		this.manifestSupplier = parent.manifestSupplier;
+		this.manifest = parent.manifest;
+		this.signed = parent.signed;
+		this.comment = parent.comment;
+	}
+
 	/**
 	 * Private constructor used to create a new {@link JarFile} either directly or from a
 	 * nested entry.
@@ -111,12 +134,13 @@ public JarFile(File file) throws IOException {
 	 */
 	private JarFile(RandomAccessDataFile rootFile, String pathFromRoot, RandomAccessData data, JarFileType type)
 			throws IOException {
-		this(rootFile, pathFromRoot, data, null, type, null);
+		this(null, rootFile, pathFromRoot, data, null, type, null);
 	}
 
-	private JarFile(RandomAccessDataFile rootFile, String pathFromRoot, RandomAccessData data, JarEntryFilter filter,
-			JarFileType type, Supplier<Manifest> manifestSupplier) throws IOException {
+	private JarFile(JarFile parent, RandomAccessDataFile rootFile, String pathFromRoot, RandomAccessData data,
+			JarEntryFilter filter, JarFileType type, Supplier<Manifest> manifestSupplier) throws IOException {
 		super(rootFile.getFile());
+		this.parent = parent;
 		this.rootFile = rootFile;
 		this.pathFromRoot = pathFromRoot;
 		CentralDirectoryParser parser = new CentralDirectoryParser();
@@ -166,6 +190,10 @@ public void visitEnd() {
 		};
 	}
 
+	JarFile getParent() {
+		return this.parent;
+	}
+
 	protected final RandomAccessDataFile getRootJarFile() {
 		return this.rootFile;
 	}
@@ -277,8 +305,9 @@ private JarFile createJarFileFromDirectoryEntry(JarEntry entry) throws IOExcepti
 			}
 			return null;
 		};
-		return new JarFile(this.rootFile, this.pathFromRoot + "!/" + entry.getName().substring(0, name.length() - 1),
-				this.data, filter, JarFileType.NESTED_DIRECTORY, this.manifestSupplier);
+		return new JarFile(this, this.rootFile,
+				this.pathFromRoot + "!/" + entry.getName().substring(0, name.length() - 1), this.data, filter,
+				JarFileType.NESTED_DIRECTORY, this.manifestSupplier);
 	}
 
 	private JarFile createJarFileFromFileEntry(JarEntry entry) throws IOException {
@@ -306,7 +335,7 @@ public int size() {
 	@Override
 	public void close() throws IOException {
 		super.close();
-		if (this.type == JarFileType.DIRECT) {
+		if (this.type == JarFileType.DIRECT && this.parent == null) {
 			this.rootFile.close();
 		}
 	}
@@ -326,10 +355,9 @@ String getUrlString() throws MalformedURLException {
 	 */
 	public URL getUrl() throws MalformedURLException {
 		if (this.url == null) {
-			Handler handler = new Handler(this);
 			String file = this.rootFile.getFile().toURI() + this.pathFromRoot + "!/";
 			file = file.replace("file:////", "file://"); // Fix UNC paths
-			this.url = new URL("jar", "", -1, file, handler);
+			this.url = new URL("jar", "", -1, file, new Handler(this));
 		}
 		return this.url;
 	}

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2019 the original author or authors.
+ * 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.
@@ -266,7 +266,7 @@ static JarURLConnection get(URL url, JarFile jarFile) throws IOException {
 				&& !jarFile.containsEntry(jarEntryName.toString())) {
 			return NOT_FOUND_CONNECTION;
 		}
-		return new JarURLConnection(url, jarFile, jarEntryName);
+		return new JarURLConnection(url, new JarFile(jarFile), jarEntryName);
 	}
 
 	private static int indexOfRootSpec(StringSequence file, String pathFromRoot) {

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

@@ -205,10 +205,10 @@ public void getUrl() throws Exception {
 		URL url = this.jarFile.getUrl();
 		assertThat(url.toString()).isEqualTo("jar:" + this.rootJarFile.toURI() + "!/");
 		JarURLConnection jarURLConnection = (JarURLConnection) url.openConnection();
-		assertThat(jarURLConnection.getJarFile()).isSameAs(this.jarFile);
+		assertThat(jarURLConnection.getJarFile().getParent()).isSameAs(this.jarFile);
 		assertThat(jarURLConnection.getJarEntry()).isNull();
 		assertThat(jarURLConnection.getContentLength()).isGreaterThan(1);
-		assertThat(jarURLConnection.getContent()).isSameAs(this.jarFile);
+		assertThat(((JarFile) jarURLConnection.getContent()).getParent()).isSameAs(this.jarFile);
 		assertThat(jarURLConnection.getContentType()).isEqualTo("x-java/jar");
 		assertThat(jarURLConnection.getJarFileURL().toURI()).isEqualTo(this.rootJarFile.toURI());
 	}
@@ -218,7 +218,7 @@ public void createEntryUrl() throws Exception {
 		URL url = new URL(this.jarFile.getUrl(), "1.dat");
 		assertThat(url.toString()).isEqualTo("jar:" + this.rootJarFile.toURI() + "!/1.dat");
 		JarURLConnection jarURLConnection = (JarURLConnection) url.openConnection();
-		assertThat(jarURLConnection.getJarFile()).isSameAs(this.jarFile);
+		assertThat(jarURLConnection.getJarFile().getParent()).isSameAs(this.jarFile);
 		assertThat(jarURLConnection.getJarEntry()).isSameAs(this.jarFile.getJarEntry("1.dat"));
 		assertThat(jarURLConnection.getContentLength()).isEqualTo(1);
 		assertThat(jarURLConnection.getContent()).isInstanceOf(InputStream.class);
@@ -273,7 +273,7 @@ public void getNestedJarFile() throws Exception {
 		URL url = nestedJarFile.getUrl();
 		assertThat(url.toString()).isEqualTo("jar:" + this.rootJarFile.toURI() + "!/nested.jar!/");
 		JarURLConnection conn = (JarURLConnection) url.openConnection();
-		assertThat(conn.getJarFile()).isSameAs(nestedJarFile);
+		assertThat(conn.getJarFile().getParent()).isSameAs(nestedJarFile);
 		assertThat(conn.getJarFileURL().toString()).isEqualTo("jar:" + this.rootJarFile.toURI() + "!/nested.jar");
 		assertThat(conn.getInputStream()).isNotNull();
 		JarInputStream jarInputStream = new JarInputStream(conn.getInputStream());
@@ -301,7 +301,7 @@ public void getNestedJarDirectory() throws Exception {
 
 		URL url = nestedJarFile.getUrl();
 		assertThat(url.toString()).isEqualTo("jar:" + this.rootJarFile.toURI() + "!/d!/");
-		assertThat(((JarURLConnection) url.openConnection()).getJarFile()).isSameAs(nestedJarFile);
+		assertThat(((JarURLConnection) url.openConnection()).getJarFile().getParent()).isSameAs(nestedJarFile);
 	}
 
 	@Test

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2019 the original author or authors.
+ * 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.
@@ -20,6 +20,7 @@
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.net.URL;
+import java.util.zip.ZipFile;
 
 import org.junit.Before;
 import org.junit.Rule;
@@ -28,6 +29,7 @@
 
 import org.springframework.boot.loader.TestJarCreator;
 import org.springframework.boot.loader.jar.JarURLConnection.JarEntryName;
+import org.springframework.test.util.ReflectionTestUtils;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
@@ -58,13 +60,15 @@ public void setup() throws Exception {
 	@Test
 	public void connectionToRootUsingAbsoluteUrl() throws Exception {
 		URL url = new URL("jar:" + this.rootJarFile.toURI().toURL() + "!/");
-		assertThat(JarURLConnection.get(url, this.jarFile).getContent()).isSameAs(this.jarFile);
+		Object content = JarURLConnection.get(url, this.jarFile).getContent();
+		assertThat(((JarFile) content).getParent()).isSameAs(this.jarFile);
 	}
 
 	@Test
 	public void connectionToRootUsingRelativeUrl() throws Exception {
 		URL url = new URL("jar:file:" + getRelativePath() + "!/");
-		assertThat(JarURLConnection.get(url, this.jarFile).getContent()).isSameAs(this.jarFile);
+		Object content = JarURLConnection.get(url, this.jarFile).getContent();
+		assertThat(((JarFile) content).getParent()).isSameAs(this.jarFile);
 	}
 
 	@Test
@@ -191,6 +195,15 @@ public void jarEntryNameWithMixtureOfEncodedAndUnencodedDoubleByteCharacters() {
 				.isEqualTo("\u00e1/b/\u00c7.class");
 	}
 
+	@Test
+	public void openConnectionCanBeClosedWithoutClosingSourceJar() throws Exception {
+		URL url = new URL("jar:" + this.rootJarFile.toURI().toURL() + "!/");
+		JarURLConnection connection = JarURLConnection.get(url, this.jarFile);
+		JarFile connectionJarFile = connection.getJarFile();
+		connectionJarFile.close();
+		assertThat((Boolean) ReflectionTestUtils.getField(this.jarFile, ZipFile.class, "closeRequested")).isFalse();
+	}
+
 	private String getRelativePath() {
 		return this.rootJarFile.getPath().replace('\\', '/');
 	}