From aac367e9c5e40733421716f2edfb3522f3480119 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 18 Aug 2020 11:36:36 -0700 Subject: [PATCH] Attempt to fix memory leak in JarFile class Create a new `JarFileWrapper` class so that we can wrap and existing `JarFile` and offer a version that can be safely closed. Prior to this commit, we provided wrapper functionality in the `JarFile` class itself. Unfortunately, because we override `close` and also create a lot of wrappers this caused memory issues when running on Java 11. With Java 11 `java.util.zip.ZipFile` class uses `FinalizableResource` for any implementation that overrides `close()`. This means that any wrapper classes will not be garbage collected until the JVM finalizer thread runs. Closes gh-22991 --- .../boot/loader/jar/AbstractJarFile.java | 78 ++++++++++ .../boot/loader/jar/JarFile.java | 71 +++------ .../boot/loader/jar/JarFileWrapper.java | 120 +++++++++++++++ .../boot/loader/jar/JarURLConnection.java | 25 ++-- .../boot/loader/jar/HandlerTests.java | 10 +- .../boot/loader/jar/JarFileTests.java | 11 +- .../boot/loader/jar/JarFileWrapperTests.java | 140 ++++++++++++++++++ .../loader/jar/JarURLConnectionTests.java | 6 +- 8 files changed, 385 insertions(+), 76 deletions(-) create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/AbstractJarFile.java create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarFileWrapper.java create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarFileWrapperTests.java diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/AbstractJarFile.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/AbstractJarFile.java new file mode 100644 index 000000000000..f6b5b43a6ff5 --- /dev/null +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/AbstractJarFile.java @@ -0,0 +1,78 @@ +/* + * 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.jar; + +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.net.MalformedURLException; +import java.net.URL; +import java.security.Permission; + +/** + * Base class for extended variants of {@link java.util.jar.JarFile}. + * + * @author Phillip Webb + */ +abstract class AbstractJarFile extends java.util.jar.JarFile { + + /** + * Create a new {@link AbstractJarFile}. + * @param file the root jar file. + * @throws IOException on IO error + */ + AbstractJarFile(File file) throws IOException { + super(file); + } + + /** + * Return a URL that can be used to access this JAR file. NOTE: the specified URL + * cannot be serialized and or cloned. + * @return the URL + * @throws MalformedURLException if the URL is malformed + */ + protected abstract URL getUrl() throws MalformedURLException; + + /** + * Return the {@link JarFileType} of this instance. + * @return the jar file type + */ + protected abstract JarFileType getType(); + + /** + * Return the security permission for this JAR. + * @return the security permission. + */ + protected abstract Permission getPermission(); + + /** + * Return an {@link InputStream} for the entire jar contents. + * @return the contents input stream + * @throws IOException on IO error + */ + protected abstract InputStream getInputStream() throws IOException; + + /** + * The type of a {@link JarFile}. + */ + enum JarFileType { + + DIRECT, NESTED_DIRECTORY, NESTED_JAR + + } + +} 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 c12e3cc2e566..2a971a5fe051 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 @@ -17,6 +17,7 @@ package org.springframework.boot.loader.jar; import java.io.File; +import java.io.FilePermission; import java.io.IOException; import java.io.InputStream; import java.lang.ref.SoftReference; @@ -24,6 +25,7 @@ import java.net.URL; import java.net.URLStreamHandler; import java.net.URLStreamHandlerFactory; +import java.security.Permission; import java.util.Enumeration; import java.util.Iterator; import java.util.function.Supplier; @@ -48,7 +50,7 @@ * @author Andy Wilkinson * @since 1.0.0 */ -public class JarFile extends java.util.jar.JarFile { +public class JarFile extends AbstractJarFile { private static final String MANIFEST_NAME = "META-INF/MANIFEST.MF"; @@ -60,7 +62,7 @@ public class JarFile extends java.util.jar.JarFile { private static final AsciiBytes SIGNATURE_FILE_EXTENSION = new AsciiBytes(".SF"); - private final JarFile parent; + private static final String READ_ACTION = "read"; private final RandomAccessDataFile rootFile; @@ -104,28 +106,6 @@ 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()); - super.close(); - 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. @@ -137,14 +117,13 @@ public JarFile(File file) throws IOException { */ private JarFile(RandomAccessDataFile rootFile, String pathFromRoot, RandomAccessData data, JarFileType type) throws IOException { - this(null, rootFile, pathFromRoot, data, null, type, null); + this(rootFile, pathFromRoot, data, null, type, null); } - private JarFile(JarFile parent, RandomAccessDataFile rootFile, String pathFromRoot, RandomAccessData data, - JarEntryFilter filter, JarFileType type, Supplier manifestSupplier) throws IOException { + private JarFile(RandomAccessDataFile rootFile, String pathFromRoot, RandomAccessData data, JarEntryFilter filter, + JarFileType type, Supplier manifestSupplier) throws IOException { super(rootFile.getFile()); super.close(); - this.parent = parent; this.rootFile = rootFile; this.pathFromRoot = pathFromRoot; CentralDirectoryParser parser = new CentralDirectoryParser(); @@ -194,8 +173,9 @@ public void visitEnd() { }; } - JarFile getParent() { - return this.parent; + @Override + protected Permission getPermission() { + return new FilePermission(this.rootFile.getFile().getPath(), READ_ACTION); } protected final RandomAccessDataFile getRootJarFile() { @@ -244,6 +224,11 @@ public ZipEntry getEntry(String name) { return this.entries.getEntry(name); } + @Override + protected InputStream getInputStream() throws IOException { + return this.data.getInputStream(); + } + @Override public synchronized InputStream getInputStream(ZipEntry entry) throws IOException { if (entry instanceof JarEntry) { @@ -296,9 +281,8 @@ private JarFile createJarFileFromDirectoryEntry(JarEntry entry) throws IOExcepti } return null; }; - return new JarFile(this, this.rootFile, - this.pathFromRoot + "!/" + entry.getName().substring(0, name.length() - 1), this.data, filter, - JarFileType.NESTED_DIRECTORY, this.manifestSupplier); + return new JarFile(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 { @@ -329,7 +313,7 @@ public void close() throws IOException { return; } this.closed = true; - if (this.type == JarFileType.DIRECT && this.parent == null) { + if (this.type == JarFileType.DIRECT) { this.rootFile.close(); } } @@ -345,12 +329,7 @@ String getUrlString() throws MalformedURLException { return this.urlString; } - /** - * Return a URL that can be used to access this JAR file. NOTE: the specified URL - * cannot be serialized and or cloned. - * @return the URL - * @throws MalformedURLException if the URL is malformed - */ + @Override public URL getUrl() throws MalformedURLException { if (this.url == null) { String file = this.rootFile.getFile().toURI() + this.pathFromRoot + "!/"; @@ -409,7 +388,8 @@ protected String getPathFromRoot() { return this.pathFromRoot; } - JarFileType getType() { + @Override + protected JarFileType getType() { return this.type; } @@ -438,15 +418,6 @@ private static void resetCachedUrlHandlers() { } } - /** - * The type of a {@link JarFile}. - */ - enum JarFileType { - - DIRECT, NESTED_DIRECTORY, NESTED_JAR - - } - /** * An {@link Enumeration} on {@linkplain java.util.jar.JarEntry jar entries}. */ diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarFileWrapper.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarFileWrapper.java new file mode 100644 index 000000000000..3608c7a26301 --- /dev/null +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarFileWrapper.java @@ -0,0 +1,120 @@ +/* + * 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.jar; + +import java.io.IOException; +import java.io.InputStream; +import java.net.MalformedURLException; +import java.net.URL; +import java.security.Permission; +import java.util.Enumeration; +import java.util.jar.JarEntry; +import java.util.jar.Manifest; +import java.util.zip.ZipEntry; + +/** + * A wrapper used to create a copy of a {@link JarFile} so that it can be safely closed + * without closing the original. + * + * @author Phillip Webb + */ +class JarFileWrapper extends AbstractJarFile { + + private final JarFile parent; + + JarFileWrapper(JarFile parent) throws IOException { + super(parent.getRootJarFile().getFile()); + this.parent = parent; + super.close(); + } + + @Override + protected URL getUrl() throws MalformedURLException { + return this.parent.getUrl(); + } + + @Override + protected JarFileType getType() { + return this.parent.getType(); + } + + @Override + protected Permission getPermission() { + return this.parent.getPermission(); + } + + @Override + public Manifest getManifest() throws IOException { + return this.parent.getManifest(); + } + + @Override + public Enumeration entries() { + return this.parent.entries(); + } + + @Override + public JarEntry getJarEntry(String name) { + return this.parent.getJarEntry(name); + } + + @Override + public ZipEntry getEntry(String name) { + return this.parent.getEntry(name); + } + + @Override + protected InputStream getInputStream() throws IOException { + return this.parent.getInputStream(); + } + + @Override + public synchronized InputStream getInputStream(ZipEntry ze) throws IOException { + return this.parent.getInputStream(ze); + } + + @Override + public String getComment() { + return this.parent.getComment(); + } + + @Override + public int size() { + return this.parent.size(); + } + + @Override + public String toString() { + return this.parent.toString(); + } + + @Override + public String getName() { + return this.parent.getName(); + } + + static JarFile unwrap(java.util.jar.JarFile jarFile) { + if (jarFile instanceof JarFile) { + return (JarFile) jarFile; + } + if (jarFile instanceof JarFileWrapper) { + return unwrap(((JarFileWrapper) jarFile).parent); + } + throw new IllegalStateException("Not a JarFile or Wrapper"); + } + +} 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 58c39990ae64..1e39966283af 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 @@ -18,7 +18,6 @@ import java.io.ByteArrayOutputStream; import java.io.FileNotFoundException; -import java.io.FilePermission; import java.io.IOException; import java.io.InputStream; import java.io.UnsupportedEncodingException; @@ -68,11 +67,9 @@ protected URLConnection openConnection(URL u) throws IOException { private static final JarEntryName EMPTY_JAR_ENTRY_NAME = new JarEntryName(new StringSequence("")); - private static final String READ_ACTION = "read"; - private static final JarURLConnection NOT_FOUND_CONNECTION = JarURLConnection.notFound(); - private final JarFile jarFile; + private final AbstractJarFile jarFile; private Permission permission; @@ -80,9 +77,9 @@ protected URLConnection openConnection(URL u) throws IOException { private final JarEntryName jarEntryName; - private JarEntry jarEntry; + private java.util.jar.JarEntry jarEntry; - private JarURLConnection(URL url, JarFile jarFile, JarEntryName jarEntryName) throws IOException { + private JarURLConnection(URL url, AbstractJarFile jarFile, JarEntryName jarEntryName) throws IOException { // What we pass to super is ultimately ignored super(EMPTY_JAR_URL); this.url = url; @@ -105,7 +102,7 @@ public void connect() throws IOException { } @Override - public JarFile getJarFile() throws IOException { + public java.util.jar.JarFile getJarFile() throws IOException { connect(); return this.jarFile; } @@ -138,7 +135,7 @@ private URL buildJarFileUrl() { } @Override - public JarEntry getJarEntry() throws IOException { + public java.util.jar.JarEntry getJarEntry() throws IOException { if (this.jarEntryName == null || this.jarEntryName.isEmpty()) { return null; } @@ -163,7 +160,7 @@ public InputStream getInputStream() throws IOException { throw new IOException("no entry name specified"); } connect(); - InputStream inputStream = (this.jarEntryName.isEmpty() ? this.jarFile.getData().getInputStream() + InputStream inputStream = (this.jarEntryName.isEmpty() ? this.jarFile.getInputStream() : this.jarFile.getInputStream(this.jarEntry)); if (inputStream == null) { throwFileNotFound(this.jarEntryName, this.jarFile); @@ -171,7 +168,7 @@ public InputStream getInputStream() throws IOException { return inputStream; } - private void throwFileNotFound(Object entry, JarFile jarFile) throws FileNotFoundException { + private void throwFileNotFound(Object entry, AbstractJarFile jarFile) throws FileNotFoundException { if (Boolean.TRUE.equals(useFastExceptions.get())) { throw FILE_NOT_FOUND_EXCEPTION; } @@ -196,7 +193,7 @@ public long getContentLengthLong() { if (this.jarEntryName.isEmpty()) { return this.jarFile.size(); } - JarEntry entry = getJarEntry(); + java.util.jar.JarEntry entry = getJarEntry(); return (entry != null) ? (int) entry.getSize() : -1; } catch (IOException ex) { @@ -221,7 +218,7 @@ public Permission getPermission() throws IOException { throw FILE_NOT_FOUND_EXCEPTION; } if (this.permission == null) { - this.permission = new FilePermission(this.jarFile.getRootJarFile().getFile().getPath(), READ_ACTION); + this.permission = this.jarFile.getPermission(); } return this.permission; } @@ -232,7 +229,7 @@ public long getLastModified() { return 0; } try { - JarEntry entry = getJarEntry(); + java.util.jar.JarEntry entry = getJarEntry(); return (entry != null) ? entry.getTime() : 0; } catch (IOException ex) { @@ -266,7 +263,7 @@ static JarURLConnection get(URL url, JarFile jarFile) throws IOException { && !jarFile.containsEntry(jarEntryName.toString())) { return NOT_FOUND_CONNECTION; } - return new JarURLConnection(url, new JarFile(jarFile), jarEntryName); + return new JarURLConnection(url, new JarFileWrapper(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/HandlerTests.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/HandlerTests.java index 5c1f3fcd18f2..82e8ab5f563a 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/HandlerTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/HandlerTests.java @@ -171,11 +171,12 @@ void whenJarHasAPlusInItsPathConnectionJarFileMatchesOriginalJarFile(@TempDir Fi TestJarCreator.createTestJar(testJar); URL url = new URL(null, "jar:" + testJar.toURI().toURL() + "!/nested.jar!/3.dat", this.handler); JarURLConnection connection = (JarURLConnection) url.openConnection(); + JarFile jarFile = JarFileWrapper.unwrap(connection.getJarFile()); try { - assertThat(connection.getJarFile().getRootJarFile().getFile()).isEqualTo(testJar); + assertThat(jarFile.getRootJarFile().getFile()).isEqualTo(testJar); } finally { - connection.getJarFile().close(); + jarFile.close(); } } @@ -185,11 +186,12 @@ void whenJarHasASpaceInItsPathConnectionJarFileMatchesOriginalJarFile(@TempDir F TestJarCreator.createTestJar(testJar); URL url = new URL(null, "jar:" + testJar.toURI().toURL() + "!/nested.jar!/3.dat", this.handler); JarURLConnection connection = (JarURLConnection) url.openConnection(); + JarFile jarFile = JarFileWrapper.unwrap(connection.getJarFile()); try { - assertThat(connection.getJarFile().getRootJarFile().getFile()).isEqualTo(testJar); + assertThat(jarFile.getRootJarFile().getFile()).isEqualTo(testJar); } finally { - connection.getJarFile().close(); + jarFile.close(); } } 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 f4b43884c41f..eba31af4e626 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 @@ -218,10 +218,10 @@ 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().getParent()).isSameAs(this.jarFile); + assertThat(JarFileWrapper.unwrap(jarURLConnection.getJarFile())).isSameAs(this.jarFile); assertThat(jarURLConnection.getJarEntry()).isNull(); assertThat(jarURLConnection.getContentLength()).isGreaterThan(1); - assertThat(((JarFile) jarURLConnection.getContent()).getParent()).isSameAs(this.jarFile); + assertThat(JarFileWrapper.unwrap((java.util.jar.JarFile) jarURLConnection.getContent())).isSameAs(this.jarFile); assertThat(jarURLConnection.getContentType()).isEqualTo("x-java/jar"); assertThat(jarURLConnection.getJarFileURL().toURI()).isEqualTo(this.rootJarFile.toURI()); } @@ -231,7 +231,7 @@ 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().getParent()).isSameAs(this.jarFile); + assertThat(JarFileWrapper.unwrap(jarURLConnection.getJarFile())).isSameAs(this.jarFile); assertThat(jarURLConnection.getJarEntry()).isSameAs(this.jarFile.getJarEntry("1.dat")); assertThat(jarURLConnection.getContentLength()).isEqualTo(1); assertThat(jarURLConnection.getContent()).isInstanceOf(InputStream.class); @@ -285,7 +285,7 @@ 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().getParent()).isSameAs(nestedJarFile); + assertThat(JarFileWrapper.unwrap(conn.getJarFile())).isSameAs(nestedJarFile); assertThat(conn.getJarFileURL().toString()).isEqualTo("jar:" + this.rootJarFile.toURI() + "!/nested.jar"); assertThat(conn.getInputStream()).isNotNull(); JarInputStream jarInputStream = new JarInputStream(conn.getInputStream()); @@ -314,7 +314,8 @@ void getNestedJarDirectory() throws Exception { URL url = nestedJarFile.getUrl(); assertThat(url.toString()).isEqualTo("jar:" + this.rootJarFile.toURI() + "!/d!/"); - assertThat(((JarURLConnection) url.openConnection()).getJarFile().getParent()).isSameAs(nestedJarFile); + JarURLConnection connection = (JarURLConnection) url.openConnection(); + assertThat(JarFileWrapper.unwrap(connection.getJarFile())).isSameAs(nestedJarFile); } } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarFileWrapperTests.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarFileWrapperTests.java new file mode 100644 index 000000000000..aa7dd0be4265 --- /dev/null +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarFileWrapperTests.java @@ -0,0 +1,140 @@ +/* + * 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.jar; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.net.MalformedURLException; +import java.util.jar.JarOutputStream; +import java.util.zip.ZipEntry; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; + +/** + * Tests for {@link JarFileWrapper}. + * + * @author Phillip Webb + */ +class JarFileWrapperTests { + + private JarFile parent; + + private JarFileWrapper wrapper; + + @BeforeEach + void setup(@TempDir File temp) throws IOException { + this.parent = spy(new JarFile(createTempJar(temp))); + this.wrapper = new JarFileWrapper(this.parent); + } + + private File createTempJar(File temp) throws IOException { + File file = new File(temp, "temp.jar"); + new JarOutputStream(new FileOutputStream(file)).close(); + return file; + } + + @Test + void getUrlDelegatesToParent() throws MalformedURLException { + this.wrapper.getUrl(); + verify(this.parent).getUrl(); + } + + @Test + void getTypeDelegatesToParent() { + this.wrapper.getType(); + verify(this.parent).getType(); + } + + @Test + void getPermissionDelegatesToParent() { + this.wrapper.getPermission(); + verify(this.parent).getPermission(); + } + + @Test + void getManifestDelegatesToParent() throws IOException { + this.wrapper.getManifest(); + verify(this.parent).getManifest(); + } + + @Test + void entriesDelegatesToParent() { + this.wrapper.entries(); + verify(this.parent).entries(); + } + + @Test + void getJarEntryDelegatesToParent() { + this.wrapper.getJarEntry("test"); + verify(this.parent).getJarEntry("test"); + } + + @Test + void getEntryDelegatesToParent() { + this.wrapper.getEntry("test"); + verify(this.parent).getEntry("test"); + } + + @Test + void getInputStreamDelegatesToParent() throws IOException { + this.wrapper.getInputStream(); + verify(this.parent).getInputStream(); + } + + @Test + void getEntryInputStreamDelegatesToParent() throws IOException { + ZipEntry entry = new ZipEntry("test"); + this.wrapper.getInputStream(entry); + verify(this.parent).getInputStream(entry); + } + + @Test + void getCommentDelegatesToParent() { + this.wrapper.getComment(); + verify(this.parent).getComment(); + } + + @Test + void sizeDelegatesToParent() { + this.wrapper.size(); + verify(this.parent).size(); + } + + @Test + void toStringDelegatesToParent() { + assertThat(this.wrapper.toString()).endsWith("/temp.jar"); + } + + @Test // gh-22991 + void wrapperMustNotImplementClose() { + // If the wrapper overrides close then on Java 11 a FinalizableResource + // instance will be used to perform cleanup. This can result in a lot + // of additional memory being used since cleanup only occurs when the + // finalizer thread runs. See gh-22991 + assertThatExceptionOfType(NoSuchMethodException.class) + .isThrownBy(() -> JarFileWrapper.class.getDeclaredMethod("close")); + } + +} 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 402ad6850e88..7ea75b2fb67e 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 @@ -62,14 +62,14 @@ void tearDown() throws Exception { void connectionToRootUsingAbsoluteUrl() throws Exception { URL url = new URL("jar:" + this.rootJarFile.toURI().toURL() + "!/"); Object content = JarURLConnection.get(url, this.jarFile).getContent(); - assertThat(((JarFile) content).getParent()).isSameAs(this.jarFile); + assertThat(JarFileWrapper.unwrap((java.util.jar.JarFile) content)).isSameAs(this.jarFile); } @Test void connectionToRootUsingRelativeUrl() throws Exception { URL url = new URL("jar:file:" + getRelativePath() + "!/"); Object content = JarURLConnection.get(url, this.jarFile).getContent(); - assertThat(((JarFile) content).getParent()).isSameAs(this.jarFile); + assertThat(JarFileWrapper.unwrap((java.util.jar.JarFile) content)).isSameAs(this.jarFile); } @Test @@ -226,7 +226,7 @@ void jarEntryNameWithMixtureOfEncodedAndUnencodedDoubleByteCharacters() { void openConnectionCanBeClosedWithoutClosingSourceJar() throws Exception { URL url = new URL("jar:" + this.rootJarFile.toURI().toURL() + "!/"); JarURLConnection connection = JarURLConnection.get(url, this.jarFile); - JarFile connectionJarFile = connection.getJarFile(); + java.util.jar.JarFile connectionJarFile = connection.getJarFile(); connectionJarFile.close(); assertThat(this.jarFile.isClosed()).isFalse(); }