Skip to content

Conversation

@matts1
Copy link
Contributor

@matts1 matts1 commented Jan 6, 2026

It was already decompressed on line 285, so attempting to decompress it again turns the delta stream into garbage.

Unfortunately I don't know enough about the git file format to make a test, the best I was able to do was to compare it to a reference implementation and see what was going wrong.

Fixes #2344

It was already decompressed on line 285, so attempting to decompress it
again turns the delta stream into garbage.

Unfortunately I don't know enough about the git file format to make a
test, the best I was able to do was to compare it to a reference
implementation and see what was going wrong.

Fixes GitoxideLabs#2344
@matts1 matts1 marked this pull request as ready for review January 6, 2026 00:48
@Byron Byron requested a review from Copilot January 6, 2026 06:30
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a buffer corruption bug in pack file delta resolution where decompressing a base object would overwrite already-decompressed delta instructions. The issue was discovered when attempting to read a specific object from the Chromium repository (issue #2344).

Key Changes

  • Fixed buffer slice calculation when decompressing base objects to prevent corruption of delta instruction data
  • The fix restricts decompression output to only the portion of the buffer designated for base object data, excluding the delta instructions area

@Byron
Copy link
Member

Byron commented Jan 6, 2026

Thanks so much for digging into this, and finding a solution. Just at a glance it makes perfect sense to bound the output buffer, to avoid the decompression to try to 'overshoot', which seems to happen now (with zlib-rs) or could always happen.

Admittedly, I couldn't try this against the chromium repository as I didn't have a clone ready in time, but did at least ran a pack verification again a relatively small linux pack via…

cargo run --release --bin gix -- free pack verify tests/fixtures/repos/linux.git/objects/pack/pack-3ee05b0f4e4c2cb59757c95c68e2d13c0a491289.pac

…, and it succeeded. Beyond that, I think the test-suite would have caught it if this change was tuned to fixing exactly this one issue.

@Byron Byron merged commit c0cf38f into GitoxideLabs:main Jan 6, 2026
36 checks passed
@Byron
Copy link
Member

Byron commented Jan 6, 2026

And I have created a patch-release for gix-pack so you'd be able to pick it up right away.

Copilot AI added a commit that referenced this pull request Jan 6, 2026
Copilot AI added a commit that referenced this pull request Jan 6, 2026
- Generated pack-regression-*.pack with large base object (52KB) and delta chains
- Updated regression test to use custom pack file
- Added comprehensive documentation explaining test limitations
- Test exercises the buffer bounding code path even though it doesn't fail without the fix
  (requires specific zlib-rs compression conditions like chromium repository)
- Provides infrastructure for adding reproducing pack file in the future

Co-authored-by: Byron <[email protected]>
@matts1 matts1 deleted the push-utkmrmunzotv branch January 6, 2026 22:59
matts1 added a commit to jj-vcs/jj that referenced this pull request Jan 6, 2026
0.64.1 contains GitoxideLabs/gitoxide#2345,
which fixes an issue where it throws an error when attempting to unpack
objects.
matts1 added a commit to jj-vcs/jj that referenced this pull request Jan 6, 2026
0.64.1 contains GitoxideLabs/gitoxide#2345,
which fixes an issue where it throws an error when attempting to unpack
objects.
github-merge-queue bot pushed a commit to jj-vcs/jj that referenced this pull request Jan 6, 2026
0.64.1 contains GitoxideLabs/gitoxide#2345,
which fixes an issue where it throws an error when attempting to unpack
objects.
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.

Unable to read object in chromium repo

2 participants