From d1c31445ec784045d4adf45cb01102ec81dbd6b2 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 29 Jul 2013 22:21:23 -0700 Subject: [PATCH] Fixup various spring-boot-loader-tools issues Fix compression of existing jars to detect nested zip entries and use the STORED method. Also fixed various test errors. Issue: #53129653 --- spring-boot-loader-tools/pom.xml | 7 + .../boot/launcher/tools/JarWriter.java | 131 +++++++++++++++--- .../boot/launcher/tools/RepackagerTests.java | 62 ++++++--- .../boot/launcher/tools/TestJarFile.java | 40 ++++-- 4 files changed, 193 insertions(+), 47 deletions(-) diff --git a/spring-boot-loader-tools/pom.xml b/spring-boot-loader-tools/pom.xml index f54981aacab..993633ee1b7 100644 --- a/spring-boot-loader-tools/pom.xml +++ b/spring-boot-loader-tools/pom.xml @@ -18,6 +18,13 @@ org.ow2.asm asm + + + ${project.groupId} + spring-boot-loader + ${project.version} + provided + org.zeroturnaround diff --git a/spring-boot-loader-tools/src/main/java/org/springframework/boot/launcher/tools/JarWriter.java b/spring-boot-loader-tools/src/main/java/org/springframework/boot/launcher/tools/JarWriter.java index 65c37ff7aa8..35da12eae83 100644 --- a/spring-boot-loader-tools/src/main/java/org/springframework/boot/launcher/tools/JarWriter.java +++ b/spring-boot-loader-tools/src/main/java/org/springframework/boot/launcher/tools/JarWriter.java @@ -16,13 +16,16 @@ package org.springframework.boot.launcher.tools; +import java.io.ByteArrayInputStream; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.FileOutputStream; +import java.io.FilterInputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.Arrays; import java.util.Enumeration; import java.util.HashSet; import java.util.Set; @@ -84,9 +87,21 @@ class JarWriter { Enumeration entries = jarFile.entries(); while (entries.hasMoreElements()) { JarEntry entry = entries.nextElement(); - EntryWriter entryWriter = new InputStreamEntryWriter( - jarFile.getInputStream(entry), true); - writeEntry(entry, entryWriter); + ZipHeaderPeekInputStream inputStream = new ZipHeaderPeekInputStream( + jarFile.getInputStream(entry)); + try { + if (inputStream.hasZipHeader() && entry.getMethod() != ZipEntry.STORED) { + new CrcAndSize(inputStream).setupStoredEntry(entry); + inputStream.close(); + inputStream = new ZipHeaderPeekInputStream( + jarFile.getInputStream(entry)); + } + EntryWriter entryWriter = new InputStreamEntryWriter(inputStream, true); + writeEntry(entry, entryWriter); + } + finally { + inputStream.close(); + } } } @@ -98,29 +113,10 @@ class JarWriter { */ public void writeNestedLibrary(String destination, File file) throws IOException { JarEntry entry = new JarEntry(destination + file.getName()); - entry.setSize(file.length()); - entry.setCompressedSize(file.length()); - entry.setCrc(getCrc(file)); - entry.setMethod(ZipEntry.STORED); + new CrcAndSize(file).setupStoredEntry(entry); writeEntry(entry, new InputStreamEntryWriter(new FileInputStream(file), true)); } - private long getCrc(File file) throws IOException { - FileInputStream inputStream = new FileInputStream(file); - try { - byte[] buffer = new byte[BUFFER_SIZE]; - CRC32 crc = new CRC32(); - int bytesRead = -1; - while ((bytesRead = inputStream.read(buffer)) != -1) { - crc.update(buffer, 0, bytesRead); - } - return crc.getValue(); - } - finally { - inputStream.close(); - } - } - /** * Write the required spring-boot-loader classes to the JAR. * @throws IOException @@ -215,4 +211,93 @@ class JarWriter { } + /** + * {@link InputStream} that can peek ahead at zip header bytes. + */ + private static class ZipHeaderPeekInputStream extends FilterInputStream { + + private static final byte[] ZIP_HEADER = new byte[] { 0x50, 0x4b, 0x03, 0x04 }; + + private byte[] header; + + private ByteArrayInputStream headerStream; + + protected ZipHeaderPeekInputStream(InputStream in) throws IOException { + super(in); + this.header = new byte[4]; + int len = in.read(this.header); + this.headerStream = new ByteArrayInputStream(this.header, 0, len); + } + + @Override + public int read() throws IOException { + int read = (this.headerStream == null ? -1 : this.headerStream.read()); + if (read != -1) { + this.headerStream = null; + return read; + } + return super.read(); + } + + @Override + public int read(byte[] b) throws IOException { + return read(b, 0, b.length); + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + int read = (this.headerStream == null ? -1 : this.headerStream.read(b, off, + len)); + if (read != -1) { + this.headerStream = null; + return read; + } + return super.read(b, off, len); + } + + public boolean hasZipHeader() { + return Arrays.equals(this.header, ZIP_HEADER); + } + } + + /** + * Data holder for CRC and Size + */ + private static class CrcAndSize { + + private final CRC32 crc = new CRC32(); + + private long size; + + public CrcAndSize(File file) throws IOException { + FileInputStream inputStream = new FileInputStream(file); + try { + load(inputStream); + } + finally { + inputStream.close(); + } + } + + public CrcAndSize(InputStream inputStream) throws IOException { + load(inputStream); + } + + private void load(InputStream inputStream) throws IOException { + byte[] buffer = new byte[BUFFER_SIZE]; + int bytesRead = -1; + while ((bytesRead = inputStream.read(buffer)) != -1) { + this.crc.update(buffer, 0, bytesRead); + this.size += bytesRead; + } + } + + public void setupStoredEntry(JarEntry entry) { + entry.setSize(this.size); + entry.setCompressedSize(this.size); + entry.setCrc(this.crc.getValue()); + entry.setMethod(ZipEntry.STORED); + } + } + } diff --git a/spring-boot-loader-tools/src/test/java/org/springframework/boot/launcher/tools/RepackagerTests.java b/spring-boot-loader-tools/src/test/java/org/springframework/boot/launcher/tools/RepackagerTests.java index 9fad583fb92..7ba3e65263b 100644 --- a/spring-boot-loader-tools/src/test/java/org/springframework/boot/launcher/tools/RepackagerTests.java +++ b/spring-boot-loader-tools/src/test/java/org/springframework/boot/launcher/tools/RepackagerTests.java @@ -20,6 +20,7 @@ import java.io.File; import java.io.IOException; import java.util.jar.JarFile; import java.util.jar.Manifest; +import java.util.zip.ZipEntry; import org.junit.Before; import org.junit.Rule; @@ -82,7 +83,7 @@ public class RepackagerTests { @Test public void specificMainClass() throws Exception { - this.testJarFile.addClass("a.b.C.class", ClassWithoutMainMethod.class); + this.testJarFile.addClass("a/b/C.class", ClassWithoutMainMethod.class); File file = this.testJarFile.getFile(); Repackager repackager = new Repackager(file); repackager.setMainClass("a.b.C"); @@ -97,7 +98,7 @@ public class RepackagerTests { @Test public void mainClassFromManifest() throws Exception { - this.testJarFile.addClass("a.b.C.class", ClassWithoutMainMethod.class); + this.testJarFile.addClass("a/b/C.class", ClassWithoutMainMethod.class); Manifest manifest = new Manifest(); manifest = new Manifest(); manifest.getMainAttributes().putValue("Manifest-Version", "1.0"); @@ -116,7 +117,7 @@ public class RepackagerTests { @Test public void mainClassFound() throws Exception { - this.testJarFile.addClass("a.b.C.class", ClassWithMainMethod.class); + this.testJarFile.addClass("a/b/C.class", ClassWithMainMethod.class); File file = this.testJarFile.getFile(); Repackager repackager = new Repackager(file); repackager.repackage(NO_LIBRARIES); @@ -130,7 +131,7 @@ public class RepackagerTests { @Test public void noMainClass() throws Exception { - this.testJarFile.addClass("a.b.C.class", ClassWithoutMainMethod.class); + this.testJarFile.addClass("a/b/C.class", ClassWithoutMainMethod.class); this.thrown.expect(IllegalStateException.class); this.thrown.expectMessage("Unable to find main class"); new Repackager(this.testJarFile.getFile()).repackage(NO_LIBRARIES); @@ -138,7 +139,7 @@ public class RepackagerTests { @Test public void sameSourceAndDestinationWithBackup() throws Exception { - this.testJarFile.addClass("a.b.C.class", ClassWithMainMethod.class); + this.testJarFile.addClass("a/b/C.class", ClassWithMainMethod.class); File file = this.testJarFile.getFile(); Repackager repackager = new Repackager(file); repackager.repackage(NO_LIBRARIES); @@ -149,7 +150,7 @@ public class RepackagerTests { @Test public void sameSourceAndDestinationWithoutBackup() throws Exception { - this.testJarFile.addClass("a.b.C.class", ClassWithMainMethod.class); + this.testJarFile.addClass("a/b/C.class", ClassWithMainMethod.class); File file = this.testJarFile.getFile(); Repackager repackager = new Repackager(file); repackager.setBackupSource(false); @@ -161,7 +162,7 @@ public class RepackagerTests { @Test public void differentDestination() throws Exception { - this.testJarFile.addClass("a.b.C.class", ClassWithMainMethod.class); + this.testJarFile.addClass("a/b/C.class", ClassWithMainMethod.class); File source = this.testJarFile.getFile(); File dest = this.temporaryFolder.newFile("different.jar"); Repackager repackager = new Repackager(source); @@ -175,7 +176,7 @@ public class RepackagerTests { @Test public void nullDestination() throws Exception { - this.testJarFile.addClass("a.b.C.class", ClassWithMainMethod.class); + this.testJarFile.addClass("a/b/C.class", ClassWithMainMethod.class); Repackager repackager = new Repackager(this.testJarFile.getFile()); this.thrown.expect(IllegalArgumentException.class); this.thrown.expectMessage("Invalid destination"); @@ -184,7 +185,7 @@ public class RepackagerTests { @Test public void destinationIsDirectory() throws Exception { - this.testJarFile.addClass("a.b.C.class", ClassWithMainMethod.class); + this.testJarFile.addClass("a/b/C.class", ClassWithMainMethod.class); Repackager repackager = new Repackager(this.testJarFile.getFile()); this.thrown.expect(IllegalArgumentException.class); this.thrown.expectMessage("Invalid destination"); @@ -193,7 +194,7 @@ public class RepackagerTests { @Test public void overwriteDestination() throws Exception { - this.testJarFile.addClass("a.b.C.class", ClassWithMainMethod.class); + this.testJarFile.addClass("a/b/C.class", ClassWithMainMethod.class); Repackager repackager = new Repackager(this.testJarFile.getFile()); File dest = this.temporaryFolder.newFile("dest.jar"); dest.createNewFile(); @@ -203,7 +204,7 @@ public class RepackagerTests { @Test public void nullLibraries() throws Exception { - this.testJarFile.addClass("a.b.C.class", ClassWithMainMethod.class); + this.testJarFile.addClass("a/b/C.class", ClassWithMainMethod.class); File file = this.testJarFile.getFile(); Repackager repackager = new Repackager(file); this.thrown.expect(IllegalArgumentException.class); @@ -214,9 +215,9 @@ public class RepackagerTests { @Test public void libraries() throws Exception { TestJarFile libJar = new TestJarFile(this.temporaryFolder); - libJar.addClass("a.b.C.class", ClassWithoutMainMethod.class); + libJar.addClass("a/b/C.class", ClassWithoutMainMethod.class); final File libJarFile = libJar.getFile(); - this.testJarFile.addClass("a.b.C.class", ClassWithMainMethod.class); + this.testJarFile.addClass("a/b/C.class", ClassWithMainMethod.class); File file = this.testJarFile.getFile(); Repackager repackager = new Repackager(file); repackager.repackage(new Libraries() { @@ -231,9 +232,9 @@ public class RepackagerTests { @Test public void customLayout() throws Exception { TestJarFile libJar = new TestJarFile(this.temporaryFolder); - libJar.addClass("a.b.C.class", ClassWithoutMainMethod.class); + libJar.addClass("a/b/C.class", ClassWithoutMainMethod.class); final File libJarFile = libJar.getFile(); - this.testJarFile.addClass("a.b.C.class", ClassWithMainMethod.class); + this.testJarFile.addClass("a/b/C.class", ClassWithMainMethod.class); File file = this.testJarFile.getFile(); Repackager repackager = new Repackager(file); Layout layout = mock(Layout.class); @@ -254,13 +255,42 @@ public class RepackagerTests { @Test public void nullCustomLayout() throws Exception { - this.testJarFile.addClass("a.b.C.class", ClassWithoutMainMethod.class); + this.testJarFile.addClass("a/b/C.class", ClassWithoutMainMethod.class); Repackager repackager = new Repackager(this.testJarFile.getFile()); this.thrown.expect(IllegalArgumentException.class); this.thrown.expectMessage("Layout must not be null"); repackager.setLayout(null); } + @Test + public void dontRecompressZips() throws Exception { + TestJarFile nested = new TestJarFile(this.temporaryFolder); + nested.addClass("a/b/C.class", ClassWithoutMainMethod.class); + final File nestedFile = nested.getFile(); + this.testJarFile.addFile("test/nested.jar", nestedFile); + this.testJarFile.addClass("A.class", ClassWithMainMethod.class); + File file = this.testJarFile.getFile(); + Repackager repackager = new Repackager(file); + repackager.repackage(new Libraries() { + @Override + public void doWithLibraries(LibraryCallback callback) throws IOException { + callback.library(nestedFile, LibraryScope.COMPILE); + } + }); + + JarFile jarFile = new JarFile(file); + try { + assertThat(jarFile.getEntry("lib/" + nestedFile.getName()).getMethod(), + equalTo(ZipEntry.STORED)); + assertThat(jarFile.getEntry("test/nested.jar").getMethod(), + equalTo(ZipEntry.STORED)); + } + finally { + jarFile.close(); + } + + } + private boolean hasLauncherClasses(File file) throws IOException { return hasEntry(file, "org/springframework/boot/") && hasEntry(file, "org/springframework/boot/loader/JarLauncher.class"); diff --git a/spring-boot-loader-tools/src/test/java/org/springframework/boot/launcher/tools/TestJarFile.java b/spring-boot-loader-tools/src/test/java/org/springframework/boot/launcher/tools/TestJarFile.java index a19f908cf29..7a5e7684786 100644 --- a/spring-boot-loader-tools/src/test/java/org/springframework/boot/launcher/tools/TestJarFile.java +++ b/spring-boot-loader-tools/src/test/java/org/springframework/boot/launcher/tools/TestJarFile.java @@ -17,6 +17,8 @@ package org.springframework.boot.launcher.tools; import java.io.File; +import java.io.FileInputStream; +import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; @@ -44,20 +46,22 @@ public class TestJarFile { } public void addClass(String filename, Class classToCopy) throws IOException { - String[] paths = filename.split("\\/"); - File file = this.jarSource; - for (String path : paths) { - file = new File(file, path); - } + File file = getFilePath(filename); file.getParentFile().mkdirs(); InputStream inputStream = getClass().getResourceAsStream( "/" + classToCopy.getName().replace(".", "/") + ".class"); - OutputStream outputStream = new FileOutputStream(file); + copyToFile(inputStream, file); + } + + public void addFile(String filename, File fileToCopy) throws IOException { + File file = getFilePath(filename); + file.getParentFile().mkdirs(); + InputStream inputStream = new FileInputStream(fileToCopy); try { - copy(inputStream, outputStream); + copyToFile(inputStream, file); } finally { - outputStream.close(); + inputStream.close(); } } @@ -73,6 +77,26 @@ public class TestJarFile { } } + private File getFilePath(String filename) { + String[] paths = filename.split("\\/"); + File file = this.jarSource; + for (String path : paths) { + file = new File(file, path); + } + return file; + } + + private void copyToFile(InputStream inputStream, File file) + throws FileNotFoundException, IOException { + OutputStream outputStream = new FileOutputStream(file); + try { + copy(inputStream, outputStream); + } + finally { + outputStream.close(); + } + } + private void copy(InputStream in, OutputStream out) throws IOException { int bytesRead = -1; while ((bytesRead = in.read(this.buffer)) != -1) {