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 PAX extended attribute reading logic to treat '=' character as valid in the value strings. #82810

Merged
merged 5 commits into from
Mar 9, 2023

Conversation

carlossanlop
Copy link
Member

Fixes #81699

We had logic in TarHeader.Read.cs that was explicitly forbidding the '=' character as part of the body of the value string in an extended attribute key-value pair. It felt natural to have such restriction in place as it is one of the two separators that the spec indicates:

  • The '=' character is used to separate a key from a value.
  • The '\n' character is used to separate a key-value-pair from another.

But the restriction is not needed: We only need to find the first '=' character to stop reading a key and start reading a value, and after that, we can keep reading any character, including any number of '=' characters, until we detect a '\n' character. Additionally, none of the specs indicated that the '=' is forbidden in values.

This issue was reported by a customer, so we would like to consider a backport to 7.0.

…. It is not handling any filesystem entries.
…ribute roundtripping when the value contains an '=' character.

Also add a null check for a subsequent GetNextEntry.
@carlossanlop
Copy link
Member Author

While working on this, I realized we should disallow adding key-value-pairs in which the key contains '=', or in which either the key or the value contain a '\n'. Both cases will cause the extended attribute reading logic to fail.

I'll add the fixes to this PR unless anyone indicates a preference to have the two issues fixed separately.

@jeffhandley
Copy link
Member

Separate sounds good to me. Would those other changes potentially be breaking?

@carlossanlop
Copy link
Member Author

Would those other changes potentially be breaking?

I don't think adding the exception would be breaking because we would be shielding the user from unexpected behavior and corrupted data due to improper use of reserved characters in the key or values.

@carlossanlop carlossanlop merged commit ae4ba7f into dotnet:main Mar 9, 2023
@carlossanlop
Copy link
Member Author

/backport to release/7.0

@github-actions
Copy link
Contributor

github-actions bot commented Mar 9, 2023

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/4370450091

@carlossanlop carlossanlop deleted the TarFixEqualSignExtAttr branch March 9, 2023 17:06
@ghost ghost locked as resolved and limited conversation to collaborators Apr 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pax Headers incorrectly treated is invalid if value contains an '=' sign
3 participants