Skip to content

Commit

Permalink
Don't close jar files early
Browse files Browse the repository at this point in the history
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
  • Loading branch information
philwebb committed Jun 15, 2022
1 parent 49b7100 commit b42f056
Show file tree
Hide file tree
Showing 9 changed files with 158 additions and 28 deletions.
@@ -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.
Expand All @@ -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;
Expand Down Expand Up @@ -93,6 +96,8 @@ public class JarFile extends AbstractJarFile implements Iterable<java.util.jar.J

private volatile JarFileWrapper wrapper;

private final List<JarFile> nestedJars = Collections.synchronizedList(new ArrayList<>());

/**
* Create a new {@link JarFile} backed by the specified file.
* @param file the root jar file
Expand Down Expand Up @@ -128,9 +133,6 @@ private JarFile(RandomAccessDataFile rootFile, String pathFromRoot, RandomAccess
private JarFile(RandomAccessDataFile rootFile, String pathFromRoot, RandomAccessData data, JarEntryFilter filter,
JarFileType type, Supplier<Manifest> manifestSupplier) throws IOException {
super(rootFile.getFile());
if (System.getSecurityManager() == null) {
super.close();
}
this.rootFile = rootFile;
this.pathFromRoot = pathFromRoot;
CentralDirectoryParser parser = new CentralDirectoryParser();
Expand All @@ -142,8 +144,7 @@ private JarFile(RandomAccessDataFile rootFile, String pathFromRoot, RandomAccess
}
catch (RuntimeException ex) {
try {
this.rootFile.close();
super.close();
close();
}
catch (IOException ioex) {
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand All @@ -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() {
Expand Down
@@ -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.
Expand Down Expand Up @@ -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
Expand Down
@@ -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.
Expand All @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
*/
Expand Down
@@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
Expand Up @@ -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"))
Expand All @@ -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
}
@@ -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'
}
@@ -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}"
}
}
}
}
@@ -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);
}

}
Expand Up @@ -51,25 +51,40 @@ 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:")
.doesNotContain("illegal").doesNotContain("jar written to temp");
}
}

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;
}

Expand Down

0 comments on commit b42f056

Please sign in to comment.