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

[release/7.0] Tar: Remove invalidation of whitespace in PAX extended attributes #78785

Merged
merged 3 commits into from
Nov 29, 2022

Conversation

carlossanlop
Copy link
Member

@carlossanlop carlossanlop commented Nov 23, 2022

Manual backport of #78465, #78707 and #78744 to release/7.0.

Customer Impact

A customer reported #78456, in which they described that when a PAX entry in a tar archive contained extended attributes with whitespace characters, these characters would get treated as the end of the key or value, or would get trimmed.

The bug surfaced when extracting an archive that contained entries with filenames longer than what could fit in the standard name metadata field. In these cases, the expected behavior is to have the full path name added to the extended attributes dictionary as the value of the path key, and the standard name field is ignored.

But when the path contains spaces, we were considering the space as the ending character of the value. If two entries with very long paths get truncated in the same space, then when we extracted them, the file path would be the same, causing our TarFile extraction methods to think we had an existing file in disk, and we would throw.

The fix consisted in removing the logic that ignored or trimmed the whitespace in the middle of the path. We already know how long the key or the value in the dictionary is supposed to be, so there's no need to truncate.

Also, the Tar specs did not explicitly specify that spaces should be disallowed in keys or values, so we are providing more usage flexibility to users.

Testing

Added unit tests to verify:

  • Roundtrip: Adding keys or values with whitespaces to pax entries, then writing them to a tar archive, then reading that archive back, and verifying the entries did not get their extended attributes trimmed or truncated.
  • Customer scenario: Added tests that verified the exact problem the customer reported with really long paths that contained spaces, to ensure we were not truncating or trimming the path, and the two files were not considered collisions.

Risk

Low. This is a bug in a new feature in 7.0, and we are removing logic that is not explicitly indicated in the Tar spec.

stephentoub and others added 3 commits November 23, 2022 13:06
…net#78465)

* Remove invalidation of whitespace in TryGetNextExtendedAttribute

* Add requested test
* Remove TrimStart in PAX extended attributes

* Adjust existing tests with two extra inline datas.

Co-authored-by: carlossanlop <[email protected]>
…cate entries when the full path is in the extended attributes. (dotnet#78744)

Co-authored-by: carlossanlop <[email protected]>
@carlossanlop carlossanlop added this to the 7.0.2 milestone Nov 23, 2022
@carlossanlop carlossanlop self-assigned this Nov 23, 2022
@carlossanlop carlossanlop marked this pull request as ready for review November 23, 2022 21:51
@carlossanlop
Copy link
Member Author

Approved by Tactics via email. CI failure is unrelated.

@carlossanlop
Copy link
Member Author

Branding has been done. Milestone is 7.0.2. Signed-off by area owners. Approved by Tactics. No OOB package authoring changes needed. CI failure unrelated (#78778).
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit 64ddf49 into dotnet:release/7.0 Nov 29, 2022
@carlossanlop carlossanlop deleted the BackportTarFix branch November 29, 2022 23:21
@ghost ghost locked as resolved and limited conversation to collaborators Dec 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Formats.Tar Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants