You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/08/12 10:26:24 UTC

[GitHub] [commons-compress] dweiss commented on a diff in pull request #306: COMPRESS-623: Speed up ZipFile's raw stream copying by not seeking for local headers until they're needed

dweiss commented on code in PR #306:
URL: https://github.com/apache/commons-compress/pull/306#discussion_r944253806


##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java:
##########
@@ -595,11 +598,16 @@ public InputStream getRawInputStream(final ZipArchiveEntry ze) {
         if (!(ze instanceof Entry)) {
             return null;
         }
-        final long start = ze.getDataOffset();
-        if (start == EntryStreamOffsets.OFFSET_UNKNOWN) {
-            return null;
+
+        try {
+            final long start = getDataOffset(ze);
+            if (start == EntryStreamOffsets.OFFSET_UNKNOWN) {
+                return null;
+            }
+            return createBoundedInputStream(start, ze.getCompressedSize());
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);

Review Comment:
   It'd be probably cleaner to change the signature of getRawInputStream to throw IOException but this would be backward-incompatible.



##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java:
##########
@@ -595,11 +598,16 @@ public InputStream getRawInputStream(final ZipArchiveEntry ze) {
         if (!(ze instanceof Entry)) {
             return null;
         }
-        final long start = ze.getDataOffset();
-        if (start == EntryStreamOffsets.OFFSET_UNKNOWN) {
-            return null;
+
+        try {
+            final long start = getDataOffset(ze);
+            if (start == EntryStreamOffsets.OFFSET_UNKNOWN) {
+                return null;
+            }
+            return createBoundedInputStream(start, ze.getCompressedSize());
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);

Review Comment:
   I also don't know but it seems like the returned offset from getDataOffset should never be unknown so the check there maybe removed.



##########
src/main/java/org/apache/commons/compress/archivers/zip/ZipFile.java:
##########
@@ -613,7 +621,7 @@ public InputStream getRawInputStream(final ZipArchiveEntry ze) {
      * @throws IOException on error
      */
     public void copyRawEntries(final ZipArchiveOutputStream target, final ZipArchiveEntryPredicate predicate)
-            throws IOException {
+        throws IOException {

Review Comment:
   Sorry for whitespace changes - I can revert these (any way to reformat automatically to conform to the convention used?).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org