Skip to content

Commit 221717b

Browse files
jozkeejozkee
authored
Tar: Use indexer setter instead of Add on ExtendedAttributes dictionary (#76404)
* Use indexer setter instead of Add on ExtendedAttributes dictionary * Add roundtrip tests * Fix TryAddStringField and always set mtime
1 parent 433457d commit 221717b

File tree

6 files changed

+294
-22
lines changed

6 files changed

+294
-22
lines changed

‎src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs‎

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -720,39 +720,33 @@ static int CountDigits(int value)
720720
// extended attributes. They get collected and saved in that dictionary, with no restrictions.
721721
private void CollectExtendedAttributesFromStandardFieldsIfNeeded()
722722
{
723-
ExtendedAttributes.Add(PaxEaName, _name);
723+
ExtendedAttributes[PaxEaName] = _name;
724+
ExtendedAttributes[PaxEaMTime] = TarHelpers.GetTimestampStringFromDateTimeOffset(_mTime);
724725

725-
if (!ExtendedAttributes.ContainsKey(PaxEaMTime))
726-
{
727-
ExtendedAttributes.Add(PaxEaMTime, TarHelpers.GetTimestampStringFromDateTimeOffset(_mTime));
728-
}
729-
730-
if (!string.IsNullOrEmpty(_gName))
731-
{
732-
TryAddStringField(ExtendedAttributes, PaxEaGName, _gName, FieldLengths.GName);
733-
}
734-
735-
if (!string.IsNullOrEmpty(_uName))
736-
{
737-
TryAddStringField(ExtendedAttributes, PaxEaUName, _uName, FieldLengths.UName);
738-
}
726+
TryAddStringField(ExtendedAttributes, PaxEaGName, _gName, FieldLengths.GName);
727+
TryAddStringField(ExtendedAttributes, PaxEaUName, _uName, FieldLengths.UName);
739728

740729
if (!string.IsNullOrEmpty(_linkName))
741730
{
742-
ExtendedAttributes.Add(PaxEaLinkName, _linkName);
731+
Debug.Assert(_typeFlag is TarEntryType.SymbolicLink or TarEntryType.HardLink);
732+
ExtendedAttributes[PaxEaLinkName] = _linkName;
743733
}
744734

745735
if (_size > 99_999_999)
746736
{
747-
ExtendedAttributes.Add(PaxEaSize, _size.ToString());
737+
ExtendedAttributes[PaxEaSize] = _size.ToString();
748738
}
749739

750-
// Adds the specified string to the dictionary if it's longer than the specified max byte length.
751-
static void TryAddStringField(Dictionary<string, string> extendedAttributes, string key, string value, int maxLength)
740+
// Sets the specified string to the dictionary if it's longer than the specified max byte length; otherwise, remove it.
741+
static void TryAddStringField(Dictionary<string, string> extendedAttributes, string key, string? value, int maxLength)
752742
{
753-
if (Encoding.UTF8.GetByteCount(value) > maxLength)
743+
if (string.IsNullOrEmpty(value) || GetUtf8TextLength(value) <= maxLength)
744+
{
745+
extendedAttributes.Remove(key);
746+
}
747+
else
754748
{
755-
extendedAttributes.Add(key, value);
749+
extendedAttributes[key] = value;
756750
}
757751
}
758752
}

‎src/libraries/System.Formats.Tar/tests/TarTestsBase.cs‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -481,7 +481,7 @@ protected TarEntryType GetTarEntryTypeForTarEntryFormat(TarEntryType entryType,
481481
return entryType;
482482
}
483483

484-
protected TarEntry InvokeTarEntryCreationConstructor(TarEntryFormat targetFormat, TarEntryType entryType, string entryName)
484+
protected static TarEntry InvokeTarEntryCreationConstructor(TarEntryFormat targetFormat, TarEntryType entryType, string entryName)
485485
=> targetFormat switch
486486
{
487487
TarEntryFormat.V7 => new V7TarEntry(entryType, entryName),
@@ -796,5 +796,18 @@ internal enum NameCapabilities
796796
NameAndPrefix,
797797
Unlimited
798798
}
799+
800+
internal static void WriteTarArchiveWithOneEntry(Stream s, TarEntryFormat entryFormat, TarEntryType entryType)
801+
{
802+
using TarWriter writer = new(s, leaveOpen: true);
803+
804+
TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, "foo");
805+
if (entryType == TarEntryType.SymbolicLink)
806+
{
807+
entry.LinkName = "bar";
808+
}
809+
810+
writer.WriteEntry(entry);
811+
}
799812
}
800813
}

