From f84323fe3e1d5ed662bfcff894a1ed47c046fa1b Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Thu, 22 Oct 2020 16:57:01 -0700 Subject: [PATCH] Prevent duplicate jar entries from being written Update the `AbstractJarWriter` so that it can directly build the layer index as entries are written. Prior to this commit, a layer tracking was handled by a decorator class which was broken because it didn't override enough methods. Since `AbstractJarWriter` has quite a complex API, it seems sensible to have it handle the layer index directly, removing the need for a decorator entirely. Fixes gh-23801 --- .../boot/loader/tools/AbstractJarWriter.java | 45 ++++++++++++++++--- .../boot/loader/tools/Packager.java | 34 +------------- 2 files changed, 40 insertions(+), 39 deletions(-) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/AbstractJarWriter.java b/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/AbstractJarWriter.java index acce43ecbdb..67b1ad5eb3d 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/AbstractJarWriter.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/AbstractJarWriter.java @@ -60,6 +60,20 @@ public abstract class AbstractJarWriter implements LoaderClassesWriter { private final Set writtenEntries = new HashSet<>(); + private Layers layers; + + private LayersIndex layersIndex; + + /** + * Update this writer to use specific layers. + * @param layers the layers to use + * @param layersIndex the layers index to update + */ + void useLayers(Layers layers, LayersIndex layersIndex) { + this.layers = layers; + this.layersIndex = layersIndex; + } + /** * Write the specified manifest. * @param manifest the manifest to write @@ -89,7 +103,7 @@ public abstract class AbstractJarWriter implements LoaderClassesWriter { EntryWriter entryWriter = new InputStreamEntryWriter(inputStream); JarArchiveEntry transformedEntry = entryTransformer.transform(entry); if (transformedEntry != null) { - writeEntry(transformedEntry, entryWriter, unpackHandler); + writeEntry(transformedEntry, entryWriter, unpackHandler, true); } } } @@ -144,7 +158,15 @@ public abstract class AbstractJarWriter implements LoaderClassesWriter { entry.setTime(getNestedLibraryTime(library)); new CrcAndSize(library::openStream).setupStoredEntry(entry); try (InputStream inputStream = library.openStream()) { - writeEntry(entry, new InputStreamEntryWriter(inputStream), new LibraryUnpackHandler(library)); + writeEntry(entry, new InputStreamEntryWriter(inputStream), new LibraryUnpackHandler(library), false); + updateLayerIndex(entry.getName(), library); + } + } + + private void updateLayerIndex(String name, Library library) { + if (this.layers != null) { + Layer layer = this.layers.getLayer(library); + this.layersIndex.add(layer, name); } } @@ -225,7 +247,7 @@ public abstract class AbstractJarWriter implements LoaderClassesWriter { } private void writeEntry(JarArchiveEntry entry, EntryWriter entryWriter) throws IOException { - writeEntry(entry, entryWriter, UnpackHandler.NEVER); + writeEntry(entry, entryWriter, UnpackHandler.NEVER, true); } /** @@ -234,10 +256,11 @@ public abstract class AbstractJarWriter implements LoaderClassesWriter { * @param entry the entry to write * @param entryWriter the entry writer or {@code null} if there is no content * @param unpackHandler handles possible unpacking for the entry + * @param updateLayerIndex if the layer index should be updated * @throws IOException in case of I/O errors */ - private void writeEntry(JarArchiveEntry entry, EntryWriter entryWriter, UnpackHandler unpackHandler) - throws IOException { + private void writeEntry(JarArchiveEntry entry, EntryWriter entryWriter, UnpackHandler unpackHandler, + boolean updateLayerIndex) throws IOException { String name = entry.getName(); writeParentDirectoryEntries(name); if (this.writtenEntries.add(name)) { @@ -248,10 +271,20 @@ public abstract class AbstractJarWriter implements LoaderClassesWriter { entry.setSize(entryWriter.size()); } entryWriter = addUnpackCommentIfNecessary(entry, entryWriter, unpackHandler); + if (updateLayerIndex) { + updateLayerIndex(entry); + } writeToArchive(entry, entryWriter); } } + private void updateLayerIndex(JarArchiveEntry entry) { + if (this.layers != null && !entry.getName().endsWith("/")) { + Layer layer = this.layers.getLayer(entry.getName()); + this.layersIndex.add(layer, entry.getName()); + } + } + protected abstract void writeToArchive(ZipEntry entry, EntryWriter entryWriter) throws IOException; private void writeParentDirectoryEntries(String name) throws IOException { @@ -259,7 +292,7 @@ public abstract class AbstractJarWriter implements LoaderClassesWriter { while (parent.lastIndexOf('/') != -1) { parent = parent.substring(0, parent.lastIndexOf('/')); if (!parent.isEmpty()) { - writeEntry(new JarArchiveEntry(parent + "/"), null, UnpackHandler.NEVER); + writeEntry(new JarArchiveEntry(parent + "/"), null, UnpackHandler.NEVER, false); } } } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Packager.java b/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Packager.java index a1b06415943..ce1a4e983e2 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Packager.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Packager.java @@ -29,7 +29,6 @@ import java.util.jar.Attributes; import java.util.jar.JarFile; import java.util.jar.Manifest; import java.util.stream.Collectors; -import java.util.zip.ZipEntry; import org.apache.commons.compress.archivers.jar.JarArchiveEntry; @@ -168,9 +167,7 @@ public abstract class Packager { protected final void write(JarFile sourceJar, Libraries libraries, AbstractJarWriter writer) throws IOException { Assert.notNull(libraries, "Libraries must not be null"); WritableLibraries writeableLibraries = new WritableLibraries(libraries); - if (this.layers != null) { - writer = new LayerTrackingEntryWriter(writer); - } + writer.useLayers(this.layers, this.layersIndex); writer.writeManifest(buildManifest(sourceJar)); writeLoaderClasses(writer); writer.writeEntries(sourceJar, getEntityTransformer(), writeableLibraries); @@ -422,35 +419,6 @@ public abstract class Packager { } - /** - * Decorator to track the layers as entries are written. - */ - private final class LayerTrackingEntryWriter extends AbstractJarWriter { - - private final AbstractJarWriter writer; - - private LayerTrackingEntryWriter(AbstractJarWriter writer) { - this.writer = writer; - } - - @Override - public void writeNestedLibrary(String location, Library library) throws IOException { - this.writer.writeNestedLibrary(location, library); - Layer layer = Packager.this.layers.getLayer(library); - Packager.this.layersIndex.add(layer, location + library.getName()); - } - - @Override - protected void writeToArchive(ZipEntry entry, EntryWriter entryWriter) throws IOException { - this.writer.writeToArchive(entry, entryWriter); - if (!entry.getName().endsWith("/")) { - Layer layer = Packager.this.layers.getLayer(entry.getName()); - Packager.this.layersIndex.add(layer, entry.getName()); - } - } - - } - /** * An {@link UnpackHandler} that determines that an entry needs to be unpacked if a * library that requires unpacking has a matching entry name.