Tar: Use indexer setter instead of Add on ExtendedAttributes dictionary

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

@@ -720,39 +720,33 @@ static int CountDigits(int value)
         // extended attributes. They get collected and saved in that dictionary, with no restrictions.
         private void CollectExtendedAttributesFromStandardFieldsIfNeeded()
         {
-            ExtendedAttributes.Add(PaxEaName, _name);
+            ExtendedAttributes[PaxEaName] = _name;
+            ExtendedAttributes[PaxEaMTime] = TarHelpers.GetTimestampStringFromDateTimeOffset(_mTime);
 
-            if (!ExtendedAttributes.ContainsKey(PaxEaMTime))
-            {
-                ExtendedAttributes.Add(PaxEaMTime, TarHelpers.GetTimestampStringFromDateTimeOffset(_mTime));
-            }
-
-            if (!string.IsNullOrEmpty(_gName))
-            {
-                TryAddStringField(ExtendedAttributes, PaxEaGName, _gName, FieldLengths.GName);
-            }
-
-            if (!string.IsNullOrEmpty(_uName))
-            {
-                TryAddStringField(ExtendedAttributes, PaxEaUName, _uName, FieldLengths.UName);
-            }
+            TryAddStringField(ExtendedAttributes, PaxEaGName, _gName, FieldLengths.GName);
+            TryAddStringField(ExtendedAttributes, PaxEaUName, _uName, FieldLengths.UName);
 
             if (!string.IsNullOrEmpty(_linkName))
             {
-                ExtendedAttributes.Add(PaxEaLinkName, _linkName);
+                Debug.Assert(_typeFlag is TarEntryType.SymbolicLink or TarEntryType.HardLink);
+                ExtendedAttributes[PaxEaLinkName] = _linkName;
             }
 
             if (_size > 99_999_999)
             {
-                ExtendedAttributes.Add(PaxEaSize, _size.ToString());
+                ExtendedAttributes[PaxEaSize] = _size.ToString();
             }
 
-            // Adds the specified string to the dictionary if it's longer than the specified max byte length.
-            static void TryAddStringField(Dictionary<string, string> extendedAttributes, string key, string value, int maxLength)
+            // 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 (Encoding.UTF8.GetByteCount(value) > maxLength)
+                if (string.IsNullOrEmpty(value) || GetUtf8TextLength(value) <= maxLength)
+                {
+                    extendedAttributes.Remove(key);
+                }
+                else
                 {
-                    extendedAttributes.Add(key, value);
+                    extendedAttributes[key] = value;
                 }
             }
         }

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

@@ -481,7 +481,7 @@ protected TarEntryType GetTarEntryTypeForTarEntryFormat(TarEntryType entryType,
             return entryType;
         }
 
-        protected TarEntry InvokeTarEntryCreationConstructor(TarEntryFormat targetFormat, TarEntryType entryType, string entryName)
+        protected static TarEntry InvokeTarEntryCreationConstructor(TarEntryFormat targetFormat, TarEntryType entryType, string entryName)
             => targetFormat switch
             {
                 TarEntryFormat.V7 => new V7TarEntry(entryType, entryName),
@@ -796,5 +796,18 @@ internal enum NameCapabilities
             NameAndPrefix,
             Unlimited
         }
+
+        internal static void WriteTarArchiveWithOneEntry(Stream s, TarEntryFormat entryFormat, TarEntryType entryType)
+        {
+            using TarWriter writer = new(s, leaveOpen: true);
+
+            TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, "foo");
+            if (entryType == TarEntryType.SymbolicLink)
+            {
+                entry.LinkName = "bar";
+            }
+
+            writer.WriteEntry(entry);
+        }
     }
 }

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

@@ -147,5 +147,103 @@ public void UserNameGroupNameRoundtrips(TarEntryFormat entryFormat, bool unseeka
             Assert.Equal(userGroupName, posixEntry.UserName);
             Assert.Equal(userGroupName, posixEntry.GroupName);
         }
