Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(tar): ignore non-tar file portion of a stream in UntarStream #6064

Merged
merged 6 commits into from
Sep 30, 2024

Conversation

BlackAsLight
Copy link
Contributor

@BlackAsLight BlackAsLight commented Sep 26, 2024

Fixes: denoland/wasmbuild#143

This pull request fixes a bug with the UntarStream class where it incorrectly assumes the contents of a stream will only contain a tar file. This is incorrect as anything after a tar file can be appended and a valid tar decoder should ignore everything after it receives its 1024 bytes of 0b00 when checking for a header.

The changes made here should check for a premature ending and if so send a cancel signal upstream so if a readable stream being pipped in intentionally had multiple stuff appended together, it can listen for that and then switch to whatever.

… be exactly the right amount of bytes

- The spec specifies that a tar file ends with 1024 bytes of `0b00`, but it says nothing about extra information being stored after this.
Copy link

codecov bot commented Sep 26, 2024

Codecov Report

Attention: Patch coverage is 93.93939% with 2 lines in your changes missing coverage. Please review.

Project coverage is 96.76%. Comparing base (361be45) to head (1ca7bd4).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
tar/untar_stream.ts 93.93% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6064   +/-   ##
=======================================
  Coverage   96.75%   96.76%           
=======================================
  Files         508      508           
  Lines       39132    39140    +8     
  Branches     5786     5793    +7     
=======================================
+ Hits        37864    37872    +8     
  Misses       1226     1226           
  Partials       42       42           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@iuioiua iuioiua changed the title fix(tar): UntarStream bug fix(tar): ignore non-tar file portions of a stream in UntarStream Sep 26, 2024
@iuioiua iuioiua changed the title fix(tar): ignore non-tar file portions of a stream in UntarStream fix(tar): ignore non-tar file portion of a stream in UntarStream Sep 26, 2024
tar/untar_stream.ts Outdated Show resolved Hide resolved
tar/untar_stream.ts Outdated Show resolved Hide resolved
tar/untar_stream_test.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

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

Thank you!

Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

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

LGTM

@kt3k kt3k merged commit e43a7df into denoland:main Sep 30, 2024
16 checks passed
@BlackAsLight BlackAsLight deleted the untar_ending_bug branch September 30, 2024 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants