From ed7a5db17441bdb37df8e90dc0fa6c52948c008e Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Tue, 21 Apr 2020 18:11:03 -0700 Subject: [PATCH] Fail operations when JarFile is closed Update `JarFile` to track when the instance has been closed and throw an exception in the same way that `ZipFile` does. Closes gh-21072 --- .../boot/loader/jar/JarFile.java | 29 +++++++- .../boot/loader/jar/JarFileEntries.java | 22 ++++++- .../boot/loader/jar/JarFileTests.java | 66 ++++++++++++++++++- 3 files changed, 112 insertions(+), 5 deletions(-) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarFile.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarFile.java index c9af0bee786..8be0ceb8be7 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarFile.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/jar/JarFile.java @@ -26,9 +26,13 @@ import java.net.URLStreamHandler; import java.net.URLStreamHandlerFactory; import java.util.Enumeration; import java.util.Iterator; +import java.util.Spliterator; +import java.util.Spliterators; import java.util.function.Supplier; import java.util.jar.JarInputStream; import java.util.jar.Manifest; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; import java.util.zip.ZipEntry; import org.springframework.boot.loader.data.RandomAccessData; @@ -84,6 +88,8 @@ public class JarFile extends java.util.jar.JarFile implements Iterable stream() { + Spliterator spliterator = Spliterators.spliterator(iterator(), size(), + Spliterator.ORDERED | Spliterator.DISTINCT | Spliterator.IMMUTABLE | Spliterator.NONNULL); + return StreamSupport.stream(spliterator, false); + } + /** * Return an iterator for the contained entries. * @see java.lang.Iterable#iterator() @@ -230,7 +243,7 @@ public class JarFile extends java.util.jar.JarFile implements Iterable iterator() { - return (Iterator) this.entries.iterator(); + return (Iterator) this.entries.iterator(this::ensureOpen); } public JarEntry getJarEntry(CharSequence name) { @@ -248,11 +261,13 @@ public class JarFile extends java.util.jar.JarFile implements Iterable { + private static final Runnable NO_VALIDATION = () -> { + }; + private static final String META_INF_PREFIX = "META-INF/"; private static final Name MULTI_RELEASE = new Name("Multi-Release"); @@ -192,7 +195,11 @@ class JarFileEntries implements CentralDirectoryVisitor, Iterable { @Override public Iterator iterator() { - return new EntryIterator(); + return new EntryIterator(NO_VALIDATION); + } + + Iterator iterator(Runnable validator) { + return new EntryIterator(validator); } boolean containsEntry(CharSequence name) { @@ -347,17 +354,26 @@ class JarFileEntries implements CentralDirectoryVisitor, Iterable { /** * Iterator for contained entries. */ - private class EntryIterator implements Iterator { + private final class EntryIterator implements Iterator { + + private final Runnable validator; private int index = 0; + private EntryIterator(Runnable validator) { + this.validator = validator; + validator.run(); + } + @Override public boolean hasNext() { + this.validator.run(); return this.index < JarFileEntries.this.size; } @Override public JarEntry next() { + this.validator.run(); if (!hasNext()) { throw new NoSuchElementException(); } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarFileTests.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarFileTests.java index f4b43884c41..34b17eb22d3 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarFileTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/jar/JarFileTests.java @@ -30,17 +30,21 @@ import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.nio.file.attribute.FileTime; import java.time.Instant; +import java.util.ArrayList; import java.util.Collections; import java.util.Enumeration; +import java.util.Iterator; import java.util.List; import java.util.jar.JarEntry; import java.util.jar.JarInputStream; import java.util.jar.JarOutputStream; import java.util.jar.Manifest; +import java.util.stream.Stream; import java.util.zip.CRC32; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; +import org.assertj.core.api.ThrowableAssert.ThrowingCallable; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -56,6 +60,7 @@ import org.springframework.util.StreamUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIOException; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; @@ -171,6 +176,12 @@ class JarFileTests { assertThat(entry.getName()).isEqualTo("1.dat"); } + @Test + void getJarEntryWhenClosed() throws Exception { + this.jarFile.close(); + assertThatZipFileClosedIsThrownBy(() -> this.jarFile.getJarEntry("1.dat")); + } + @Test void getInputStream() throws Exception { InputStream inputStream = this.jarFile.getInputStream(this.jarFile.getEntry("1.dat")); @@ -180,23 +191,41 @@ class JarFileTests { assertThat(inputStream.read()).isEqualTo(-1); } + @Test + void getInputStreamWhenClosed() throws Exception { + this.jarFile.close(); + assertThatZipFileClosedIsThrownBy(() -> this.jarFile.getInputStream(this.jarFile.getEntry("1.dat"))); + } + @Test void getComment() { assertThat(this.jarFile.getComment()).isEqualTo("outer"); } + @Test + void getCommentWhenClosed() throws Exception { + this.jarFile.close(); + assertThatZipFileClosedIsThrownBy(() -> this.jarFile.getComment()); + } + @Test void getName() { assertThat(this.jarFile.getName()).isEqualTo(this.rootJarFile.getPath()); } @Test - void getSize() throws Exception { + void size() throws Exception { try (ZipFile zip = new ZipFile(this.rootJarFile)) { assertThat(this.jarFile.size()).isEqualTo(zip.size()); } } + @Test + void sizeWhenClosed() throws Exception { + this.jarFile.close(); + assertThatZipFileClosedIsThrownBy(() -> this.jarFile.size()); + } + @Test void getEntryTime() throws Exception { java.util.jar.JarFile jdkJarFile = new java.util.jar.JarFile(this.rootJarFile); @@ -603,6 +632,41 @@ class JarFileTests { } } + @Test + void iterator() { + Iterator iterator = this.jarFile.iterator(); + List names = new ArrayList(); + while (iterator.hasNext()) { + names.add(iterator.next().getName()); + } + assertThat(names).hasSize(12).contains("1.dat"); + } + + @Test + void iteratorWhenClosed() throws IOException { + this.jarFile.close(); + assertThatZipFileClosedIsThrownBy(() -> this.jarFile.iterator()); + } + + @Test + void iteratorWhenClosedLater() throws IOException { + Iterator iterator = this.jarFile.iterator(); + iterator.next(); + this.jarFile.close(); + assertThatZipFileClosedIsThrownBy(() -> iterator.hasNext()); + } + + @Test + void stream() { + Stream stream = this.jarFile.stream().map(JarEntry::getName); + assertThat(stream).hasSize(12).contains("1.dat"); + + } + + private void assertThatZipFileClosedIsThrownBy(ThrowingCallable throwingCallable) { + assertThatIllegalStateException().isThrownBy(throwingCallable).withMessage("zip file closed"); + } + private int getJavaVersion() { try { Object runtimeVersion = Runtime.class.getMethod("version").invoke(null);