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 PGET Expected Byte Check #236

Merged
merged 1 commit into from
Dec 3, 2024
Merged

Conversation

tempusfrangit
Copy link
Contributor

Tar files have padding. Verify the end of the file is null and only error if there is non-null or we haven't matched content-length from origin.

@tempusfrangit tempusfrangit requested review from meatballhat and a team December 3, 2024 18:00
Copy link
Contributor

@philandstuff philandstuff left a comment

Choose a reason for hiding this comment

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

I think this could be simpler with io.ReadAll() - we wouldn't need the outer for loop - but this is fine

@tempusfrangit tempusfrangit force-pushed the fix-pget-expected-byte-checker branch from 5f80b75 to ab1ab9d Compare December 3, 2024 18:31
@tempusfrangit
Copy link
Contributor Author

I think this could be simpler with io.ReadAll() - we wouldn't need the outer for loop - but this is fine

Good point. I'll verify that works and get an update.

Tar files have padding. Verify the end of the file is null and only
error if there is non-null or we haven't matched content-length from
origin.

This behavior is an artifact of multi-readers if the final multireader
is potentially all null bytes. New test mimics observed behavior.
@tempusfrangit tempusfrangit force-pushed the fix-pget-expected-byte-checker branch from ab1ab9d to 1e8bc92 Compare December 3, 2024 18:34
@tempusfrangit
Copy link
Contributor Author

The only risk here is if someone creates a malicious tar with massive padding it could force an oomkill.

I am generally ok with that trade off.

@tempusfrangit tempusfrangit merged commit f71c128 into main Dec 3, 2024
5 checks passed
@tempusfrangit tempusfrangit deleted the fix-pget-expected-byte-checker branch December 3, 2024 18:37
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.

2 participants