COMPRESS-644 -- do not allow zero by entries when detecting magic-less tar #386
Conversation
…tars -- look for the first non-directory entry with length > 0 and a valid checksum
Codecov Report
@@ Coverage Diff @@
## master #386 +/- ##
============================================
- Coverage 80.45% 80.44% -0.01%
- Complexity 6723 6724 +1
============================================
Files 343 343
Lines 25312 25316 +4
Branches 4107 4109 +2
============================================
+ Hits 20365 20366 +1
+ Misses 3366 3365 -1
- Partials 1581 1585 +4
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
src/test/java/org/apache/commons/compress/DetectArchiverTestCase.java
Outdated
Show resolved
Hide resolved
src/main/java/org/apache/commons/compress/archivers/ArchiveStreamFactory.java
Outdated
Show resolved
Hide resolved
| // COMPRESS-644 - do not allow zero byte file entries | ||
| TarArchiveEntry tae = tais.getNextTarEntry(); | ||
| //try to find the first non-directory entry within the first 10 entries. | ||
| int max = 10; |
There was a problem hiding this comment.
What makes 10 special? Needs a comment or a better a Javadoc comment for a new constant.
There was a problem hiding this comment.
Completely arbitrary. I improved documentation. I'm completely open to the idea of dropping this attempt to iterate through the first x directory entries.
ICO file - Based on PR #386, modified for old tar format - Add test for old tar format archive - Add test for empty TAR archives (which we can't detect)
There was a problem hiding this comment.
Hello @tballison
Thanks for your patience.
The PR breaks detecting old format tar archives in an improved test since this PR was created.
I tweaked the detection from this PR and pushed a different fix in git master.
| @@ -141,4 +145,14 @@ public void testEmptyJarArchive() throws Exception { | |||
| public void testEmptyZipArchive() throws Exception { | |||
| checkEmptyArchive("zip"); | |||
| } | |||
|
|
|||
| @Test | |||
| public void ignoreZeroByteEntryInTarDetect_COMPRESS644() throws IOException { | |||
There was a problem hiding this comment.
IOException is not thrown here, so it shouldn't be in the method signature.
| TarArchiveEntry tae = tais.getNextTarEntry(); | ||
| //try to find the first non-directory entry within the first 10 entries. | ||
| int count = 0; | ||
| while (tae != null && tae.isDirectory() && count++ < MAX_TAR_RECORDS) { |
There was a problem hiding this comment.
This causes detection to fail for src/test/resources/COMPRESS-612/test-times-star-folder.tar (once you rebase on git master which tests more files).
ICO file #386 More test fixtures
When relying on reading the first record from a tar file and then verifying the checksum as proof that the file is a tar file, require that the record have more than zero bytes.