From b2dad7f1c4839fab008e473171dfaa4eda614ded Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 1 Feb 2018 11:22:41 +0000 Subject: [PATCH] Rework entry ordering of repackaged archives Previously, the Repackager would write entries in the following order: - Libraries that require unpacking - Existing entries - Application classes - WEB-INF/lib jars in a war - Libraries that do not require unpacking - Loader classes Libraries that require unpacking were written before existing entries so that, when repackaging a war, an entry in WEB-INF/lib would not get in first and prevent a library with same location from being unpacked. However, this had the unwanted side-effect of changing the classpath order when an entry requires unpacking. This commit reworks the handling of existing entries and libraries that require unpacking so that existing entries can be written first while also marking any that match a library that requires unpacking as requiring unpacking. Additionally, loader classes are now written first. They are the first classes in the jar that will be used so it seems to make sense for them to appear first. This aligns Maven-based repackaging with the Gradle plugin's behaviour and with the structure documented in the reference documentation's "The Executable Jar Format" appendix. The net result of the changes described above is that entries are now written in the following order: - Loader classes - Existing entries - Application classes - WEB-INF/lib jars in a war marked for unpacking if needed - Libraries Closes gh-11695 Closes gh-11696 --- .../boot/loader/tools/JarWriter.java | 94 +++++++++++++-- .../boot/loader/tools/Repackager.java | 114 ++++++++++-------- .../boot/loader/tools/RepackagerTests.java | 67 +++++++++- 3 files changed, 214 insertions(+), 61 deletions(-) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/JarWriter.java b/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/JarWriter.java index 29dcd0b7493..fa0463936f1 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/JarWriter.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/JarWriter.java @@ -18,6 +18,7 @@ package org.springframework.boot.loader.tools; import java.io.BufferedInputStream; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; @@ -54,6 +55,8 @@ import org.apache.commons.compress.archivers.zip.UnixStat; */ public class JarWriter implements LoaderClassesWriter, AutoCloseable { + private static final UnpackHandler NEVER_UNPACK = new NeverUnpackHandler(); + private static final String NESTED_LOADER_JAR = "META-INF/loader/spring-boot-loader.jar"; private static final int BUFFER_SIZE = 32 * 1024; @@ -119,11 +122,15 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable { * @throws IOException if the entries cannot be written */ public void writeEntries(JarFile jarFile) throws IOException { - this.writeEntries(jarFile, new IdentityEntryTransformer()); + this.writeEntries(jarFile, new IdentityEntryTransformer(), NEVER_UNPACK); } - void writeEntries(JarFile jarFile, EntryTransformer entryTransformer) - throws IOException { + void writeEntries(JarFile jarFile, UnpackHandler unpackHandler) throws IOException { + this.writeEntries(jarFile, new IdentityEntryTransformer(), unpackHandler); + } + + void writeEntries(JarFile jarFile, EntryTransformer entryTransformer, + UnpackHandler unpackHandler) throws IOException { Enumeration entries = jarFile.entries(); while (entries.hasMoreElements()) { JarArchiveEntry entry = new JarArchiveEntry(entries.nextElement()); @@ -133,7 +140,7 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable { EntryWriter entryWriter = new InputStreamEntryWriter(inputStream, true); JarArchiveEntry transformedEntry = entryTransformer.transform(entry); if (transformedEntry != null) { - writeEntry(transformedEntry, entryWriter); + writeEntry(transformedEntry, entryWriter, unpackHandler); } } } @@ -172,11 +179,9 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable { File file = library.getFile(); JarArchiveEntry entry = new JarArchiveEntry(destination + library.getName()); entry.setTime(getNestedLibraryTime(file)); - if (library.isUnpackRequired()) { - entry.setComment("UNPACK:" + FileUtils.sha1Hash(file)); - } new CrcAndSize(file).setupStoredEntry(entry); - writeEntry(entry, new InputStreamEntryWriter(new FileInputStream(file), true)); + writeEntry(entry, new InputStreamEntryWriter(new FileInputStream(file), true), + new LibraryUnpackHandler(library)); } private long getNestedLibraryTime(File file) { @@ -236,15 +241,21 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable { this.jarOutput.close(); } + private void writeEntry(JarArchiveEntry entry, EntryWriter entryWriter) + throws IOException { + writeEntry(entry, entryWriter, NEVER_UNPACK); + } + /** - * Perform the actual write of a {@link JarEntry}. All other {@code write} method + * Perform the actual write of a {@link JarEntry}. All other {@code write} methods * delegate to this one. * @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 * @throws IOException in case of I/O errors */ - private void writeEntry(JarArchiveEntry entry, EntryWriter entryWriter) - throws IOException { + private void writeEntry(JarArchiveEntry entry, EntryWriter entryWriter, + UnpackHandler unpackHandler) throws IOException { String parent = entry.getName(); if (parent.endsWith("/")) { parent = parent.substring(0, parent.length() - 1); @@ -256,11 +267,12 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable { if (parent.lastIndexOf('/') != -1) { parent = parent.substring(0, parent.lastIndexOf('/') + 1); if (!parent.isEmpty()) { - writeEntry(new JarArchiveEntry(parent), null); + writeEntry(new JarArchiveEntry(parent), null, unpackHandler); } } if (this.writtenEntries.add(entry.getName())) { + entryWriter = addUnpackCommentIfNecessary(entry, entryWriter, unpackHandler); this.jarOutput.putArchiveEntry(entry); if (entryWriter != null) { entryWriter.write(this.jarOutput); @@ -269,6 +281,18 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable { } } + private EntryWriter addUnpackCommentIfNecessary(JarArchiveEntry entry, + EntryWriter entryWriter, UnpackHandler unpackHandler) throws IOException { + if (entryWriter == null || !unpackHandler.requiresUnpack(entry.getName())) { + return entryWriter; + } + ByteArrayOutputStream output = new ByteArrayOutputStream(); + entryWriter.write(output); + entry.setComment("UNPACK:" + unpackHandler.sha1Hash(entry.getName())); + return new InputStreamEntryWriter(new ByteArrayInputStream(output.toByteArray()), + true); + } + /** * Interface used to write jar entry date. */ @@ -421,4 +445,50 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable { } + /** + * An {@code UnpackHandler} determines whether or not unpacking is required and + * provides a SHA1 hash if required. + */ + interface UnpackHandler { + + boolean requiresUnpack(String name); + + String sha1Hash(String name) throws IOException; + + } + + private static final class NeverUnpackHandler implements UnpackHandler { + + @Override + public boolean requiresUnpack(String name) { + return false; + } + + @Override + public String sha1Hash(String name) { + throw new UnsupportedOperationException(); + } + + } + + private static final class LibraryUnpackHandler implements UnpackHandler { + + private final Library library; + + private LibraryUnpackHandler(Library library) { + this.library = library; + } + + @Override + public boolean requiresUnpack(String name) { + return this.library.isUnpackRequired(); + } + + @Override + public String sha1Hash(String name) throws IOException { + return FileUtils.sha1Hash(this.library.getFile()); + } + + } + } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Repackager.java b/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Repackager.java index 4c8fd0e95f2..dd4487a30e7 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Repackager.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Repackager.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 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. @@ -21,9 +21,10 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; -import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; -import java.util.Set; +import java.util.Map; +import java.util.Map.Entry; import java.util.concurrent.TimeUnit; import java.util.jar.JarFile; import java.util.jar.Manifest; @@ -31,6 +32,7 @@ import java.util.jar.Manifest; import org.apache.commons.compress.archivers.jar.JarArchiveEntry; import org.springframework.boot.loader.tools.JarWriter.EntryTransformer; +import org.springframework.boot.loader.tools.JarWriter.UnpackHandler; import org.springframework.core.io.support.SpringFactoriesLoader; import org.springframework.util.Assert; import org.springframework.util.StringUtils; @@ -231,53 +233,19 @@ public class Repackager { private void repackage(JarFile sourceJar, File destination, Libraries libraries, LaunchScript launchScript) throws IOException { + WritableLibraries writeableLibraries = new WritableLibraries(libraries); try (JarWriter writer = new JarWriter(destination, launchScript)) { - final List unpackLibraries = new ArrayList<>(); - final List standardLibraries = new ArrayList<>(); - libraries.doWithLibraries((library) -> { - File file = library.getFile(); - if (isZip(file)) { - if (library.isUnpackRequired()) { - unpackLibraries.add(library); - } - else { - standardLibraries.add(library); - } - } - }); - repackage(sourceJar, writer, unpackLibraries, standardLibraries); - } - } - - private void repackage(JarFile sourceJar, JarWriter writer, - final List unpackLibraries, final List standardLibraries) - throws IOException { - writer.writeManifest(buildManifest(sourceJar)); - Set seen = new HashSet<>(); - writeNestedLibraries(unpackLibraries, seen, writer); - if (this.layout instanceof RepackagingLayout) { - writer.writeEntries(sourceJar, new RenamingEntryTransformer( - ((RepackagingLayout) this.layout).getRepackagedClassesLocation())); - } - else { - writer.writeEntries(sourceJar); - } - writeNestedLibraries(standardLibraries, seen, writer); - writeLoaderClasses(writer); - } - - private void writeNestedLibraries(List libraries, Set alreadySeen, - JarWriter writer) throws IOException { - for (Library library : libraries) { - String destination = Repackager.this.layout - .getLibraryDestination(library.getName(), library.getScope()); - if (destination != null) { - if (!alreadySeen.add(destination + library.getName())) { - throw new IllegalStateException( - "Duplicate library " + library.getName()); - } - writer.writeNestedLibrary(destination, library); + writer.writeManifest(buildManifest(sourceJar)); + writeLoaderClasses(writer); + if (this.layout instanceof RepackagingLayout) { + writer.writeEntries(sourceJar, new RenamingEntryTransformer( + ((RepackagingLayout) this.layout).getRepackagedClassesLocation()), + writeableLibraries); } + else { + writer.writeEntries(sourceJar, writeableLibraries); + } + writeableLibraries.write(writer); } } @@ -443,4 +411,54 @@ public class Repackager { } + /** + * An {@link UnpackHandler} that determines that an entry needs to be unpacked if a + * library that requires unpacking has a matching entry name. + */ + private final class WritableLibraries implements UnpackHandler { + + private final Map libraryEntryNames = new LinkedHashMap<>(); + + private WritableLibraries(Libraries libraries) throws IOException { + libraries.doWithLibraries((library) -> { + if (isZip(library.getFile())) { + String libraryDestination = Repackager.this.layout + .getLibraryDestination(library.getName(), library.getScope()) + + library.getName(); + Library existing = this.libraryEntryNames + .putIfAbsent(libraryDestination, library); + if (existing != null) { + throw new IllegalStateException( + "Duplicate library " + library.getName()); + } + } + }); + } + + @Override + public boolean requiresUnpack(String name) { + Library library = this.libraryEntryNames.get(name); + return library != null && library.isUnpackRequired(); + } + + @Override + public String sha1Hash(String name) throws IOException { + Library library = this.libraryEntryNames.get(name); + if (library == null) { + throw new IllegalArgumentException( + "No library found for entry name '" + name + "'"); + } + return FileUtils.sha1Hash(library.getFile()); + } + + private void write(JarWriter writer) throws IOException { + for (Entry entry : this.libraryEntryNames.entrySet()) { + writer.writeNestedLibrary( + entry.getKey().substring(0, entry.getKey().lastIndexOf('/') + 1), + entry.getValue()); + } + } + + } + } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/test/java/org/springframework/boot/loader/tools/RepackagerTests.java b/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/test/java/org/springframework/boot/loader/tools/RepackagerTests.java index 81001a5b61c..24451d7ad4b 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/test/java/org/springframework/boot/loader/tools/RepackagerTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader-tools/src/test/java/org/springframework/boot/loader/tools/RepackagerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 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. @@ -21,8 +21,10 @@ import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.attribute.PosixFilePermission; +import java.util.ArrayList; import java.util.Calendar; import java.util.Enumeration; +import java.util.List; import java.util.jar.Attributes; import java.util.jar.JarEntry; import java.util.jar.JarFile; @@ -575,6 +577,58 @@ public class RepackagerTests { } } + @Test + public void loaderIsWrittenFirstThenApplicationClassesThenLibraries() + throws IOException { + this.testJarFile.addClass("com/example/Application.class", + ClassWithMainMethod.class); + File source = this.testJarFile.getFile(); + File dest = this.temporaryFolder.newFile("dest.jar"); + File libraryOne = createLibrary(); + File libraryTwo = createLibrary(); + File libraryThree = createLibrary(); + Repackager repackager = new Repackager(source); + repackager.repackage(dest, (callback) -> { + callback.library(new Library(libraryOne, LibraryScope.COMPILE, false)); + callback.library(new Library(libraryTwo, LibraryScope.COMPILE, true)); + callback.library(new Library(libraryThree, LibraryScope.COMPILE, false)); + }); + assertThat(getEntryNames(dest)).containsSubsequence( + "org/springframework/boot/loader/", + "BOOT-INF/classes/com/example/Application.class", + "BOOT-INF/lib/" + libraryOne.getName(), + "BOOT-INF/lib/" + libraryTwo.getName(), + "BOOT-INF/lib/" + libraryThree.getName()); + } + + @Test + public void existingEntryThatMatchesUnpackLibraryIsMarkedForUnpack() + throws IOException { + File library = createLibrary(); + this.testJarFile.addClass("WEB-INF/classes/com/example/Application.class", + ClassWithMainMethod.class); + this.testJarFile.addFile("WEB-INF/lib/" + library.getName(), library); + File source = this.testJarFile.getFile("war"); + File dest = this.temporaryFolder.newFile("dest.war"); + Repackager repackager = new Repackager(source); + repackager.setLayout(new Layouts.War()); + repackager.repackage(dest, (callback) -> callback + .library(new Library(library, LibraryScope.COMPILE, true))); + assertThat(getEntryNames(dest)).containsSubsequence( + "org/springframework/boot/loader/", + "WEB-INF/classes/com/example/Application.class", + "WEB-INF/lib/" + library.getName()); + JarEntry unpackLibrary = getEntry(dest, "WEB-INF/lib/" + library.getName()); + assertThat(unpackLibrary.getComment()).startsWith("UNPACK:"); + } + + private File createLibrary() throws IOException { + TestJarFile library = new TestJarFile(this.temporaryFolder); + library.addClass("com/example/library/Library.class", + ClassWithoutMainMethod.class); + return library.getFile(); + } + private boolean hasLauncherClasses(File file) throws IOException { return hasEntry(file, "org/springframework/boot/") && hasEntry(file, "org/springframework/boot/loader/JarLauncher.class"); @@ -596,6 +650,17 @@ public class RepackagerTests { } } + private List getEntryNames(File file) throws IOException { + List entryNames = new ArrayList<>(); + try (JarFile jarFile = new JarFile(file)) { + Enumeration entries = jarFile.entries(); + while (entries.hasMoreElements()) { + entryNames.add(entries.nextElement().getName()); + } + } + return entryNames; + } + private static class MockLauncherScript implements LaunchScript { private final byte[] bytes;