From 674022d4014a668f9ff3f2f7837b716667ff6e30 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 17 Aug 2022 11:37:19 -0700 Subject: [PATCH 1/2] Revert "Don't close nested jars or wrapper when parent is closed" This reverts commit 360eb027befd51012614f5938fb9cdc9f557a577. --- .../springframework/boot/loader/jar/JarFile.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) 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 65450ce3958c..65727df35d0f 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 @@ -26,8 +26,11 @@ import java.net.URLStreamHandler; import java.net.URLStreamHandlerFactory; import java.security.Permission; +import java.util.ArrayList; +import java.util.Collections; import java.util.Enumeration; import java.util.Iterator; +import java.util.List; import java.util.Spliterator; import java.util.Spliterators; import java.util.function.Supplier; @@ -93,6 +96,8 @@ public class JarFile extends AbstractJarFile implements Iterable nestedJars = Collections.synchronizedList(new ArrayList<>()); + /** * Create a new {@link JarFile} backed by the specified file. * @param file the root jar file @@ -335,8 +340,10 @@ private JarFile createJarFileFromFileEntry(JarEntry entry) throws IOException { + "mechanism used to create your executable jar file"); } RandomAccessData entryData = this.entries.getEntryData(entry.getName()); - return new JarFile(this.rootFile, this.pathFromRoot + "!/" + entry.getName(), entryData, + JarFile nestedJar = new JarFile(this.rootFile, this.pathFromRoot + "!/" + entry.getName(), entryData, JarFileType.NESTED_JAR); + this.nestedJars.add(nestedJar); + return nestedJar; } @Override @@ -361,6 +368,12 @@ public void close() throws IOException { if (this.type == JarFileType.DIRECT) { this.rootFile.close(); } + if (this.wrapper != null) { + this.wrapper.close(); + } + for (JarFile nestedJar : this.nestedJars) { + nestedJar.close(); + } this.closed = true; } } From bd74344025c1302c6700aa6b20f3e3951aeca32f Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 17 Aug 2022 11:38:42 -0700 Subject: [PATCH 2/2] Revert "Don't close jar files early" This reverts commit b42f056ddbfd5041ef80d2d909dd2f5e51ec3ff0. --- .../boot/loader/jar/JarFile.java | 42 ++++++------------- .../boot/loader/jar/JarFileWrapper.java | 5 ++- .../boot/loader/jar/JarURLConnection.java | 18 +------- .../boot/loader/jar/JarFileWrapperTests.java | 3 +- .../spring-boot-loader-tests/build.gradle | 16 +------ .../build.gradle | 22 ---------- .../settings.gradle | 15 ------- .../LoaderSignedJarTestApplication.java | 36 ---------------- .../boot/loader/LoaderIntegrationTests.java | 29 ++++--------- 9 files changed, 28 insertions(+), 158 deletions(-) delete mode 100644 spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/spring-boot-loader-tests-signed-jar-unpack-app/build.gradle delete mode 100644 spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/spring-boot-loader-tests-signed-jar-unpack-app/settings.gradle delete mode 100644 spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/spring-boot-loader-tests-signed-jar-unpack-app/src/main/java/org/springframework/boot/loaderapp/LoaderSignedJarTestApplication.java 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 65727df35d0f..81386386f931 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-2022 the original author or authors. + * Copyright 2012-2021 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. @@ -26,11 +26,8 @@ import java.net.URLStreamHandler; import java.net.URLStreamHandlerFactory; import java.security.Permission; -import java.util.ArrayList; -import java.util.Collections; import java.util.Enumeration; import java.util.Iterator; -import java.util.List; import java.util.Spliterator; import java.util.Spliterators; import java.util.function.Supplier; @@ -96,8 +93,6 @@ public class JarFile extends AbstractJarFile implements Iterable nestedJars = Collections.synchronizedList(new ArrayList<>()); - /** * Create a new {@link JarFile} backed by the specified file. * @param file the root jar file @@ -133,6 +128,9 @@ private JarFile(RandomAccessDataFile rootFile, String pathFromRoot, RandomAccess private JarFile(RandomAccessDataFile rootFile, String pathFromRoot, RandomAccessData data, JarEntryFilter filter, JarFileType type, Supplier manifestSupplier) throws IOException { super(rootFile.getFile()); + if (System.getSecurityManager() == null) { + super.close(); + } this.rootFile = rootFile; this.pathFromRoot = pathFromRoot; CentralDirectoryParser parser = new CentralDirectoryParser(); @@ -144,7 +142,8 @@ private JarFile(RandomAccessDataFile rootFile, String pathFromRoot, RandomAccess } catch (RuntimeException ex) { try { - close(); + this.rootFile.close(); + super.close(); } catch (IOException ioex) { } @@ -189,13 +188,8 @@ public void visitEnd() { JarFileWrapper getWrapper() throws IOException { JarFileWrapper wrapper = this.wrapper; if (wrapper == null) { - synchronized (this) { - if (this.wrapper != null) { - return this.wrapper; - } - wrapper = new JarFileWrapper(this); - this.wrapper = wrapper; - } + wrapper = new JarFileWrapper(this); + this.wrapper = wrapper; } return wrapper; } @@ -340,10 +334,8 @@ private JarFile createJarFileFromFileEntry(JarEntry entry) throws IOException { + "mechanism used to create your executable jar file"); } RandomAccessData entryData = this.entries.getEntryData(entry.getName()); - JarFile nestedJar = new JarFile(this.rootFile, this.pathFromRoot + "!/" + entry.getName(), entryData, + return new JarFile(this.rootFile, this.pathFromRoot + "!/" + entry.getName(), entryData, JarFileType.NESTED_JAR); - this.nestedJars.add(nestedJar); - return nestedJar; } @Override @@ -363,19 +355,11 @@ public void close() throws IOException { if (this.closed) { return; } - synchronized (this) { - super.close(); - if (this.type == JarFileType.DIRECT) { - this.rootFile.close(); - } - if (this.wrapper != null) { - this.wrapper.close(); - } - for (JarFile nestedJar : this.nestedJars) { - nestedJar.close(); - } - this.closed = true; + super.close(); + if (this.type == JarFileType.DIRECT) { + this.rootFile.close(); } + this.closed = true; } private void ensureOpen() { 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 index d5c2f2ca4763..ebc897985553 100644 --- 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 @@ -1,5 +1,5 @@ /* - * Copyright 2012-2022 the original author or authors. + * Copyright 2012-2021 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. @@ -40,6 +40,9 @@ class JarFileWrapper extends AbstractJarFile { JarFileWrapper(JarFile parent) throws IOException { super(parent.getRootJarFile().getFile()); this.parent = parent; + if (System.getSecurityManager() == null) { + super.close(); + } } @Override 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 a7ba89923182..5e3a4f7a80c8 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-2022 the original author or authors. + * Copyright 2012-2021 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. @@ -18,7 +18,6 @@ import java.io.ByteArrayOutputStream; import java.io.FileNotFoundException; -import java.io.FilterInputStream; import java.io.IOException; import java.io.InputStream; import java.io.UnsupportedEncodingException; @@ -166,7 +165,7 @@ public InputStream getInputStream() throws IOException { if (inputStream == null) { throwFileNotFound(this.jarEntryName, this.jarFile); } - return new ConnectionInputStream(inputStream); + return inputStream; } private void throwFileNotFound(Object entry, AbstractJarFile jarFile) throws FileNotFoundException { @@ -291,19 +290,6 @@ private static JarURLConnection notFound(JarFile jarFile, JarEntryName jarEntryN return new JarURLConnection(null, jarFile, jarEntryName); } - private class ConnectionInputStream extends FilterInputStream { - - ConnectionInputStream(InputStream in) { - super(in); - } - - @Override - public void close() throws IOException { - JarURLConnection.this.jarFile.close(); - } - - } - /** * A JarEntryName parsed from a URL String. */ 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 index 6cc4eb28d240..7da6716b00a9 100644 --- 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 @@ -1,5 +1,5 @@ /* - * Copyright 2012-2022 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. @@ -61,7 +61,6 @@ void setup(@TempDir File temp) throws Exception { @AfterEach void cleanup() throws Exception { this.parent.close(); - this.wrapper.close(); } private File createTempJar(File temp) throws IOException { diff --git a/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/build.gradle b/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/build.gradle index 8cb06000b48f..7c4095f73b1a 100644 --- a/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/build.gradle +++ b/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/build.gradle @@ -14,8 +14,6 @@ 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") - app project(path: ":spring-boot-project:spring-boot-starters:spring-boot-starter", configuration: "mavenRepository") - app("org.bouncycastle:bcprov-jdk15on:1.70") intTestImplementation(enforcedPlatform(project(":spring-boot-project:spring-boot-parent"))) intTestImplementation(project(":spring-boot-project:spring-boot-tools:spring-boot-test-support")) @@ -41,18 +39,6 @@ task buildApp(type: GradleBuild) { tasks = ["build"] } -task syncSignedJarUnpackAppSource(type: org.springframework.boot.build.SyncAppSource) { - sourceDirectory = file("spring-boot-loader-tests-signed-jar-unpack-app") - destinationDirectory = file("${buildDir}/spring-boot-loader-tests-signed-jar-unpack-app") -} - -task buildSignedJarUnpackApp(type: GradleBuild) { - dependsOn syncSignedJarUnpackAppSource, syncMavenRepository - dir = "${buildDir}/spring-boot-loader-tests-signed-jar-unpack-app" - startParameter.buildCacheEnabled = false - tasks = ["build"] -} - intTest { - dependsOn buildApp, buildSignedJarUnpackApp + dependsOn buildApp } \ No newline at end of file diff --git a/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/spring-boot-loader-tests-signed-jar-unpack-app/build.gradle b/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/spring-boot-loader-tests-signed-jar-unpack-app/build.gradle deleted file mode 100644 index 88c4a0e3d877..000000000000 --- a/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/spring-boot-loader-tests-signed-jar-unpack-app/build.gradle +++ /dev/null @@ -1,22 +0,0 @@ -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") - implementation("org.bouncycastle:bcprov-jdk18on:1.71") -} - -bootJar { - requiresUnpack '**/bcprov-jdk18on-*.jar' -} \ No newline at end of file diff --git a/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/spring-boot-loader-tests-signed-jar-unpack-app/settings.gradle b/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/spring-boot-loader-tests-signed-jar-unpack-app/settings.gradle deleted file mode 100644 index 06d9554ad0d6..000000000000 --- a/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/spring-boot-loader-tests-signed-jar-unpack-app/settings.gradle +++ /dev/null @@ -1,15 +0,0 @@ -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}" - } - } - } -} \ No newline at end of file diff --git a/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/spring-boot-loader-tests-signed-jar-unpack-app/src/main/java/org/springframework/boot/loaderapp/LoaderSignedJarTestApplication.java b/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/spring-boot-loader-tests-signed-jar-unpack-app/src/main/java/org/springframework/boot/loaderapp/LoaderSignedJarTestApplication.java deleted file mode 100644 index ce050ba586cb..000000000000 --- a/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/spring-boot-loader-tests-signed-jar-unpack-app/src/main/java/org/springframework/boot/loaderapp/LoaderSignedJarTestApplication.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright 2012-2022 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.security.Security; -import javax.crypto.Cipher; -import org.bouncycastle.jce.provider.BouncyCastleProvider; - -import org.springframework.boot.SpringApplication; -import org.springframework.boot.autoconfigure.SpringBootApplication; - -@SpringBootApplication -public class LoaderSignedJarTestApplication { - - public static void main(String[] args) throws Exception { - Security.addProvider(new BouncyCastleProvider()); - Cipher.getInstance("AES/CBC/PKCS5Padding","BC"); - System.out.println("Legion of the Bouncy Castle"); - SpringApplication.run(LoaderSignedJarTestApplication.class, args); - } - -} diff --git a/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/src/intTest/java/org/springframework/boot/loader/LoaderIntegrationTests.java b/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/src/intTest/java/org/springframework/boot/loader/LoaderIntegrationTests.java index e4bce38694fe..8aa27b2efdcd 100644 --- a/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/src/intTest/java/org/springframework/boot/loader/LoaderIntegrationTests.java +++ b/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/src/intTest/java/org/springframework/boot/loader/LoaderIntegrationTests.java @@ -51,7 +51,7 @@ class LoaderIntegrationTests { @ParameterizedTest @MethodSource("javaRuntimes") void readUrlsWithoutWarning(JavaRuntime javaRuntime) { - try (GenericContainer container = createContainer(javaRuntime, "spring-boot-loader-tests-app")) { + try (GenericContainer container = createContainer(javaRuntime)) { container.start(); System.out.println(this.output.toUtf8String()); assertThat(this.output.toUtf8String()).contains(">>>>> 287649 BYTES from").doesNotContain("WARNING:") @@ -59,32 +59,17 @@ void readUrlsWithoutWarning(JavaRuntime javaRuntime) { } } - @ParameterizedTest - @MethodSource("javaRuntimes") - void runSignedJarWhenUnpacked(JavaRuntime javaRuntime) { - try (GenericContainer container = createContainer(javaRuntime, - "spring-boot-loader-tests-signed-jar-unpack-app")) { - container.start(); - System.out.println(this.output.toUtf8String()); - assertThat(this.output.toUtf8String()).contains("Legion of the Bouncy Castle"); - } - } - - private GenericContainer createContainer(JavaRuntime javaRuntime, String name) { + private GenericContainer createContainer(JavaRuntime javaRuntime) { return javaRuntime.getContainer().withLogConsumer(this.output) - .withCopyFileToContainer(findApplication(name), "/app.jar") + .withCopyFileToContainer(MountableFile.forHostPath(findApplication().toPath()), "/app.jar") .withStartupCheckStrategy(new OneShotStartupCheckStrategy().withTimeout(Duration.ofMinutes(5))) .withCommand("java", "-jar", "app.jar"); } - private MountableFile findApplication(String name) { - return MountableFile.forHostPath(findJarFile(name).toPath()); - } - - private File findJarFile(String name) { - String path = String.format("build/%1$s/build/libs/%1$s.jar", name); - File jar = new File(path); - Assert.state(jar.isFile(), () -> "Could not find " + path + ". Have you built it?"); + private File findApplication() { + String name = String.format("build/%1$s/build/libs/%1$s.jar", "spring-boot-loader-tests-app"); + File jar = new File(name); + Assert.state(jar.isFile(), () -> "Could not find " + name + ". Have you built it?"); return jar; }