+
+        [Theory]
+        [InlineData(TarEntryType.RegularFile)]
+        [InlineData(TarEntryType.Directory)]
+        [InlineData(TarEntryType.HardLink)]
+        [InlineData(TarEntryType.SymbolicLink)]
+        public void PaxExtendedAttributes_DoNotOverwritePublicProperties_WhenTheyFitOnLegacyFields(TarEntryType entryType)
+        {
+            Dictionary<string, string> extendedAttributes = new();
+            extendedAttributes[PaxEaName] = "ea_name";
+            extendedAttributes[PaxEaGName] = "ea_gname";
+            extendedAttributes[PaxEaUName] = "ea_uname";
+            extendedAttributes[PaxEaMTime] = GetTimestampStringFromDateTimeOffset(TestModificationTime);
+
+            if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
+            {
+                extendedAttributes[PaxEaLinkName] = "ea_linkname";
+            }
+
+            PaxTarEntry writeEntry = new PaxTarEntry(entryType, "name", extendedAttributes);
+            writeEntry.Name = new string('a', 100);
+            // GName and UName must be longer than 32 to be written as extended attribute.
+            writeEntry.GroupName = new string('b', 32);
+            writeEntry.UserName = new string('c', 32);
+            // There's no limit on MTime, we just ensure it roundtrips.
+            writeEntry.ModificationTime = TestModificationTime.AddDays(1);
+
+            if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
+            {
+                writeEntry.LinkName = new string('d', 100);
+            }
+
+            MemoryStream ms = new();
+            using (TarWriter w = new(ms, leaveOpen: true))
+            {
+                w.WriteEntry(writeEntry);
+            }
+            ms.Position = 0;
+
+            using TarReader r = new(ms);
+            PaxTarEntry readEntry = Assert.IsType<PaxTarEntry>(r.GetNextEntry());
+            Assert.Null(r.GetNextEntry());
+
+            Assert.Equal(writeEntry.Name, readEntry.Name);
+            Assert.Equal(writeEntry.GroupName, readEntry.GroupName);
+            Assert.Equal(writeEntry.UserName, readEntry.UserName);
+            Assert.Equal(writeEntry.ModificationTime, readEntry.ModificationTime);
+            Assert.Equal(writeEntry.LinkName, readEntry.LinkName);
+        }
+
+        [Theory]
+        [InlineData(TarEntryType.RegularFile)]
+        [InlineData(TarEntryType.Directory)]
+        [InlineData(TarEntryType.HardLink)]
+        [InlineData(TarEntryType.SymbolicLink)]
+        public void PaxExtendedAttributes_DoNotOverwritePublicProperties_WhenLargerThanLegacyFields(TarEntryType entryType)
+        {
+            Dictionary<string, string> extendedAttributes = new();
+            extendedAttributes[PaxEaName] = "ea_name";
+            extendedAttributes[PaxEaGName] = "ea_gname";
+            extendedAttributes[PaxEaUName] = "ea_uname";
+            extendedAttributes[PaxEaMTime] = GetTimestampStringFromDateTimeOffset(TestModificationTime);
+
+            if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
+            {
+                extendedAttributes[PaxEaLinkName] = "ea_linkname";
+            }
+
+            PaxTarEntry writeEntry = new PaxTarEntry(entryType, "name", extendedAttributes);
+            writeEntry.Name = new string('a', MaxPathComponent);
+            // GName and UName must be longer than 32 to be written as extended attribute.
+            writeEntry.GroupName = new string('b', 32 + 1);
+            writeEntry.UserName = new string('c', 32 + 1);
+            // There's no limit on MTime, we just ensure it roundtrips.
+            writeEntry.ModificationTime = TestModificationTime.AddDays(1);
+
+            if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
+            {
+                writeEntry.LinkName = new string('d', 100 + 1);
+            }
+
+            MemoryStream ms = new();
+            using (TarWriter w = new(ms, leaveOpen: true))
+            {
+                w.WriteEntry(writeEntry);
+            }
+            ms.Position = 0;
+
+            using TarReader r = new(ms);
+            PaxTarEntry readEntry = Assert.IsType<PaxTarEntry>(r.GetNextEntry());
+            Assert.Null(r.GetNextEntry());
+
+            Assert.Equal(writeEntry.Name, readEntry.Name);
+            Assert.Equal(writeEntry.GroupName, readEntry.GroupName);
+            Assert.Equal(writeEntry.UserName, readEntry.UserName);
+            Assert.Equal(writeEntry.ModificationTime, readEntry.ModificationTime);
+            Assert.Equal(writeEntry.LinkName, readEntry.LinkName);
+        }
     }
 }

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

@@ -450,5 +450,45 @@ public void WriteEntry_TooLongGroupName_Throws(TarEntryFormat entryFormat, strin
 
             Assert.Throws<ArgumentException>("entry", () => writer.WriteEntry(entry));
         }
