This is an automated email from the ASF dual-hosted git repository. peterlee pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-compress.git
The following commit(s) were added to refs/heads/master by this push: new 36773d9 COMPRESS-529 : properly throw Exceptions for tar 36773d9 is described below commit 36773d948e9a220c1dc3abec3d2028b0879a7766 Author: PeterAlfredLee <peteralfred...@gmail.com> AuthorDate: Mon Jun 1 16:48:05 2020 +0800 COMPRESS-529 : properly throw Exceptions for tar Throw expected IOException instead of NumberFormatException if it encounters non-numbers in tar pax headers. Throw IllegalArgumentException if the file name is too long with the default long file mode LONGFILE_ERROR --- src/changes/changes.xml | 8 ++++++++ .../commons/compress/archivers/tar/TarArchiveEntry.java | 1 + .../compress/archivers/tar/TarArchiveInputStream.java | 12 ++++++++---- .../compress/archivers/tar/TarArchiveOutputStream.java | 5 ++++- .../archivers/tar/TarArchiveInputStreamTest.java | 8 ++++++++ .../archivers/tar/TarArchiveOutputStreamTest.java | 10 ++++++++++ src/test/resources/COMPRESS-529.tar | Bin 0 -> 1536 bytes 7 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 9c6d8b5..0c0e3f2 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -108,6 +108,14 @@ The <action> type attribute can be add,update,fix,remove. throw the expected IOException rather than obscure RuntimeExceptions. </action> + <action issue="COMPRESS-529" type="fix" date="2020-06-01"> + Throw expected IOException instead of NumberFormatException if + it encounters non-numbers when parsing pax headers for tarball. + + Throw IllegalArgumentException instead of RuntimeExceptions if + the file name is longer than 100 bytes with the longFileMode + of LONGFILE_ERROR, and address this in java docs. + </action> </release> <release version="1.20" date="2020-02-08" description="Release 1.20"> 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 f180780..d562336 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 @@ -1112,6 +1112,7 @@ public class TarArchiveEntry implements ArchiveEntry, TarConstants { * @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 * @since 1.15 */ private void processPaxHeader(String key, String val, Map<String, String> headers) { diff --git a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveInputStream.java b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveInputStream.java index c112ac7..320fc9d 100644 --- a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveInputStream.java +++ b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveInputStream.java @@ -406,10 +406,14 @@ public class TarArchiveInputStream extends ArchiveInputStream { readGlobalPaxHeaders(); } - if (currEntry.isPaxHeader()){ // Process Pax headers - paxHeaders(); - } else if (!globalPaxHeaders.isEmpty()) { - applyPaxHeadersToCurrentEntry(globalPaxHeaders, globalSparseHeaders); + try { + if (currEntry.isPaxHeader()){ // Process Pax headers + paxHeaders(); + } else if (!globalPaxHeaders.isEmpty()) { + applyPaxHeadersToCurrentEntry(globalPaxHeaders, globalSparseHeaders); + } + } catch (NumberFormatException e) { + throw new IOException("Error detected parsing the pax header", e); } if (currEntry.isOldGNUSparse()){ // Process sparse files 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 758f673..0816976 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 @@ -654,6 +654,9 @@ public class TarArchiveOutputStream extends ArchiveOutputStream { * @param paxHeaderName name of the pax header to write * @param linkType type of the GNU entry to write * @param fieldName the name of the field + * @throws IllegalArgumentException if the {@link TarArchiveOutputStream#longFileMode} equals + * {@link TarArchiveOutputStream#LONGFILE_ERROR} and the file + * name is too long * @return whether a pax header has been written. */ private boolean handleLongName(final TarArchiveEntry entry, final String name, @@ -680,7 +683,7 @@ public class TarArchiveOutputStream extends ArchiveOutputStream { write(0); // NUL terminator closeArchiveEntry(); } else if (longFileMode != LONGFILE_TRUNCATE) { - throw new RuntimeException(fieldName + " '" + name //NOSONAR + throw new IllegalArgumentException(fieldName + " '" + name //NOSONAR + "' is too long ( > " + TarConstants.NAMELEN + " bytes)"); } diff --git a/src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveInputStreamTest.java b/src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveInputStreamTest.java index 095ec2c..df404ec 100644 --- a/src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveInputStreamTest.java +++ b/src/test/java/org/apache/commons/compress/archivers/tar/TarArchiveInputStreamTest.java @@ -433,6 +433,14 @@ public class TarArchiveInputStreamTest extends AbstractTestCase { } } + @Test(expected = IOException.class) + public void testParseTarWithNonNumberPaxHeaders() throws IOException { + try (FileInputStream in = new FileInputStream(getFile("COMPRESS-529.tar")); + TarArchiveInputStream archive = new TarArchiveInputStream(in)) { + archive.getNextEntry(); + } + } + private TarArchiveInputStream getTestStream(final String name) { return new TarArchiveInputStream( TarArchiveInputStreamTest.class.getResourceAsStream(name)); 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 3bdfe9d..71f49cd 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 @@ -777,6 +777,16 @@ public class TarArchiveOutputStreamTest extends AbstractTestCase { tarIn.close(); } + @Test(expected = IllegalArgumentException.class) + public void testWriteLongFileNameThrowsException() throws Exception { + final String n = "01234567890123456789012345678901234567890123456789" + + "01234567890123456789012345678901234567890123456789" + + "01234567890123456789012345678901234567890123456789"; + final TarArchiveEntry t = new TarArchiveEntry(n); + final TarArchiveOutputStream tos = new TarArchiveOutputStream(new ByteArrayOutputStream(), "ASCII"); + tos.putArchiveEntry(t); + } + private static byte[] createTarArchiveContainingOneDirectory(final String fname, final Date modificationDate) throws IOException { final ByteArrayOutputStream baos = new ByteArrayOutputStream(); diff --git a/src/test/resources/COMPRESS-529.tar b/src/test/resources/COMPRESS-529.tar new file mode 100644 index 0000000..351908d Binary files /dev/null and b/src/test/resources/COMPRESS-529.tar differ