Repository navigation
Conversation
- 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.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6430 +/- ##
==========================================
+ Coverage 84.79% 84.92% +0.13%
==========================================
Files 358 358
Lines 38957 38994 +37
Branches 11020 11064 +44
==========================================
+ Hits 33032 33116 +84
+ Misses 5489 5442 -47
Partials 436 436 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.