Skip to content

Commit

Permalink
Fail operations when JarFile is closed
Browse files Browse the repository at this point in the history
Update `JarFile` to track when the instance has been closed and throw
an exception in the same way that `ZipFile` does.

Closes spring-projectsgh-21072
  • Loading branch information
philwebb committed Apr 22, 2020
1 parent cc33e23 commit ed7a5db
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,13 @@
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;
Expand Down Expand Up @@ -84,6 +88,8 @@ public class JarFile extends java.util.jar.JarFile implements Iterable<java.util

private String comment;

private volatile boolean closed;

/**
* Create a new {@link JarFile} backed by the specified file.
* @param file the root jar file
Expand Down Expand Up @@ -222,6 +228,13 @@ public Enumeration<java.util.jar.JarEntry> entries() {
return new JarEntryEnumeration(this.entries.iterator());
}

@Override
public Stream<java.util.jar.JarEntry> stream() {
Spliterator<java.util.jar.JarEntry> 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()
Expand All @@ -230,7 +243,7 @@ public Enumeration<java.util.jar.JarEntry> entries() {
@Override
@SuppressWarnings({ "unchecked", "rawtypes" })
public Iterator<java.util.jar.JarEntry> iterator() {
return (Iterator) this.entries.iterator();
return (Iterator) this.entries.iterator(this::ensureOpen);
}

public JarEntry getJarEntry(CharSequence name) {
Expand All @@ -248,11 +261,13 @@ public boolean containsEntry(String name) {

@Override
public ZipEntry getEntry(String name) {
ensureOpen();
return this.entries.getEntry(name);
}

@Override
public synchronized InputStream getInputStream(ZipEntry entry) throws IOException {
ensureOpen();
if (entry instanceof JarEntry) {
return this.entries.getInputStream((JarEntry) entry);
}
Expand Down Expand Up @@ -322,22 +337,34 @@ private JarFile createJarFileFromFileEntry(JarEntry entry) throws IOException {

@Override
public String getComment() {
ensureOpen();
return this.comment;
}

@Override
public int size() {
ensureOpen();
return this.entries.getSize();
}

@Override
public void close() throws IOException {
if (this.closed) {
return;
}
this.closed = true;
super.close();
if (this.type == JarFileType.DIRECT && this.parent == null) {
this.rootFile.close();
}
}

private void ensureOpen() {
if (this.closed) {
throw new IllegalStateException("zip file closed");
}
}

String getUrlString() throws MalformedURLException {
if (this.urlString == null) {
this.urlString = getUrl().toString();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2012-2019 the original author or authors.
* Copyright 2012-2020 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -47,6 +47,9 @@
*/
class JarFileEntries implements CentralDirectoryVisitor, Iterable<JarEntry> {

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");
Expand Down Expand Up @@ -192,7 +195,11 @@ private void swap(int[] array, int i, int j) {

@Override
public Iterator<JarEntry> iterator() {
return new EntryIterator();
return new EntryIterator(NO_VALIDATION);
}

Iterator<JarEntry> iterator(Runnable validator) {
return new EntryIterator(validator);
}

boolean containsEntry(CharSequence name) {
Expand Down Expand Up @@ -347,17 +354,26 @@ private AsciiBytes applyFilter(AsciiBytes name) {
/**
* Iterator for contained entries.
*/
private class EntryIterator implements Iterator<JarEntry> {
private final class EntryIterator implements Iterator<JarEntry> {

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();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,21 @@
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;
Expand All @@ -56,6 +60,7 @@
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;

Expand Down Expand Up @@ -171,6 +176,12 @@ void getJarEntry() {
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"));
Expand All @@ -180,23 +191,41 @@ void getInputStream() throws Exception {
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);
Expand Down Expand Up @@ -603,6 +632,41 @@ void jarFileEntryWithEpochTimeOfZeroShouldNotFail() throws Exception {
}
}

@Test
void iterator() {
Iterator<JarEntry> iterator = this.jarFile.iterator();
List<String> names = new ArrayList<String>();
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<JarEntry> iterator = this.jarFile.iterator();
iterator.next();
this.jarFile.close();
assertThatZipFileClosedIsThrownBy(() -> iterator.hasNext());
}

@Test
void stream() {
Stream<String> 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);
Expand Down

0 comments on commit ed7a5db

Please sign in to comment.