You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by pe...@apache.org on 2020/04/22 03:09:43 UTC

[commons-compress] branch master updated: COMPRESS-510 : fix for multiple retrievals of InputStream for same SevenZFile entry fails

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 69de512  COMPRESS-510 : fix for multiple retrievals of InputStream for same SevenZFile entry fails
69de512 is described below

commit 69de512db43c9ca35da11664a1502702353a6fdd
Author: PeterAlfredLee <pe...@gmail.com>
AuthorDate: Wed Apr 22 11:08:22 2020 +0800

    COMPRESS-510 : fix for multiple retrievals of InputStream for same SevenZFile entry fails
    
    fix for multiple retrievals of InputStream for same SevenZFile entry fails
---
 src/changes/changes.xml                            |  2 +-
 .../compress/archivers/sevenz/SevenZFile.java      | 47 +++++++++++++---------
 .../commons/compress/utils/BoundedInputStream.java |  8 ++++
 .../utils/ChecksumVerifyingInputStream.java        |  4 ++
 .../compress/archivers/sevenz/SevenZFileTest.java  | 24 +++++++++++
 5 files changed, 65 insertions(+), 20 deletions(-)

diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 90804e8..4ebd470 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -62,7 +62,7 @@ The <action> type attribute can be add,update,fix,remove.
       </action>
       <action issue="COMPRESS-510" type="fix" date="2020-04-18">
         Fix bugs in random access of 7z. Exceptions are thrown
-        when reading the first entry multiable times by random
+        when reading the first entry multiple times by random
         access.
       </action>
     </release>
