fix upload error when cookbook tarball uid/gid is very large #1810
Conversation
This file contains 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
robbkidd
added
Aspect: Correctness
Priority: Critical
Type: Bug
robbkidd
force-pushed
the
robb/tars-are-weird
branch
from
73ed8ec
to
ceb7d57
Compare
robbkidd
force-pushed
the
robb/tars-are-weird
branch
4 times, most recently
from
00d941a
to
e73e455
Compare
|
@teknofire Look past all the force pushing and this is where I would up after our pairing on Friday. Thoughts? |
tyler-ball
approved these changes
May 14, 2019
teknofire
reviewed
May 14, 2019
robbkidd
force-pushed
the
robb/tars-are-weird
branch
2 times, most recently
from
4507051
to
37fddd0
Compare
Fixes #1806 RubyGems' Gem::Package::TarReader has some strict octal checking that is incompatible with pax format tar files with large (greater than 8^8) uid or gid entries (see rubygems/rubygems#2213). This can happen particularly when publishing a cookbook on a Windows workstation that is a member of a domain; the group IDs there can be very large numbers. This change swaps out the handling of tar files from RubyGems' library to Chef's own ffi-libarchive. The previous logic also assumed that cookbook tarballs must be GZipped which is probably an assumption made elsewhere in the Chef ecosystem. Because libarchive is pretty forgiving of the handling of multiple formats of tar files, the requirement for GZipped tar files is preserved and implemented with FileMagic, already a dependency of Supermarket via the Fieri engine subcomponent. Signed-off-by: Robb Kidd <rkidd@chef.io>
|
I missed adding |
Signed-off-by: Robb Kidd <rkidd@chef.io>
Signed-off-by: Robb Kidd <rkidd@chef.io>
robbkidd
force-pushed
the
robb/tars-are-weird
branch
from
37fddd0
to
10326ac
Compare
|
OK. |
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.
Description
RubyGems' Gem::Package::TarReader has some strict octal checking that is incompatible with pax format tar files with large (greater than 8^8) uid or gid entries (see rubygems/rubygems#2213). This can happen particularly when publishing a cookbook on a Windows workstation that is a member of a domain; the group IDs there can be very large numbers.
This change swaps out the handling of tar files from RubyGems' library to Chef's own ffi-libarchive. The previous logic also assumed that cookbook tarballs must be GZipped which is probably an assumption made elsewhere in the Chef ecosystem. Because libarchive is pretty forgiving of the handling of multiple formats of tar files, the requirement for GZipped tar files is preserved and implemented with FileMagic, already a dependency of Supermarket via the Fieri engine subcomponent.
Related Issue
Fixes #1806
Types of changes
New feature (non-breaking change which adds functionality)Breaking change (fix or feature that would cause existing functionality to change)Chore (non-breaking change that does not add functionality or fix an issue)Checklist:
I have updated the documentation accordingly.