You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2024/01/17 13:32:53 UTC
(commons-compress) 03/03: Add and use ArchiveEntry.resolveIn(Path)
This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-compress.git
commit 3d80677a86615888f3f0808e7c46f3f66bf5869e
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Wed Jan 17 08:32:44 2024 -0500
Add and use ArchiveEntry.resolveIn(Path)
---
src/changes/changes.xml | 7 ++++---
.../commons/compress/archivers/ArchiveEntry.java | 20 ++++++++++++++++++++
.../compress/archivers/ArchiveException.java | 10 +++++-----
.../compress/archivers/examples/Expander.java | 9 ++-------
.../org/apache/commons/compress/AbstractTest.java | 21 +++++++++------------
.../commons/compress/archivers/zip/Lister.java | 17 ++++++++++-------
6 files changed, 50 insertions(+), 34 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index c458e6bd2..c0819d2e4 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -75,9 +75,10 @@ The <action> type attribute can be add,update,fix,remove.
<action type="fix" dev="ggregory" due-to="Gary Gregory">Deprecate ByteUtils.fromLittleEndian(InputStream, int).</action>
<action type="fix" dev="ggregory" due-to="Gary Gregory">Deprecate ByteUtils.toLittleEndian(DataOutput, long, int).</action>
<!-- ADD -->
- <action type="add" dev="ggregory" due-to="Gary Gregory">Add ZipFile.builder(), add ZipFile.Builder, deprecate constructors.</action>
- <action type="add" dev="ggregory" due-to="Gary Gregory">Add SevenZFile.builder(), add SevenZFile.Builder, deprecate constructors.</action>
- <action type="add" dev="ggregory" due-to="Gary Gregory">Add ArchiveInputStream.getCharset().</action>
+ <action type="add" dev="ggregory" due-to="Gary Gregory">Add and use ZipFile.builder(), ZipFile.Builder, and deprecate constructors.</action>
+ <action type="add" dev="ggregory" due-to="Gary Gregory">Add and use SevenZFile.builder(), SevenZFile.Builder, and deprecate constructors.</action>
+ <action type="add" dev="ggregory" due-to="Gary Gregory">Add and use ArchiveInputStream.getCharset().</action>
+ <action type="add" dev="ggregory" due-to="Gary Gregory">Add and use ArchiveEntry.resolveIn(Path).</action>
<!-- UPDATE -->
<action type="update" dev="ggregory" due-to="Gary Gregory">Bump commons-lang3 from 3.13.0 to 3.14.0.</action>
<action type="update" dev="ggregory" due-to="Dependabot">Bump com.github.marschall:memoryfilesystem from 2.6.1 to 2.8.0 #444, #458.</action>
diff --git a/src/main/java/org/apache/commons/compress/archivers/ArchiveEntry.java b/src/main/java/org/apache/commons/compress/archivers/ArchiveEntry.java
index 2015f76e1..d329868c2 100644
--- a/src/main/java/org/apache/commons/compress/archivers/ArchiveEntry.java
+++ b/src/main/java/org/apache/commons/compress/archivers/ArchiveEntry.java
@@ -18,6 +18,8 @@
*/
package org.apache.commons.compress.archivers;
+import java.io.IOException;
+import java.nio.file.Path;
import java.util.Date;
/**
@@ -62,4 +64,22 @@ public interface ArchiveEntry {
* @return true if this entry refers to a directory.
*/
boolean isDirectory();
+
+ /**
+ * Resolves this entry in the given parent Path.
+ *
+ * @param parentPath the {@link Path#resolve(Path)} receiver.
+ * @return a resolved and normalized Path.
+ * @throws IOException if this method detects a Zip slip.
+ * @since 1.26.0
+ */
+ default Path resolveIn(final Path parentPath) throws IOException {
+ final String name = getName();
+ final Path outputFile = parentPath.resolve(name).normalize();
+ if (!outputFile.startsWith(parentPath)) {
+ throw new IOException(String.format("Zip slip '%s' + '%s' -> '%s'", parentPath, name, outputFile));
+ }
+ return outputFile;
+ }
+
}
diff --git a/src/main/java/org/apache/commons/compress/archivers/ArchiveException.java b/src/main/java/org/apache/commons/compress/archivers/ArchiveException.java
index 1c2248ccf..de22b5725 100644
--- a/src/main/java/org/apache/commons/compress/archivers/ArchiveException.java
+++ b/src/main/java/org/apache/commons/compress/archivers/ArchiveException.java
@@ -19,17 +19,17 @@
package org.apache.commons.compress.archivers;
/**
- * Archiver related Exception
+ * Archiver related Exception.
*/
public class ArchiveException extends Exception {
- /** Serial */
+ /** Serial. */
private static final long serialVersionUID = 2772690708123267100L;
/**
* Constructs a new exception with the specified detail message. The cause is not initialized.
*
- * @param message the detail message
+ * @param message the detail message.
*/
public ArchiveException(final String message) {
super(message);
@@ -38,8 +38,8 @@ public class ArchiveException extends Exception {
/**
* Constructs a new exception with the specified detail message and cause.
*
- * @param message the detail message
- * @param cause the cause
+ * @param message the detail message.
+ * @param cause the cause.
*/
public ArchiveException(final String message, final Exception cause) {
super(message, cause);
diff --git a/src/main/java/org/apache/commons/compress/archivers/examples/Expander.java b/src/main/java/org/apache/commons/compress/archivers/examples/Expander.java
index 29ad6ec83..bd91a8336 100644
--- a/src/main/java/org/apache/commons/compress/archivers/examples/Expander.java
+++ b/src/main/java/org/apache/commons/compress/archivers/examples/Expander.java
@@ -41,7 +41,7 @@ import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarFile;
import org.apache.commons.compress.archivers.zip.ZipArchiveEntry;
import org.apache.commons.compress.archivers.zip.ZipFile;
-import org.apache.commons.compress.utils.IOUtils;
+import org.apache.commons.io.IOUtils;
import org.apache.commons.io.output.NullOutputStream;
/**
@@ -70,12 +70,7 @@ public class Expander {
final Path targetDirPath = nullTarget ? null : targetDirectory.normalize();
T nextEntry = supplier.get();
while (nextEntry != null) {
- final Path targetPath = nullTarget ? null : targetDirectory.resolve(nextEntry.getName());
- // check if targetDirectory and f are the same path - this may
- // happen if the nextEntry.getName() is "./"
- if (!nullTarget && !targetPath.normalize().startsWith(targetDirPath) && !Files.isSameFile(targetDirectory, targetPath)) {
- throw new IOException("Expanding " + nextEntry.getName() + " would create file outside of " + targetDirectory);
- }
+ final Path targetPath = nullTarget ? null : nextEntry.resolveIn(targetDirPath);
if (nextEntry.isDirectory()) {
if (!nullTarget && !Files.isDirectory(targetPath) && Files.createDirectories(targetPath) == null) {
throw new IOException("Failed to create directory " + targetPath);
diff --git a/src/test/java/org/apache/commons/compress/AbstractTest.java b/src/test/java/org/apache/commons/compress/AbstractTest.java
index a4070ebe9..1772b81ca 100644
--- a/src/test/java/org/apache/commons/compress/AbstractTest.java
+++ b/src/test/java/org/apache/commons/compress/AbstractTest.java
@@ -32,7 +32,6 @@ import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.Files;
import java.nio.file.Path;
-import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
@@ -152,27 +151,25 @@ public abstract class AbstractTest extends AbstractTempDirTest {
* @throws Exception
*/
protected File checkArchiveContent(final ArchiveInputStream<?> inputStream, final List<String> expected, final boolean cleanUp) throws Exception {
- final Path result = createTempDirectory("dir-result");
+ final Path targetDir = createTempDirectory("dir-result");
+ final Path result = targetDir.resolve("result");
try {
ArchiveEntry entry;
while ((entry = inputStream.getNextEntry()) != null) {
- final Path output = Paths.get(result.toString(), "result", entry.getName()).normalize();
- if (!output.startsWith(result)) {
- throw new IOException("Zip slip " + output);
- }
+ final Path outputFile = entry.resolveIn(result);
long bytesCopied = 0;
if (entry.isDirectory()) {
- Files.createDirectories(output);
+ Files.createDirectories(outputFile);
} else {
- Files.createDirectories(output.getParent());
- bytesCopied = Files.copy(inputStream, output);
+ Files.createDirectories(outputFile.getParent());
+ bytesCopied = Files.copy(inputStream, outputFile);
}
final long size = entry.getSize();
if (size != ArchiveEntry.SIZE_UNKNOWN) {
assertEquals(size, bytesCopied, "Entry.size should equal bytes read.");
}
- if (!Files.exists(output)) {
+ if (!Files.exists(outputFile)) {
fail("Extraction failed: " + entry.getName());
}
if (expected != null && !expected.remove(getExpectedString(entry))) {
@@ -188,10 +185,10 @@ public abstract class AbstractTest extends AbstractTempDirTest {
}
} finally {
if (cleanUp) {
- forceDelete(result);
+ forceDelete(targetDir);
}
}
- return result.toFile();
+ return targetDir.toFile();
}
/**
diff --git a/src/test/java/org/apache/commons/compress/archivers/zip/Lister.java b/src/test/java/org/apache/commons/compress/archivers/zip/Lister.java
index 7c4192435..ef4f9a11b 100644
--- a/src/test/java/org/apache/commons/compress/archivers/zip/Lister.java
+++ b/src/test/java/org/apache/commons/compress/archivers/zip/Lister.java
@@ -22,6 +22,8 @@ import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
import java.util.Enumeration;
import org.apache.commons.compress.archivers.ArchiveEntry;
@@ -42,15 +44,16 @@ public final class Lister {
boolean useStream = false;
String encoding;
boolean allowStoredEntriesWithDataDescriptor = false;
- String dir;
+ Path dir;
}
- private static void extract(final String dirName, final ZipArchiveEntry entry, final InputStream inputStream) throws IOException {
- final File file = new File(dirName, entry.getName());
- if (!file.getParentFile().exists()) {
- file.getParentFile().mkdirs();
+ private static void extract(final Path targetDir, final ZipArchiveEntry entry, final InputStream inputStream) throws IOException {
+ final Path outputFile = entry.resolveIn(targetDir);
+ final Path parent = outputFile.getParent();
+ if (parent != null && !Files.exists(parent)) {
+ Files.createDirectories(parent);
}
- Files.copy(inputStream, file.toPath());
+ Files.copy(inputStream, outputFile);
}
private static void list(final ZipArchiveEntry entry) {
@@ -104,7 +107,7 @@ public final class Lister {
}
} else if (args[i].equals("-extract")) {
if (argsLength > i + 1) {
- cl.dir = args[++i];
+ cl.dir = Paths.get(args[++i]);
} else {
System.err.println("missing argument to -extract");
error = true;