You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2026/08/09 22:47:45 UTC
(commons-compress) 02/04: Some TarArchiveEntry code now throw ArchiveException instead of IllegalArgumentException.
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 <ga...@gmail.com>
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