Skip to content

Enhance ZipUtils #2148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jodastephen merged 1 commit into from
Apr 23, 2020
Merged

Enhance ZipUtils #2148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 47 additions & 17 deletions modules/collect/src/main/java/com/opengamma/strata/collect/io/ZipUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.io.UncheckedIOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import java.util.Base64;
import java.util.HashSet;
Expand All @@ -21,18 +22,21 @@
import java.util.function.BiConsumer;
import java.util.zip.GZIPInputStream;
import java.util.zip.ZipEntry;
import java.util.zip.ZipException;
import java.util.zip.ZipInputStream;
import java.util.zip.ZipOutputStream;

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.io.ByteStreams;
import com.opengamma.strata.collect.ArgChecker;

/**
* Utility class to simplify accessing and creating zip files, and other packed formats.
*/
public final class ZipUtils {
// need to watch out for ZIP slip attack when unzipping to file system
// https://github.com/snyk/zip-slip-vulnerability
private static final Path DUMMY_PATH = Paths.get("/dummy/");

private ZipUtils() {
}
Expand All @@ -53,6 +57,7 @@ public static Set<String> unzipPathNames(BeanByteSource source) {
try (ZipInputStream in = new ZipInputStream(source.openStream())) {
ZipEntry entry = in.getNextEntry();
while (entry != null) {
validateZipPathName(DUMMY_PATH, entry);
if (!entry.isDirectory()) {
entryNames.add(entry.getName());
}
Expand Down Expand Up @@ -82,8 +87,10 @@ public static Optional<ArrayByteSource> unzipPathNameInMemory(BeanByteSource sou
try (ZipInputStream in = new ZipInputStream(source.openStream())) {
ZipEntry entry = in.getNextEntry();
while (entry != null) {
validateZipPathName(DUMMY_PATH, entry);
if (!entry.isDirectory() && entry.getName().equals(relativePathName)) {
return Optional.of(ArrayByteSource.ofUnsafe(ByteStreams.toByteArray(in)).withFileName(entry.getName()));
ArrayByteSource extractedBytes = extractInputStream(in, entry.getName());
return Optional.of(extractedBytes);
}
in.closeEntry();
entry = in.getNextEntry();
Expand All @@ -103,15 +110,17 @@ public static Optional<ArrayByteSource> unzipPathNameInMemory(BeanByteSource sou
* @param source the byte source to unzip
* @param path the path to unzip to
* @throws UncheckedIOException if an IO error occurs
* @throws SecurityException if the path is not absolute and the calling thread cannot access system property "user.dir"
*/
public static void unzip(BeanByteSource source, Path path) {
Path absolutePath = path.toAbsolutePath();
Set<ZipKey> deduplicate = new HashSet<>();
try (ZipInputStream in = new ZipInputStream(source.openStream())) {
ZipEntry entry = in.getNextEntry();
while (entry != null) {
Path resolved = validateZipPathName(absolutePath, entry);
if (!entry.isDirectory()) {
if (deduplicate.add(new ZipKey(entry))) {
Path resolved = path.resolve(entry.getName());
if (deduplicate.add(new ZipKey(entry, resolved))) {
Files.createDirectories(resolved);
Files.copy(in, resolved, StandardCopyOption.REPLACE_EXISTING);
}
Expand Down Expand Up @@ -182,6 +191,7 @@ public static Map<String, ArrayByteSource> unpackInMemory(BeanByteSource source)
* <p>
* Unpacking handles ZIP, GZ and BASE64 formats based entirely on the suffix of the input file name.
* If the input suffix is not recognized as a packed format, the consumer is invoked with the original file.
* This method is not recursive.
*
* @param source the byte source to unpack
* @param consumer the consumer, which is passed the relative path name and content for each entry
Expand All @@ -198,8 +208,8 @@ public static void unpackInMemory(BeanByteSource source, BiConsumer<String, Arra
} else if (suffixMatches(fileName, ".base64")) {
try (InputStream in = Base64.getDecoder().wrap(source.openBufferedStream())) {
String shortFileName = fileName.substring(0, fileName.length() - 7);
ArrayByteSource unbase64 = ArrayByteSource.from(in).withFileName(shortFileName);
consumer.accept(shortFileName, unbase64);
ArrayByteSource extracted = extractInputStream(in, shortFileName);
consumer.accept(shortFileName, extracted);
} catch (IOException ex) {
throw new UncheckedIOException(ex);
}
Expand Down Expand Up @@ -244,10 +254,11 @@ public static void unzipInMemory(BeanByteSource source, BiConsumer<String, Array
try (ZipInputStream in = new ZipInputStream(source.openStream())) {
ZipEntry entry = in.getNextEntry();
while (entry != null) {
Path resolved = validateZipPathName(DUMMY_PATH, entry);
if (!entry.isDirectory()) {
if (deduplicate.add(new ZipKey(entry))) {
ArrayByteSource entrySource = ArrayByteSource.ofUnsafe(ByteStreams.toByteArray(in)).withFileName(entry.getName());
consumer.accept(entry.getName(), entrySource);
if (deduplicate.add(new ZipKey(entry, resolved))) {
ArrayByteSource extractedBytes = extractInputStream(in, entry.getName());
consumer.accept(entry.getName(), extractedBytes);
}
}
in.closeEntry();
Expand All @@ -263,8 +274,8 @@ public static void unzipInMemory(BeanByteSource source, BiConsumer<String, Array
private static void ungzInMemory(BeanByteSource source, String fileName, BiConsumer<String, ArrayByteSource> consumer) {
try (GZIPInputStream in = new GZIPInputStream(source.openStream())) {
String shortFileName = fileName.substring(0, fileName.length() - 3);
ArrayByteSource entrySource = ArrayByteSource.ofUnsafe(ByteStreams.toByteArray(in)).withFileName(shortFileName);
consumer.accept(shortFileName, entrySource);
ArrayByteSource extractedBytes = extractInputStream(in, shortFileName);
consumer.accept(shortFileName, extractedBytes);
} catch (IOException ex) {
throw new UncheckedIOException(ex);
}
Expand All @@ -275,25 +286,44 @@ private static boolean suffixMatches(String name, String suffix) {
return name.regionMatches(true, name.length() - suffix.length(), suffix, 0, suffix.length());
}

// unzip the input, trying to recover from large files or ZIP bombs
private static ArrayByteSource extractInputStream(InputStream in, String fileName) throws IOException {
try {
return ArrayByteSource.from(in).withFileName(fileName);
} catch (OutOfMemoryError ex) {
System.gc();
throw new IOException("Unzipped input too large: " + fileName);
}
}

// prevent ZIP slip attack
private static Path validateZipPathName(Path rootPath, ZipEntry entry) throws ZipException {
Path resolved = rootPath.normalize().resolve(entry.getName()).normalize();
if (!resolved.startsWith(rootPath)) {
throw new ZipException("ZIP file contains illegal file name: " + entry.getName());
}
return resolved;
}

//-------------------------------------------------------------------------
// handle duplicate entries in a zip file
// while such a zip file is stupid, it is apparently valid
private static class ZipKey {
private final String fileName;
private final Path resolvedPath;
private final long crc;
private final long size;

private ZipKey(ZipEntry entry) {
fileName = entry.getName();
crc = entry.getCrc();
size = entry.getSize();
private ZipKey(ZipEntry entry, Path resolvedPath) {
this.resolvedPath = resolvedPath;
this.crc = entry.getCrc();
this.size = entry.getSize();
}

@Override
public boolean equals(Object obj) {
if (obj instanceof ZipKey) {
ZipKey key = (ZipKey) obj;
return fileName.equals(key.fileName) &&
return resolvedPath.equals(key.resolvedPath) &&
crc == key.crc &&
size == key.size;
}
Expand Down
42 changes: 41 additions & 1 deletion modules/collect/src/test/java/com/opengamma/strata/collect/io/ZipUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@
package com.opengamma.strata.collect.io;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.fail;
import static org.assertj.core.api.Assertions.within;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
Expand Down Expand Up @@ -62,6 +64,14 @@ public void test_unzipPathNames() {
assertThat(names).containsOnly("test/alpha/Alpha.txt", "test/beta/Beta1.txt", "test/beta/Beta2.txt");
}

@Test
public void test_unzipPathNames_zipSlip() {
ArrayByteSource zipFile = ResourceLocator.ofClasspath(ZipUtilsTest.class, "zip-slip.zip").getByteSource().load();

assertThatExceptionOfType(UncheckedIOException.class)
.isThrownBy(() -> ZipUtils.unzipPathNames(zipFile));
}

//-------------------------------------------------------------------------
@Test
public void test_unzipPathNameInMemory() {
Expand All @@ -78,6 +88,14 @@ public void test_unzipPathNameInMemory() {
.hasValue("BETA2");
}

@Test
public void test_unzipPathNameInMemory_zipSlip() {
ArrayByteSource zipFile = ResourceLocator.ofClasspath(ZipUtilsTest.class, "zip-slip.zip").getByteSource().load();

assertThatExceptionOfType(UncheckedIOException.class)
.isThrownBy(() -> ZipUtils.unzipPathNameInMemory(zipFile, "test/alpha/Alpha.txt"));
}

//-------------------------------------------------------------------------
@Test
public void test_unzip_toPath() {
Expand All @@ -102,6 +120,14 @@ public void test_unzip_toPath_withFolders() {
assertThat(tmpDir.resolve("test/beta/Beta2.txt")).hasContent("BETA2");
}

@Test
public void test_unzip_toPath_zipSlip() {
ArrayByteSource zipFile = ResourceLocator.ofClasspath(ZipUtilsTest.class, "zip-slip.zip").getByteSource().load();

assertThatExceptionOfType(UncheckedIOException.class)
.isThrownBy(() -> ZipUtils.unzip(zipFile, tmpDir));
}

//-------------------------------------------------------------------------
@Test
public void test_zipInMemory() throws Exception {
Expand Down Expand Up @@ -143,7 +169,7 @@ public void test_unzipInMemory_toMap_zip() {

//-------------------------------------------------------------------------
@Test
public void test_unzipInMemory_zip() {
public void test_unzipInMemory() {
ArrayByteSource source1 = ArrayByteSource.ofUtf8("Hello World").withFileName("TestFile1.txt");
ArrayByteSource source2 = ArrayByteSource.ofUtf8("Hello Planet").withFileName("TestFile2.txt");
ArrayByteSource zipFile = ZipUtils.zipInMemory(ImmutableList.of(source1, source2)).withFileName("Test.foo");
Expand All @@ -164,6 +190,20 @@ public void test_unzipInMemory_zip() {
assertThat(counter).hasValue(2);
}

@Test
public void test_unzipInMemory_zipSlip() {
ArrayByteSource zipFile = ResourceLocator.ofClasspath(ZipUtilsTest.class, "zip-slip.zip").getByteSource().load();

assertThatExceptionOfType(UncheckedIOException.class)
.isThrownBy(() -> ZipUtils.unzipInMemory(
zipFile,
(name, extracted) -> {
if (!name.equals("good.txt")) {
fail("Should not get here: " + name);
}
}));
}

//-------------------------------------------------------------------------
@Test
public void test_unpackInMemory_toMap_zip() {
Expand Down
Binary file added modules/collect/src/test/resources/com/opengamma/strata/collect/io/zip-slip.zip
View file
Open in desktop
Binary file not shown.