Repository navigation
Fix tar parser correctness issues - #6430
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6430 +/- ##
==========================================
+ Coverage 84.82% 84.95% +0.13%
==========================================
Files 359 359
Lines 39106 39137 +31
Branches 11076 11091 +15
==========================================
+ Hits 33170 33249 +79
+ Misses 5499 5451 -48
Partials 437 437 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
canova
approved these changes
Nov 6, 2026
canova
left a comment
canova
left a comment
Member
There was a problem hiding this comment.
It looks good to me overall, thanks!
- Parse octal fields with a stricter parser: Compared to parseInt, this parser won't accept negative values or numbers with trailing garbage. - Use the octal parser for the checksum and device number fields. These were incorrectly parsed as decimal. - Verify the header checksum. - Fix end-of-archive detection: The end is marked by a full zero block. We were trusting the size written in the header and not detecting truncated input.
TextDecoder's 'ascii' label actually means windows-1252, so UTF-8 names written by cargo were garbled and never matched the requested path. With fatal: true, some non-ASCII bytes also made the whole archive fail.
Replace the recursive _readNextFile with a loop that collects a pending pax 'x' header and then resolves the next member's name, linkname and size before reading its data. Previously, the pax records were patched onto the entry after it had been read, so: - a pax 'size' record changed entry.size but not the buffer or the position of the next header, desynchronizing the parse; - with two stacked 'x' headers, the earlier one won; - a tombstoned 'path' left entry.name undefined; - every record was copied onto the entry object, so 'buffer=1' replaced the file contents, and all-digit paths became numbers; - many extension headers in a row overflowed the stack. Now only the 'path', 'linkpath' and 'size' keywords are recognized and other records are ignored. A second 'x' header for the same member, an empty resolved name, an 'x' header at the end of the archive, duplicate records and malformed records are rejected. Global 'g' headers may not contain any of the recognized keywords, so their records don't need to be kept. Since hasNext() can no longer know whether a member follows without parsing extension headers, next() now returns null at the end of the archive. TarFileEntry is reduced to the fields that are resolved properly and that callers can use.
The parser treated any header containing 'ustar' in its magic field as UStar, including GNU headers. GNU headers don't have a name prefix field; those bytes hold atime, ctime and other fields, so an archive with atime set would produce names like '<octal atime>/<name>'. Classify each header by its exact magic and version as UStar, GNU or v7, and only read the prefix for UStar. Reject unknown formats, archives that mix formats, and pax headers in non-UStar archives.
cargo writes crates in the GNU tar format. In that format, names longer than 100 bytes are stored in a preceding 'L' entry, and long link names in a 'K' entry. We treated these as ordinary entries and returned the following member with its name truncated to 100 bytes, so files with long paths, such as 95 of the files in aws-sdk-iam-1.132.0, could never be found in the archive. Apply the payload of 'L' and 'K' entries to the next member. Reject more than one of each per member, and 'L'/'K' entries in non-GNU archives.
mstange
force-pushed
the
push-umouvtxtptkz
branch
from
November 6, 2026 19:16
4823eb8 to
e5ea81f
Compare
mstange
enabled auto-merge
November 6, 2026 19:16
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
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.
Main | Deploy preview
In #3778 I added a tar parser in order to support
cargo:...paths.A few days ago I read https://astral.sh/blog/tar-codec and thought it might be a good idea to do an audit on our parser.
The biggest issue found is that we weren't supporting long filenames (member names >= 100 bytes), which means that we weren't able to load some source files, such as the one in this profile: https://share.firefox.dev/4ziKT9t
This PR fixes the long path issue. It contains other correctness fixes as well, but they didn't really have any impact on the usability of the profiler.