fix: validate object fields before use - #77
Merged
Conversation
Several values from an object header were used without checking them first, so malformed input surfaced later as a confusing failure rather than a decode error at the point it was read. - blob: reject a negative declared size, and grow the read buffer from the bytes actually received instead of reserving the declared size before any body arrives. io.EOF and io.ErrUnexpectedEOF are still returned exactly where io.ReadFull returned them. - personinfo: guard both indexes in the email loop, matching the name loop above. An email containing repeated spaces splits into an empty part, so "author Name <a b@c>", which git commit-tree writes, now decodes instead of erroring. - util: shaToCid rejects a reference that is not 20 bytes and stops discarding mh.Encode's error. The error propagates through commit, tag and tree decode, so a short reference fails while being read rather than when the node is encoded again.
decodeGpgSig and readMergeTag appended to a string in a loop, which reallocates and copies the whole accumulated value on every line. Both now use a strings.Builder, as the person info email loop already does. Decoding a commit carrying a 2 MB signature drops from 3.056s to 3ms, and the cost becomes linear in the signature length rather than quadratic.
TestObjectParse walks .git/objects and reads each file it finds as a loose git object. It skipped the pack and info directories by checking the folder a file sits in directly, so it only caught files one level down. Git can also write a commit-graph into a folder inside info. Those files are a level deeper, so the check missed them, and the test tried to read a commit-graph as a git object and failed with "zlib: invalid header". Git writes a split commit-graph on every commit under some configurations, so the test failed on a clean checkout for anyone with that turned on. Skip both directories and everything inside them instead.
gammazero
approved these changes
Jul 27, 2026
WalkDir hands the callback an fs.DirEntry instead of an os.FileInfo, so it does not stat every entry it visits. All three walks over .git/objects now use it. The two benchmarks also skipped pack and info by looking at the folder a file sits in directly, so a commit-graph one level deeper reached the parser and failed them with "zlib: invalid header". They now skip both directories whole, as TestObjectParse does, which also drops a path split per file. Co-authored-by: Andrew Gillis <11790789+gammazero@users.noreply.github.com> Suggested in #77 (comment)
|
Suggested tag:
Cutting a Release (and modifying code files)This PR is modifying both Automatically created GitHub ReleaseA draft GitHub Release has been created. |
lidel
added a commit
to ipfs/kubo
that referenced
this pull request
Jul 27, 2026
* fix: recover from panics in detached goroutines ls, dag get and dag export each hand their work to a goroutine and read the result back over a channel or pipe. Decoding and encoding there runs whatever codec a block's CID names, so it runs third-party code, and a panic on a detached goroutine ends the daemon rather than the command. Each now recovers, logs, and reports through the channel it already uses. In dag export the recover is registered after the existing cleanup defer so it runs first, while errCh is still open. * chore: update boxo, go-ipld-git and go-unixfsnode Picks up the CAR streaming and object parsing work from ipfs/boxo#1197, ipfs/go-ipld-git#77 and ipfs/go-unixfsnode#100. All three are pinned to their branches for now; swap for the tagged releases before merging. * chore: update boxo, go-ipld-git and go-unixfsnode go-ipld-git and go-unixfsnode are on their tagged releases. boxo is pinned to main, which carries ipfs/boxo#1197 but has not been released yet, so this still needs a boxo release before it can merge. Notes the CAR truncation marker in the v0.43 changelog, since that is the user-visible part of the boxo update. Also stops a slow Ubuntu mirror from failing the ipfs-webui job. That job installed Playwright OS dependencies for every browser although it declares no projects and so only ever runs chromium, and the install had no timeout of its own. When the mirror served 10.7 MB of package indices at 39 kB/s, apt-get update alone outlasted the job's 20 minute budget and the run was cancelled before any test started. The install is now scoped to chromium, capped, and best effort: the runner image already ships what headless chromium needs, and a library that really is missing surfaces when the browser fails to launch.
lidel
added a commit
to ipfs/kubo
that referenced
this pull request
Jul 27, 2026
* fix: recover from panics in detached goroutines ls, dag get and dag export each hand their work to a goroutine and read the result back over a channel or pipe. Decoding and encoding there runs whatever codec a block's CID names, so it runs third-party code, and a panic on a detached goroutine ends the daemon rather than the command. Each now recovers, logs, and reports through the channel it already uses. In dag export the recover is registered after the existing cleanup defer so it runs first, while errCh is still open. * chore: update boxo, go-ipld-git and go-unixfsnode Picks up the CAR streaming and object parsing work from ipfs/boxo#1197, ipfs/go-ipld-git#77 and ipfs/go-unixfsnode#100. All three are pinned to their branches for now; swap for the tagged releases before merging. * chore: update boxo, go-ipld-git and go-unixfsnode go-ipld-git and go-unixfsnode are on their tagged releases. boxo is pinned to main, which carries ipfs/boxo#1197 but has not been released yet, so this still needs a boxo release before it can merge. Notes the CAR truncation marker in the v0.43 changelog, since that is the user-visible part of the boxo update. Also stops a slow Ubuntu mirror from failing the ipfs-webui job. That job installed Playwright OS dependencies for every browser although it declares no projects and so only ever runs chromium, and the install had no timeout of its own. When the mirror served 10.7 MB of package indices at 39 kB/s, apt-get update alone outlasted the job's 20 minute budget and the run was cancelled before any test started. The install is now scoped to chromium, capped, and best effort: the runner image already ships what headless chromium needs, and a library that really is missing surfaces when the browser fails to launch. (cherry picked from commit f9baf8f)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Papercuts: the decoder trusts what an object header says. A blob's declared size is set aside in full before any data arrives. The author and committer parser breaks on an email with two spaces in a row, which
git commit-treewrites. A reference that is not 20 bytes is accepted when read, and fails only when the object is written back. Signatures are built by adding to a string in a loop, which is slow for large ones.Fix
author Name <a b@c>decodes.strings.Builder.Good objects decode as before. Also bumps the version, and fixes a test that failed when git had written a commit-graph.