+
+        public static IEnumerable<object[]> WriteEntry_UsingTarEntry_FromTarReader_IntoTarWriter_TheoryData()
+        {
+            foreach (var entryFormat in new[] { TarEntryFormat.V7, TarEntryFormat.Ustar, TarEntryFormat.Pax, TarEntryFormat.Gnu })
+            {
+                foreach (var entryType in new[] { entryFormat == TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile, TarEntryType.Directory, TarEntryType.SymbolicLink })
+                {
+                    foreach (bool unseekableStream in new[] { false, true })
+                    {
+                        yield return new object[] { entryFormat, entryType, unseekableStream };
+                    }
+                }
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(WriteEntry_UsingTarEntry_FromTarReader_IntoTarWriter_TheoryData))]
+        public void WriteEntry_UsingTarEntry_FromTarReader_IntoTarWriter(TarEntryFormat entryFormat, TarEntryType entryType, bool unseekableStream)
+        {
+            MemoryStream msSource = new();
+            MemoryStream msDestination = new();
+
+            WriteTarArchiveWithOneEntry(msSource, entryFormat, entryType);
+            msSource.Position = 0;
+
+            Stream source = new WrappedStream(msSource, msSource.CanRead, msSource.CanWrite, canSeek: !unseekableStream);
+            Stream destination = new WrappedStream(msDestination, msDestination.CanRead, msDestination.CanWrite, canSeek: !unseekableStream);
+
+            using (TarReader reader = new(source))
+            using (TarWriter writer = new(destination))
+            {
+                TarEntry entry;
+                while ((entry = reader.GetNextEntry()) != null)
+                {
+                    writer.WriteEntry(entry);
+                }
+            }
+
+            AssertExtensions.SequenceEqual(msSource.ToArray(), msDestination.ToArray());
+        }
     }
 }

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

@@ -96,5 +96,103 @@ public async Task UserNameGroupNameRoundtripsAsync(TarEntryFormat entryFormat, b
             Assert.Equal(userGroupName, posixEntry.UserName);
             Assert.Equal(userGroupName, posixEntry.GroupName);
         }
