From b42f056ddbfd5041ef80d2d909dd2f5e51ec3ff0 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 14 Jun 2022 20:46:51 -0700 Subject: [PATCH] Don't close jar files early Update `JarFile` and related classes so that `close()` is not longer called early. Prior to this commit, we would always immediately close the underlying jar file to prevent file locking issues with our build. This causes issues on certain JVMs when they attempt to verify a signed jar. The file lock issues have now been solved by returning a custom input stream from `JarUrlConnection` which captures and delegates the close method. Fixes gh-29356 --- .../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, 158 insertions(+), 28 deletions(-) create mode 100644 spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/spring-boot-loader-tests-signed-jar-unpack-app/build.gradle create mode 100644 spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/spring-boot-loader-tests-signed-jar-unpack-app/settings.gradle create 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 81386386f931..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 @@ -1,5 +1,5 @@ /* - * Copyright 2012-2021 the original author or authors. + * 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. @@ -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 @@ -128,9 +133,6 @@ 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(); @@ -142,8 +144,7 @@ private JarFile(RandomAccessDataFile rootFile, String pathFromRoot, RandomAccess } catch (RuntimeException ex) { try { - this.rootFile.close(); - super.close(); + close(); } catch (IOException ioex) { } @@ -188,8 +189,13 @@ public void visitEnd() { JarFileWrapper getWrapper() throws IOException { JarFileWrapper wrapper = this.wrapper; if (wrapper == null) { - wrapper = new JarFileWrapper(this); - this.wrapper = wrapper; + synchronized (this) { + if (this.wrapper != null) { + return this.wrapper; + } + wrapper = new JarFileWrapper(this); + this.wrapper = wrapper; + } } return wrapper; } @@ -334,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 @@ -355,11 +363,19 @@ public void close() throws IOException { if (this.closed) { return; } - super.close(); - if (this.type == JarFileType.DIRECT) { - this.rootFile.close(); + 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; } - 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 ebc897985553..d5c2f2ca4763 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-2021 the original author or authors. + * 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. @@ -40,9 +40,6 @@ 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 5e3a4f7a80c8..a7ba89923182 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-2021 the original author or authors. + * 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. @@ -18,6 +18,7 @@ import java.io.ByteArrayOutputStream; import java.io.FileNotFoundException; +import java.io.FilterInputStream; import java.io.IOException; import java.io.InputStream; import java.io.UnsupportedEncodingException; @@ -165,7 +166,7 @@ public InputStream getInputStream() throws IOException { if (inputStream == null) { throwFileNotFound(this.jarEntryName, this.jarFile); } - return inputStream; + return new ConnectionInputStream(inputStream); } private void throwFileNotFound(Object entry, AbstractJarFile jarFile) throws FileNotFoundException { @@ -290,6 +291,19 @@ 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 7da6716b00a9..6cc4eb28d240 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-2020 the original author or authors. + * 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. @@ -61,6 +61,7 @@ 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 7c4095f73b1a..8cb06000b48f 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,6 +14,8 @@ 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")) @@ -39,6 +41,18 @@ 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 + dependsOn buildApp, buildSignedJarUnpackApp } \ 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 new file mode 100644 index 000000000000..a7787a1b8444 --- /dev/null +++ b/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/spring-boot-loader-tests-signed-jar-unpack-app/build.gradle @@ -0,0 +1,22 @@ +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-jdk15on:1.70") +} + +bootJar { + requiresUnpack '**/bcprov-jdk15on-*.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 new file mode 100644 index 000000000000..06d9554ad0d6 --- /dev/null +++ b/spring-boot-tests/spring-boot-integration-tests/spring-boot-loader-tests/spring-boot-loader-tests-signed-jar-unpack-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}" + } + } + } +} \ 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 new file mode 100644 index 000000000000..ce050ba586cb --- /dev/null +++ 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 @@ -0,0 +1,36 @@ +/* + * 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 8aa27b2efdcd..e4bce38694fe 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)) { + try (GenericContainer container = createContainer(javaRuntime, "spring-boot-loader-tests-app")) { container.start(); System.out.println(this.output.toUtf8String()); assertThat(this.output.toUtf8String()).contains(">>>>> 287649 BYTES from").doesNotContain("WARNING:") @@ -59,17 +59,32 @@ void readUrlsWithoutWarning(JavaRuntime javaRuntime) { } } - private GenericContainer createContainer(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) { return javaRuntime.getContainer().withLogConsumer(this.output) - .withCopyFileToContainer(MountableFile.forHostPath(findApplication().toPath()), "/app.jar") + .withCopyFileToContainer(findApplication(name), "/app.jar") .withStartupCheckStrategy(new OneShotStartupCheckStrategy().withTimeout(Duration.ofMinutes(5))) .withCommand("java", "-jar", "app.jar"); } - 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?"); + 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?"); return jar; }