root
/
spring-boot
mirror of https://github.com/spring-projects/spring-boot.git
1
0
Fork 0
Code Issues Actions 12 Packages Projects Releases Wiki Activity

Polish 'Fix signed jar performance issues' 

Update the performance improvements to push certificate loading
and storage into the `JarFileEntries` class. This allows us to
keep certificates without needing to cache all entry data. We
now also keep certificates and code signers in a dedicated class
which is set whenever the full jar stream as been read, even if
the contained values are `null`. The logic that assumes META-INF
entries are not signed has been removed in favor of delegating to
the streamed entry results.

See gh-19041
This commit is contained in:
Phillip Webb 2020-09-13 13:59:26 -07:00
parent 4d053e15d8
commit c6a9696dd1
5 changed files with 148 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -1,5 +1,5 @@
/*
* Copyright 2012-2019 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.
@ -32,20 +32,21 @@ import java.util.jar.Manifest;
*/
class JarEntry extends java.util.jar.JarEntry implements FileHeader {
private final int index;
private final AsciiBytes name;
private final AsciiBytes headerName;
private Certificate[] certificates;
private CodeSigner[] codeSigners;
private final JarFile jarFile;
private long localHeaderOffset;
JarEntry(JarFile jarFile, CentralDirectoryFileHeader header, AsciiBytes nameAlias) {
private volatile JarEntryCertification certification;
JarEntry(JarFile jarFile, int index, CentralDirectoryFileHeader header, AsciiBytes nameAlias) {
super((nameAlias != null) ? nameAlias.toString() : header.getName().toString());
this.index = index;
this.name = (nameAlias != null) ? nameAlias : header.getName();
this.headerName = header.getName();
this.jarFile = jarFile;
@ -59,6 +60,10 @@ class JarEntry extends java.util.jar.JarEntry implements FileHeader {
setExtra(header.getExtra());
}
int getIndex() {
return this.index;
}
AsciiBytes getAsciiBytesName() {
return this.name;
}
@ -85,23 +90,24 @@ class JarEntry extends java.util.jar.JarEntry implements FileHeader {
@Override
public Certificate[] getCertificates() {
if (this.jarFile.isSigned() && this.certificates == null && isSignable()) {
this.jarFile.setupEntryCertificates();
}
return this.certificates;
return getCertification().getCertificates();
}
@Override
public CodeSigner[] getCodeSigners() {
if (this.jarFile.isSigned() && this.codeSigners == null && isSignable()) {
this.jarFile.setupEntryCertificates();
}
return this.codeSigners;
return getCertification().getCodeSigners();
}
void setCertificates(java.util.jar.JarEntry entry) {
this.certificates = entry.getCertificates();
this.codeSigners = entry.getCodeSigners();
private JarEntryCertification getCertification() {
if (!this.jarFile.isSigned()) {
return JarEntryCertification.NONE;
}
JarEntryCertification certification = this.certification;
if (certification == null) {
certification = this.jarFile.getCertification(this);
this.certification = certification;
}
return certification;
}
@Override
@ -109,8 +115,4 @@ class JarEntry extends java.util.jar.JarEntry implements FileHeader {
return this.localHeaderOffset;
}
private boolean isSignable() {
return !isDirectory() && !getName().startsWith("META-INF");
}
}

58
spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarEntryCertification.java Normal file
View File

@ -0,0 +1,58 @@
/*
* 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.
* 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.loader.jar;
import java.security.CodeSigner;
import java.security.cert.Certificate;
/**
* {@link Certificate} and {@link CodeSigner} details for a {@link JarEntry} from a signed
* {@link JarFile}.
*
* @author Phillip Webb
*/
class JarEntryCertification {
static final JarEntryCertification NONE = new JarEntryCertification(null, null);
private final Certificate[] certificates;
private final CodeSigner[] codeSigners;
JarEntryCertification(Certificate[] certificates, CodeSigner[] codeSigners) {
this.certificates = certificates;
this.codeSigners = codeSigners;
}
Certificate[] getCertificates() {
return (this.certificates != null) ? this.certificates.clone() : null;
}
CodeSigner[] getCodeSigners() {
return (this.codeSigners != null) ? this.codeSigners.clone() : null;
}
public static JarEntryCertification from(java.util.jar.JarEntry certifiedEntry) {
Certificate[] certificates = (certifiedEntry != null) ? certifiedEntry.getCertificates() : null;
CodeSigner[] codeSigners = (certifiedEntry != null) ? certifiedEntry.getCodeSigners() : null;
if (certificates == null && codeSigners == null) {
return NONE;
}
return new JarEntryCertification(certificates, codeSigners);
}
}

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

@ -27,7 +27,6 @@ import java.net.URLStreamHandlerFactory;
import java.util.Enumeration;
import java.util.Iterator;
import java.util.function.Supplier;
import java.util.jar.JarInputStream;
import java.util.jar.Manifest;
import java.util.zip.ZipEntry;
@ -387,29 +386,15 @@ public class JarFile extends java.util.jar.JarFile {
return this.signed;
}
void setupEntryCertificates() {
// Fallback to JarInputStream to obtain certificates, not fast but hopefully not
// happening that often.
JarEntryCertification getCertification(JarEntry entry) {
try {
try (JarInputStream jarStream = new JarInputStream(getData().getInputStream())) {
java.util.jar.JarEntry certEntry = null;
while ((certEntry = jarStream.getNextJarEntry()) != null) {
jarStream.closeEntry();
setCertificates(getJarEntry(certEntry.getName()), certEntry);
}
}
return this.entries.getCertification(entry);
}
catch (IOException ex) {
throw new IllegalStateException(ex);
}
}
private void setCertificates(JarEntry entry, java.util.jar.JarEntry certEntry) {
if (entry != null) {
entry.setCertificates(certEntry);
}
}
public void clearCache() {
this.entries.clearCache();
}

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

@ -1,5 +1,5 @@
/*
* Copyright 2012-2019 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.
@ -26,6 +26,7 @@ import java.util.Map;
import java.util.NoSuchElementException;
import java.util.jar.Attributes;
import java.util.jar.Attributes.Name;
import java.util.jar.JarInputStream;
import java.util.jar.Manifest;
import java.util.zip.ZipEntry;
@ -91,14 +92,13 @@ class JarFileEntries implements CentralDirectoryVisitor, Iterable<JarEntry> {
private Boolean multiReleaseJar;
private JarEntryCertification[] certifications;
private final Map<Integer, FileHeader> entriesCache = Collections
.synchronizedMap(new LinkedHashMap<Integer, FileHeader>(16, 0.75f, true) {
@Override
protected boolean removeEldestEntry(Map.Entry<Integer, FileHeader> eldest) {
if (JarFileEntries.this.jarFile.isSigned()) {
return false;
}
return size() >= ENTRY_CACHE_SIZE;
}
@ -313,7 +313,7 @@ class JarFileEntries implements CentralDirectoryVisitor, Iterable<JarEntry> {
FileHeader entry = (cached != null) ? cached : CentralDirectoryFileHeader
.fromRandomAccessData(this.centralDirectoryData, this.centralDirectoryOffsets[index], this.filter);
if (CentralDirectoryFileHeader.class.equals(entry.getClass()) && type.equals(JarEntry.class)) {
entry = new JarEntry(this.jarFile, (CentralDirectoryFileHeader) entry, nameAlias);
entry = new JarEntry(this.jarFile, index, (CentralDirectoryFileHeader) entry, nameAlias);
}
if (cacheEntry && cached != entry) {
this.entriesCache.put(index, entry);
@ -344,6 +344,41 @@ class JarFileEntries implements CentralDirectoryVisitor, Iterable<JarEntry> {
return (this.filter != null) ? this.filter.apply(name) : name;
}
JarEntryCertification getCertification(JarEntry entry) throws IOException {
JarEntryCertification[] certifications = this.certifications;
if (certifications == null) {
certifications = new JarEntryCertification[this.size];
// We fallback to use JarInputStream to obtain the certs. This isn't that
// fast, but hopefully doesn't happen too often.
try (JarInputStream certifiedJarStream = new JarInputStream(this.jarFile.getData().getInputStream())) {
java.util.jar.JarEntry certifiedEntry = null;
while ((certifiedEntry = certifiedJarStream.getNextJarEntry()) != null) {
int index = getEntryIndex(certifiedEntry.getName());
if (index != -1) {
certifications[index] = JarEntryCertification.from(certifiedEntry);
}
certifiedJarStream.closeEntry();
}
}
this.certifications = certifications;
}
JarEntryCertification certification = certifications[entry.getIndex()];
return (certification != null) ? certification : JarEntryCertification.NONE;
}
private int getEntryIndex(CharSequence name) {
int hashCode = AsciiBytes.hashCode(name);
int index = getFirstIndex(hashCode);
while (index >= 0 && index < this.size && this.hashCodes[index] == hashCode) {
CentralDirectoryFileHeader candidate = getEntry(index, CentralDirectoryFileHeader.class, false, null);
if (candidate.hasName(name, NO_SUFFIX)) {
return index;
}
index++;
}
return -1;
}
/**
* Iterator for contained entries.
*/
@ -363,7 +398,7 @@ class JarFileEntries implements CentralDirectoryVisitor, Iterable<JarEntry> {
}
int entryIndex = JarFileEntries.this.positions[this.index];
this.index++;
return getEntry(entryIndex, JarEntry.class, true, null);
return getEntry(entryIndex, JarEntry.class, false, null);
}
}

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

@ -26,9 +26,7 @@ import java.net.URL;
import java.net.URLClassLoader;
import java.nio.charset.Charset;
import java.nio.file.attribute.FileTime;
import java.security.cert.Certificate;
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.Enumeration;
import java.util.jar.JarEntry;
import java.util.jar.JarInputStream;
@ -46,6 +44,7 @@ import org.springframework.boot.loader.TestJarCreator;
import org.springframework.boot.loader.data.RandomAccessDataFile;
import org.springframework.test.util.ReflectionTestUtils;
import org.springframework.util.FileCopyUtils;
import org.springframework.util.StopWatch;
import org.springframework.util.StreamUtils;
import static org.assertj.core.api.Assertions.assertThat;
@ -377,39 +376,33 @@ public class JarFileTests {
@Test
public void verifySignedJar() throws Exception {
String classpath = System.getProperty("java.class.path");
String[] entries = classpath.split(System.getProperty("path.separator"));
String signedJarFile = null;
File signedJarFile = getSignedJarFile();
assertThat(signedJarFile).exists();
try (java.util.jar.JarFile expected = new java.util.jar.JarFile(signedJarFile)) {
try (JarFile actual = new JarFile(signedJarFile)) {
StopWatch stopWatch = new StopWatch();
Enumeration<JarEntry> actualEntries = actual.entries();
while (actualEntries.hasMoreElements()) {
JarEntry actualEntry = actualEntries.nextElement();
java.util.jar.JarEntry expectedEntry = expected.getJarEntry(actualEntry.getName());
assertThat(actualEntry.getCertificates()).as(actualEntry.getName())
.isEqualTo(expectedEntry.getCertificates());
assertThat(actualEntry.getCodeSigners()).as(actualEntry.getName())
.isEqualTo(expectedEntry.getCodeSigners());
}
assertThat(stopWatch.getTotalTimeSeconds()).isLessThan(3.0);
}
}
}
private File getSignedJarFile() {
String[] entries = System.getProperty("java.class.path").split(System.getProperty("path.separator"));
for (String entry : entries) {
if (entry.contains("bcprov")) {
signedJarFile = entry;
return new File(entry);
}
}
assertThat(signedJarFile).isNotNull();
java.util.jar.JarFile jarFile = new JarFile(new File(signedJarFile));
jarFile.getManifest();
Enumeration<JarEntry> jarEntries = jarFile.entries();
// Make sure this whole certificates routine runs in an acceptable time (few
// seconds at most)
// Some signed jars took from 30s to 5 min depending on the implementation.
Instant start = Instant.now();
while (jarEntries.hasMoreElements()) {
JarEntry jarEntry = jarEntries.nextElement();
InputStream inputStream = jarFile.getInputStream(jarEntry);
inputStream.skip(Long.MAX_VALUE);
inputStream.close();
Certificate[] certs = jarEntry.getCertificates();
if (!jarEntry.getName().startsWith("META-INF") && !jarEntry.isDirectory()
&& !jarEntry.getName().endsWith("TigerDigest.class")) {
assertThat(certs).isNotNull();
}
}
jarFile.close();
// 3 seconds is still quite long, but low enough to catch most problems
assertThat(ChronoUnit.SECONDS.between(start, Instant.now())).isLessThanOrEqualTo(3L);
return null;
}
@Test