+
+        [Theory]
+        [InlineData(TarEntryType.RegularFile)]
+        [InlineData(TarEntryType.Directory)]
+        [InlineData(TarEntryType.HardLink)]
+        [InlineData(TarEntryType.SymbolicLink)]
+        public async Task PaxExtendedAttributes_DoNotOverwritePublicProperties_WhenTheyFitOnLegacyFieldsAsync(TarEntryType entryType)
+        {
+            Dictionary<string, string> extendedAttributes = new();
+            extendedAttributes[PaxEaName] = "ea_name";
+            extendedAttributes[PaxEaGName] = "ea_gname";
+            extendedAttributes[PaxEaUName] = "ea_uname";
+            extendedAttributes[PaxEaMTime] = GetTimestampStringFromDateTimeOffset(TestModificationTime);
+
+            if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
+            {
+                extendedAttributes[PaxEaLinkName] = "ea_linkname";
+            }
+
+            PaxTarEntry writeEntry = new PaxTarEntry(entryType, "name", extendedAttributes);
+            writeEntry.Name = new string('a', 100);
+            // GName and UName must be longer than 32 to be written as extended attribute.
+            writeEntry.GroupName = new string('b', 32);
+            writeEntry.UserName = new string('c', 32);
+            // There's no limit on MTime, we just ensure it roundtrips.
+            writeEntry.ModificationTime = TestModificationTime.AddDays(1);
+
+            if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
+            {
+                writeEntry.LinkName = new string('d', 100);
+            }
+
+            MemoryStream ms = new();
+            await using (TarWriter w = new(ms, leaveOpen: true))
+            {
+                await w.WriteEntryAsync(writeEntry);
+            }
+            ms.Position = 0;
+
+            await using TarReader r = new(ms);
+            PaxTarEntry readEntry = Assert.IsType<PaxTarEntry>(await r.GetNextEntryAsync());
+            Assert.Null(await r.GetNextEntryAsync());
+
+            Assert.Equal(writeEntry.Name, readEntry.Name);
+            Assert.Equal(writeEntry.GroupName, readEntry.GroupName);
+            Assert.Equal(writeEntry.UserName, readEntry.UserName);
+            Assert.Equal(writeEntry.ModificationTime, readEntry.ModificationTime);
+            Assert.Equal(writeEntry.LinkName, readEntry.LinkName);
+        }
+
+        [Theory]
+        [InlineData(TarEntryType.RegularFile)]
+        [InlineData(TarEntryType.Directory)]
+        [InlineData(TarEntryType.HardLink)]
+        [InlineData(TarEntryType.SymbolicLink)]
+        public async Task PaxExtendedAttributes_DoNotOverwritePublicProperties_WhenLargerThanLegacyFieldsAsync(TarEntryType entryType)
+        {
+            Dictionary<string, string> extendedAttributes = new();
+            extendedAttributes[PaxEaName] = "ea_name";
+            extendedAttributes[PaxEaGName] = "ea_gname";
+            extendedAttributes[PaxEaUName] = "ea_uname";
+            extendedAttributes[PaxEaMTime] = GetTimestampStringFromDateTimeOffset(TestModificationTime);
+
+            if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
+            {
+                extendedAttributes[PaxEaLinkName] = "ea_linkname";
+            }
+
+            PaxTarEntry writeEntry = new PaxTarEntry(entryType, "name", extendedAttributes);
+            writeEntry.Name = new string('a', MaxPathComponent);
+            // GName and UName must be longer than 32 to be written as extended attribute.
+            writeEntry.GroupName = new string('b', 32 + 1);
+            writeEntry.UserName = new string('c', 32 + 1);
+            // There's no limit on MTime, we just ensure it roundtrips.
+            writeEntry.ModificationTime = TestModificationTime.AddDays(1);
+
+            if (entryType is TarEntryType.HardLink or TarEntryType.SymbolicLink)
+            {
+                writeEntry.LinkName = new string('d', 100 + 1);
+            }
+
+            MemoryStream ms = new();
+            await using (TarWriter w = new(ms, leaveOpen: true))
+            {
+                await w.WriteEntryAsync(writeEntry);
+            }
+            ms.Position = 0;
+
+            await using TarReader r = new(ms);
+            PaxTarEntry readEntry = Assert.IsType<PaxTarEntry>(await r.GetNextEntryAsync());
+            Assert.Null(await r.GetNextEntryAsync());
+
+            Assert.Equal(writeEntry.Name, readEntry.Name);
+            Assert.Equal(writeEntry.GroupName, readEntry.GroupName);
+            Assert.Equal(writeEntry.UserName, readEntry.UserName);
+            Assert.Equal(writeEntry.ModificationTime, readEntry.ModificationTime);
+            Assert.Equal(writeEntry.LinkName, readEntry.LinkName);
+        }
     }
 }

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

@@ -379,5 +379,34 @@ public async Task WriteEntry_TooLongGroupName_Throws_Async(TarEntryFormat entryF
 
             await Assert.ThrowsAsync<ArgumentException>("entry", () => writer.WriteEntryAsync(entry));
         }
+
+        public static IEnumerable<object[]> WriteEntry_UsingTarEntry_FromTarReader_IntoTarWriter_Async_TheoryData()
+            => TarWriter_WriteEntry_Tests.WriteEntry_UsingTarEntry_FromTarReader_IntoTarWriter_TheoryData();
+
+        [Theory]
+        [MemberData(nameof(WriteEntry_UsingTarEntry_FromTarReader_IntoTarWriter_Async_TheoryData))]
+        public async Task WriteEntry_UsingTarEntry_FromTarReader_IntoTarWriter_Async(TarEntryFormat entryFormat, TarEntryType entryType, bool unseekableStream)
+        {
+            using MemoryStream msSource = new();
+            using MemoryStream msDestination = new();
+
+            WriteTarArchiveWithOneEntry(msSource, entryFormat, entryType);
+            msSource.Position = 0;
+
+            Stream source = new WrappedStream(msSource, msSource.CanRead, msSource.CanWrite, canSeek: !unseekableStream);
+            Stream destination = new WrappedStream(msDestination, msDestination.CanRead, msDestination.CanWrite, canSeek: !unseekableStream);
+
+            await using (TarReader reader = new(source))
+            await using (TarWriter writer = new(destination))
+            {
+                TarEntry entry;
+                while ((entry = await reader.GetNextEntryAsync()) != null)
+                {
+                    await writer.WriteEntryAsync(entry);
+                }
+            }
+
+            AssertExtensions.SequenceEqual(msSource.ToArray(), msDestination.ToArray());
+        }
     }
 }