‎src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Entry.Roundtrip.Tests.cs‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,5 +147,103 @@ public void UserNameGroupNameRoundtrips(TarEntryFormat entryFormat, bool unseeka
147147
Assert.Equal(userGroupName, posixEntry.UserName);
148148
Assert.Equal(userGroupName, posixEntry.GroupName);
149149
}
150+
151+
[Theory]
152+
[InlineData(TarEntryType.RegularFile)]
153+
[InlineData(TarEntryType.Directory)]
154+
[InlineData(TarEntryType.HardLink)]
155+
[InlineData(TarEntryType.SymbolicLink)]
156+
public void PaxExtendedAttributes_DoNotOverwritePublicProperties_WhenTheyFitOnLegacyFields(TarEntryType entryType)
157+
{
158+
Dictionary<string, string> extendedAttributes = new();
159+
extendedAttributes[PaxEaName] = "ea_name";
160+
extendedAttributes[PaxEaGName] = "ea_gname";
161+
extendedAttributes[PaxEaUName] = "ea_uname";
162+
extendedAttributes[PaxEaMTime] = GetTimestampStringFromDateTimeOffset(TestModificationTime);
163+
164+
if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
165+
{
166+
extendedAttributes[PaxEaLinkName] = "ea_linkname";
167+
}
168+
169+
PaxTarEntry writeEntry = new PaxTarEntry(entryType, "name", extendedAttributes);
170+
writeEntry.Name = new string('a', 100);
171+
// GName and UName must be longer than 32 to be written as extended attribute.
172+
writeEntry.GroupName = new string('b', 32);
173+
writeEntry.UserName = new string('c', 32);
174+
// There's no limit on MTime, we just ensure it roundtrips.
175+
writeEntry.ModificationTime = TestModificationTime.AddDays(1);
176+
177+
if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
178+
{
179+
writeEntry.LinkName = new string('d', 100);
180+
}
181+
182+
MemoryStream ms = new();
183+
using (TarWriter w = new(ms, leaveOpen: true))
184+
{
185+
w.WriteEntry(writeEntry);
186+
}
187+
ms.Position = 0;
188+
189+
using TarReader r = new(ms);
190+
PaxTarEntry readEntry = Assert.IsType<PaxTarEntry>(r.GetNextEntry());
191+
Assert.Null(r.GetNextEntry());
192+
193+
Assert.Equal(writeEntry.Name, readEntry.Name);
194+
Assert.Equal(writeEntry.GroupName, readEntry.GroupName);
195+
Assert.Equal(writeEntry.UserName, readEntry.UserName);
196+
Assert.Equal(writeEntry.ModificationTime, readEntry.ModificationTime);
197+
Assert.Equal(writeEntry.LinkName, readEntry.LinkName);
198+
}
199+
200+
[Theory]
201+
[InlineData(TarEntryType.RegularFile)]
202+
[InlineData(TarEntryType.Directory)]
203+
[InlineData(TarEntryType.HardLink)]
204+
[InlineData(TarEntryType.SymbolicLink)]
205+
public void PaxExtendedAttributes_DoNotOverwritePublicProperties_WhenLargerThanLegacyFields(TarEntryType entryType)
206+
{
207+
Dictionary<string, string> extendedAttributes = new();
208+
extendedAttributes[PaxEaName] = "ea_name";
209+
extendedAttributes[PaxEaGName] = "ea_gname";
210+
extendedAttributes[PaxEaUName] = "ea_uname";
211+
extendedAttributes[PaxEaMTime] = GetTimestampStringFromDateTimeOffset(TestModificationTime);
212+
213+
if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
214+
{
215+
extendedAttributes[PaxEaLinkName] = "ea_linkname";
216+
}
217+
218+
PaxTarEntry writeEntry = new PaxTarEntry(entryType, "name", extendedAttributes);
219+
writeEntry.Name = new string('a', MaxPathComponent);
220+
// GName and UName must be longer than 32 to be written as extended attribute.
221+
writeEntry.GroupName = new string('b', 32 + 1);
222+
writeEntry.UserName = new string('c', 32 + 1);
223+
// There's no limit on MTime, we just ensure it roundtrips.
224+
writeEntry.ModificationTime = TestModificationTime.AddDays(1);
225+
226+
if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
227+
{
228+
writeEntry.LinkName = new string('d', 100 + 1);
229+
}
230+
231+
MemoryStream ms = new();
232+
using (TarWriter w = new(ms, leaveOpen: true))
233+
{
234+
w.WriteEntry(writeEntry);
235+
}
236+
ms.Position = 0;
237+
238+
using TarReader r = new(ms);
239+
PaxTarEntry readEntry = Assert.IsType<PaxTarEntry>(r.GetNextEntry());
240+
Assert.Null(r.GetNextEntry());
241+
242+
Assert.Equal(writeEntry.Name, readEntry.Name);
243+
Assert.Equal(writeEntry.GroupName, readEntry.GroupName);
244+
Assert.Equal(writeEntry.UserName, readEntry.UserName);
245+
Assert.Equal(writeEntry.ModificationTime, readEntry.ModificationTime);
246+
Assert.Equal(writeEntry.LinkName, readEntry.LinkName);
247+
}
150248
}
151249
}

‎src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Tests.cs‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,5 +450,45 @@ public void WriteEntry_TooLongGroupName_Throws(TarEntryFormat entryFormat, strin
450450

451451
Assert.Throws<ArgumentException>("entry", () => writer.WriteEntry(entry));
452452
}
453+
454+
public static IEnumerable<object[]> WriteEntry_UsingTarEntry_FromTarReader_IntoTarWriter_TheoryData()
455+
{
456+
foreach (var entryFormat in new[] { TarEntryFormat.V7, TarEntryFormat.Ustar, TarEntryFormat.Pax, TarEntryFormat.Gnu })
457+
{
458+
foreach (var entryType in new[] { entryFormat == TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile, TarEntryType.Directory, TarEntryType.SymbolicLink })
459+
{
460+
foreach (bool unseekableStream in new[] { false, true })
461+
{
462+
yield return new object[] { entryFormat, entryType, unseekableStream };
463+
}
464+
}
465+
}
466+
}
467+
468+
[Theory]
469+
[MemberData(nameof(WriteEntry_UsingTarEntry_FromTarReader_IntoTarWriter_TheoryData))]
470+
public void WriteEntry_UsingTarEntry_FromTarReader_IntoTarWriter(TarEntryFormat entryFormat, TarEntryType entryType, bool unseekableStream)
471+
{
472+
MemoryStream msSource = new();
473+
MemoryStream msDestination = new();
474+
475+
WriteTarArchiveWithOneEntry(msSource, entryFormat, entryType);
476+
msSource.Position = 0;
477+
478+
Stream source = new WrappedStream(msSource, msSource.CanRead, msSource.CanWrite, canSeek: !unseekableStream);
479+
Stream destination = new WrappedStream(msDestination, msDestination.CanRead, msDestination.CanWrite, canSeek: !unseekableStream);
480+
481+
using (TarReader reader = new(source))
482+
using (TarWriter writer = new(destination))
483+
{
484+
TarEntry entry;
485+
while ((entry = reader.GetNextEntry()) != null)
486+
{
487+
writer.WriteEntry(entry);
488+
}
489+
}
490+
491+
AssertExtensions.SequenceEqual(msSource.ToArray(), msDestination.ToArray());
492+
}
453493
}
454494
}

‎src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.Roundtrip.Tests.cs‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,5 +96,103 @@ public async Task UserNameGroupNameRoundtripsAsync(TarEntryFormat entryFormat, b
9696
Assert.Equal(userGroupName, posixEntry.UserName);
9797
Assert.Equal(userGroupName, posixEntry.GroupName);
9898
}
99+
100+
[Theory]
101+
[InlineData(TarEntryType.RegularFile)]
102+
[InlineData(TarEntryType.Directory)]
103+
[InlineData(TarEntryType.HardLink)]
104+
[InlineData(TarEntryType.SymbolicLink)]
105+
public async Task PaxExtendedAttributes_DoNotOverwritePublicProperties_WhenTheyFitOnLegacyFieldsAsync(TarEntryType entryType)
106+
{
107+
Dictionary<string, string> extendedAttributes = new();
108+
extendedAttributes[PaxEaName] = "ea_name";
109+
extendedAttributes[PaxEaGName] = "ea_gname";
110+
extendedAttributes[PaxEaUName] = "ea_uname";
111+
extendedAttributes[PaxEaMTime] = GetTimestampStringFromDateTimeOffset(TestModificationTime);
112+
113+
if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
114+
{
115+
extendedAttributes[PaxEaLinkName] = "ea_linkname";
116+
}
117+
118+
PaxTarEntry writeEntry = new PaxTarEntry(entryType, "name", extendedAttributes);
119+
writeEntry.Name = new string('a', 100);
120+
// GName and UName must be longer than 32 to be written as extended attribute.
121+
writeEntry.GroupName = new string('b', 32);
122+
writeEntry.UserName = new string('c', 32);
123+
// There's no limit on MTime, we just ensure it roundtrips.
124+
writeEntry.ModificationTime = TestModificationTime.AddDays(1);
125+
126+
if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
127+
{
128+
writeEntry.LinkName = new string('d', 100);
129+
}
130+
131+
MemoryStream ms = new();
132+
await using (TarWriter w = new(ms, leaveOpen: true))
133+
{
134+
await w.WriteEntryAsync(writeEntry);
135+
}
136+
ms.Position = 0;
137+
138+
await using TarReader r = new(ms);
139+
PaxTarEntry readEntry = Assert.IsType<PaxTarEntry>(await r.GetNextEntryAsync());
140+
Assert.Null(await r.GetNextEntryAsync());
141+
142+
Assert.Equal(writeEntry.Name, readEntry.Name);
143+
Assert.Equal(writeEntry.GroupName, readEntry.GroupName);
144+
Assert.Equal(writeEntry.UserName, readEntry.UserName);
145+
Assert.Equal(writeEntry.ModificationTime, readEntry.ModificationTime);
146+
Assert.Equal(writeEntry.LinkName, readEntry.LinkName);
147+
}
148+
149+
[Theory]
150+
[InlineData(TarEntryType.RegularFile)]
151+
[InlineData(TarEntryType.Directory)]
152+
[InlineData(TarEntryType.HardLink)]
153+
[InlineData(TarEntryType.SymbolicLink)]
154+
public async Task PaxExtendedAttributes_DoNotOverwritePublicProperties_WhenLargerThanLegacyFieldsAsync(TarEntryType entryType)
155+
{
156+
Dictionary<string, string> extendedAttributes = new();
157+
extendedAttributes[PaxEaName] = "ea_name";
158+
extendedAttributes[PaxEaGName] = "ea_gname";
159+
extendedAttributes[PaxEaUName] = "ea_uname";
160+
extendedAttributes[PaxEaMTime] = GetTimestampStringFromDateTimeOffset(TestModificationTime);
161+
162+
if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
163+
{
164+
extendedAttributes[PaxEaLinkName] = "ea_linkname";
165+
}
166+
167+
PaxTarEntry writeEntry = new PaxTarEntry(entryType, "name", extendedAttributes);
168+
writeEntry.Name = new string('a', MaxPathComponent);
169+
// GName and UName must be longer than 32 to be written as extended attribute.
170+
writeEntry.GroupName = new string('b', 32 + 1);
171+
writeEntry.UserName = new string('c', 32 + 1);
172+
// There's no limit on MTime, we just ensure it roundtrips.
173+
writeEntry.ModificationTime = TestModificationTime.AddDays(1);
174+
175+
if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
176+
{
177+
writeEntry.LinkName = new string('d', 100 + 1);
178+
}
179+
180+
MemoryStream ms = new();
181+
await using (TarWriter w = new(ms, leaveOpen: true))
182+
{
183+
await w.WriteEntryAsync(writeEntry);
184+
}
185+
ms.Position = 0;
186+
187+
await using TarReader r = new(ms);
188+
PaxTarEntry readEntry = Assert.IsType<PaxTarEntry>(await r.GetNextEntryAsync());
189+
Assert.Null(await r.GetNextEntryAsync());
190+
191+
Assert.Equal(writeEntry.Name, readEntry.Name);
192+
Assert.Equal(writeEntry.GroupName, readEntry.GroupName);
193+
Assert.Equal(writeEntry.UserName, readEntry.UserName);
194+
Assert.Equal(writeEntry.ModificationTime, readEntry.ModificationTime);
195+
Assert.Equal(writeEntry.LinkName, readEntry.LinkName);
196+
}
99197
}
100198
}

‎src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Tests.cs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,5 +379,34 @@ public async Task WriteEntry_TooLongGroupName_Throws_Async(TarEntryFormat entryF
379379

380380
await Assert.ThrowsAsync<ArgumentException>("entry", () => writer.WriteEntryAsync(entry));
381381
}
382+
383+
public static IEnumerable<object[]> WriteEntry_UsingTarEntry_FromTarReader_IntoTarWriter_Async_TheoryData()
384+
=> TarWriter_WriteEntry_Tests.WriteEntry_UsingTarEntry_FromTarReader_IntoTarWriter_TheoryData();
385+
386+
[Theory]
387+
[MemberData(nameof(WriteEntry_UsingTarEntry_FromTarReader_IntoTarWriter_Async_TheoryData))]
388+
public async Task WriteEntry_UsingTarEntry_FromTarReader_IntoTarWriter_Async(TarEntryFormat entryFormat, TarEntryType entryType, bool unseekableStream)
389+
{
390+
using MemoryStream msSource = new();
391+
using MemoryStream msDestination = new();
392+
393+
WriteTarArchiveWithOneEntry(msSource, entryFormat, entryType);
394+
msSource.Position = 0;
395+
396+
Stream source = new WrappedStream(msSource, msSource.CanRead, msSource.CanWrite, canSeek: !unseekableStream);
397+
Stream destination = new WrappedStream(msDestination, msDestination.CanRead, msDestination.CanWrite, canSeek: !unseekableStream);
398+
399+
await using (TarReader reader = new(source))
400+
await using (TarWriter writer = new(destination))
401+
{
402+
TarEntry entry;
403+
while ((entry = await reader.GetNextEntryAsync()) != null)
404+
{
405+
await writer.WriteEntryAsync(entry);
406+
}
407+
}
408+
409+
AssertExtensions.SequenceEqual(msSource.ToArray(), msDestination.ToArray());
410+
}
382411
}
383412
}

0 commit comments

Comments
 (0)