From c85918b8b35ee6215583020c2ef4287c4f7eca8d Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 21 Apr 2020 16:08:54 -0700 Subject: [PATCH] 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 --- .../boot/loader/jar/JarFile.java | 46 +++++++++++++++---- .../boot/loader/jar/JarURLConnection.java | 4 +- .../boot/loader/jar/JarFileTests.java | 10 ++-- .../loader/jar/JarURLConnectionTests.java | 19 ++++++-- 4 files changed, 60 insertions(+), 19 deletions(-) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarFile.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarFile.java index 26d62978eb..82bc85cef8 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarFile.java +++ b/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 class JarFile extends java.util.jar.JarFile { 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 class JarFile extends java.util.jar.JarFile { */ 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 manifestSupplier) throws IOException { + private JarFile(JarFile parent, RandomAccessDataFile rootFile, String pathFromRoot, RandomAccessData data, + JarEntryFilter filter, JarFileType type, Supplier manifestSupplier) throws IOException { super(rootFile.getFile()); + this.parent = parent; this.rootFile = rootFile; this.pathFromRoot = pathFromRoot; CentralDirectoryParser parser = new CentralDirectoryParser(); @@ -166,6 +190,10 @@ public class JarFile extends java.util.jar.JarFile { }; } + JarFile getParent() { + return this.parent; + } + protected final RandomAccessDataFile getRootJarFile() { return this.rootFile; } @@ -277,8 +305,9 @@ public class JarFile extends java.util.jar.JarFile { } 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 class JarFile extends java.util.jar.JarFile { @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 @@ public class JarFile extends java.util.jar.JarFile { */ 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; } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarURLConnection.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarURLConnection.java index 8b173346ea..5c35b21382 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarURLConnection.java +++ b/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 @@ final class JarURLConnection extends java.net.JarURLConnection { && !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) { diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarFileTests.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarFileTests.java index 4c74f6f71c..3459f76c18 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarFileTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarFileTests.java @@ -205,10 +205,10 @@ public class JarFileTests { 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 class JarFileTests { 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 class JarFileTests { 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 class JarFileTests { 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 diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarURLConnectionTests.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarURLConnectionTests.java index cdf4a20ad5..fcaa296dda 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarURLConnectionTests.java +++ b/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.ByteArrayInputStream; 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.junit.rules.TemporaryFolder; 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 class JarURLConnectionTests { @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 class JarURLConnectionTests { .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('\\', '/'); }