From 38ccf140b370a0273acc1e74914884bbb718af2c Mon Sep 17 00:00:00 2001 From: "David M. Lloyd" Date: Wed, 30 Sep 2026 14:13:13 -0500 Subject: [PATCH] No longer allow duplicate entries by default You can still add them using an explicit option though. --- .../common/io/archive/ArchiveBuilder.java | 42 +++++++++- .../smallrye/common/io/archive/ZipOption.java | 6 ++ .../io/archive/ArchiveBuilderTests.java | 77 +++++++++++++++++++ 3 files changed, 121 insertions(+), 4 deletions(-) diff --git a/io/src/main/java/io/smallrye/common/io/archive/ArchiveBuilder.java b/io/src/main/java/io/smallrye/common/io/archive/ArchiveBuilder.java index 21db7442..f9756995 100644 --- a/io/src/main/java/io/smallrye/common/io/archive/ArchiveBuilder.java +++ b/io/src/main/java/io/smallrye/common/io/archive/ArchiveBuilder.java @@ -25,6 +25,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.EnumSet; +import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.zip.CRC32; @@ -89,17 +90,40 @@ public final class ArchiveBuilder implements Closeable { private final long startOffset; private final int defaultMethod; private final boolean defaultZip64; + /** + * Indicates whether duplicate entry names are permitted. + * If {@code false}, adding a duplicate entry name will throw an {@link IllegalArgumentException}. + */ + private final boolean allowDuplicateEntries; + /** + * The set of entry names that have already been added to this archive. + * Used to prevent duplicate entry names when {@link #allowDuplicateEntries} is {@code false}. + * This field is {@code null} if duplicate entry names are permitted. + */ + private final Set addedNames; private final List entries = new ArrayList<>(); private Deflater deflater; private CRC32 crc; private Closeable activeEntry; private boolean closed; - private ArchiveBuilder(BufferedFile file, long startOffset, int defaultMethod, boolean defaultZip64) { + /** + * Construct a new {@code ArchiveBuilder} instance. + * + * @param file the buffered file to write the archive to (must not be {@code null}) + * @param startOffset the file position where the archive begins (must be non-negative) + * @param defaultMethod the default compression method ({@link Constants#METHOD_STORED} or {@link Constants#METHOD_DEFLATE}) + * @param defaultZip64 whether to use ZIP64 by default + * @param allowDuplicateEntries whether duplicate entry names are permitted + */ + private ArchiveBuilder(BufferedFile file, long startOffset, int defaultMethod, boolean defaultZip64, + boolean allowDuplicateEntries) { this.file = file; this.startOffset = startOffset; this.defaultMethod = defaultMethod; this.defaultZip64 = defaultZip64; + this.allowDuplicateEntries = allowDuplicateEntries; + this.addedNames = allowDuplicateEntries ? null : new HashSet<>(); } /** @@ -181,7 +205,7 @@ public static ArchiveBuilder open(Path path, Collection op for (OpenOption opt : options) { if (opt instanceof ZipOption zo) { switch (zo) { - case STORED, DEFLATED, ZIP64 -> { + case STORED, DEFLATED, ZIP64, ALLOW_DUPLICATE_ENTRIES -> { } } } else if (opt instanceof StandardOpenOption soo) { @@ -264,7 +288,7 @@ private static ArchiveBuilder openTrusted(final BufferedFile file, int parsed = parseEntryOptions(options, true); int method = parsed & PARSED_METHOD_MASK; return new ArchiveBuilder(file, file.filePosition(), method == PARSED_NO_METHOD ? METHOD_DEFLATE : method, - (parsed & PARSED_ZIP64) != 0); + (parsed & PARSED_ZIP64) != 0, (parsed & PARSED_ALLOW_DUPLICATE_ENTRIES) != 0); } /** @@ -1282,7 +1306,8 @@ public ArchiveBuilder addArchive(String name, Collection o // create the inner builder; ownsFile = true so closing it closes the nested file, // which triggers the close action for CRC/LFH patching on the outer entry return new ArchiveBuilder(nested, nested.filePosition(), - innerMethod == PARSED_NO_METHOD ? METHOD_DEFLATE : innerMethod, zip64); + innerMethod == PARSED_NO_METHOD ? METHOD_DEFLATE : innerMethod, zip64, + (parsed & PARSED_ALLOW_DUPLICATE_ENTRIES) != 0 || allowDuplicateEntries); } // ── Raw data methods ──────────────────────────────────────────────── @@ -1441,6 +1466,11 @@ private static int setMethod(int current, int requested) { */ private static final int PARSED_ZIP64 = 1 << 8; + /** + * Flag bit indicating that {@link ZipOption#ALLOW_DUPLICATE_ENTRIES} was present in the parsed options. + */ + private static final int PARSED_ALLOW_DUPLICATE_ENTRIES = 1 << 9; + /** * Parse {@link ZipOption} and {@link StandardOpenOption} values from an option collection, * returning the compression method and ZIP64 flag packed into a single {@code int}. @@ -1475,6 +1505,7 @@ private static int parseEntryOptions(Collection options, b method = setMethod(method, METHOD_DEFLATE); } case ZIP64 -> flags |= PARSED_ZIP64; + case ALLOW_DUPLICATE_ENTRIES -> flags |= PARSED_ALLOW_DUPLICATE_ENTRIES; } } else if (opt instanceof StandardOpenOption soo) { switch (soo) { @@ -1514,6 +1545,9 @@ private CdEntry initEntry(String name, int method, boolean zip64, if (fileType != S_IFDIR && name.endsWith("/")) { throw new IllegalArgumentException("Non-directory entry name must not end with '/': " + name); } + if (!allowDuplicateEntries && !addedNames.add(name)) { + throw new IllegalArgumentException("Duplicate entry: " + name); + } CdEntry entry = new CdEntry(); entry.fileName = name.getBytes(StandardCharsets.UTF_8); entry.method = method; diff --git a/io/src/main/java/io/smallrye/common/io/archive/ZipOption.java b/io/src/main/java/io/smallrye/common/io/archive/ZipOption.java index a2c6fc68..182f1aad 100644 --- a/io/src/main/java/io/smallrye/common/io/archive/ZipOption.java +++ b/io/src/main/java/io/smallrye/common/io/archive/ZipOption.java @@ -37,5 +37,11 @@ public enum ZipOption implements OpenOption { * exceeds 4 GB), regardless of this option. */ ZIP64, + /** + * Allow duplicate entries to be added to the archive. + * When this option is not specified, attempting to add multiple entries with the same + * name will throw an {@link IllegalArgumentException}. + */ + ALLOW_DUPLICATE_ENTRIES, ; } diff --git a/io/src/test/java/io/smallrye/common/io/archive/ArchiveBuilderTests.java b/io/src/test/java/io/smallrye/common/io/archive/ArchiveBuilderTests.java index 5362b1de..1b832773 100644 --- a/io/src/test/java/io/smallrye/common/io/archive/ArchiveBuilderTests.java +++ b/io/src/test/java/io/smallrye/common/io/archive/ArchiveBuilderTests.java @@ -1995,4 +1995,81 @@ public void testLastEntryReflectsSecondEntry() throws Exception { assertEquals((int) secondCrc.getValue(), builder.lastEntryCrc32()); } } + + /** + * Test that adding a duplicate entry name without {@link ZipOption#ALLOW_DUPLICATE_ENTRIES} + * throws an {@link IllegalArgumentException}. + * + * @throws Exception if an unexpected error occurs + */ + @Test + public void testDuplicateEntryBlockedByDefault() throws Exception { + Path file = tempDir.resolve("duplicate-blocked.zip"); + try (ArchiveBuilder builder = ArchiveBuilder.open(file)) { + builder.addEntry("hello.txt", "hello".getBytes(StandardCharsets.UTF_8)); + assertThrows(IllegalArgumentException.class, () -> { + builder.addEntry("hello.txt", "world".getBytes(StandardCharsets.UTF_8)); + }); + } + } + + /** + * Test that adding a duplicate entry name with {@link ZipOption#ALLOW_DUPLICATE_ENTRIES} + * is permitted and creates duplicate entries successfully. + * + * @throws Exception if an unexpected error occurs + */ + @Test + public void testDuplicateEntryAllowedWithOptions() throws Exception { + Path file = tempDir.resolve("duplicate-allowed.zip"); + try (ArchiveBuilder builder = ArchiveBuilder.open(file, ZipOption.ALLOW_DUPLICATE_ENTRIES)) { + builder.addEntry("hello.txt", "hello".getBytes(StandardCharsets.UTF_8)); + assertDoesNotThrow(() -> { + builder.addEntry("hello.txt", "world".getBytes(StandardCharsets.UTF_8)); + }); + } + + // Verify that we actually have 2 entries with the same name in the zip + try (ZipFile zf = new ZipFile(file.toFile())) { + assertEquals(2, zf.size()); + } + } + + /** + * Test that adding duplicate directory entry names without {@link ZipOption#ALLOW_DUPLICATE_ENTRIES} + * throws an {@link IllegalArgumentException}. + * + * @throws Exception if an unexpected error occurs + */ + @Test + public void testDuplicateDirectoryBlockedByDefault() throws Exception { + Path file = tempDir.resolve("duplicate-dir-blocked.zip"); + try (ArchiveBuilder builder = ArchiveBuilder.open(file)) { + builder.addDirectory("META-INF"); + assertThrows(IllegalArgumentException.class, () -> { + builder.addDirectory("META-INF"); + }); + } + } + + /** + * Test that adding a duplicate nested archive entry name without {@link ZipOption#ALLOW_DUPLICATE_ENTRIES} + * throws an {@link IllegalArgumentException}. + * + * @throws Exception if an unexpected error occurs + */ + @Test + public void testDuplicateNestedArchiveBlockedByDefault() throws Exception { + Path file = tempDir.resolve("duplicate-archive-blocked.zip"); + try (ArchiveBuilder builder = ArchiveBuilder.open(file)) { + try (ArchiveBuilder inner1 = builder.addArchive("inner.jar")) { + inner1.addEntry("foo.txt", "foo".getBytes(StandardCharsets.UTF_8)); + } + assertThrows(IllegalArgumentException.class, () -> { + try (ArchiveBuilder inner2 = builder.addArchive("inner.jar")) { + inner2.addEntry("bar.txt", "bar".getBytes(StandardCharsets.UTF_8)); + } + }); + } + } }