Skip to content

[COMPRESS-724] Fix PAX 1.x sparse header alignment#775

Open
OldTruckDriver wants to merge 1 commit into
apache:masterfrom
OldTruckDriver:fix/COMPRESS-724_pax1x_sparse_alignment
Open

[COMPRESS-724] Fix PAX 1.x sparse header alignment#775
OldTruckDriver wants to merge 1 commit into
apache:masterfrom
OldTruckDriver:fix/COMPRESS-724_pax1x_sparse_alignment

Conversation

@OldTruckDriver

Copy link
Copy Markdown

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

Do not skip an extra tar record when parsePAX1XSparseHeaders(InputStream, int) finishes reading sparse headers exactly on a record boundary.

Add a regression test that keeps the first data byte visible after an aligned sparse header block.

Reviewed-by: OpenAI Codex
Reviewed-by: Anthropic Claude Code

@garydgregory garydgregory left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @OldTruckDriver

Thank you for the PR.

The tests contain too many magic numbers.

For example:

        final byte[] fileData = new byte[513];
        Arrays.fill(fileData, 0, 512, (byte) 'A');
        fileData[512] = 'B';

Above 513 and 512 are clearly related.

In pax1xSparseHeaderAlignedToSingleRecord, why 200 and 108. Either use local vars with clear names or write comments.

TY!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants