Skip to content

Tar: Use indexer setter instead of Add on ExtendedAttributes dictionary #76404

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 3, 2022
Merged

Tar: Use indexer setter instead of Add on ExtendedAttributes dictionary #76404

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
335ace1
Use indexer setter instead of Add on ExtendedAttributes dictionary
jozkee Sep 29, 2022
d472c64
Add roundtrip tests
jozkee Sep 30, 2022
9d4d7f3
Fix TryAddStringField and always set mtime
jozkee Sep 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -721,17 +721,14 @@ static int CountDigits(int value)
private void CollectExtendedAttributesFromStandardFieldsIfNeeded()
{
ExtendedAttributes[PaxEaName] = _name;

if (!ExtendedAttributes.ContainsKey(PaxEaMTime))
{
ExtendedAttributes.Add(PaxEaMTime, TarHelpers.GetTimestampStringFromDateTimeOffset(_mTime));
}
ExtendedAttributes[PaxEaMTime] = TarHelpers.GetTimestampStringFromDateTimeOffset(_mTime);

TryAddStringField(ExtendedAttributes, PaxEaGName, _gName, FieldLengths.GName);
TryAddStringField(ExtendedAttributes, PaxEaUName, _uName, FieldLengths.UName);

if (!string.IsNullOrEmpty(_linkName))
{
Debug.Assert(_typeFlag is TarEntryType.SymbolicLink or TarEntryType.HardLink);
Copy link
Contributor

@carlossanlop carlossanlop Oct 3, 2022

Choose a reason for hiding this comment

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

Nice.

ExtendedAttributes[PaxEaLinkName] = _linkName;
}

Expand All @@ -740,12 +737,16 @@ private void CollectExtendedAttributesFromStandardFieldsIfNeeded()
ExtendedAttributes[PaxEaSize] = _size.ToString();
}
Copy link
Contributor

@carlossanlop carlossanlop Sep 29, 2022

Choose a reason for hiding this comment

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

We could add an else here, in case the data stream gets reduced below the max allowed size, in which case, we can remove the PaxEaSize value from the dictionary entry.

Copy link
Contributor

@carlossanlop carlossanlop Sep 30, 2022

Choose a reason for hiding this comment

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

In my other PR which was replaced by this one, I had a fake stream implementation that simulated having 99_999_999 length. You can reuse that to test this.


// Adds the specified string to the dictionary if it's longer than the specified max byte length.
// Sets the specified string to the dictionary if it's longer than the specified max byte length; otherwise, remove it.
static void TryAddStringField(Dictionary<string, string> extendedAttributes, string key, string? value, int maxLength)
{
if (!string.IsNullOrEmpty(value) && GetUtf8TextLength(value) > maxLength)
if (string.IsNullOrEmpty(value) || GetUtf8TextLength(value) <= maxLength)
{
extendedAttributes.Remove(key);
}
else
{
extendedAttributes.Add(key, value);
extendedAttributes[key] = value;
}
}
}
Expand Down