This is an automated email from the ASF dual-hosted git repository. garydgregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-compress.git
commit 4b95cbb4fc58f7c280ef4e43ad3af4674cddc881 Author: Gary Gregory <[email protected]> AuthorDate: Mon Aug 10 17:35:30 2026 -0400 ArchiveStreamFactory now throws ArchiveException instead of IllegalArgumentException. TarArchiveOutputStream now throws ArchiveException instead of IllegalArgumentException. --- src/changes/changes.xml | 2 + .../compress/archivers/ArchiveStreamFactory.java | 97 ++++++++++------------ .../archivers/tar/TarArchiveOutputStream.java | 12 +-- .../archivers/ArchiveStreamFactoryTest.java | 4 +- .../compress/archivers/ExceptionMessageTest.java | 19 ++--- .../archivers/tar/TarArchiveOutputStreamTest.java | 7 +- 6 files changed, 64 insertions(+), 77 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 659b8c7ae..eddf0b565 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -102,6 +102,7 @@ The <action> type attribute can be add,update,fix,remove. <action type="fix" dev="ggregory" due-to="Gary Gregory">TarArchiveEntry.setSize(long) now throw ArchiveException instead of IllegalArgumentException.</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">TarArchiveEntry.addPaxHeader(String, String) now throws ArchiveException instead of IllegalArgumentException.</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">TAR ParsingUtils now throws the IOException subclass CompressException.</action> + <action type="fix" dev="ggregory" due-to="Gary Gregory">TarArchiveOutputStream now throws ArchiveException instead of IllegalArgumentException..</action> <!-- FIX ar --> <action type="fix" dev="ggregory" due-to="Gary Gregory">ArArchiveInputStream.readGNUStringTable(byte[], int, int) now provides a better exception message, wrapping the underlying exception.</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">ArArchiveInputStream.read(byte[], int, int) now throws ArchiveException instead of ArithmeticException.</action> @@ -172,6 +173,7 @@ The <action> type attribute can be add,update,fix,remove. <action type="fix" dev="ggregory" due-to="Gary Gregory">Add and use ArchiveException.requireNonNegative(int, Supplier<String>)</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">Add and use ArchiveException.requireNonNegative(long, String)</action> <action type="fix" dev="ggregory" due-to="Gary Gregory">Add and use ArchiveException.requireNonNegative(long, Supplier<String>)</action> +<!-- <action type="fix" dev="ggregory" due-to="Gary Gregory">ArchiveStreamFactory now throws ArchiveException instead of IllegalArgumentException.</action> --> <!-- ADD --> <action type="add" dev="ggregory" due-to="Gary Gregory">Add MemoryLimitException.MemoryLimitException(long, long).</action> <action type="add" dev="ggregory" due-to="Gary Gregory">Add CompressException.CompressException(String, Object...).</action> diff --git a/src/main/java/org/apache/commons/compress/archivers/ArchiveStreamFactory.java b/src/main/java/org/apache/commons/compress/archivers/ArchiveStreamFactory.java index a4f86aba6..c745b1070 100644 --- a/src/main/java/org/apache/commons/compress/archivers/ArchiveStreamFactory.java +++ b/src/main/java/org/apache/commons/compress/archivers/ArchiveStreamFactory.java @@ -212,24 +212,22 @@ private static Iterable<ArchiveStreamProvider> archiveStreamProviderIterable() { /** * Try to determine the type of Archiver * - * @param in input stream. + * @param inputStream input stream. * @return type of archiver if found. * @throws ArchiveException if an archiver cannot be detected in the stream. * @since 1.14 */ - public static String detect(final InputStream in) throws ArchiveException { - if (in == null) { - throw new IllegalArgumentException("Stream must not be null."); - } - if (!in.markSupported()) { + public static String detect(final InputStream inputStream) throws ArchiveException { + ArchiveException.requireNonNull(inputStream, "null inputStream"); + if (!inputStream.markSupported()) { throw new IllegalArgumentException("Mark is not supported."); } final byte[] signature = new byte[SIGNATURE_SIZE]; - in.mark(signature.length); + inputStream.mark(signature.length); int signatureLength = -1; try { - signatureLength = IOUtils.read(in, signature); - in.reset(); + signatureLength = IOUtils.read(inputStream, signature); + inputStream.reset(); } catch (final IOException e) { throw new ArchiveException("Failure reading signature.", (Throwable) e); } @@ -255,10 +253,10 @@ public static String detect(final InputStream in) throws ArchiveException { } // Dump needs a bigger buffer to check the signature; final byte[] dumpsig = new byte[DUMP_SIGNATURE_SIZE]; - in.mark(dumpsig.length); + inputStream.mark(dumpsig.length); try { - signatureLength = IOUtils.read(in, dumpsig); - in.reset(); + signatureLength = IOUtils.read(inputStream, dumpsig); + inputStream.reset(); } catch (final IOException e) { throw new ArchiveException("IOException while reading dump signature", (Throwable) e); } @@ -267,10 +265,10 @@ public static String detect(final InputStream in) throws ArchiveException { } // Tar needs an even bigger buffer to check the signature; read the first block final byte[] tarHeader = new byte[TAR_HEADER_SIZE]; - in.mark(tarHeader.length); + inputStream.mark(tarHeader.length); try { - signatureLength = IOUtils.read(in, tarHeader); - in.reset(); + signatureLength = IOUtils.read(inputStream, tarHeader); + inputStream.reset(); } catch (final IOException e) { throw new ArchiveException("IOException while reading tar signature", (Throwable) e); } @@ -279,14 +277,13 @@ public static String detect(final InputStream in) throws ArchiveException { } // COMPRESS-117 if (signatureLength >= TAR_HEADER_SIZE) { - try (TarArchiveInputStream inputStream = - TarArchiveInputStream.builder().setByteArray(tarHeader).get()) { + try (TarArchiveInputStream tarIn = TarArchiveInputStream.builder().setByteArray(tarHeader).get()) { // COMPRESS-191 - verify the header checksum - TarArchiveEntry entry = inputStream.getNextEntry(); + TarArchiveEntry entry = tarIn.getNextEntry(); // try to find the first non-directory entry within the first 10 entries. int count = 0; while (entry != null && entry.isDirectory() && entry.isCheckSumOK() && count++ < TAR_TEST_ENTRY_COUNT) { - entry = inputStream.getNextEntry(); + entry = tarIn.getNextEntry(); } if (entry != null && entry.isCheckSumOK() && !entry.isDirectory() && isName(entry.getGroupName()) && isName(entry.getName()) && isName(entry.getUserName()) || count > 0) { @@ -299,10 +296,10 @@ && isName(entry.getUserName()) || count > 0) { // LHA has no magic signature, so its detection is heuristic. It is checked last so that // formats with a stronger signature are not shadowed by a false positive LHA match. final byte[] lhasig = new byte[LHA_SIGNATURE_SIZE]; - in.mark(lhasig.length); + inputStream.mark(lhasig.length); try { - signatureLength = IOUtils.read(in, lhasig); - in.reset(); + signatureLength = IOUtils.read(inputStream, lhasig); + inputStream.reset(); } catch (final IOException e) { throw new ArchiveException("IOException while reading LHA signature", (Throwable) e); } @@ -443,41 +440,37 @@ public <I extends ArchiveInputStream<? extends ArchiveEntry>> I createArchiveInp @SuppressWarnings("unchecked") @Override - public <I extends ArchiveInputStream<? extends ArchiveEntry>> I createArchiveInputStream(final String archiverName, final InputStream in, + public <I extends ArchiveInputStream<? extends ArchiveEntry>> I createArchiveInputStream(final String archiverName, final InputStream inputStream, final String actualEncoding) throws ArchiveException { - if (archiverName == null) { - throw new IllegalArgumentException("Archiver name must not be null."); - } - if (in == null) { - throw new IllegalArgumentException("InputStream must not be null."); - } + ArchiveException.requireNonNull(archiverName, "null archiverName"); + ArchiveException.requireNonNull(inputStream, "null inputStream"); try { if (AR.equalsIgnoreCase(archiverName)) { - return (I) ArArchiveInputStream.builder().setInputStream(in).get(); + return (I) ArArchiveInputStream.builder().setInputStream(inputStream).get(); } if (ARJ.equalsIgnoreCase(archiverName)) { - final ArjArchiveInputStream.Builder arjBuilder = ArjArchiveInputStream.builder().setInputStream(in); + final ArjArchiveInputStream.Builder arjBuilder = ArjArchiveInputStream.builder().setInputStream(inputStream); if (actualEncoding != null) { arjBuilder.setCharset(actualEncoding); } return (I) arjBuilder.get(); } if (LHA.equalsIgnoreCase(archiverName)) { - final LhaArchiveInputStream.Builder lhaBuilder = LhaArchiveInputStream.builder().setInputStream(in); + final LhaArchiveInputStream.Builder lhaBuilder = LhaArchiveInputStream.builder().setInputStream(inputStream); if (actualEncoding != null) { lhaBuilder.setCharset(actualEncoding); } return (I) lhaBuilder.get(); } if (ZIP.equalsIgnoreCase(archiverName)) { - final ZipArchiveInputStream.Builder zipBuilder = ZipArchiveInputStream.builder().setInputStream(in); + final ZipArchiveInputStream.Builder zipBuilder = ZipArchiveInputStream.builder().setInputStream(inputStream); if (actualEncoding != null) { zipBuilder.setCharset(actualEncoding); } return (I) zipBuilder.get(); } if (TAR.equalsIgnoreCase(archiverName)) { - final TarArchiveInputStream.Builder tarBuilder = TarArchiveInputStream.builder().setInputStream(in); + final TarArchiveInputStream.Builder tarBuilder = TarArchiveInputStream.builder().setInputStream(inputStream); if (actualEncoding != null) { tarBuilder.setCharset(actualEncoding); } @@ -485,21 +478,21 @@ public <I extends ArchiveInputStream<? extends ArchiveEntry>> I createArchiveInp } if (JAR.equalsIgnoreCase(archiverName) || APK.equalsIgnoreCase(archiverName)) { final JarArchiveInputStream.Builder jarBuilder = - JarArchiveInputStream.jarInputStreamBuilder().setInputStream(in); + JarArchiveInputStream.jarInputStreamBuilder().setInputStream(inputStream); if (actualEncoding != null) { jarBuilder.setCharset(actualEncoding); } return (I) jarBuilder.get(); } if (CPIO.equalsIgnoreCase(archiverName)) { - final CpioArchiveInputStream.Builder cpioBuilder = CpioArchiveInputStream.builder().setInputStream(in); + final CpioArchiveInputStream.Builder cpioBuilder = CpioArchiveInputStream.builder().setInputStream(inputStream); if (actualEncoding != null) { cpioBuilder.setCharset(actualEncoding); } return (I) cpioBuilder.get(); } if (DUMP.equalsIgnoreCase(archiverName)) { - final DumpArchiveInputStream.Builder dumpBuilder = DumpArchiveInputStream.builder().setInputStream(in); + final DumpArchiveInputStream.Builder dumpBuilder = DumpArchiveInputStream.builder().setInputStream(inputStream); if (actualEncoding != null) { dumpBuilder.setCharset(actualEncoding); } @@ -510,7 +503,7 @@ public <I extends ArchiveInputStream<? extends ArchiveEntry>> I createArchiveInp } final ArchiveStreamProvider archiveStreamProvider = getArchiveInputStreamProviders().get(toKey(archiverName)); if (archiveStreamProvider != null) { - return archiveStreamProvider.createArchiveInputStream(archiverName, in, actualEncoding); + return archiveStreamProvider.createArchiveInputStream(archiverName, inputStream, actualEncoding); } throw new ArchiveException("Archiver: %s not found.", archiverName); } catch (final ArchiveException e) { @@ -539,19 +532,15 @@ public <O extends ArchiveOutputStream<? extends ArchiveEntry>> O createArchiveOu @SuppressWarnings("unchecked") @Override - public <O extends ArchiveOutputStream<? extends ArchiveEntry>> O createArchiveOutputStream(final String archiverName, final OutputStream out, + public <O extends ArchiveOutputStream<? extends ArchiveEntry>> O createArchiveOutputStream(final String archiverName, final OutputStream outputStream, final String actualEncoding) throws ArchiveException { - if (archiverName == null) { - throw new IllegalArgumentException("Archiver name must not be null."); - } - if (out == null) { - throw new IllegalArgumentException("OutputStream must not be null."); - } + ArchiveException.requireNonNull(archiverName, "null archiverName"); + ArchiveException.requireNonNull(outputStream, "null outputStream"); if (AR.equalsIgnoreCase(archiverName)) { - return (O) new ArArchiveOutputStream(out); + return (O) new ArArchiveOutputStream(outputStream); } if (ZIP.equalsIgnoreCase(archiverName)) { - final ZipArchiveOutputStream zip = new ZipArchiveOutputStream(out); + final ZipArchiveOutputStream zip = new ZipArchiveOutputStream(outputStream); if (actualEncoding != null) { zip.setEncoding(actualEncoding); } @@ -559,28 +548,28 @@ public <O extends ArchiveOutputStream<? extends ArchiveEntry>> O createArchiveOu } if (TAR.equalsIgnoreCase(archiverName)) { if (actualEncoding != null) { - return (O) new TarArchiveOutputStream(out, actualEncoding); + return (O) new TarArchiveOutputStream(outputStream, actualEncoding); } - return (O) new TarArchiveOutputStream(out); + return (O) new TarArchiveOutputStream(outputStream); } if (JAR.equalsIgnoreCase(archiverName)) { if (actualEncoding != null) { - return (O) new JarArchiveOutputStream(out, actualEncoding); + return (O) new JarArchiveOutputStream(outputStream, actualEncoding); } - return (O) new JarArchiveOutputStream(out); + return (O) new JarArchiveOutputStream(outputStream); } if (CPIO.equalsIgnoreCase(archiverName)) { if (actualEncoding != null) { - return (O) new CpioArchiveOutputStream(out, actualEncoding); + return (O) new CpioArchiveOutputStream(outputStream, actualEncoding); } - return (O) new CpioArchiveOutputStream(out); + return (O) new CpioArchiveOutputStream(outputStream); } if (SEVEN_Z.equalsIgnoreCase(archiverName)) { throw new StreamingNotSupportedException(SEVEN_Z); } final ArchiveStreamProvider archiveStreamProvider = getArchiveOutputStreamProviders().get(toKey(archiverName)); if (archiveStreamProvider != null) { - return archiveStreamProvider.createArchiveOutputStream(archiverName, out, actualEncoding); + return archiveStreamProvider.createArchiveOutputStream(archiverName, outputStream, actualEncoding); } throw new ArchiveException("Archiver: %s not found.", archiverName); } diff --git a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java index a3385f7a4..8df01c701 100644 --- a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java +++ b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStream.java @@ -267,7 +267,7 @@ private void addPaxHeaderForBigNumber(final Map<String, String> paxHeaders, fina } } - private void addPaxHeadersForBigNumbers(final Map<String, String> paxHeaders, final TarArchiveEntry entry) { + private void addPaxHeadersForBigNumbers(final Map<String, String> paxHeaders, final TarArchiveEntry entry) throws ArchiveException { addPaxHeaderForBigNumber(paxHeaders, "size", entry.getSize(), TarConstants.MAXSIZE); addPaxHeaderForBigNumber(paxHeaders, "gid", entry.getLongGroupId(), TarConstants.MAXID); addFileTimePaxHeaderForBigNumber(paxHeaders, "mtime", entry.getLastModifiedTime(), TarConstants.MAXSIZE); @@ -363,18 +363,18 @@ private byte[] encodeExtendedPaxHeadersContents(final Map<String, String> header return toUtf8Bytes(w.toString()); } - private void failForBigNumber(final String field, final long value, final long maxValue) { + private void failForBigNumber(final String field, final long value, final long maxValue) throws ArchiveException { failForBigNumber(field, value, maxValue, ""); } - private void failForBigNumber(final String field, final long value, final long maxValue, final String additionalMsg) { + private void failForBigNumber(final String field, final long value, final long maxValue, final String additionalMsg) throws ArchiveException { if (value < 0 || value > maxValue) { - throw new IllegalArgumentException(field + " '" + value // NOSONAR + throw new ArchiveException(field + " '" + value // NOSONAR + "' is too big ( > " + maxValue + " )." + additionalMsg); } } - private void failForBigNumbers(final TarArchiveEntry entry) { + private void failForBigNumbers(final TarArchiveEntry entry) throws ArchiveException { failForBigNumber("entry size", entry.getSize(), TarConstants.MAXSIZE); failForBigNumberWithPosixMessage("group id", entry.getLongGroupId(), TarConstants.MAXID); failForBigNumber("last modification time", FileTimes.toUnixTime(entry.getLastModifiedTime()), TarConstants.MAXSIZE); @@ -384,7 +384,7 @@ private void failForBigNumbers(final TarArchiveEntry entry) { failForBigNumber("minor device number", entry.getDevMinor(), TarConstants.MAXID); } - private void failForBigNumberWithPosixMessage(final String field, final long value, final long maxValue) { + private void failForBigNumberWithPosixMessage(final String field, final long value, final long maxValue) throws ArchiveException { failForBigNumber(field, value, maxValue, " Use STAR or POSIX extensions to overcome this limit"); } diff --git a/src/test/java/org/apache/commons/compress/archivers/ArchiveStreamFactoryTest.java b/src/test/java/org/apache/commons/compress/archivers/ArchiveStreamFactoryTest.java index 2f45d89a6..a94482e22 100644 --- a/src/test/java/org/apache/commons/compress/archivers/ArchiveStreamFactoryTest.java +++ b/src/test/java/org/apache/commons/compress/archivers/ArchiveStreamFactoryTest.java @@ -281,9 +281,9 @@ void testDetect() throws Exception { "shouldn't be able to detect empty stream"); assertEquals("No Archiver found for the stream signature", e1.getMessage()); - final IllegalArgumentException e2 = assertThrows(IllegalArgumentException.class, () -> ArchiveStreamFactory.detect(null), + final ArchiveException e2 = assertThrows(ArchiveException.class, () -> ArchiveStreamFactory.detect(null), "shouldn't be able to detect null stream"); - assertEquals("Stream must not be null.", e2.getMessage()); + assertEquals("null inputStream", e2.getMessage()); final ArchiveException e3 = assertThrows(ArchiveException.class, () -> ArchiveStreamFactory.detect(new BufferedInputStream(new BrokenInputStream())), "Expected ArchiveException"); diff --git a/src/test/java/org/apache/commons/compress/archivers/ExceptionMessageTest.java b/src/test/java/org/apache/commons/compress/archivers/ExceptionMessageTest.java index d2c1d7645..9035d0178 100644 --- a/src/test/java/org/apache/commons/compress/archivers/ExceptionMessageTest.java +++ b/src/test/java/org/apache/commons/compress/archivers/ExceptionMessageTest.java @@ -26,38 +26,33 @@ class ExceptionMessageTest { - private static final String ARCHIVER_NULL_MESSAGE = "Archiver name must not be null."; + private static final String ARCHIVER_NULL_MESSAGE = "null archiverName"; - private static final String INPUTSTREAM_NULL_MESSAGE = "InputStream must not be null."; + private static final String INPUTSTREAM_NULL_MESSAGE = "null inputStream"; - private static final String OUTPUTSTREAM_NULL_MESSAGE = "OutputStream must not be null."; + private static final String OUTPUTSTREAM_NULL_MESSAGE = "null outputStream"; @Test void testMessageWhenArchiverNameIsNull_1() { - final IllegalArgumentException e = assertThrows(IllegalArgumentException.class, - () -> ArchiveStreamFactory.DEFAULT.createArchiveInputStream(null, System.in), "Should raise an IllegalArgumentException."); + final ArchiveException e = assertThrows(ArchiveException.class, () -> ArchiveStreamFactory.DEFAULT.createArchiveInputStream(null, System.in)); assertEquals(ARCHIVER_NULL_MESSAGE, e.getMessage()); } @Test void testMessageWhenArchiverNameIsNull_2() { - final IllegalArgumentException e = assertThrows(IllegalArgumentException.class, - () -> ArchiveStreamFactory.DEFAULT.createArchiveOutputStream(null, System.out), "Should raise an IllegalArgumentException."); + final ArchiveException e = assertThrows(ArchiveException.class, () -> ArchiveStreamFactory.DEFAULT.createArchiveOutputStream(null, System.out)); assertEquals(ARCHIVER_NULL_MESSAGE, e.getMessage()); } @Test void testMessageWhenInputStreamIsNull() { - final IllegalArgumentException e = assertThrows(IllegalArgumentException.class, - () -> ArchiveStreamFactory.DEFAULT.createArchiveInputStream("zip", null), "Should raise an IllegalArgumentException."); + final ArchiveException e = assertThrows(ArchiveException.class, () -> ArchiveStreamFactory.DEFAULT.createArchiveInputStream("zip", null)); assertEquals(INPUTSTREAM_NULL_MESSAGE, e.getMessage()); } @Test void testMessageWhenOutputStreamIsNull() { - final IllegalArgumentException e = assertThrows(IllegalArgumentException.class, - () -> ArchiveStreamFactory.DEFAULT.createArchiveOutputStream("zip", null), "Should raise an IllegalArgumentException."); + final ArchiveException e = assertThrows(ArchiveException.class, () -> ArchiveStreamFactory.DEFAULT.createArchiveOutputStream("zip", null)); assertEquals(OUTPUTSTREAM_NULL_MESSAGE, e.getMessage()); } - } diff --git a/src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStreamTest.java b/src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStreamTest.java index 38097564f..3782cbef7 100644 --- a/src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStreamTest.java +++ b/src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveOutputStreamTest.java @@ -45,6 +45,7 @@ import org.apache.commons.compress.AbstractTest; import org.apache.commons.compress.archivers.ArchiveEntry; +import org.apache.commons.compress.archivers.ArchiveException; import org.apache.commons.compress.archivers.ArchiveOutputStream; import org.apache.commons.compress.archivers.ArchiveStreamFactory; import org.apache.commons.io.IOUtils; @@ -92,7 +93,7 @@ void testBigNumberErrorMode() throws Exception { t.setSize(0100000000000L); final ByteArrayOutputStream bos = new ByteArrayOutputStream(); try (TarArchiveOutputStream tos = new TarArchiveOutputStream(bos)) { - assertThrows(IllegalArgumentException.class, () -> tos.putArchiveEntry(t)); + assertThrows(ArchiveException.class, () -> tos.putArchiveEntry(t)); } } @@ -226,7 +227,7 @@ void testMaxFileSizeError() throws Exception { tos1.putArchiveEntry(t); t.setSize(0100000000000L); final TarArchiveOutputStream tos2 = new TarArchiveOutputStream(new ByteArrayOutputStream()); - assertThrows(RuntimeException.class, () -> tos2.putArchiveEntry(t), "Should have generated RuntimeException"); + assertThrows(ArchiveException.class, () -> tos2.putArchiveEntry(t)); } @Test @@ -235,7 +236,7 @@ void testOldEntryError() throws Exception { t.setSize(Integer.MAX_VALUE); t.setModTime(-1000); try (TarArchiveOutputStream tos = new TarArchiveOutputStream(new ByteArrayOutputStream())) { - assertThrows(RuntimeException.class, () -> tos.putArchiveEntry(t)); + assertThrows(ArchiveException.class, () -> tos.putArchiveEntry(t)); } }
