Skip to content

Commit 4d053e1

Browse files
mathieufortin01philwebb
authored andcommitted
Fix signed jar performance issues
Update Spring Boot nested JarFile support to improve the performance of signed jars. Prior to this commit, `certificates` and `codeSigners` were read by streaming the entire jar whenever the existing values were `null`. Unfortunately, the contract for `getCertificates` and get `getCodeSigners` states that `null` is a valid return value. This meant that full jar streaming would occur whenever either method was called on an entry that had no result. The problem was further exacerbated by the fact that entries might not be cached. See gh-19041
1 parent 6bf1bd5 commit 4d053e1

File tree

4 files changed

+27
-15
lines changed

4 files changed

+27
-15
lines changed

spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarEntry.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,16 +85,16 @@ public Attributes getAttributes() throws IOException {
8585

8686
@Override
8787
public Certificate[] getCertificates() {
88-
if (this.jarFile.isSigned() && this.certificates == null) {
89-
this.jarFile.setupEntryCertificates(this);
88+
if (this.jarFile.isSigned() && this.certificates == null && isSignable()) {
89+
this.jarFile.setupEntryCertificates();
9090
}
9191
return this.certificates;
9292
}
9393

9494
@Override
9595
public CodeSigner[] getCodeSigners() {
96-
if (this.jarFile.isSigned() && this.codeSigners == null) {
97-
this.jarFile.setupEntryCertificates(this);
96+
if (this.jarFile.isSigned() && this.codeSigners == null && isSignable()) {
97+
this.jarFile.setupEntryCertificates();
9898
}
9999
return this.codeSigners;
100100
}
@@ -109,4 +109,8 @@ public long getLocalHeaderOffset() {
109109
return this.localHeaderOffset;
110110
}
111111

112+
private boolean isSignable() {
113+
return !isDirectory() && !getName().startsWith("META-INF");
114+
}
115+
112116
}

spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarFile.java

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -387,19 +387,15 @@ boolean isSigned() {
387387
return this.signed;
388388
}
389389

390-
void setupEntryCertificates(JarEntry entry) {
390+
void setupEntryCertificates() {
391391
// Fallback to JarInputStream to obtain certificates, not fast but hopefully not
392392
// happening that often.
393393
try {
394-
try (JarInputStream inputStream = new JarInputStream(getData().getInputStream())) {
395-
java.util.jar.JarEntry certEntry = inputStream.getNextJarEntry();
396-
while (certEntry != null) {
397-
inputStream.closeEntry();
398-
if (entry.getName().equals(certEntry.getName())) {
399-
setCertificates(entry, certEntry);
400-
}
394+
try (JarInputStream jarStream = new JarInputStream(getData().getInputStream())) {
395+
java.util.jar.JarEntry certEntry = null;
396+
while ((certEntry = jarStream.getNextJarEntry()) != null) {
397+
jarStream.closeEntry();
401398
setCertificates(getJarEntry(certEntry.getName()), certEntry);
402-
certEntry = inputStream.getNextJarEntry();
403399
}
404400
}
405401
}

spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarFileEntries.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ public JarEntry next() {
363363
}
364364
int entryIndex = JarFileEntries.this.positions[this.index];
365365
this.index++;
366-
return getEntry(entryIndex, JarEntry.class, false, null);
366+
return getEntry(entryIndex, JarEntry.class, true, null);
367367
}
368368

369369
}

spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarFileTests.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@
2626
import java.net.URLClassLoader;
2727
import java.nio.charset.Charset;
2828
import java.nio.file.attribute.FileTime;
29+
import java.security.cert.Certificate;
2930
import java.time.Instant;
31+
import java.time.temporal.ChronoUnit;
3032
import java.util.Enumeration;
3133
import java.util.jar.JarEntry;
3234
import java.util.jar.JarInputStream;
@@ -387,17 +389,27 @@ public void verifySignedJar() throws Exception {
387389
java.util.jar.JarFile jarFile = new JarFile(new File(signedJarFile));
388390
jarFile.getManifest();
389391
Enumeration<JarEntry> jarEntries = jarFile.entries();
392+
393+
// Make sure this whole certificates routine runs in an acceptable time (few
394+
// seconds at most)
395+
// Some signed jars took from 30s to 5 min depending on the implementation.
396+
Instant start = Instant.now();
390397
while (jarEntries.hasMoreElements()) {
391398
JarEntry jarEntry = jarEntries.nextElement();
392399
InputStream inputStream = jarFile.getInputStream(jarEntry);
393400
inputStream.skip(Long.MAX_VALUE);
394401
inputStream.close();
402+
403+
Certificate[] certs = jarEntry.getCertificates();
395404
if (!jarEntry.getName().startsWith("META-INF") && !jarEntry.isDirectory()
396405
&& !jarEntry.getName().endsWith("TigerDigest.class")) {
397-
assertThat(jarEntry.getCertificates()).isNotNull();
406+
assertThat(certs).isNotNull();
398407
}
399408
}
400409
jarFile.close();
410+
411+
// 3 seconds is still quite long, but low enough to catch most problems
412+
assertThat(ChronoUnit.SECONDS.between(start, Instant.now())).isLessThanOrEqualTo(3L);
401413
}
402414

403415
@Test

0 commit comments

Comments
 (0)