From 38585bf3b64a35c2e8b43c05202cf5f7f7969261 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Mon, 17 Mar 2014 13:37:33 +0000 Subject: [PATCH] Omit any file that is not a zip when repackaging When repackaging an archive, the files in the resulting lib directory must be zip files. If they're not zip files, the resulting archive may fail to run (#324). The previous approach was to consider an artifact's type when deciding whether or not it should be packaged. The type is a string and, while there are a number of well-known values, it can essentially be anything. This caused a problem with an artifact incorrectly being identified as being unsuitable for inclusion (#489). This commit changes the approach. Rather than looking at an artifact's type, it looks at the first four bytes of the archive's file. Only if these header bytes matche that of a zip file is the artifact included. This is a better match for the requirement that all files in lib be zip files. Fixes #489 --- .../boot/gradle/task/ProjectLibraries.java | 10 +---- .../boot/loader/tools/Repackager.java | 39 +++++++++++++++++-- .../boot/loader/tools/RepackagerTests.java | 5 +++ .../boot/maven/ArtifactsLibraries.java | 15 ++----- .../boot/maven/ArtifactsLibrariesTest.java | 10 ----- 5 files changed, 45 insertions(+), 34 deletions(-) diff --git a/spring-boot-tools/spring-boot-gradle-plugin/src/main/groovy/org/springframework/boot/gradle/task/ProjectLibraries.java b/spring-boot-tools/spring-boot-gradle-plugin/src/main/groovy/org/springframework/boot/gradle/task/ProjectLibraries.java index 782166b225..3197b21b6e 100644 --- a/spring-boot-tools/spring-boot-gradle-plugin/src/main/groovy/org/springframework/boot/gradle/task/ProjectLibraries.java +++ b/spring-boot-tools/spring-boot-gradle-plugin/src/main/groovy/org/springframework/boot/gradle/task/ProjectLibraries.java @@ -17,9 +17,7 @@ package org.springframework.boot.gradle.task; import java.io.IOException; -import java.util.Arrays; import java.util.Collections; -import java.util.HashSet; import java.util.Set; import org.gradle.api.Project; @@ -37,10 +35,6 @@ import org.springframework.boot.loader.tools.LibraryScope; */ class ProjectLibraries implements Libraries { - private static final Set SUPPORTED_TYPES = Collections - .unmodifiableSet(new HashSet(Arrays.asList("jar", "ejb", - "ejb-client", "test-jar", "bundle"))); - private final Project project; private String providedConfigurationName = "providedRuntime"; @@ -109,9 +103,7 @@ class ProjectLibraries implements Libraries { private void libraries(LibraryScope scope, Set artifacts, LibraryCallback callback) throws IOException { for (ResolvedArtifact artifact : artifacts) { - if (SUPPORTED_TYPES.contains(artifact.getType())) { - callback.library(artifact.getFile(), scope); - } + callback.library(artifact.getFile(), scope); } } } diff --git a/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Repackager.java b/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Repackager.java index 54341ea3ea..ab04bbd428 100644 --- a/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Repackager.java +++ b/spring-boot-tools/spring-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/Repackager.java @@ -17,8 +17,10 @@ package org.springframework.boot.loader.tools; import java.io.File; +import java.io.FileInputStream; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.jar.JarFile; import java.util.jar.Manifest; @@ -33,6 +35,8 @@ import org.springframework.boot.loader.tools.MainClassFinder.ClassNameCallback; */ public class Repackager { + private static final byte[] ZIP_FILE_HEADER = new byte[] { 'P', 'K', 3, 4 }; + private static final String MAIN_CLASS_ATTRIBUTE = "Main-Class"; private static final String START_CLASS_ATTRIBUTE = "Start-Class"; @@ -142,10 +146,37 @@ public class Repackager { @Override public void library(File file, LibraryScope scope) throws IOException { - String destination = Repackager.this.layout.getLibraryDestination( - file.getName(), scope); - if (destination != null) { - writer.writeNestedLibrary(destination, file); + + if (isZipFile(file)) { + String destination = Repackager.this.layout + .getLibraryDestination(file.getName(), scope); + if (destination != null) { + writer.writeNestedLibrary(destination, file); + } + } + } + + private boolean isZipFile(File file) { + byte[] buffer = new byte[4]; + FileInputStream fis = null; + try { + fis = new FileInputStream(file); + int read = fis.read(buffer); + + return (read == 4 && Arrays.equals(buffer, ZIP_FILE_HEADER)); + } + catch (IOException ioe) { + return false; + } + finally { + if (fis != null) { + try { + fis.close(); + } + catch (IOException ioe) { + // Close quietly + } + } } } }); diff --git a/spring-boot-tools/spring-boot-loader-tools/src/test/java/org/springframework/boot/loader/tools/RepackagerTests.java b/spring-boot-tools/spring-boot-loader-tools/src/test/java/org/springframework/boot/loader/tools/RepackagerTests.java index b80e12e4d5..1a83738a47 100644 --- a/spring-boot-tools/spring-boot-loader-tools/src/test/java/org/springframework/boot/loader/tools/RepackagerTests.java +++ b/spring-boot-tools/spring-boot-loader-tools/src/test/java/org/springframework/boot/loader/tools/RepackagerTests.java @@ -30,6 +30,7 @@ import org.junit.rules.ExpectedException; import org.junit.rules.TemporaryFolder; import org.springframework.boot.loader.tools.sample.ClassWithMainMethod; import org.springframework.boot.loader.tools.sample.ClassWithoutMainMethod; +import org.springframework.util.FileCopyUtils; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertThat; @@ -256,6 +257,8 @@ public class RepackagerTests { TestJarFile libJar = new TestJarFile(this.temporaryFolder); libJar.addClass("a/b/C.class", ClassWithoutMainMethod.class); final File libJarFile = libJar.getFile(); + final File libNonJarFile = this.temporaryFolder.newFile(); + FileCopyUtils.copy(new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8 }, libNonJarFile); this.testJarFile.addClass("a/b/C.class", ClassWithMainMethod.class); File file = this.testJarFile.getFile(); Repackager repackager = new Repackager(file); @@ -263,9 +266,11 @@ public class RepackagerTests { @Override public void doWithLibraries(LibraryCallback callback) throws IOException { callback.library(libJarFile, LibraryScope.COMPILE); + callback.library(libNonJarFile, LibraryScope.COMPILE); } }); assertThat(hasEntry(file, "lib/" + libJarFile.getName()), equalTo(true)); + assertThat(hasEntry(file, "lib/" + libNonJarFile.getName()), equalTo(false)); } @Test diff --git a/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/ArtifactsLibraries.java b/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/ArtifactsLibraries.java index de5758b285..2e672703e2 100644 --- a/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/ArtifactsLibraries.java +++ b/spring-boot-tools/spring-boot-maven-plugin/src/main/java/org/springframework/boot/maven/ArtifactsLibraries.java @@ -17,10 +17,8 @@ package org.springframework.boot.maven; import java.io.IOException; -import java.util.Arrays; import java.util.Collections; import java.util.HashMap; -import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -33,13 +31,10 @@ import org.springframework.boot.loader.tools.LibraryScope; * {@link Libraries} backed by Maven {@link Artifact}s * * @author Phillip Webb + * @author Andy Wilkinson */ public class ArtifactsLibraries implements Libraries { - private static final Set SUPPORTED_TYPES = Collections - .unmodifiableSet(new HashSet(Arrays.asList("jar", "ejb", - "ejb-client", "test-jar", "bundle"))); - private static final Map SCOPES; static { Map scopes = new HashMap(); @@ -58,11 +53,9 @@ public class ArtifactsLibraries implements Libraries { @Override public void doWithLibraries(LibraryCallback callback) throws IOException { for (Artifact artifact : this.artifacts) { - if (SUPPORTED_TYPES.contains(artifact.getType())) { - LibraryScope scope = SCOPES.get(artifact.getScope()); - if (scope != null && artifact.getFile() != null) { - callback.library(artifact.getFile(), scope); - } + LibraryScope scope = SCOPES.get(artifact.getScope()); + if (scope != null && artifact.getFile() != null) { + callback.library(artifact.getFile(), scope); } } } diff --git a/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/ArtifactsLibrariesTest.java b/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/ArtifactsLibrariesTest.java index 9c4e35d2b3..3faf8f3a48 100644 --- a/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/ArtifactsLibrariesTest.java +++ b/spring-boot-tools/spring-boot-maven-plugin/src/test/java/org/springframework/boot/maven/ArtifactsLibrariesTest.java @@ -30,7 +30,6 @@ import org.springframework.boot.loader.tools.LibraryScope; import static org.mockito.BDDMockito.given; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; /** * Tests for {@link ArtifactsLibraries}. @@ -66,13 +65,4 @@ public class ArtifactsLibrariesTest { this.libs.doWithLibraries(this.callback); verify(this.callback).library(this.file, LibraryScope.COMPILE); } - - @Test - public void doesNotIncludePoms() throws Exception { - given(this.artifact.getType()).willReturn("pom"); - given(this.artifact.getScope()).willReturn("compile"); - this.libs.doWithLibraries(this.callback); - verifyZeroInteractions(this.callback); - } - }