From 558d811b0a92c3bf24bbf688c7bd579137e593d0 Mon Sep 17 00:00:00 2001 From: Moritz Halbritter Date: Tue, 19 Mar 2024 15:12:13 +0100 Subject: [PATCH] Use JarFile instead of ZipInputStream ZipInputStream can't cope with some non-deflated entries, see https://bugs.openjdk.org/browse/JDK-8143613. JarFile works better, but it doesn't support creation time / access time. See gh-38276 --- .../boot/jarmode/tools/ExtractCommand.java | 86 ++++++++++++------- .../jarmode/tools/ExtractCommandTests.java | 38 ++------ .../tools/ExtractLayersCommandTests.java | 27 ------ 3 files changed, 61 insertions(+), 90 deletions(-) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/main/java/org/springframework/boot/jarmode/tools/ExtractCommand.java b/spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/main/java/org/springframework/boot/jarmode/tools/ExtractCommand.java index 4821d686b4..b9c190b31f 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/main/java/org/springframework/boot/jarmode/tools/ExtractCommand.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/main/java/org/springframework/boot/jarmode/tools/ExtractCommand.java @@ -20,17 +20,20 @@ import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.OutputStream; import java.io.PrintStream; import java.io.UncheckedIOException; import java.nio.file.Files; import java.nio.file.attribute.BasicFileAttributeView; import java.nio.file.attribute.FileTime; +import java.util.Enumeration; import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; import java.util.jar.JarEntry; +import java.util.jar.JarFile; import java.util.jar.JarOutputStream; import java.util.jar.Manifest; import java.util.zip.ZipEntry; @@ -157,8 +160,8 @@ class ExtractCommand extends Command { private void extractLibraries(FileResolver fileResolver, JarStructure jarStructure, Map options) throws IOException { String librariesDirectory = getLibrariesDirectory(options); - extractArchive(fileResolver, (zipEntry) -> { - Entry entry = jarStructure.resolve(zipEntry); + extractArchive(fileResolver, (jarEntry) -> { + Entry entry = jarStructure.resolve(jarEntry); if (isType(entry, Type.LIBRARY)) { return librariesDirectory + entry.location(); } @@ -212,22 +215,22 @@ class ExtractCommand extends Command { } private void extractArchive(FileResolver fileResolver) throws IOException { - extractArchive(fileResolver, ZipEntry::getName); + extractArchive(fileResolver, JarEntry::getName); } private void extractArchive(FileResolver fileResolver, EntryNameTransformer entryNameTransformer) throws IOException { - withZipEntries(this.context.getArchiveFile(), (stream, zipEntry) -> { - if (zipEntry.isDirectory()) { + withJarEntries(this.context.getArchiveFile(), (stream, jarEntry) -> { + if (jarEntry.isDirectory()) { return; } - String name = entryNameTransformer.getName(zipEntry); + String name = entryNameTransformer.getName(jarEntry); if (name == null) { return; } - File file = fileResolver.resolve(zipEntry, name); + File file = fileResolver.resolve(jarEntry, name); if (file != null) { - extractEntry(stream, zipEntry, file); + extractEntry(stream, jarEntry, file); } }); } @@ -249,11 +252,11 @@ class ExtractCommand extends Command { Manifest manifest = jarStructure.createLauncherManifest((library) -> librariesDirectory + library); mkDirs(file.getParentFile()); try (JarOutputStream output = new JarOutputStream(new FileOutputStream(file), manifest)) { - withZipEntries(this.context.getArchiveFile(), ((stream, zipEntry) -> { - Entry entry = jarStructure.resolve(zipEntry); + withJarEntries(this.context.getArchiveFile(), ((stream, jarEntry) -> { + Entry entry = jarStructure.resolve(jarEntry); if (isType(entry, Type.APPLICATION_CLASS_OR_RESOURCE) && StringUtils.hasLength(entry.location())) { - JarEntry jarEntry = createJarEntry(entry.location(), zipEntry); - output.putNextEntry(jarEntry); + JarEntry newJarEntry = createJarEntry(entry.location(), jarEntry); + output.putNextEntry(newJarEntry); StreamUtils.copy(stream, output); output.closeEntry(); } @@ -275,51 +278,74 @@ class ExtractCommand extends Command { return entry.type() == type; } - private static void extractEntry(ZipInputStream zip, ZipEntry entry, File file) throws IOException { + private static void extractEntry(InputStream stream, JarEntry entry, File file) throws IOException { mkDirs(file.getParentFile()); try (OutputStream out = new FileOutputStream(file)) { - StreamUtils.copy(zip, out); + StreamUtils.copy(stream, out); } try { Files.getFileAttributeView(file.toPath(), BasicFileAttributeView.class) - .setTimes(entry.getLastModifiedTime(), entry.getLastAccessTime(), entry.getCreationTime()); + .setTimes(getLastModifiedTime(entry), getLastAccessTime(entry), getCreationTime(entry)); } catch (IOException ex) { // File system does not support setting time attributes. Continue. } } + private static FileTime getCreationTime(JarEntry entry) { + if (entry.getCreationTime() != null) { + return entry.getCreationTime(); + } + return entry.getLastModifiedTime(); + } + + private static FileTime getLastAccessTime(JarEntry entry) { + if (entry.getLastAccessTime() != null) { + return entry.getLastAccessTime(); + } + return getLastModifiedTime(entry); + } + + private static FileTime getLastModifiedTime(JarEntry entry) { + if (entry.getLastModifiedTime() != null) { + return entry.getLastModifiedTime(); + } + return entry.getCreationTime(); + } + private static void mkDirs(File file) throws IOException { if (!file.exists() && !file.mkdirs()) { throw new IOException("Unable to create directory " + file); } } - private static JarEntry createJarEntry(String location, ZipEntry originalEntry) { + private static JarEntry createJarEntry(String location, JarEntry originalEntry) { JarEntry jarEntry = new JarEntry(location); - FileTime lastModifiedTime = originalEntry.getLastModifiedTime(); + FileTime lastModifiedTime = getLastModifiedTime(originalEntry); if (lastModifiedTime != null) { jarEntry.setLastModifiedTime(lastModifiedTime); } - FileTime lastAccessTime = originalEntry.getLastAccessTime(); + FileTime lastAccessTime = getLastAccessTime(originalEntry); if (lastAccessTime != null) { jarEntry.setLastAccessTime(lastAccessTime); } - FileTime creationTime = originalEntry.getCreationTime(); + FileTime creationTime = getCreationTime(originalEntry); if (creationTime != null) { jarEntry.setCreationTime(creationTime); } return jarEntry; } - private static void withZipEntries(File file, ThrowingConsumer callback) throws IOException { - try (ZipInputStream stream = new ZipInputStream(new FileInputStream(file))) { - ZipEntry entry = stream.getNextEntry(); - while (entry != null) { + private static void withJarEntries(File file, ThrowingConsumer callback) throws IOException { + try (JarFile jarFile = new JarFile(file)) { + Enumeration entries = jarFile.entries(); + while (entries.hasMoreElements()) { + JarEntry entry = entries.nextElement(); if (StringUtils.hasLength(entry.getName())) { - callback.accept(stream, entry); + try (InputStream stream = jarFile.getInputStream(entry)) { + callback.accept(stream, entry); + } } - entry = stream.getNextEntry(); } } } @@ -336,14 +362,14 @@ class ExtractCommand extends Command { @FunctionalInterface private interface EntryNameTransformer { - String getName(ZipEntry entry); + String getName(JarEntry entry); } @FunctionalInterface private interface ThrowingConsumer { - void accept(ZipInputStream stream, ZipEntry entry) throws IOException; + void accept(InputStream stream, JarEntry entry) throws IOException; } @@ -356,14 +382,14 @@ class ExtractCommand extends Command { void createDirectories() throws IOException; /** - * Resolves the given {@link ZipEntry} to a file. - * @param entry the zip entry + * Resolves the given {@link JarEntry} to a file. + * @param entry the jar entry * @param newName the new name of the file * @return file where the contents should be written or {@code null} if this entry * should be skipped * @throws IOException if something went wrong */ - default File resolve(ZipEntry entry, String newName) throws IOException { + default File resolve(JarEntry entry, String newName) throws IOException { return resolve(entry.getName(), newName); } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/test/java/org/springframework/boot/jarmode/tools/ExtractCommandTests.java b/spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/test/java/org/springframework/boot/jarmode/tools/ExtractCommandTests.java index c706764bb1..4a8d565610 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/test/java/org/springframework/boot/jarmode/tools/ExtractCommandTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/test/java/org/springframework/boot/jarmode/tools/ExtractCommandTests.java @@ -19,13 +19,11 @@ package org.springframework.boot.jarmode.tools; import java.io.File; import java.io.FileWriter; import java.io.IOException; -import java.lang.Runtime.Version; import java.nio.file.Files; import java.nio.file.attribute.BasicFileAttributeView; import java.nio.file.attribute.BasicFileAttributes; import java.time.Instant; import java.time.temporal.ChronoUnit; -import java.util.EnumSet; import java.util.List; import java.util.Map; import java.util.jar.Manifest; @@ -33,8 +31,6 @@ import java.util.jar.Manifest; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.condition.JRE; -import org.junit.jupiter.api.condition.OS; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; @@ -46,13 +42,11 @@ import static org.assertj.core.api.Assertions.assertThatIllegalStateException; */ class ExtractCommandTests extends AbstractTests { - private static final Instant NOW = Instant.now(); + private static final Instant CREATION_TIME = Instant.parse("2020-01-01T00:00:00Z"); - private static final Instant CREATION_TIME = NOW.minus(3, ChronoUnit.DAYS); + private static final Instant LAST_MODIFIED_TIME = Instant.parse("2021-01-01T00:00:00Z"); - private static final Instant LAST_MODIFIED_TIME = NOW.minus(2, ChronoUnit.DAYS); - - private static final Instant LAST_ACCESS_TIME = NOW.minus(1, ChronoUnit.DAYS); + private static final Instant LAST_ACCESS_TIME = Instant.parse("2022-01-01T00:00:00Z"); private File archive; @@ -85,35 +79,13 @@ class ExtractCommandTests extends AbstractTests { .getFileAttributeView(file.toPath(), BasicFileAttributeView.class) .readAttributes(); assertThat(basicAttributes.lastModifiedTime().toInstant().truncatedTo(ChronoUnit.SECONDS)) + .as("last modified time") .isEqualTo(LAST_MODIFIED_TIME.truncatedTo(ChronoUnit.SECONDS)); - Instant expectedCreationTime = expectedCreationTime(); - if (expectedCreationTime != null) { - assertThat(basicAttributes.creationTime().toInstant().truncatedTo(ChronoUnit.SECONDS)) - .isEqualTo(expectedCreationTime.truncatedTo(ChronoUnit.SECONDS)); - } - assertThat(basicAttributes.lastAccessTime().toInstant().truncatedTo(ChronoUnit.SECONDS)) - .isEqualTo(LAST_ACCESS_TIME.truncatedTo(ChronoUnit.SECONDS)); } catch (IOException ex) { throw new RuntimeException(ex); } - } - - private Instant expectedCreationTime() { - // macOS uses last modified time until Java 20 where it uses creation time. - // https://github.com/openjdk/jdk21u-dev/commit/6397d564a5dab07f81bf4c69b116ebfabb2446ba - if (OS.MAC.isCurrentOs()) { - return (EnumSet.range(JRE.JAVA_17, JRE.JAVA_19).contains(JRE.currentVersion())) ? LAST_MODIFIED_TIME - : CREATION_TIME; - } - if (OS.LINUX.isCurrentOs()) { - // Linux uses the modified time until Java 21.0.2 where a bug means that it - // uses the birth time which it has not set, preventing us from verifying it. - // https://github.com/openjdk/jdk21u-dev/commit/4cf572e3b99b675418e456e7815fb6fd79245e30 - return (Runtime.version().compareTo(Version.parse("21.0.2")) >= 0) ? null : LAST_MODIFIED_TIME; - } - return CREATION_TIME; - } + }; @Nested class Extract { diff --git a/spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/test/java/org/springframework/boot/jarmode/tools/ExtractLayersCommandTests.java b/spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/test/java/org/springframework/boot/jarmode/tools/ExtractLayersCommandTests.java index ca22d6cb1e..d3baba1630 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/test/java/org/springframework/boot/jarmode/tools/ExtractLayersCommandTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-jarmode-tools/src/test/java/org/springframework/boot/jarmode/tools/ExtractLayersCommandTests.java @@ -21,7 +21,6 @@ import java.io.FileOutputStream; import java.io.FileWriter; import java.io.IOException; import java.io.InputStreamReader; -import java.lang.Runtime.Version; import java.nio.file.Files; import java.nio.file.attribute.BasicFileAttributeView; import java.nio.file.attribute.BasicFileAttributes; @@ -30,7 +29,6 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.Arrays; import java.util.Collections; -import java.util.EnumSet; import java.util.Iterator; import java.util.concurrent.TimeUnit; import java.util.function.Consumer; @@ -39,8 +37,6 @@ import java.util.zip.ZipOutputStream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.condition.JRE; -import org.junit.jupiter.api.condition.OS; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.TempDir; import org.mockito.Mock; @@ -112,35 +108,12 @@ class ExtractLayersCommandTests { .readAttributes(); assertThat(basicAttributes.lastModifiedTime().to(TimeUnit.SECONDS)) .isEqualTo(LAST_MODIFIED_TIME.to(TimeUnit.SECONDS)); - FileTime expectedCreationTime = expectedCreationTime(); - if (expectedCreationTime != null) { - assertThat(basicAttributes.creationTime().to(TimeUnit.SECONDS)) - .isEqualTo(expectedCreationTime.to(TimeUnit.SECONDS)); - } - assertThat(basicAttributes.lastAccessTime().to(TimeUnit.SECONDS)) - .isEqualTo(LAST_ACCESS_TIME.to(TimeUnit.SECONDS)); } catch (IOException ex) { throw new RuntimeException(ex); } } - private FileTime expectedCreationTime() { - // macOS uses last modified time until Java 20 where it uses creation time. - // https://github.com/openjdk/jdk21u-dev/commit/6397d564a5dab07f81bf4c69b116ebfabb2446ba - if (OS.MAC.isCurrentOs()) { - return (EnumSet.range(JRE.JAVA_17, JRE.JAVA_19).contains(JRE.currentVersion())) ? LAST_MODIFIED_TIME - : CREATION_TIME; - } - if (OS.LINUX.isCurrentOs()) { - // Linux uses the modified time until Java 21.0.2 where a bug means that it - // uses the birth time which it has not set, preventing us from verifying it. - // https://github.com/openjdk/jdk21u-dev/commit/4cf572e3b99b675418e456e7815fb6fd79245e30 - return (Runtime.version().compareTo(Version.parse("21.0.2")) >= 0) ? null : LAST_MODIFIED_TIME; - } - return CREATION_TIME; - } - @Test void runWhenHasDestinationOptionExtractsLayers() { given(this.context.getArchiveFile()).willReturn(this.jarFile);