Commit f0b7e7cf authored by Andy Wilkinson's avatar Andy Wilkinson

Ensure that fat jars and wars do not corrupt UTF-8 entry names

Previously, both Repackager and the Grade plugin used the JRE's
standard ZipOutputStream when creating a fat jar or war file. This
resulted in entry names that needed UTF-8 encoding to become
corrupted.

This commit updates both to use Commons Compress'
ZipArchiveOutputStream and to configure the stream's encoding and
each entry's Unix mode. This ensures that names are encoded using
UTF-8 and can be read back in correctly by common zip tools.

Closes gh-9405
parent 885e2993
......@@ -85,6 +85,11 @@
<artifactId>jopt-simple</artifactId>
<version>4.6</version>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-compress</artifactId>
<version>1.14</version>
</dependency>
<dependency>
<groupId>org.apache.ivy</groupId>
<artifactId>ivy</artifactId>
......
......@@ -38,7 +38,6 @@ dependencies {
compile localGroovy()
compile gradleApi()
testCompile gradleTestKit()
testCompile 'org.apache.commons:commons-compress:1.13'
}
jar {
......
......@@ -29,6 +29,10 @@
<groupId>io.spring.gradle</groupId>
<artifactId>dependency-management-plugin</artifactId>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-compress</artifactId>
</dependency>
</dependencies>
<build>
<plugins>
......
......@@ -83,7 +83,7 @@ class BootArchiveSupport {
CopyAction copyAction = new BootZipCopyAction(jar.getArchivePath(),
jar.isPreserveFileTimestamps(), isUsingDefaultLoader(jar),
this.requiresUnpack.getAsSpec(), this.exclusions.getAsExcludeSpec(),
this.launchScript, this.compressionResolver);
this.launchScript, this.compressionResolver, jar.getMetadataCharset());
if (!jar.isReproducibleFileOrder()) {
return copyAction;
}
......
......@@ -24,10 +24,11 @@ import java.util.HashSet;
import java.util.Set;
import java.util.function.Function;
import java.util.zip.CRC32;
import java.util.zip.ZipEntry;
import java.util.zip.ZipInputStream;
import java.util.zip.ZipOutputStream;
import org.apache.commons.compress.archivers.zip.UnixStat;
import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream;
import org.gradle.api.GradleException;
import org.gradle.api.file.FileCopyDetails;
import org.gradle.api.file.FileTreeElement;
......@@ -65,10 +66,13 @@ class BootZipCopyAction implements CopyAction {
private final Function<FileCopyDetails, ZipCompression> compressionResolver;
private final String encoding;
BootZipCopyAction(File output, boolean preserveFileTimestamps,
boolean includeDefaultLoader, Spec<FileTreeElement> requiresUnpack,
Spec<FileTreeElement> exclusions, LaunchScriptConfiguration launchScript,
Function<FileCopyDetails, ZipCompression> compressionResolver) {
Function<FileCopyDetails, ZipCompression> compressionResolver,
String encoding) {
this.output = output;
this.preserveFileTimestamps = preserveFileTimestamps;
this.includeDefaultLoader = includeDefaultLoader;
......@@ -76,16 +80,20 @@ class BootZipCopyAction implements CopyAction {
this.exclusions = exclusions;
this.launchScript = launchScript;
this.compressionResolver = compressionResolver;
this.encoding = encoding;
}
@Override
public WorkResult execute(CopyActionProcessingStream stream) {
ZipOutputStream zipStream;
ZipArchiveOutputStream zipStream;
Spec<FileTreeElement> loaderEntries;
try {
FileOutputStream fileStream = new FileOutputStream(this.output);
writeLaunchScriptIfNecessary(fileStream);
zipStream = new ZipOutputStream(fileStream);
zipStream = new ZipArchiveOutputStream(fileStream);
if (this.encoding != null) {
zipStream.setEncoding(this.encoding);
}
loaderEntries = writeLoaderClassesIfNecessary(zipStream);
}
catch (IOException ex) {
......@@ -113,25 +121,26 @@ class BootZipCopyAction implements CopyAction {
return Specs.union(loaderEntries, this.exclusions);
}
private Spec<FileTreeElement> writeLoaderClassesIfNecessary(ZipOutputStream out) {
private Spec<FileTreeElement> writeLoaderClassesIfNecessary(
ZipArchiveOutputStream out) {
if (!this.includeDefaultLoader) {
return Specs.satisfyNone();
}
return writeLoaderClasses(out);
}
private Spec<FileTreeElement> writeLoaderClasses(ZipOutputStream out) {
private Spec<FileTreeElement> writeLoaderClasses(ZipArchiveOutputStream out) {
try (ZipInputStream in = new ZipInputStream(getClass()
.getResourceAsStream("/META-INF/loader/spring-boot-loader.jar"))) {
Set<String> entries = new HashSet<String>();
ZipEntry entry;
java.util.zip.ZipEntry entry;
while ((entry = in.getNextEntry()) != null) {
if (entry.isDirectory() && !entry.getName().startsWith("META-INF/")) {
writeDirectory(entry, out);
writeDirectory(new ZipArchiveEntry(entry), out);
entries.add(entry.getName());
}
else if (entry.getName().endsWith(".class")) {
writeClass(entry, in, out);
writeClass(new ZipArchiveEntry(entry), in, out);
}
}
return (element) -> {
......@@ -147,26 +156,29 @@ class BootZipCopyAction implements CopyAction {
}
}
private void writeDirectory(ZipEntry entry, ZipOutputStream out) throws IOException {
private void writeDirectory(ZipArchiveEntry entry, ZipArchiveOutputStream out)
throws IOException {
if (!this.preserveFileTimestamps) {
entry.setTime(GUtil.CONSTANT_TIME_FOR_ZIP_ENTRIES);
}
out.putNextEntry(entry);
out.closeEntry();
entry.setUnixMode(UnixStat.DIR_FLAG | UnixStat.DEFAULT_DIR_PERM);
out.putArchiveEntry(entry);
out.closeArchiveEntry();
}
private void writeClass(ZipEntry entry, ZipInputStream in, ZipOutputStream out)
throws IOException {
private void writeClass(ZipArchiveEntry entry, ZipInputStream in,
ZipArchiveOutputStream out) throws IOException {
if (!this.preserveFileTimestamps) {
entry.setTime(GUtil.CONSTANT_TIME_FOR_ZIP_ENTRIES);
}
out.putNextEntry(entry);
entry.setUnixMode(UnixStat.FILE_FLAG | UnixStat.DEFAULT_FILE_PERM);
out.putArchiveEntry(entry);
byte[] buffer = new byte[4096];
int read;
while ((read = in.read(buffer)) > 0) {
out.write(buffer, 0, read);
}
out.closeEntry();
out.closeArchiveEntry();
}
private void writeLaunchScriptIfNecessary(FileOutputStream fileStream) {
......@@ -185,7 +197,7 @@ class BootZipCopyAction implements CopyAction {
private static final class ZipStreamAction
implements CopyActionProcessingStreamAction {
private final ZipOutputStream zipStream;
private final ZipArchiveOutputStream zipStream;
private final File output;
......@@ -197,7 +209,7 @@ class BootZipCopyAction implements CopyAction {
private final Function<FileCopyDetails, ZipCompression> compressionType;
private ZipStreamAction(ZipOutputStream zipStream, File output,
private ZipStreamAction(ZipArchiveOutputStream zipStream, File output,
boolean preserveFileTimestamps, Spec<FileTreeElement> requiresUnpack,
Spec<FileTreeElement> exclusions,
Function<FileCopyDetails, ZipCompression> compressionType) {
......@@ -229,29 +241,31 @@ class BootZipCopyAction implements CopyAction {
}
private void createDirectory(FileCopyDetailsInternal details) throws IOException {
ZipEntry archiveEntry = new ZipEntry(
ZipArchiveEntry archiveEntry = new ZipArchiveEntry(
details.getRelativePath().getPathString() + '/');
archiveEntry.setUnixMode(UnixStat.DIR_FLAG | details.getMode());
archiveEntry.setTime(getTime(details));
this.zipStream.putNextEntry(archiveEntry);
this.zipStream.closeEntry();
this.zipStream.putArchiveEntry(archiveEntry);
this.zipStream.closeArchiveEntry();
}
private void createFile(FileCopyDetailsInternal details) throws IOException {
String relativePath = details.getRelativePath().getPathString();
ZipEntry archiveEntry = new ZipEntry(relativePath);
ZipArchiveEntry archiveEntry = new ZipArchiveEntry(relativePath);
archiveEntry.setUnixMode(UnixStat.FILE_FLAG | details.getMode());
archiveEntry.setTime(getTime(details));
ZipCompression compression = this.compressionType.apply(details);
if (compression == ZipCompression.STORED) {
prepareStoredEntry(details, archiveEntry);
}
this.zipStream.putNextEntry(archiveEntry);
this.zipStream.putArchiveEntry(archiveEntry);
details.copyTo(this.zipStream);
this.zipStream.closeEntry();
this.zipStream.closeArchiveEntry();
}
private void prepareStoredEntry(FileCopyDetailsInternal details,
ZipEntry archiveEntry) throws IOException {
archiveEntry.setMethod(ZipEntry.STORED);
ZipArchiveEntry archiveEntry) throws IOException {
archiveEntry.setMethod(java.util.zip.ZipEntry.STORED);
archiveEntry.setSize(details.getSize());
archiveEntry.setCompressedSize(details.getSize());
Crc32OutputStream crcStream = new Crc32OutputStream();
......
......@@ -27,6 +27,8 @@ import java.util.List;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
import org.apache.commons.compress.archivers.zip.ZipFile;
import org.gradle.api.Project;
import org.gradle.api.tasks.bundling.AbstractArchiveTask;
import org.gradle.api.tasks.bundling.Jar;
......@@ -306,6 +308,27 @@ public abstract class AbstractBootArchiveTests<T extends Jar & BootArchive> {
}
}
@Test
public void allEntriesUseUnixPlatformAndUtf8NameEncoding() throws IOException {
this.task.setMainClass("com.example.Main");
this.task.setMetadataCharset("UTF-8");
File classpathFolder = this.temp.newFolder();
File resource = new File(classpathFolder, "some-resource.xml");
resource.getParentFile().mkdirs();
resource.createNewFile();
this.task.classpath(classpathFolder);
this.task.execute();
File archivePath = this.task.getArchivePath();
try (ZipFile zip = new ZipFile(archivePath)) {
Enumeration<ZipArchiveEntry> entries = zip.getEntries();
while (entries.hasMoreElements()) {
ZipArchiveEntry entry = entries.nextElement();
assertThat(entry.getPlatform()).isEqualTo(ZipArchiveEntry.PLATFORM_UNIX);
assertThat(entry.getGeneralPurposeBit().usesUTF8ForNames()).isTrue();
}
}
}
private T configure(T task) throws IOException {
AbstractArchiveTask archiveTask = task;
archiveTask.setBaseName("test");
......
......@@ -30,6 +30,7 @@ import javax.xml.xpath.XPathExpression;
import javax.xml.xpath.XPathFactory;
import io.spring.gradle.dependencymanagement.DependencyManagementPlugin;
import org.apache.commons.compress.archivers.ArchiveEntry;
import org.gradle.testkit.runner.BuildResult;
import org.gradle.testkit.runner.GradleRunner;
import org.junit.rules.TemporaryFolder;
......@@ -114,7 +115,8 @@ public class GradleBuild implements TestRule {
+ absolutePath("build/resources/main") + ","
+ pathOfJarContaining(LaunchScript.class) + ","
+ pathOfJarContaining(ClassVisitor.class) + ","
+ pathOfJarContaining(DependencyManagementPlugin.class);
+ pathOfJarContaining(DependencyManagementPlugin.class) + ","
+ pathOfJarContaining(ArchiveEntry.class);
}
private String absolutePath(String path) {
......
......@@ -23,6 +23,10 @@
<groupId>org.springframework</groupId>
<artifactId>spring-core</artifactId>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-compress</artifactId>
</dependency>
<!-- Provided -->
<dependency>
<groupId>org.springframework.boot</groupId>
......
......@@ -37,11 +37,14 @@ import java.util.Set;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
import java.util.jar.JarInputStream;
import java.util.jar.JarOutputStream;
import java.util.jar.Manifest;
import java.util.zip.CRC32;
import java.util.zip.ZipEntry;
import org.apache.commons.compress.archivers.jar.JarArchiveEntry;
import org.apache.commons.compress.archivers.jar.JarArchiveOutputStream;
import org.apache.commons.compress.archivers.zip.UnixStat;
/**
* Writes JAR content, ensuring valid directory entries are always create and duplicate
* items are ignored.
......@@ -55,7 +58,7 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable {
private static final int BUFFER_SIZE = 32 * 1024;
private final JarOutputStream jarOutput;
private final JarArchiveOutputStream jarOutput;
private final Set<String> writtenEntries = new HashSet<>();
......@@ -83,7 +86,8 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable {
fileOutputStream.write(launchScript.toByteArray());
setExecutableFilePermission(file);
}
this.jarOutput = new JarOutputStream(fileOutputStream);
this.jarOutput = new JarArchiveOutputStream(fileOutputStream);
this.jarOutput.setEncoding("UTF-8");
}
private void setExecutableFilePermission(File file) {
......@@ -105,7 +109,7 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable {
* @throws IOException of the manifest cannot be written
*/
public void writeManifest(final Manifest manifest) throws IOException {
JarEntry entry = new JarEntry("META-INF/MANIFEST.MF");
JarArchiveEntry entry = new JarArchiveEntry("META-INF/MANIFEST.MF");
writeEntry(entry, new EntryWriter() {
@Override
public void write(OutputStream outputStream) throws IOException {
......@@ -127,12 +131,12 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable {
throws IOException {
Enumeration<JarEntry> entries = jarFile.entries();
while (entries.hasMoreElements()) {
JarEntry entry = entries.nextElement();
JarArchiveEntry entry = new JarArchiveEntry(entries.nextElement());
setUpStoredEntryIfNecessary(jarFile, entry);
try (ZipHeaderPeekInputStream inputStream = new ZipHeaderPeekInputStream(
jarFile.getInputStream(entry))) {
EntryWriter entryWriter = new InputStreamEntryWriter(inputStream, true);
JarEntry transformedEntry = entryTransformer.transform(entry);
JarArchiveEntry transformedEntry = entryTransformer.transform(entry);
if (transformedEntry != null) {
writeEntry(transformedEntry, entryWriter);
}
......@@ -140,7 +144,7 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable {
}
}
private void setUpStoredEntryIfNecessary(JarFile jarFile, JarEntry entry)
private void setUpStoredEntryIfNecessary(JarFile jarFile, JarArchiveEntry entry)
throws IOException {
try (ZipHeaderPeekInputStream inputStream = new ZipHeaderPeekInputStream(
jarFile.getInputStream(entry))) {
......@@ -158,7 +162,7 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable {
*/
@Override
public void writeEntry(String entryName, InputStream inputStream) throws IOException {
JarEntry entry = new JarEntry(entryName);
JarArchiveEntry entry = new JarArchiveEntry(entryName);
writeEntry(entry, new InputStreamEntryWriter(inputStream, true));
}
......@@ -171,7 +175,7 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable {
public void writeNestedLibrary(String destination, Library library)
throws IOException {
File file = library.getFile();
JarEntry entry = new JarEntry(destination + library.getName());
JarArchiveEntry entry = new JarArchiveEntry(destination + library.getName());
entry.setTime(getNestedLibraryTime(file));
if (library.isUnpackRequired()) {
entry.setComment("UNPACK:" + FileUtils.sha1Hash(file));
......@@ -225,7 +229,8 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable {
JarEntry entry;
while ((entry = inputStream.getNextJarEntry()) != null) {
if (entry.getName().endsWith(".class")) {
writeEntry(entry, new InputStreamEntryWriter(inputStream, false));
writeEntry(new JarArchiveEntry(entry),
new InputStreamEntryWriter(inputStream, false));
}
}
inputStream.close();
......@@ -247,24 +252,29 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable {
* @param entryWriter the entry writer or {@code null} if there is no content
* @throws IOException in case of I/O errors
*/
private void writeEntry(JarEntry entry, EntryWriter entryWriter) throws IOException {
private void writeEntry(JarArchiveEntry entry, EntryWriter entryWriter)
throws IOException {
String parent = entry.getName();
if (parent.endsWith("/")) {
parent = parent.substring(0, parent.length() - 1);
entry.setUnixMode(UnixStat.DIR_FLAG | UnixStat.DEFAULT_DIR_PERM);
}
else {
entry.setUnixMode(UnixStat.FILE_FLAG | UnixStat.DEFAULT_FILE_PERM);
}
if (parent.lastIndexOf("/") != -1) {
parent = parent.substring(0, parent.lastIndexOf("/") + 1);
if (parent.length() > 0) {
writeEntry(new JarEntry(parent), null);
writeEntry(new JarArchiveEntry(parent), null);
}
}
if (this.writtenEntries.add(entry.getName())) {
this.jarOutput.putNextEntry(entry);
this.jarOutput.putArchiveEntry(entry);
if (entryWriter != null) {
entryWriter.write(this.jarOutput);
}
this.jarOutput.closeEntry();
this.jarOutput.closeArchiveEntry();
}
}
......@@ -393,7 +403,7 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable {
}
}
public void setupStoredEntry(JarEntry entry) {
public void setupStoredEntry(JarArchiveEntry entry) {
entry.setSize(this.size);
entry.setCompressedSize(this.size);
entry.setCrc(this.crc.getValue());
......@@ -408,7 +418,7 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable {
*/
interface EntryTransformer {
JarEntry transform(JarEntry jarEntry);
JarArchiveEntry transform(JarArchiveEntry jarEntry);
}
......@@ -418,7 +428,7 @@ public class JarWriter implements LoaderClassesWriter, AutoCloseable {
private static final class IdentityEntryTransformer implements EntryTransformer {
@Override
public JarEntry transform(JarEntry jarEntry) {
public JarArchiveEntry transform(JarArchiveEntry jarEntry) {
return jarEntry;
}
......
......@@ -25,10 +25,11 @@ import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
import java.util.jar.Manifest;
import org.apache.commons.compress.archivers.jar.JarArchiveEntry;
import org.springframework.boot.loader.tools.JarWriter.EntryTransformer;
import org.springframework.core.io.support.SpringFactoriesLoader;
import org.springframework.util.Assert;
......@@ -407,7 +408,7 @@ public class Repackager {
}
@Override
public JarEntry transform(JarEntry entry) {
public JarArchiveEntry transform(JarArchiveEntry entry) {
if (entry.getName().equals("META-INF/INDEX.LIST")) {
return null;
}
......@@ -416,7 +417,8 @@ public class Repackager {
|| entry.getName().startsWith("BOOT-INF/")) {
return entry;
}
JarEntry renamedEntry = new JarEntry(this.namePrefix + entry.getName());
JarArchiveEntry renamedEntry = new JarArchiveEntry(
this.namePrefix + entry.getName());
renamedEntry.setTime(entry.getTime());
renamedEntry.setSize(entry.getSize());
renamedEntry.setMethod(entry.getMethod());
......
......@@ -22,12 +22,15 @@ import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.attribute.PosixFilePermission;
import java.util.Calendar;
import java.util.Enumeration;
import java.util.jar.Attributes;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
import java.util.jar.Manifest;
import java.util.zip.ZipEntry;
import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
import org.apache.commons.compress.archivers.zip.ZipFile;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
......@@ -595,6 +598,23 @@ public class RepackagerTests {
}
}
@Test
public void allEntriesUseUnixPlatformAndUtf8NameEncoding() throws IOException {
this.testJarFile.addClass("A.class", ClassWithMainMethod.class);
File source = this.testJarFile.getFile();
File dest = this.temporaryFolder.newFile("dest.jar");
Repackager repackager = new Repackager(source);
repackager.repackage(dest, NO_LIBRARIES);
try (ZipFile zip = new ZipFile(dest)) {
Enumeration<ZipArchiveEntry> entries = zip.getEntries();
while (entries.hasMoreElements()) {
ZipArchiveEntry entry = entries.nextElement();
assertThat(entry.getPlatform()).isEqualTo(ZipArchiveEntry.PLATFORM_UNIX);
assertThat(entry.getGeneralPurposeBit().usesUTF8ForNames()).isTrue();
}
}
}
private boolean hasLauncherClasses(File file) throws IOException {
return hasEntry(file, "org/springframework/boot/")
&& hasEntry(file, "org/springframework/boot/loader/JarLauncher.class");
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment