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 6146a93548bcbd79ddeea11cff696ce21199ece5 Author: Gary Gregory <[email protected]> AuthorDate: Sun Aug 9 18:26:54 2026 -0400 Some TarArchiveEntry code now throw ArchiveException instead of IllegalArgumentException. - Add and use ArchiveException.requireNonNegative(int, String) - Add and use ArchiveException.requireNonNegative(int, Supplier<String>) - Add and use ArchiveException.requireNonNegative(long, String) - Add and use ArchiveException.requireNonNegative(long, Supplier<String>) - The following TarArchiveEntry methods now throw ArchiveException instead of IllegalArgumentException - setDataOffset(long) - setDevMajor(int) - setDevMinor(int) - setSize(long) --- .../compress/archivers/ArchiveException.java | 32 ++++ .../archivers/ar/ArArchiveInputStream.java | 3 +- .../compress/archivers/tar/TarArchiveEntry.java | 162 ++++++++++----------- .../commons/compress/archivers/tar/TarUtils.java | 25 ++-- .../archivers/tar/TarArchiveEntryTest.java | 8 +- 5 files changed, 127 insertions(+), 103 deletions(-) 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 3cd8dd31f..6c47ad5f0 100644 --- a/src/main/java/org/apache/commons/compress/archivers/ArchiveException.java +++ b/src/main/java/org/apache/commons/compress/archivers/ArchiveException.java @@ -81,6 +81,22 @@ public static int requireNonNegative(final int value, final String message) thro return value; } + /** + * Checks that the specified value is not negative and throws a customized {@link ArchiveException} if it is. + * + * @param value The value to check for negativity. + * @param message The detail message to be used in the event that a {@code ArchiveException} is thrown. + * @return {@code value} if not negative. + * @throws ArchiveException if {@code value} is negative. + * @since 1.29.0 + */ + public static int requireNonNegative(final int value, final Supplier<String> message) throws ArchiveException { + if (value < 0) { + throw new ArchiveException(message.get()); + } + return value; + } + /** * Checks that the specified value is not negative and throws a customized {@link ArchiveException} if it is. * @@ -97,6 +113,22 @@ public static long requireNonNegative(final long value, final String message) th return value; } + /** + * Checks that the specified value is not negative and throws a customized {@link ArchiveException} if it is. + * + * @param value The value to check for negativity. + * @param message The detail message to be used in the event that a {@code ArchiveException} is thrown. + * @return {@code value} if not negative. + * @throws ArchiveException if {@code value} is negative. + * @since 1.29.0 + */ + public static long requireNonNegative(final long value, final Supplier<String> message) throws ArchiveException { + if (value < 0) { + throw new ArchiveException(message.get()); + } + return value; + } + /** * Checks that the specified object reference is not {@code null} and throws a customized {@link ArchiveException} if it is. * * diff --git a/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStream.java b/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStream.java index 7dacf6df8..b25c71640 100644 --- a/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStream.java +++ b/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStream.java @@ -420,8 +420,7 @@ private boolean isGNULongName(final String name) { private ArArchiveEntry parseEntry(final byte[] headerBuf) throws IOException { // Parse the entry metadata from the header buffer try { - final String name = - ArchiveUtils.toAsciiString(headerBuf, NAME_OFFSET, NAME_LEN).trim(); + final String name = ArchiveUtils.toAsciiString(headerBuf, NAME_OFFSET, NAME_LEN).trim(); final long length = asLong(headerBuf, LENGTH_OFFSET, LENGTH_LEN); // The remaining fields in the GNU string table entry are not used and may be blank. if (GNU_STRING_TABLE_NAME.equals(name)) { diff --git a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java index 34159b13d..335c66c73 100644 --- a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java +++ b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java @@ -280,7 +280,7 @@ private static String normalizeFileName(String fileName, final boolean preserveA return fileName; } - private static Instant parseInstantFromDecimalSeconds(final String value) throws IOException { + private static Instant parseInstantFromDecimalSeconds(final String value) throws ArchiveException { // Validate field values to prevent denial of service attacks with BigDecimal values (see JDK-6560193) if (!PAX_EXTENDED_HEADER_FILE_TIMES_PATTERN.matcher(value).matches()) { throw new ArchiveException("Corrupted PAX header. Time field value is invalid '%s'", value); @@ -297,18 +297,20 @@ private static Instant parseInstantFromDecimalSeconds(final String value) throws } } - private static int requireNonNegative(final int value, final Supplier<String> message) { - if (value < 0) { - throw new IllegalArgumentException(message.get()); + private static long parseOctalOrBinary(final byte[] header, final int offset, final int length, final boolean lenient) { + if (lenient) { + try { + return TarUtils.parseOctalOrBinary(header, offset, length); + } catch (final IllegalArgumentException ex) { // NOSONAR + return UNKNOWN; + } } - return value; + return TarUtils.parseOctalOrBinary(header, offset, length); } - private static long requireNonNegative(final long value, final Supplier<String> message) { - if (value < 0) { - throw new IllegalArgumentException(message.get()); - } - return value; + private static int parseOctalOrBinaryAsInt(final byte[] header, final int offset, final int length, final boolean lenient) throws ArchiveException { + //return ArchiveException.toIntExact(parseOctalOrBinary(header, offset, length, lenient)); + return (int) parseOctalOrBinary(header, offset, length, lenient); } /** The entry's name. */ @@ -1443,17 +1445,6 @@ private boolean isXstar(final Map<String, String> globalPaxHeaders, final byte[] return true; } - private long parseOctalOrBinary(final byte[] header, final int offset, final int length, final boolean lenient) { - if (lenient) { - try { - return TarUtils.parseOctalOrBinary(header, offset, length); - } catch (final IllegalArgumentException ex) { // NOSONAR - return UNKNOWN; - } - } - return TarUtils.parseOctalOrBinary(header, offset, length); - } - /** * Parses an entry's header information from a header buffer. * @@ -1515,13 +1506,13 @@ private int parseTarHeaderBlock(final byte[] header, final ZipEncoding encoding, int offset = 0; name = oldStyle ? TarUtils.parseName(header, offset, NAMELEN) : TarUtils.parseName(header, offset, NAMELEN, encoding); offset += NAMELEN; - mode = (int) parseOctalOrBinary(header, offset, MODELEN, lenient); + mode = parseOctalOrBinaryAsInt(header, offset, MODELEN, lenient); offset += MODELEN; - userId = (int) parseOctalOrBinary(header, offset, UIDLEN, lenient); + userId = parseOctalOrBinaryAsInt(header, offset, UIDLEN, lenient); offset += UIDLEN; - groupId = (int) parseOctalOrBinary(header, offset, GIDLEN, lenient); + groupId = parseOctalOrBinaryAsInt(header, offset, GIDLEN, lenient); offset += GIDLEN; - size = TarUtils.parseOctalOrBinary(header, offset, SIZELEN); + setSize(TarUtils.parseOctalOrBinary(header, offset, SIZELEN)); ArchiveException.requireNonNegative(size, "Broken archive, entry with negative size"); offset += SIZELEN; mTime = FileTimes.fromUnixTime(parseOctalOrBinary(header, offset, MODTIMELEN, lenient)); @@ -1568,61 +1559,57 @@ private void parseUstarHeaderBlock(final Map<String, String> globalPaxHeaders, f groupName = oldStyle ? TarUtils.parseName(header, offset, GNAMELEN) : TarUtils.parseName(header, offset, GNAMELEN, encoding); offset += GNAMELEN; if (linkFlag == LF_CHR || linkFlag == LF_BLK) { - devMajor = (int) parseOctalOrBinary(header, offset, DEVLEN, lenient); + setDevMajor(parseOctalOrBinaryAsInt(header, offset, DEVLEN, lenient)); offset += DEVLEN; - devMinor = (int) parseOctalOrBinary(header, offset, DEVLEN, lenient); + setDevMinor(parseOctalOrBinaryAsInt(header, offset, DEVLEN, lenient)); offset += DEVLEN; } else { offset += 2 * DEVLEN; } final int type = evaluateType(globalPaxHeaders, header); switch (type) { - // GNU format as before 1.12 - case FORMAT_OLDGNU: { - aTime = fileTimeFromOptionalSeconds(parseOctalOrBinary(header, offset, ATIMELEN_GNU, lenient)); - offset += ATIMELEN_GNU; - cTime = fileTimeFromOptionalSeconds(parseOctalOrBinary(header, offset, CTIMELEN_GNU, lenient)); - offset += CTIMELEN_GNU; - offset += OFFSETLEN_GNU; - offset += LONGNAMESLEN_GNU; - offset += PAD2LEN_GNU; - sparseHeaders = - new ArrayList<>(TarUtils.readSparseStructs(header, offset, SPARSE_HEADERS_IN_OLDGNU_HEADER)); - offset += SPARSELEN_GNU; - isExtended = TarUtils.parseBoolean(header, offset); - offset += ISEXTENDEDLEN_GNU; - realSize = TarUtils.parseOctal(header, offset, REALSIZELEN_GNU, "parseUstarHeaderBlock()", false); - break; + // GNU format as before 1.12 + case FORMAT_OLDGNU: { + aTime = fileTimeFromOptionalSeconds(parseOctalOrBinary(header, offset, ATIMELEN_GNU, lenient)); + offset += ATIMELEN_GNU; + cTime = fileTimeFromOptionalSeconds(parseOctalOrBinary(header, offset, CTIMELEN_GNU, lenient)); + offset += CTIMELEN_GNU; + offset += OFFSETLEN_GNU; + offset += LONGNAMESLEN_GNU; + offset += PAD2LEN_GNU; + sparseHeaders = new ArrayList<>(TarUtils.readSparseStructs(header, offset, SPARSE_HEADERS_IN_OLDGNU_HEADER)); + offset += SPARSELEN_GNU; + isExtended = TarUtils.parseBoolean(header, offset); + offset += ISEXTENDEDLEN_GNU; + realSize = TarUtils.parseOctal(header, offset, REALSIZELEN_GNU, "parseUstarHeaderBlock()", false); + break; + } + // Star format (Schily tar) + case FORMAT_XSTAR: { + final String xstarPrefix = oldStyle ? TarUtils.parseName(header, offset, PREFIXLEN_XSTAR) + : TarUtils.parseName(header, offset, PREFIXLEN_XSTAR, encoding); + offset += PREFIXLEN_XSTAR; + if (!xstarPrefix.isEmpty()) { + name = xstarPrefix + "/" + name; } - // Star format (Schily tar) - case FORMAT_XSTAR: { - final String xstarPrefix = oldStyle - ? TarUtils.parseName(header, offset, PREFIXLEN_XSTAR) - : TarUtils.parseName(header, offset, PREFIXLEN_XSTAR, encoding); - offset += PREFIXLEN_XSTAR; - if (!xstarPrefix.isEmpty()) { - name = xstarPrefix + "/" + name; - } - aTime = fileTimeFromOptionalSeconds(parseOctalOrBinary(header, offset, ATIMELEN_XSTAR, lenient)); - offset += ATIMELEN_XSTAR; - cTime = fileTimeFromOptionalSeconds(parseOctalOrBinary(header, offset, CTIMELEN_XSTAR, lenient)); - break; + aTime = fileTimeFromOptionalSeconds(parseOctalOrBinary(header, offset, ATIMELEN_XSTAR, lenient)); + offset += ATIMELEN_XSTAR; + cTime = fileTimeFromOptionalSeconds(parseOctalOrBinary(header, offset, CTIMELEN_XSTAR, lenient)); + break; + } + // Pure POSIX.1-1988 UStar format + case FORMAT_POSIX: + default: { + final String prefix = oldStyle ? TarUtils.parseName(header, offset, PREFIXLEN) : TarUtils.parseName(header, offset, PREFIXLEN, encoding); + // SunOS tar -E does not add / to directory names, so fix up to be consistent + if (isDirectory() && !name.endsWith("/")) { + name += "/"; } - // Pure POSIX.1-1988 UStar format - case FORMAT_POSIX: - default: { - final String prefix = oldStyle - ? TarUtils.parseName(header, offset, PREFIXLEN) - : TarUtils.parseName(header, offset, PREFIXLEN, encoding); - // SunOS tar -E does not add / to directory names, so fix up to be consistent - if (isDirectory() && !name.endsWith("/")) { - name += "/"; - } - if (!prefix.isEmpty()) { - name = prefix + "/" + name; - } + if (!prefix.isEmpty()) { + name = prefix + "/" + name; } } + } } /** @@ -1642,7 +1629,7 @@ private void processPaxHeader(final String key, final String val) throws IOExcep * @param key The header name. * @param val The header value. * @param headers map of headers used for dealing with sparse file. - * @throws NumberFormatException if encountered errors when parsing the numbers. + * @throws IOException if encountered errors when parsing the numbers. * @since 1.15 */ private void processPaxHeader(final String key, final String val, final Map<String, String> headers) throws IOException { @@ -1676,7 +1663,7 @@ private void processPaxHeader(final String key, final String val, final Map<Stri setUserName(val); break; case "size": - setSize(ArchiveException.requireNonNegative(ParsingUtils.parseLongValue(val), "Corrupted TAR archive. Entry size is negative")); + setSize(ParsingUtils.parseLongValue(val)); break; case "mtime": setLastModifiedTime(FileTime.from(parseInstantFromDecimalSeconds(val))); @@ -1691,10 +1678,10 @@ private void processPaxHeader(final String key, final String val, final Map<Stri setCreationTime(FileTime.from(parseInstantFromDecimalSeconds(val))); break; case "SCHILY.devminor": - setDevMinor(ArchiveException.requireNonNegative(ParsingUtils.parseIntValue(val), "Corrupted TAR archive. Dev-Minor is negative")); + setDevMinor(ParsingUtils.parseIntValue(val)); break; case "SCHILY.devmajor": - setDevMajor(ArchiveException.requireNonNegative(ParsingUtils.parseIntValue(val), "Corrupted TAR archive. Dev-Major is negative")); + setDevMajor(ParsingUtils.parseIntValue(val)); break; case TarGnuSparseKeys.SIZE: fillGNUSparse0xData(headers); @@ -1778,32 +1765,33 @@ public void setCreationTime(final FileTime birthTime) { * Sets the offset of the data for the tar entry. * * @param dataOffset The position of the data in the tar. + * @throws ArchiveException if the dataOffset is < 0. * @since 1.21 */ - public void setDataOffset(final long dataOffset) { - this.dataOffset = requireNonNegative(dataOffset, () -> "The offset cannot be smaller than 0"); + public void setDataOffset(final long dataOffset) throws ArchiveException { + this.dataOffset = ArchiveException.requireNonNegative(dataOffset, (Supplier<String>) () -> "The offset cannot be smaller than 0"); } /** * Sets this entry's major device number. * * @param devNo This entry's major device number. - * @throws IllegalArgumentException if the devNo is < 0. + * @throws ArchiveException if the devNo is < 0. * @since 1.4 */ - public void setDevMajor(final int devNo) { - this.devMajor = requireNonNegative(devNo, () -> "Major device number is out of range: " + devNo); + public void setDevMajor(final int devNo) throws ArchiveException { + this.devMajor = ArchiveException.requireNonNegative(devNo, (Supplier<String>) () -> "Major device number is out of range: " + devNo); } /** * Sets this entry's minor device number. * * @param devNo This entry's minor device number. - * @throws IllegalArgumentException if the devNo is < 0. + * @throws ArchiveException if the devNo is < 0. * @since 1.4 */ - public void setDevMinor(final int devNo) { - this.devMinor = requireNonNegative(devNo, () -> "Minor device number is out of range: " + devNo); + public void setDevMinor(final int devNo) throws ArchiveException { + this.devMinor = ArchiveException.requireNonNegative(devNo, (Supplier<String>) () -> "Minor device number is out of range: " + devNo); } /** @@ -1939,10 +1927,10 @@ public void setNames(final String userName, final String groupName) { * Sets this entry's file size. * * @param size This entry's new file size. - * @throws IllegalArgumentException if the size is < 0. + * @throws ArchiveException if the size is < 0. */ - public void setSize(final long size) { - this.size = requireNonNegative(size, () -> "Size is out of range: " + size); + public void setSize(final long size) throws ArchiveException { + this.size = ArchiveException.requireNonNegative(size, (Supplier<String>) () -> "Size is out of range: " + size); } /** @@ -1958,11 +1946,11 @@ public void setSparseHeaders(final List<TarArchiveStructSparse> sparseHeaders) { /** * Sets this entry's status change time. * - * @param time This entry's new status change time. + * @param cTime This entry's new status change time. * @since 1.22 */ - public void setStatusChangeTime(final FileTime time) { - cTime = time; + public void setStatusChangeTime(final FileTime cTime) { + this.cTime = cTime; } /** diff --git a/src/main/java/org/apache/commons/compress/archivers/tar/TarUtils.java b/src/main/java/org/apache/commons/compress/archivers/tar/TarUtils.java index c5e01ff4a..d7aa17d4e 100644 --- a/src/main/java/org/apache/commons/compress/archivers/tar/TarUtils.java +++ b/src/main/java/org/apache/commons/compress/archivers/tar/TarUtils.java @@ -50,18 +50,10 @@ */ public final class TarUtils { - private static final Pattern HEADER_STRINGS_PATTERN = Pattern.compile(","); - - private static final BigInteger NEG_1_BIG_INT = BigInteger.valueOf(-1); - - private static final int BYTE_MASK = 255; - - static final ZipEncoding DEFAULT_ENCODING = ZipEncodingHelper.getZipEncoding(Charset.defaultCharset()); - /** * Encapsulates the algorithms used up to Commons Compress 1.3 as ZipEncoding. */ - static final ZipEncoding FALLBACK_ENCODING = new ZipEncoding() { + private static final class FallbackZipEncoding implements ZipEncoding { @Override public boolean canEncode(final String name) { @@ -85,7 +77,20 @@ public String decode(final byte[] buffer) { public ByteBuffer encode(final String name) { return ByteBuffer.wrap(ArchiveUtils.toAsciiBytes(name)); } - }; + } + + private static final Pattern HEADER_STRINGS_PATTERN = Pattern.compile(","); + + private static final BigInteger NEG_1_BIG_INT = BigInteger.valueOf(-1); + + private static final int BYTE_MASK = 255; + + static final ZipEncoding DEFAULT_ENCODING = ZipEncodingHelper.getZipEncoding(Charset.defaultCharset()); + + /** + * Encapsulates the algorithms used up to Commons Compress 1.3 as ZipEncoding. + */ + static final FallbackZipEncoding FALLBACK_ENCODING = new FallbackZipEncoding(); /** * Applies the PAX headers and sparse headers to the given tar entry. diff --git a/src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveEntryTest.java b/src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveEntryTest.java index 0f15c80dd..efe476608 100644 --- a/src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveEntryTest.java +++ b/src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveEntryTest.java @@ -214,11 +214,11 @@ void testLinuxFileInformationFromPath() throws IOException { } @Test - void testMaxFileSize() { + void testMaxFileSize() throws ArchiveException { final TarArchiveEntry t = new TarArchiveEntry(""); t.setSize(0); t.setSize(1); - assertThrows(IllegalArgumentException.class, () -> t.setSize(-1)); + assertThrows(ArchiveException.class, () -> t.setSize(-1)); t.setSize(077777777777L); t.setSize(0100000000000L); } @@ -248,13 +248,13 @@ void testNegativeOffsetInConstructorNotAllowed() { + "\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000" + "\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000\u0000").getBytes(UTF_8); // @formatter:on - assertThrows(IllegalArgumentException.class, + assertThrows(ArchiveException.class, () -> new TarArchiveEntry(entryContent, ZipEncodingHelper.getZipEncoding(StandardCharsets.ISO_8859_1.name()), false, -1)); } @Test void testNegativeOffsetInSetterNotAllowed() { - assertThrows(IllegalArgumentException.class, () -> new TarArchiveEntry("test").setDataOffset(-1)); + assertThrows(ArchiveException.class, () -> new TarArchiveEntry("test").setDataOffset(-1)); } @Test
