You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "OldTruckDriver (via GitHub)" <gi...@apache.org> on 2026/06/17 15:58:30 UTC

[PR] [COMPRESS-724] Fix PAX 1.x sparse header alignment [commons-compress]

OldTruckDriver opened a new pull request, #775:
URL: https://github.com/apache/commons-compress/pull/775

   ## Summary
   
   Fix `TarUtils.parsePAX1XSparseHeaders(InputStream, int)` so it does not skip an extra tar record when GNU PAX 1.x sparse headers end exactly on a record boundary.
   
   ## Details
   
   Previously, the padding calculation used `recordSize - bytesRead % recordSize`. When `bytesRead` was already aligned to `recordSize`, this computed a full-record skip instead of `0`, advancing the stream past real entry data.
   
   This change uses modulo padding calculation so aligned sparse headers leave the stream positioned at the first data byte.
   
   ## Tests
   
   - `mvn -q -Dtest=org.apache.commons.compress.archivers.tar.TarUtilsTest test`
   - `mvn -q`


-- 
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

Re: [PR] [COMPRESS-724] Fix PAX 1.x sparse header alignment [commons-compress]

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #775:
URL: https://github.com/apache/commons-compress/pull/775#issuecomment-4946928522

   @OldTruckDriver Thank you, merged 🚀 


-- 
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

Re: [PR] [COMPRESS-724] Fix PAX 1.x sparse header alignment [commons-compress]

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #775:
URL: https://github.com/apache/commons-compress/pull/775


-- 
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

Re: [PR] [COMPRESS-724] Fix PAX 1.x sparse header alignment [commons-compress]

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #775:
URL: https://github.com/apache/commons-compress/pull/775#issuecomment-4946130855

   @OldTruckDriver ping 🔔 See comments from myself and copiolot.


-- 
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

Re: [PR] [COMPRESS-724] Fix PAX 1.x sparse header alignment [commons-compress]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #775:
URL: https://github.com/apache/commons-compress/pull/775#discussion_r3551399080


##########
src/test/java/org/apache/commons/compress/archivers/tar/TarUtilsTest.java:
##########
@@ -109,6 +109,16 @@ private static Map<String, String> parsePaxHeaders(final byte[] data, final List
         return TarUtils.parsePaxHeaders(new ByteArrayInputStream(data), globalPaxHeaders, data.length, Short.MAX_VALUE, sparseHeaders);
     }
 
+    private static byte[] pax1xSparseHeaderAlignedToSingleRecord() {
+        final StringBuilder header = new StringBuilder("100\n");
+        for (int i = 0; i < 200; i++) {
+            header.append(i < 108 ? "00\n" : "0\n");
+        }
+        final byte[] bytes = header.toString().getBytes(UTF_8);
+        assertEquals(512, bytes.length);
+        return bytes;
+    }

Review Comment:
   This helper relies on the hard-coded threshold `i < 108` to make the sparse-header bytes land exactly on a 512-byte boundary. That magic number is non-obvious and brittle if the entry count or record size ever changes; it would be clearer to derive the number of extra digits needed from `recordSize` and the chosen sparse entry count.



-- 
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

Re: [PR] [COMPRESS-724] Fix PAX 1.x sparse header alignment [commons-compress]

Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on code in PR #775:
URL: https://github.com/apache/commons-compress/pull/775#discussion_r3553315978


##########
src/test/java/org/apache/commons/compress/archivers/tar/TarUtilsTest.java:
##########
@@ -328,6 +338,19 @@ void testParsePAX1XSparseHeaders() throws Exception {
         }
     }
 
+    @Test
+    void testParsePAX1XSparseHeadersDoesNotSkipAlignedDataRecord() throws Exception {
+        final byte[] fileData = new byte[513];
+        Arrays.fill(fileData, 0, 512, (byte) 'A');

Review Comment:
   Hello @OldTruckDriver 
   
   What are all these magic numbers?
   
   Is 512 supposed to be `TarConstants.DEFAULT_RCDSIZE` or something else?
   
   Are all the 512 values the same as a coincidence?
   
   Is 513 supposed to be `TarConstants.DEFAULT_RCDSIZE + 1`?
   
   You get the idea...
   
   Please use meaningful local variable names or references.
   
   



-- 
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

Re: [PR] [COMPRESS-724] Fix PAX 1.x sparse header alignment [commons-compress]

Posted by "OldTruckDriver (via GitHub)" <gi...@apache.org>.
OldTruckDriver commented on PR #775:
URL: https://github.com/apache/commons-compress/pull/775#issuecomment-4946758404

   Thanks for the review @garydgregory. You're right, those numbers should be named. To clarify the intent:
   
   1. 512 is TarConstants.DEFAULT_RCDSIZE and 513 is DEFAULT_RCDSIZE + 1 — the extra byte is what exposes the bug: with the old formula, the reader skips a whole extra record and lands on that trailing byte instead of the first data byte.
   2. 100 is the sparse entry count, 200 is the resulting number of offset/size lines, and 108 is only there to pad the header to exactly one record so it's already aligned.
   I'll push a commit replacing these with named constants/comments shortly. Sorry for the late reply.


-- 
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