diff --git a/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java b/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java
index 04a01e8..5de9667 100644
--- a/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java
+++ b/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java
@@ -1267,25 +1267,9 @@ public class SevenZFile implements Closeable {
      */
     private boolean skipEntriesWhenNeeded(int entryIndex, boolean isInSameFolder, int folderIndex) throws IOException {
         final SevenZArchiveEntry file = archive.files[entryIndex];
-        final boolean isNeedToSkipEntries;
-        boolean hasCurrentEntryBeenRead = false;
-        if (currentEntryIndex != entryIndex) {
-            // this means there are some entries to be skipped(currentEntryIndex < entryIndex)
-            // or the entry has already been read(currentEntryIndex > entryIndex)
-            isNeedToSkipEntries = true;
-        } else {
-            if (deferredBlockStreams.size() > 0) {
-                CRC32VerifyingInputStream currentEntryInputStream = (CRC32VerifyingInputStream) deferredBlockStreams.get(deferredBlockStreams.size() - 1);
-                hasCurrentEntryBeenRead = currentEntryInputStream.getBytesRemaining() != archive.files[currentEntryIndex].getSize();
-            }
-
-            // if the entry to be read is the current entry, but some data of it has
-            // been read before, then we need to reopen the stream of the folder and
-            // skip all the entries before the current entries
-            isNeedToSkipEntries = hasCurrentEntryBeenRead;
-        }
-
-        if (!isNeedToSkipEntries) {
+        // if the entry to be read is the current entry, and the entry has not
+        // been read yet, then there's nothing we need to do
+        if (currentEntryIndex == entryIndex && !hasCurrentEntryBeenRead()) {
             return false;
         }
 
@@ -1321,6 +1305,31 @@ public class SevenZFile implements Closeable {
         return true;
     }
 
+    /**
+     * Find out if any data of current entry has been read or not.
+     * This is achieved by comparing the bytes remaining to read
+     * and the size of the file.
+     *
+     * @return true if any data of current entry has been read
+     * @since 1.21
+     */
+    private boolean hasCurrentEntryBeenRead() {
+        boolean hasCurrentEntryBeenRead = false;
+        if (deferredBlockStreams.size() > 0) {
+            InputStream currentEntryInputStream = deferredBlockStreams.get(deferredBlockStreams.size() - 1);
+            // get the bytes remaining to read, and compare it with the size of
+            // the file to figure out if the file has been read
+            if (currentEntryInputStream instanceof CRC32VerifyingInputStream) {
+                hasCurrentEntryBeenRead = ((CRC32VerifyingInputStream) currentEntryInputStream).getBytesRemaining() != archive.files[currentEntryIndex].getSize();
+            }
+
+            if (currentEntryInputStream instanceof BoundedInputStream) {
+                hasCurrentEntryBeenRead = ((BoundedInputStream) currentEntryInputStream).getBytesRemaining() != archive.files[currentEntryIndex].getSize();
+            }
+        }
+        return hasCurrentEntryBeenRead;
+    }
+
     private InputStream buildDecoderStack(final Folder folder, final long folderOffset,
                 final int firstPackStreamIndex, final SevenZArchiveEntry entry) throws IOException {
         channel.position(folderOffset);
diff --git a/src/main/java/org/apache/commons/compress/utils/BoundedInputStream.java b/src/main/java/org/apache/commons/compress/utils/BoundedInputStream.java
index b6b3622..1db436a 100644
--- a/src/main/java/org/apache/commons/compress/utils/BoundedInputStream.java
+++ b/src/main/java/org/apache/commons/compress/utils/BoundedInputStream.java
@@ -85,4 +85,12 @@ public class BoundedInputStream extends InputStream {
 
         return bytesSkipped;
     }
+
+    /**
+     * @return bytes remaining to read
+     * @since 1.21
+     */
+    public long getBytesRemaining() {
+        return bytesRemaining;
+    }
 }
diff --git a/src/main/java/org/apache/commons/compress/utils/ChecksumVerifyingInputStream.java b/src/main/java/org/apache/commons/compress/utils/ChecksumVerifyingInputStream.java
index 1887334..80bbabc 100644
--- a/src/main/java/org/apache/commons/compress/utils/ChecksumVerifyingInputStream.java
+++ b/src/main/java/org/apache/commons/compress/utils/ChecksumVerifyingInputStream.java
@@ -110,6 +110,10 @@ public class ChecksumVerifyingInputStream extends InputStream {
         in.close();
     }
 
+    /**
+     * @return bytes remaining to read
+     * @since 1.21
+     */
     public long getBytesRemaining() {
         return bytesRemaining;
     }
diff --git a/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZFileTest.java b/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZFileTest.java
index 68b478c..0839d0e 100644
--- a/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZFileTest.java
+++ b/src/test/java/org/apache/commons/compress/archivers/sevenz/SevenZFileTest.java
@@ -24,7 +24,9 @@ import java.io.File;
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
+import java.nio.file.Path;
 import java.security.NoSuchAlgorithmException;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -684,6 +686,28 @@ public class SevenZFileTest extends AbstractTestCase {
         }
     }
 
+    @Test
+    public void retrieveInputStreamForAllEntriesWithoutCRCMultipleTimes() throws IOException {
+        try (final SevenZOutputFile out = new SevenZOutputFile(new File(dir, "test.7z"))) {
+            final Path inputFile = Files.createTempFile("SevenZTestTemp", "");
+
+            SevenZArchiveEntry entry = out.createArchiveEntry(inputFile.toFile(), "test.txt");
+            out.putArchiveEntry(entry);
+            out.write("Test".getBytes(StandardCharsets.UTF_8));
+            out.closeArchiveEntry();
+
+            Files.deleteIfExists(inputFile);
+        }
+
+        try (SevenZFile sevenZFile = new SevenZFile(new File(dir, "test.7z"))) {
+            for (SevenZArchiveEntry entry : sevenZFile.getEntries()) {
+                byte[] firstRead = IOUtils.toByteArray(sevenZFile.getInputStream(entry));
+                byte[] secondRead = IOUtils.toByteArray(sevenZFile.getInputStream(entry));
+                assertArrayEquals(firstRead, secondRead);
+            }
+        }
+    }
+
     private void test7zUnarchive(final File f, final SevenZMethod m, final byte[] password) throws Exception {
         try (SevenZFile sevenZFile = new SevenZFile(f, password)) {
             test7zUnarchive(sevenZFile, m);