From b080fcc03b775746f06ac76af0cbbdaa377b7847 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Fri, 5 Dec 2025 14:07:59 -0500 Subject: [PATCH 1/6] [COMPRESS-714] Internal IllegalArgumentException in ZipFile and TarFile creation is not caught --- .../archivers/tar/TarCompress714Test.java | 54 +++++++++++++++++++ .../archivers/zip/ZipCompress714Test.java | 41 ++++++++++++++ 2 files changed, 95 insertions(+) create mode 100644 src/test/java/org/apache/commons/compress/archivers/tar/TarCompress714Test.java create mode 100644 src/test/java/org/apache/commons/compress/archivers/zip/ZipCompress714Test.java diff --git a/src/test/java/org/apache/commons/compress/archivers/tar/TarCompress714Test.java b/src/test/java/org/apache/commons/compress/archivers/tar/TarCompress714Test.java new file mode 100644 index 00000000000..d0b7b6ff9f8 --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/tar/TarCompress714Test.java @@ -0,0 +1,54 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.commons.compress.archivers.tar; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; + +import org.apache.commons.compress.archivers.ArchiveException; +import org.apache.commons.compress.utils.SeekableInMemoryByteChannel; +import org.junit.jupiter.api.Test; + +/** + * Tests https://issues.apache.org/jira/browse/COMPRESS-714 + */ +public class TarCompress714Test { + + @Test + public void testIllegalPosition() throws IOException { + final byte[] data = { 46, 101, 97, 115, 97, 47, 120, -64, 72, 98, -18, 2, 53, 101, 112, 0, 0, 115, 8, 0, 0, 0, 112, 115, 40, 1, 0, 36, 2, 108, 0, -1, + -1, 0, 0, 0, 74, 0, 0, 0, 0, -1, 0, 1, 67, -2, 8, 0, 0, 0, 0, -64, -1, -1, -34, 9, 0, -120, -120, -120, -120, -120, -120, -120, -120, -120, + -120, -120, -120, -120, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 7, 0, 0, 0, 63, 0, 0, 0, 0, 0, 0, -65, -1, -126, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -128, + -1, -1, -1, -1, 0, -122, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -128, 0, 0, 0, 0, 35, 0, 53, 0, 0, 0, 0, 0, 0, 32, 56, 0, 0, 0, 0, 0, 0, -120, 55, 55, + 55, 55, 55, 55, 55, 55, 55, 55, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -105, 0, 0, 0, 49, 36, -1, -1, -1, -1, -1, -1, -126, 0, 0, 0, 0, 0, + 0, 0, 53, 0, 0, 0, -128, 0, 0, 0, 0, 16, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 46, 101, 97, 115, 97, 47, 120, -64, 72, 98, -18, 2, 53, + 101, 112, 0, 0, 115, 8, 0, 0, 0, 112, 115, 40, 1, 0, 2, 108, 0, -1, -1, 0, 0, 0, 74, 0, 0, 0, 0, -1, 0, 1, 67, -2, 8, 0, 0, 0, 0, -64, -1, -1, + -34, 9, 0, -120, -120, -120, -120, -120, -120, -120, -120, -120, -120, -120, -120, -120, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 7, 0, 0, 0, 63, 0, 0, + 0, 0, 0, 0, -65, -1, -126, 0, 26, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -128, -1, -1, -1, -1, 0, -122, 0, 0, 0, 0, 0, 0, -128, 0, 0, 0, 0, 35, 0, 53, 0, + 0, 0, 0, 0, 0, 32, 56, 0, 0, 0, 0, 0, 0, -120, 55, 55, 55, 55, 55, 55, 55, 55, 55, 55, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -105, 0, 0, + 0, 49, 36, -1, -1, -1, -1, -1, -1, -126, 0, 0, 0, 0, 0, 0, 0, 53, 0, 0, 0, -128, 0, 0, 0, 0, 16, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, + -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -65, + -1, -126, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -128, -1, -1, -1, -1, 0, -122, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -128, 0, 0, 0, 0, 35, 0, 53, 0, 0, 0, + 0, 0, 0, 32, 56, 0, 0, 0, 0, 0, 0, 0 }; + assertThrows(ArchiveException.class, () -> TarFile.builder().setChannel(new SeekableInMemoryByteChannel(data)).get()); + assertThrows(ArchiveException.class, () -> TarFile.builder().setByteArray(data).get()); + } +} diff --git a/src/test/java/org/apache/commons/compress/archivers/zip/ZipCompress714Test.java b/src/test/java/org/apache/commons/compress/archivers/zip/ZipCompress714Test.java new file mode 100644 index 00000000000..50d5f5de56e --- /dev/null +++ b/src/test/java/org/apache/commons/compress/archivers/zip/ZipCompress714Test.java @@ -0,0 +1,41 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.commons.compress.archivers.zip; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.io.IOException; + +import org.apache.commons.compress.archivers.ArchiveException; +import org.apache.commons.compress.utils.SeekableInMemoryByteChannel; +import org.junit.jupiter.api.Test; + +/** + * Tests https://issues.apache.org/jira/browse/COMPRESS-714 + */ +public class ZipCompress714Test { + + @Test + public void testIllegalPosition() throws IOException { + final byte[] data = { 80, 75, 5, 6, -127, 80, 75, 5, 6, 7, -127, -127, -127, 80, 74, 7, 8, -127, -127, -127, -127, -127 }; + assertThrows(ArchiveException.class, () -> ZipFile.builder().setChannel(new SeekableInMemoryByteChannel(data)).get()); + assertThrows(ArchiveException.class, () -> ZipFile.builder().setByteArray(data).get()); + } +} From ee088c655cd1e57b96c4086a93b17530453533fd Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Thu, 11 Dec 2025 08:31:41 -0500 Subject: [PATCH 2/6] The bug fix for [COMPRESS-714] is better done in Commons IO --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 45a8077d39b..c93507a77d3 100644 --- a/pom.xml +++ b/pom.xml @@ -207,7 +207,7 @@ Brotli, Zstandard and ar, cpio, jar, tar, zip, dump, 7z, arj. commons-io commons-io - 2.21.0 + 2.21.1-SNAPSHOT org.apache.commons From 20cbcd97f1ee7794fb6ed3b01579cfeaef67b039 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Thu, 11 Dec 2025 08:39:57 -0500 Subject: [PATCH 3/6] TEMP in PR until IO is released. --- pom.xml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pom.xml b/pom.xml index c93507a77d3..ce766e2fcdb 100644 --- a/pom.xml +++ b/pom.xml @@ -408,6 +408,20 @@ Brotli, Zstandard and ar, cpio, jar, tar, zip, dump, 7z, arj. + + + + apache.snapshots + Apache Development Snapshot Repository + https://repository.apache.org/content/repositories/snapshots/ + + false + + + true + + + From 391f90ba32e536b9c959d89c4d782226cb88890d Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Thu, 11 Dec 2025 08:44:35 -0500 Subject: [PATCH 4/6] The bug fix for [COMPRESS-714] is better done in Commons IO --- .../commons/compress/archivers/zip/ZipCompress714Test.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/apache/commons/compress/archivers/zip/ZipCompress714Test.java b/src/test/java/org/apache/commons/compress/archivers/zip/ZipCompress714Test.java index 50d5f5de56e..31269941a76 100644 --- a/src/test/java/org/apache/commons/compress/archivers/zip/ZipCompress714Test.java +++ b/src/test/java/org/apache/commons/compress/archivers/zip/ZipCompress714Test.java @@ -21,9 +21,9 @@ import static org.junit.jupiter.api.Assertions.assertThrows; +import java.io.EOFException; import java.io.IOException; -import org.apache.commons.compress.archivers.ArchiveException; import org.apache.commons.compress.utils.SeekableInMemoryByteChannel; import org.junit.jupiter.api.Test; @@ -35,7 +35,7 @@ public class ZipCompress714Test { @Test public void testIllegalPosition() throws IOException { final byte[] data = { 80, 75, 5, 6, -127, 80, 75, 5, 6, 7, -127, -127, -127, 80, 74, 7, 8, -127, -127, -127, -127, -127 }; - assertThrows(ArchiveException.class, () -> ZipFile.builder().setChannel(new SeekableInMemoryByteChannel(data)).get()); - assertThrows(ArchiveException.class, () -> ZipFile.builder().setByteArray(data).get()); + assertThrows(EOFException.class, () -> ZipFile.builder().setChannel(new SeekableInMemoryByteChannel(data)).get()); + assertThrows(EOFException.class, () -> ZipFile.builder().setByteArray(data).get()); } } From 0cfaec92de3c63fa8bc4f7b7d9ffffa8eb1b009e Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Thu, 11 Dec 2025 08:55:05 -0500 Subject: [PATCH 5/6] The bug fix for [COMPRESS-714] is better done in Commons IO --- .../commons/compress/archivers/zip/ZipCompress714Test.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/apache/commons/compress/archivers/zip/ZipCompress714Test.java b/src/test/java/org/apache/commons/compress/archivers/zip/ZipCompress714Test.java index 31269941a76..50d5f5de56e 100644 --- a/src/test/java/org/apache/commons/compress/archivers/zip/ZipCompress714Test.java +++ b/src/test/java/org/apache/commons/compress/archivers/zip/ZipCompress714Test.java @@ -21,9 +21,9 @@ import static org.junit.jupiter.api.Assertions.assertThrows; -import java.io.EOFException; import java.io.IOException; +import org.apache.commons.compress.archivers.ArchiveException; import org.apache.commons.compress.utils.SeekableInMemoryByteChannel; import org.junit.jupiter.api.Test; @@ -35,7 +35,7 @@ public class ZipCompress714Test { @Test public void testIllegalPosition() throws IOException { final byte[] data = { 80, 75, 5, 6, -127, 80, 75, 5, 6, 7, -127, -127, -127, 80, 74, 7, 8, -127, -127, -127, -127, -127 }; - assertThrows(EOFException.class, () -> ZipFile.builder().setChannel(new SeekableInMemoryByteChannel(data)).get()); - assertThrows(EOFException.class, () -> ZipFile.builder().setByteArray(data).get()); + assertThrows(ArchiveException.class, () -> ZipFile.builder().setChannel(new SeekableInMemoryByteChannel(data)).get()); + assertThrows(ArchiveException.class, () -> ZipFile.builder().setByteArray(data).get()); } } From cab216b87e73374c8633aa3c682df84cd5c4e5c0 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Thu, 11 Dec 2025 18:44:37 -0500 Subject: [PATCH 6/6] Use IOUtils.close[Quietly]() --- pom.xml | 2 +- .../commons/compress/archivers/sevenz/SevenZFile.java | 4 +--- .../org/apache/commons/compress/archivers/tar/TarFile.java | 5 ++--- .../org/apache/commons/compress/archivers/zip/ZipFile.java | 6 +----- .../commons/compress/archivers/tar/TarCompress714Test.java | 6 +++--- .../commons/compress/archivers/zip/ZipCompress714Test.java | 6 +++--- 6 files changed, 11 insertions(+), 18 deletions(-) diff --git a/pom.xml b/pom.xml index ce766e2fcdb..c14944c51cd 100644 --- a/pom.xml +++ b/pom.xml @@ -207,7 +207,7 @@ Brotli, Zstandard and ar, cpio, jar, tar, zip, dump, 7z, arj. commons-io commons-io - 2.21.1-SNAPSHOT + 2.22.0-SNAPSHOT org.apache.commons diff --git a/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java b/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java index 3f6a3079a9a..1ebab3299be 100644 --- a/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java +++ b/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java @@ -629,9 +629,7 @@ private SevenZFile(final Builder builder) throws IOException { archive = readHeaders(password); this.password = password != null ? Arrays.copyOf(password, password.length) : null; } catch (final ArithmeticException | IllegalArgumentException e) { - final ArchiveException archiveException = new ArchiveException(e); - IOUtils.close(channel, archiveException::addSuppressed); - throw archiveException; + throw IOUtils.closeQuietly(channel, new ArchiveException(e)); } } diff --git a/src/main/java/org/apache/commons/compress/archivers/tar/TarFile.java b/src/main/java/org/apache/commons/compress/archivers/tar/TarFile.java index c1dc46bc25c..bf90ec428de 100644 --- a/src/main/java/org/apache/commons/compress/archivers/tar/TarFile.java +++ b/src/main/java/org/apache/commons/compress/archivers/tar/TarFile.java @@ -192,9 +192,8 @@ private TarFile(final Builder builder) throws IOException { while ((entry = getNextTarEntry()) != null) { entries.add(entry); } - } catch (final IOException ex) { - IOUtils.close(archive, ex::addSuppressed); - throw ex; + } catch (final IOException e) { + throw IOUtils.closeQuietly(archive, e); } } diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java index 07f91acf078..241d01bf7f0 100644 --- a/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java +++ b/src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java @@ -756,12 +756,8 @@ private ZipFile(final Builder builder) throws IOException { } fillNameMap(); } catch (final IOException e) { - final ArchiveException archiveException = e instanceof ArchiveException - ? (ArchiveException) e - : new ArchiveException("Error reading Zip content from " + builder.getName(), (Throwable) e); this.closed = true; - IOUtils.close(archive, archiveException::addSuppressed); - throw archiveException; + throw IOUtils.closeQuietly(archive, e); } } diff --git a/src/test/java/org/apache/commons/compress/archivers/tar/TarCompress714Test.java b/src/test/java/org/apache/commons/compress/archivers/tar/TarCompress714Test.java index d0b7b6ff9f8..edc58743ba3 100644 --- a/src/test/java/org/apache/commons/compress/archivers/tar/TarCompress714Test.java +++ b/src/test/java/org/apache/commons/compress/archivers/tar/TarCompress714Test.java @@ -21,9 +21,9 @@ import static org.junit.jupiter.api.Assertions.assertThrows; +import java.io.EOFException; import java.io.IOException; -import org.apache.commons.compress.archivers.ArchiveException; import org.apache.commons.compress.utils.SeekableInMemoryByteChannel; import org.junit.jupiter.api.Test; @@ -48,7 +48,7 @@ public void testIllegalPosition() throws IOException { -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -65, -1, -126, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -128, -1, -1, -1, -1, 0, -122, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, -128, 0, 0, 0, 0, 35, 0, 53, 0, 0, 0, 0, 0, 0, 32, 56, 0, 0, 0, 0, 0, 0, 0 }; - assertThrows(ArchiveException.class, () -> TarFile.builder().setChannel(new SeekableInMemoryByteChannel(data)).get()); - assertThrows(ArchiveException.class, () -> TarFile.builder().setByteArray(data).get()); + assertThrows(EOFException.class, () -> TarFile.builder().setChannel(new SeekableInMemoryByteChannel(data)).get()); + assertThrows(EOFException.class, () -> TarFile.builder().setByteArray(data).get()); } } diff --git a/src/test/java/org/apache/commons/compress/archivers/zip/ZipCompress714Test.java b/src/test/java/org/apache/commons/compress/archivers/zip/ZipCompress714Test.java index 50d5f5de56e..31269941a76 100644 --- a/src/test/java/org/apache/commons/compress/archivers/zip/ZipCompress714Test.java +++ b/src/test/java/org/apache/commons/compress/archivers/zip/ZipCompress714Test.java @@ -21,9 +21,9 @@ import static org.junit.jupiter.api.Assertions.assertThrows; +import java.io.EOFException; import java.io.IOException; -import org.apache.commons.compress.archivers.ArchiveException; import org.apache.commons.compress.utils.SeekableInMemoryByteChannel; import org.junit.jupiter.api.Test; @@ -35,7 +35,7 @@ public class ZipCompress714Test { @Test public void testIllegalPosition() throws IOException { final byte[] data = { 80, 75, 5, 6, -127, 80, 75, 5, 6, 7, -127, -127, -127, 80, 74, 7, 8, -127, -127, -127, -127, -127 }; - assertThrows(ArchiveException.class, () -> ZipFile.builder().setChannel(new SeekableInMemoryByteChannel(data)).get()); - assertThrows(ArchiveException.class, () -> ZipFile.builder().setByteArray(data).get()); + assertThrows(EOFException.class, () -> ZipFile.builder().setChannel(new SeekableInMemoryByteChannel(data)).get()); + assertThrows(EOFException.class, () -> ZipFile.builder().setByteArray(data).get()); } }