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 headers with a 101 character name #1612

Merged
merged 2 commits into from
Jun 5, 2016

Conversation

ptomulik
Copy link
Contributor

@ptomulik ptomulik commented May 10, 2016

Description:

This attempts to fix #1608. The algorithm used by Gem::Package::TarWritter#split_name is revised and changed such that for names longer than 100 bytes (when split is required) it:

  • guarantees that non-empty name is returned,
  • guarantees that non-empty prefix is returned (if we split name into new name and prefix, and prefix appears to be empty, then the result is incorrect as we put the new name plus no prefix to TarHeader but the new name differs from the original one, so the resultant path is incorrect - this happens to the current implementation of split_name found in master, for example when the path is 101-characters long and starts with "/"),
  • tries to push as much of the name to prefix as possible (but not more than 155 bytes) - this is how the GNU tar works, at least from my observation,

The PR consists initially of two commits, first with tests only (to show, where the old implementation fails) and second containing the new implementation of #split_name.

Tasks:

  • Describe the problem / feature
  • Write tests
  • Write code to solve the problem
  • Get code review from coworkers / friends
  • Squash commits

I will abide by the code of conduct.

@segiddins
Copy link
Member

This looks good to me, especially since it has such good test coverage 👍

@ptomulik
Copy link
Contributor Author

Thanks.

@segiddins segiddins changed the title Fix 1608 Fix tar headers with a 101 character name May 21, 2016
@segiddins segiddins merged commit dae2484 into rubygems:master Jun 5, 2016
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.

Absolute names of length 101 characters are not handled properly by Gem::Package::TarWriter
3 participants