garydgregory commented on code in PR #715:
URL: https://github.com/apache/commons-compress/pull/715#discussion_r2434149606


##########
src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveInputStream.java:
##########
@@ -820,37 +731,15 @@ public long skip(final long n) throws IOException {
      * @throws IOException if a truncated tar archive is detected.
      */
     private void skipRecordPadding() throws IOException {
-        if (!isDirectory() && this.entrySize > 0 && this.entrySize % 
getRecordSize() != 0) {
-            final long available = in.available();
-            final long numRecords = this.entrySize / getRecordSize() + 1;
-            final long padding = numRecords * getRecordSize() - this.entrySize;
-            long skipped = IOUtils.skip(in, padding);
-            skipped = getActuallySkipped(available, skipped, padding);
+        final long entrySize = currEntry != null ? currEntry.getSize() : 0;
+        if (!isDirectory() && entrySize > 0 && entrySize % getRecordSize() != 
0) {
+            final long padding = getRecordSize() - (entrySize % 
getRecordSize());
+            final long skipped = org.apache.commons.io.IOUtils.skip(in, 
padding);
             count(skipped);
-        }
-    }
-
-    /**
-     * Skip n bytes from current input stream, if the current input stream 
doesn't have enough data to skip, jump to the next input stream and skip the 
rest
-     * bytes, keep doing this until total n bytes are skipped or the input 
streams are all skipped
-     *
-     * @param n bytes of data to skip.
-     * @return actual bytes of data skipped.
-     * @throws IOException if an I/O error occurs.
-     */
-    private long skipSparse(final long n) throws IOException {
-        if (sparseInputStreams == null || sparseInputStreams.isEmpty()) {
-            return in.skip(n);
-        }
-        long bytesSkipped = 0;
-        while (bytesSkipped < n && currentSparseInputStreamIndex < 
sparseInputStreams.size()) {
-            final InputStream currentInputStream = 
sparseInputStreams.get(currentSparseInputStreamIndex);
-            bytesSkipped += currentInputStream.skip(n - bytesSkipped);
-            if (bytesSkipped < n) {
-                currentSparseInputStreamIndex++;
+            if (skipped != padding) {
+                throw new EOFException(String.format("Truncated TAR archive: 
failed to skip record padding for entry '%s'", currEntry.getName()));

Review Comment:
   > If the number of padding bytes doesn’t match what we expect,
   
   Hm, but does that mean we are actually very near the end of the stream/file, 
or, could it be that we are in the middle of it, or even toward the start, and 
that the archive is completely corrupted?
   
   If we know we are at the or near the end, then EOFE seems OK, but not if we 
are in the middle of the file and we are really in a broken file situation. 
   
   Not that big of a deal, but it's nice to know the EOF really means "I fell 
off the end".
   



##########
src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveInputStream.java:
##########
@@ -820,37 +731,15 @@ public long skip(final long n) throws IOException {
      * @throws IOException if a truncated tar archive is detected.
      */
     private void skipRecordPadding() throws IOException {
-        if (!isDirectory() && this.entrySize > 0 && this.entrySize % 
getRecordSize() != 0) {
-            final long available = in.available();
-            final long numRecords = this.entrySize / getRecordSize() + 1;
-            final long padding = numRecords * getRecordSize() - this.entrySize;
-            long skipped = IOUtils.skip(in, padding);
-            skipped = getActuallySkipped(available, skipped, padding);
+        final long entrySize = currEntry != null ? currEntry.getSize() : 0;
+        if (!isDirectory() && entrySize > 0 && entrySize % getRecordSize() != 
0) {
+            final long padding = getRecordSize() - (entrySize % 
getRecordSize());
+            final long skipped = org.apache.commons.io.IOUtils.skip(in, 
padding);
             count(skipped);
-        }
-    }
-
-    /**
-     * Skip n bytes from current input stream, if the current input stream 
doesn't have enough data to skip, jump to the next input stream and skip the 
rest
-     * bytes, keep doing this until total n bytes are skipped or the input 
streams are all skipped
-     *
-     * @param n bytes of data to skip.
-     * @return actual bytes of data skipped.
-     * @throws IOException if an I/O error occurs.
-     */
-    private long skipSparse(final long n) throws IOException {
-        if (sparseInputStreams == null || sparseInputStreams.isEmpty()) {
-            return in.skip(n);
-        }
-        long bytesSkipped = 0;
-        while (bytesSkipped < n && currentSparseInputStreamIndex < 
sparseInputStreams.size()) {
-            final InputStream currentInputStream = 
sparseInputStreams.get(currentSparseInputStreamIndex);
-            bytesSkipped += currentInputStream.skip(n - bytesSkipped);
-            if (bytesSkipped < n) {
-                currentSparseInputStreamIndex++;
+            if (skipped != padding) {
+                throw new EOFException(String.format("Truncated TAR archive: 
failed to skip record padding for entry '%s'", currEntry.getName()));

Review Comment:
   > If the number of padding bytes doesn’t match what we expect,
   
   Hm, but does that mean we are actually very near the end of the stream/file, 
or, could it be that we are in the middle of it, or even toward the start, and 
that the archive is completely corrupted?
   
   If we know we are at or near the end, then EOFE seems OK, but not if we are 
in the middle of the file and we are really in a broken file situation. 
   
   Not that big of a deal, but it's nice to know the EOF really means "I fell 
off the end".
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to