Skip to content

Commit 895ff9c

Browse files
committed
Merge pull request #19041 from mathieufortin01
* pr/19041: Polish 'Fix signed jar performance issues' Fix signed jar performance issues Ignore Visual Studio Code Files Closes gh-19041
2 parents b3960ca + c6a9696 commit 895ff9c

File tree

6 files changed

+149
-63
lines changed

6 files changed

+149
-63
lines changed

.gitignore

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
.settings
2020
.springBeans
2121
/build
22+
.vscode
2223
/code
2324
MANIFEST.MF
2425
_site/
@@ -38,4 +39,4 @@ transaction-logs
3839
secrets.yml
3940
.gradletasknamecache
4041
.sts4-cache
41-
.mvn/.gradle-enterprise/gradle-enterprise-workspace-id
42+
.mvn/.gradle-enterprise/gradle-enterprise-workspace-id

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

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2019 the original author or authors.
2+
* Copyright 2012-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -32,20 +32,21 @@
3232
*/
3333
class JarEntry extends java.util.jar.JarEntry implements FileHeader {
3434

35+
private final int index;
36+
3537
private final AsciiBytes name;
3638

3739
private final AsciiBytes headerName;
3840

39-
private Certificate[] certificates;
40-
41-
private CodeSigner[] codeSigners;
42-
4341
private final JarFile jarFile;
4442

4543
private long localHeaderOffset;
4644

47-
JarEntry(JarFile jarFile, CentralDirectoryFileHeader header, AsciiBytes nameAlias) {
45+
private volatile JarEntryCertification certification;
46+
47+
JarEntry(JarFile jarFile, int index, CentralDirectoryFileHeader header, AsciiBytes nameAlias) {
4848
super((nameAlias != null) ? nameAlias.toString() : header.getName().toString());
49+
this.index = index;
4950
this.name = (nameAlias != null) ? nameAlias : header.getName();
5051
this.headerName = header.getName();
5152
this.jarFile = jarFile;
@@ -59,6 +60,10 @@ class JarEntry extends java.util.jar.JarEntry implements FileHeader {
5960
setExtra(header.getExtra());
6061
}
6162

63+
int getIndex() {
64+
return this.index;
65+
}
66+
6267
AsciiBytes getAsciiBytesName() {
6368
return this.name;
6469
}
@@ -85,23 +90,24 @@ public Attributes getAttributes() throws IOException {
8590

8691
@Override
8792
public Certificate[] getCertificates() {
88-
if (this.jarFile.isSigned() && this.certificates == null) {
89-
this.jarFile.setupEntryCertificates(this);
90-
}
91-
return this.certificates;
93+
return getCertification().getCertificates();
9294
}
9395

9496
@Override
9597
public CodeSigner[] getCodeSigners() {
96-
if (this.jarFile.isSigned() && this.codeSigners == null) {
97-
this.jarFile.setupEntryCertificates(this);
98-
}
99-
return this.codeSigners;
98+
return getCertification().getCodeSigners();
10099
}
101100

102-
void setCertificates(java.util.jar.JarEntry entry) {
103-
this.certificates = entry.getCertificates();
104-
this.codeSigners = entry.getCodeSigners();
101+
private JarEntryCertification getCertification() {
102+
if (!this.jarFile.isSigned()) {
103+
return JarEntryCertification.NONE;
104+
}
105+
JarEntryCertification certification = this.certification;
106+
if (certification == null) {
107+
certification = this.jarFile.getCertification(this);
108+
this.certification = certification;
109+
}
110+
return certification;
105111
}
106112

107113
@Override
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/*
2+
* Copyright 2012-2020 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.boot.loader.jar;
18+
19+
import java.security.CodeSigner;
20+
import java.security.cert.Certificate;
21+
22+
/**
23+
* {@link Certificate} and {@link CodeSigner} details for a {@link JarEntry} from a signed
24+
* {@link JarFile}.
25+
*
26+
* @author Phillip Webb
27+
*/
28+
class JarEntryCertification {
29+
30+
static final JarEntryCertification NONE = new JarEntryCertification(null, null);
31+
32+
private final Certificate[] certificates;
33+
34+
private final CodeSigner[] codeSigners;
35+
36+
JarEntryCertification(Certificate[] certificates, CodeSigner[] codeSigners) {
37+
this.certificates = certificates;
38+
this.codeSigners = codeSigners;
39+
}
40+
41+
Certificate[] getCertificates() {
42+
return (this.certificates != null) ? this.certificates.clone() : null;
43+
}
44+
45+
CodeSigner[] getCodeSigners() {
46+
return (this.codeSigners != null) ? this.codeSigners.clone() : null;
47+
}
48+
49+
static JarEntryCertification from(java.util.jar.JarEntry certifiedEntry) {
50+
Certificate[] certificates = (certifiedEntry != null) ? certifiedEntry.getCertificates() : null;
51+
CodeSigner[] codeSigners = (certifiedEntry != null) ? certifiedEntry.getCodeSigners() : null;
52+
if (certificates == null && codeSigners == null) {
53+
return NONE;
54+
}
55+
return new JarEntryCertification(certificates, codeSigners);
56+
}
57+
58+
}

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

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import java.util.Enumeration;
2828
import java.util.Iterator;
2929
import java.util.function.Supplier;
30-
import java.util.jar.JarInputStream;
3130
import java.util.jar.Manifest;
3231
import java.util.zip.ZipEntry;
3332

@@ -387,33 +386,15 @@ boolean isSigned() {
387386
return this.signed;
388387
}
389388

390-
void setupEntryCertificates(JarEntry entry) {
391-
// Fallback to JarInputStream to obtain certificates, not fast but hopefully not
392-
// happening that often.
389+
JarEntryCertification getCertification(JarEntry entry) {
393390
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-
}
401-
setCertificates(getJarEntry(certEntry.getName()), certEntry);
402-
certEntry = inputStream.getNextJarEntry();
403-
}
404-
}
391+
return this.entries.getCertification(entry);
405392
}
406393
catch (IOException ex) {
407394
throw new IllegalStateException(ex);
408395
}
409396
}
410397

411-
private void setCertificates(JarEntry entry, java.util.jar.JarEntry certEntry) {
412-
if (entry != null) {
413-
entry.setCertificates(certEntry);
414-
}
415-
}
416-
417398
public void clearCache() {
418399
this.entries.clearCache();
419400
}

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

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2012-2019 the original author or authors.
2+
* Copyright 2012-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -26,6 +26,7 @@
2626
import java.util.NoSuchElementException;
2727
import java.util.jar.Attributes;
2828
import java.util.jar.Attributes.Name;
29+
import java.util.jar.JarInputStream;
2930
import java.util.jar.Manifest;
3031
import java.util.zip.ZipEntry;
3132

@@ -91,14 +92,13 @@ class JarFileEntries implements CentralDirectoryVisitor, Iterable<JarEntry> {
9192

9293
private Boolean multiReleaseJar;
9394

95+
private JarEntryCertification[] certifications;
96+
9497
private final Map<Integer, FileHeader> entriesCache = Collections
9598
.synchronizedMap(new LinkedHashMap<Integer, FileHeader>(16, 0.75f, true) {
9699

97100
@Override
98101
protected boolean removeEldestEntry(Map.Entry<Integer, FileHeader> eldest) {
99-
if (JarFileEntries.this.jarFile.isSigned()) {
100-
return false;
101-
}
102102
return size() >= ENTRY_CACHE_SIZE;
103103
}
104104

@@ -313,7 +313,7 @@ private <T extends FileHeader> T getEntry(int index, Class<T> type, boolean cach
313313
FileHeader entry = (cached != null) ? cached : CentralDirectoryFileHeader
314314
.fromRandomAccessData(this.centralDirectoryData, this.centralDirectoryOffsets[index], this.filter);
315315
if (CentralDirectoryFileHeader.class.equals(entry.getClass()) && type.equals(JarEntry.class)) {
316-
entry = new JarEntry(this.jarFile, (CentralDirectoryFileHeader) entry, nameAlias);
316+
entry = new JarEntry(this.jarFile, index, (CentralDirectoryFileHeader) entry, nameAlias);
317317
}
318318
if (cacheEntry && cached != entry) {
319319
this.entriesCache.put(index, entry);
@@ -344,6 +344,41 @@ private AsciiBytes applyFilter(AsciiBytes name) {
344344
return (this.filter != null) ? this.filter.apply(name) : name;
345345
}
346346

347+
JarEntryCertification getCertification(JarEntry entry) throws IOException {
348+
JarEntryCertification[] certifications = this.certifications;
349+
if (certifications == null) {
350+
certifications = new JarEntryCertification[this.size];
351+
// We fallback to use JarInputStream to obtain the certs. This isn't that
352+
// fast, but hopefully doesn't happen too often.
353+
try (JarInputStream certifiedJarStream = new JarInputStream(this.jarFile.getData().getInputStream())) {
354+
java.util.jar.JarEntry certifiedEntry = null;
355+
while ((certifiedEntry = certifiedJarStream.getNextJarEntry()) != null) {
356+
int index = getEntryIndex(certifiedEntry.getName());
357+
if (index != -1) {
358+
certifications[index] = JarEntryCertification.from(certifiedEntry);
359+
}
360+
certifiedJarStream.closeEntry();
361+
}
362+
}
363+
this.certifications = certifications;
364+
}
365+
JarEntryCertification certification = certifications[entry.getIndex()];
366+
return (certification != null) ? certification : JarEntryCertification.NONE;
367+
}
368+
369+
private int getEntryIndex(CharSequence name) {
370+
int hashCode = AsciiBytes.hashCode(name);
371+
int index = getFirstIndex(hashCode);
372+
while (index >= 0 && index < this.size && this.hashCodes[index] == hashCode) {
373+
CentralDirectoryFileHeader candidate = getEntry(index, CentralDirectoryFileHeader.class, false, null);
374+
if (candidate.hasName(name, NO_SUFFIX)) {
375+
return index;
376+
}
377+
index++;
378+
}
379+
return -1;
380+
}
381+
347382
/**
348383
* Iterator for contained entries.
349384
*/

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

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.springframework.boot.loader.data.RandomAccessDataFile;
4545
import org.springframework.test.util.ReflectionTestUtils;
4646
import org.springframework.util.FileCopyUtils;
47+
import org.springframework.util.StopWatch;
4748
import org.springframework.util.StreamUtils;
4849

4950
import static org.assertj.core.api.Assertions.assertThat;
@@ -375,29 +376,33 @@ public void sensibleToString() throws Exception {
375376

376377
@Test
377378
public void verifySignedJar() throws Exception {
378-
String classpath = System.getProperty("java.class.path");
379-
String[] entries = classpath.split(System.getProperty("path.separator"));
380-
String signedJarFile = null;
381-
for (String entry : entries) {
382-
if (entry.contains("bcprov")) {
383-
signedJarFile = entry;
379+
File signedJarFile = getSignedJarFile();
380+
assertThat(signedJarFile).exists();
381+
try (java.util.jar.JarFile expected = new java.util.jar.JarFile(signedJarFile)) {
382+
try (JarFile actual = new JarFile(signedJarFile)) {
383+
StopWatch stopWatch = new StopWatch();
384+
Enumeration<JarEntry> actualEntries = actual.entries();
385+
while (actualEntries.hasMoreElements()) {
386+
JarEntry actualEntry = actualEntries.nextElement();
387+
java.util.jar.JarEntry expectedEntry = expected.getJarEntry(actualEntry.getName());
388+
assertThat(actualEntry.getCertificates()).as(actualEntry.getName())
389+
.isEqualTo(expectedEntry.getCertificates());
390+
assertThat(actualEntry.getCodeSigners()).as(actualEntry.getName())
391+
.isEqualTo(expectedEntry.getCodeSigners());
392+
}
393+
assertThat(stopWatch.getTotalTimeSeconds()).isLessThan(3.0);
384394
}
385395
}
386-
assertThat(signedJarFile).isNotNull();
387-
java.util.jar.JarFile jarFile = new JarFile(new File(signedJarFile));
388-
jarFile.getManifest();
389-
Enumeration<JarEntry> jarEntries = jarFile.entries();
390-
while (jarEntries.hasMoreElements()) {
391-
JarEntry jarEntry = jarEntries.nextElement();
392-
InputStream inputStream = jarFile.getInputStream(jarEntry);
393-
inputStream.skip(Long.MAX_VALUE);
394-
inputStream.close();
395-
if (!jarEntry.getName().startsWith("META-INF") && !jarEntry.isDirectory()
396-
&& !jarEntry.getName().endsWith("TigerDigest.class")) {
397-
assertThat(jarEntry.getCertificates()).isNotNull();
396+
}
397+
398+
private File getSignedJarFile() {
399+
String[] entries = System.getProperty("java.class.path").split(System.getProperty("path.separator"));
400+
for (String entry : entries) {
401+
if (entry.contains("bcprov")) {
402+
return new File(entry);
398403
}
399404
}
400-
jarFile.close();
405+
return null;
401406
}
402407

403408
@Test

0 commit comments

Comments
 (0)