From 9c031c5ac1948968075ddd37b657d6f8268bedae Mon Sep 17 00:00:00 2001 From: Glavo Date: Tue, 8 Sep 2026 16:37:31 +0800 Subject: [PATCH] =?UTF-8?q?=E4=BC=98=E5=8C=96=20ArchiveFileTree=20?= =?UTF-8?q?=E6=9E=84=E5=BB=BA=E6=80=A7=E8=83=BD=20(#6838)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../hmcl/util/tree/ArchiveFileTree.java | 104 ++++++++------ .../jackhuang/hmcl/util/tree/TarFileTree.java | 37 ++--- .../jackhuang/hmcl/util/tree/ZipFileTree.java | 32 +++-- .../hmcl/util/tree/ArchiveFileTreeTest.java | 132 ++++++++++++++++++ 4 files changed, 227 insertions(+), 78 deletions(-) create mode 100644 HMCLCore/src/test/java/org/jackhuang/hmcl/util/tree/ArchiveFileTreeTest.java diff --git a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/tree/ArchiveFileTree.java b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/tree/ArchiveFileTree.java index 23cffa83f4..f1cfc2d192 100644 --- a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/tree/ArchiveFileTree.java +++ b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/tree/ArchiveFileTree.java @@ -53,7 +53,6 @@ public abstract class ArchiveFileTree implements Clos } protected final R reader; - protected final Dir root = new Dir<>("", ""); public ArchiveFileTree(R reader) { this.reader = reader; @@ -63,11 +62,10 @@ public abstract class ArchiveFileTree implements Clos return reader; } - public Dir getRoot() { - return root; - } + public abstract Dir getRoot(); public @Nullable E getEntry(@NotNull String entryPath) { + Dir root = getRoot(); Dir dir = root; if (entryPath.indexOf('/') < 0) { return dir.getFiles().get(entryPath); @@ -91,7 +89,7 @@ public abstract class ArchiveFileTree implements Clos } public @Nullable Dir getDirectory(@NotNull String dirPath) { - Dir dir = root; + Dir dir = getRoot(); if (dirPath.isEmpty()) { return dir; } @@ -106,45 +104,66 @@ public abstract class ArchiveFileTree implements Clos return dir; } - protected void addEntry(E entry) throws IOException { - String[] path = entry.getName().split("/"); - List pathList = Arrays.asList(path); - + /// Adds an entry, creating missing parent directories. + /// Empty and `.` directory components are skipped. Entries with `..` directory + /// components, duplicate files, and file/directory conflicts are skipped with a warning. + /// Parent directories created before a skipped entry is detected remain in the tree. + /// + /// @param entry the archive entry to add + protected static void addEntry(Dir root, @NotNull E entry) { + String name = entry.getName(); Dir dir = root; - for (int i = 0, end = entry.isDirectory() ? path.length : path.length - 1; i < end; i++) { - String item = path[i]; - if (item.equals(".")) + int start = 0; + while (start < name.length()) { + int end = name.indexOf('/', start); + boolean isLastPart = end < 0 || end == name.length() - 1; + String item = end >= 0 ? name.substring(start, end) : name.substring(start); + + // A final component may be skipped, so the cursor must still reach the end. + start = end < 0 ? name.length() : end + 1; + + if (isLastPart && !entry.isDirectory()) { + if (dir.getSubDirs().containsKey(item)) { + LOG.warning("A file and a directory have the same name: " + name); + return; + } + + if (dir.getFiles().containsKey(item)) { + LOG.warning("Duplicate entry: " + entry.getName()); + return; + } + + if (dir.files.isEmpty()) + dir.files = new HashMap<>(); + dir.files.put(item, entry); + break; + } + + if (item.equals(".") || item.isEmpty()) continue; - if (item.equals("..") || item.isEmpty()) - throw new IOException("Invalid entry: " + entry.getName()); - - if (dir.files.containsKey(item)) { - throw new IOException("A file and a directory have the same name: " + entry.getName()); + if (item.equals("..")) { + LOG.warning("Invalid entry name: " + name); + return; } - final int nameEnd = i + 1; - dir = dir.subDirs.computeIfAbsent(item, name -> - new Dir<>(name, String.join("/", pathList.subList(0, nameEnd)))); - } - - if (entry.isDirectory()) { - if (dir.entry == null) - dir.entry = entry; - else if (!dir.entry.isDirectory()) - throw new IOException("A file and a directory have the same name: " + entry.getName()); - } else { - String fileName = path[path.length - 1]; - - if (dir.subDirs.containsKey(fileName)) { - throw new IOException("A file and a directory have the same name: " + entry.getName()); + if (dir.getFiles().containsKey(item)) { + LOG.warning("A file and a directory have the same name: " + name); + return; } - if (dir.files.containsKey(fileName)) { - throw new IOException("Duplicate entry: " + entry.getName()); - } + if (dir.subDirs.isEmpty()) + dir.subDirs = new HashMap<>(); - dir.files.put(fileName, entry); + dir = dir.subDirs.computeIfAbsent(item, Dir::new); + + if (isLastPart) { + if (dir.entry == null) + dir.entry = entry; + else if (!dir.entry.isDirectory()) + LOG.warning("A file and a directory have the same name: " + entry.getName()); + break; + } } } @@ -221,15 +240,13 @@ public abstract class ArchiveFileTree implements Clos public static final class Dir { private final String name; - private final String fullName; private E entry; - final Map> subDirs = new HashMap<>(); - final Map files = new HashMap<>(); + Map> subDirs = Map.of(); + Map files = Map.of(); - public Dir(String name, String fullName) { + public Dir(String name) { this.name = name; - this.fullName = fullName; } public boolean isRoot() { @@ -240,11 +257,6 @@ public abstract class ArchiveFileTree implements Clos return name; } - /// Get the normalized full path. Leading `/` and all `.` in the path will be removed. - public @NotNull String getFullName() { - return fullName; - } - public @Nullable E getEntry() { return entry; } diff --git a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/tree/TarFileTree.java b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/tree/TarFileTree.java index cab531e79e..3c53e07a99 100644 --- a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/tree/TarFileTree.java +++ b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/tree/TarFileTree.java @@ -20,6 +20,7 @@ package org.jackhuang.hmcl.util.tree; import kala.compress.archivers.tar.TarArchiveEntry; import kala.compress.archivers.tar.TarArchiveReader; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.io.*; import java.nio.file.Files; @@ -61,31 +62,11 @@ public final class TarFileTree extends ArchiveFileTree root; - public TarFileTree(TarArchiveReader file, Path tempFile) throws IOException { + public TarFileTree(TarArchiveReader file, Path tempFile) { super(file); this.tempFile = tempFile; - try { - for (TarArchiveEntry entry : file.getEntries()) { - addEntry(entry); - } - } catch (Throwable e) { - try { - file.close(); - } catch (Throwable e2) { - e.addSuppressed(e2); - } - - if (tempFile != null) { - try { - Files.deleteIfExists(tempFile); - } catch (Throwable e2) { - e.addSuppressed(e2); - } - } - - throw e; - } if (tempFile != null) { this.shutdownHook = new Thread(() -> { @@ -99,6 +80,18 @@ public final class TarFileTree extends ArchiveFileTree getRoot() { + if (root == null) { + root = new Dir<>(""); + for (TarArchiveEntry entry : reader.getEntries()) { + addEntry(root, entry); + } + } + + return root; + } + @Override protected void copyAttributes(@NotNull TarArchiveEntry source, @NotNull Path targetFile) throws IOException { var fileAttributeView = Files.getFileAttributeView(targetFile, BasicFileAttributeView.class); diff --git a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/tree/ZipFileTree.java b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/tree/ZipFileTree.java index a0fd4494bc..e3cb4e257e 100644 --- a/HMCLCore/src/main/java/org/jackhuang/hmcl/util/tree/ZipFileTree.java +++ b/HMCLCore/src/main/java/org/jackhuang/hmcl/util/tree/ZipFileTree.java @@ -19,8 +19,8 @@ package org.jackhuang.hmcl.util.tree; import kala.compress.archivers.zip.ZipArchiveEntry; import kala.compress.archivers.zip.ZipArchiveReader; -import org.jackhuang.hmcl.util.io.IOUtils; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import java.io.IOException; import java.io.InputStream; @@ -37,23 +37,26 @@ import java.util.Set; */ public final class ZipFileTree extends ArchiveFileTree { private final boolean closeReader; + private @Nullable Dir root; - public ZipFileTree(ZipArchiveReader file) throws IOException { + public ZipFileTree(ZipArchiveReader file) { this(file, true); } - public ZipFileTree(ZipArchiveReader file, boolean closeReader) throws IOException { + public ZipFileTree(ZipArchiveReader file, boolean closeReader) { super(file); this.closeReader = closeReader; - try { - for (ZipArchiveEntry zipArchiveEntry : file.getEntries()) { - addEntry(zipArchiveEntry); + } + + @Override + public Dir getRoot() { + if (root == null) { + root = new Dir<>(""); + for (ZipArchiveEntry zipArchiveEntry : reader.getEntries()) { + addEntry(root, zipArchiveEntry); } - } catch (Throwable e) { - if (closeReader) - IOUtils.closeQuietly(file, e); - throw e; } + return root; } @Override @@ -103,6 +106,15 @@ public final class ZipFileTree extends ArchiveFileTree and contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.jackhuang.hmcl.util.tree; + +import kala.compress.archivers.tar.TarArchiveEntry; +import kala.compress.archivers.tar.TarArchiveOutputStream; +import kala.compress.archivers.tar.TarArchiveReader; +import kala.compress.archivers.tar.TarConstants; +import org.jetbrains.annotations.NotNullByDefault; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; + +import static org.junit.jupiter.api.Assertions.*; + +/// Verifies archive path traversal and handling of skipped entries. +@NotNullByDefault +public final class ArchiveFileTreeTest { + + /// Creates an empty TAR tree whose entries can be added directly by each test. + private static TarFileTree openTree(Path directory) throws IOException { + Path archive = directory.resolve("test.tar"); + try (var output = new TarArchiveOutputStream(Files.newOutputStream(archive))) { + output.finish(); + } + return new TarFileTree(new TarArchiveReader(archive), null); + } + + /// Completes traversal when the final directory component is a dot, with or without a slash. + @ParameterizedTest + @ValueSource(strings = {".", "./", "a/.", "a/./"}) + public void testDotDirectoryTerminates(String name, @TempDir Path directory) throws IOException { + try (var tree = openTree(directory)) { + // A TAR directory flag does not require its name to end in a slash. + var entry = new TarArchiveEntry(name, TarConstants.LF_DIR); + assertTimeoutPreemptively(Duration.ofSeconds(2), () -> ArchiveFileTree.addEntry(tree.getRoot(), entry)); + assertTrue(tree.getRoot().getFiles().isEmpty()); + if (name.startsWith("a/")) { + assertEquals(1, tree.getRoot().getSubDirs().size()); + assertNotNull(tree.getDirectory("a")); + } else { + assertTrue(tree.getRoot().getSubDirs().isEmpty()); + } + } + } + + /// Preserves lookup of files beneath dot and empty directory components. + @Test + public void testNormalizedFilePath(@TempDir Path directory) throws IOException { + try (var tree = openTree(directory)) { + var entry = new TarArchiveEntry("./a//./file.txt"); + ArchiveFileTree.addEntry(tree.getRoot(), entry); + assertSame(entry, tree.getEntry("a/file.txt")); + assertTrue(tree.getRoot().getFiles().isEmpty()); + } + } + + /// Skips the whole entry when a parent component is a file, retaining the existing file. + @ParameterizedTest + @ValueSource(strings = {"a/", "a/file.txt"}) + public void testFileConflictSkipsEntry(String name, @TempDir Path directory) throws IOException { + try (var tree = openTree(directory)) { + var existing = new TarArchiveEntry("a"); + ArchiveFileTree.addEntry(tree.getRoot(), existing); + ArchiveFileTree.addEntry(tree.getRoot(), new TarArchiveEntry(name)); + assertSame(existing, tree.getEntry("a")); + assertEquals(1, tree.getRoot().getFiles().size()); + assertTrue(tree.getRoot().getSubDirs().isEmpty()); + } + } + + /// Preserves explicit and implicit directories when a file has the same name. + @ParameterizedTest + @ValueSource(strings = {"a/", "a/file.txt"}) + public void testDirectoryConflictSkipsFile(String name, @TempDir Path directory) throws IOException { + try (var tree = openTree(directory)) { + ArchiveFileTree.addEntry(tree.getRoot(), new TarArchiveEntry(name)); + ArchiveFileTree.addEntry(tree.getRoot(), new TarArchiveEntry("a")); + assertNotNull(tree.getDirectory("a")); + assertTrue(tree.getRoot().getFiles().isEmpty()); + if (name.endsWith("file.txt")) + assertNotNull(tree.getEntry("a/file.txt")); + } + } + + /// Keeps the first file entry when another file has the same path. + @Test + public void testDuplicateFileIsSkipped(@TempDir Path directory) throws IOException { + try (var tree = openTree(directory)) { + var existing = new TarArchiveEntry("file.txt"); + ArchiveFileTree.addEntry(tree.getRoot(), existing); + ArchiveFileTree.addEntry(tree.getRoot(), new TarArchiveEntry("file.txt")); + assertSame(existing, tree.getEntry("file.txt")); + assertEquals(1, tree.getRoot().getFiles().size()); + } + } + + /// Skips entries containing parent directory components without remapping their files. + @ParameterizedTest + @ValueSource(strings = {"../file.txt", "a/../file.txt"}) + public void testParentDirectorySkipsEntry(String name, @TempDir Path directory) throws IOException { + try (var tree = openTree(directory)) { + ArchiveFileTree.addEntry(tree.getRoot(), new TarArchiveEntry(name)); + assertNull(tree.getEntry("file.txt")); + assertNull(tree.getEntry("a/file.txt")); + var valid = new TarArchiveEntry("valid.txt"); + ArchiveFileTree.addEntry(tree.getRoot(), valid); + assertSame(valid, tree.getEntry("valid.txt")); + } + } +}