From 335ace1b99517627058acb7d5bc0568ffa033306 Mon Sep 17 00:00:00 2001 From: David Cantu Date: Thu, 29 Sep 2022 16:39:20 -0500 Subject: [PATCH 1/3] Use indexer setter instead of Add on ExtendedAttributes dictionary --- .../src/System/Formats/Tar/TarHeader.Write.cs | 21 ++++------ .../System.Formats.Tar/tests/TarTestsBase.cs | 15 ++++++- .../TarWriter/TarWriter.WriteEntry.Tests.cs | 40 +++++++++++++++++++ .../TarWriter.WriteEntryAsync.Tests.cs | 29 ++++++++++++++ 4 files changed, 90 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs index 32fe09310ae521..30d0f3dfe47f39 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs @@ -720,37 +720,30 @@ 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; 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); + 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 extendedAttributes, string key, string value, int maxLength) + static void TryAddStringField(Dictionary extendedAttributes, string key, string? value, int maxLength) { - if (Encoding.UTF8.GetByteCount(value) > maxLength) + if (!string.IsNullOrEmpty(value) && GetUtf8TextLength(value) > maxLength) { extendedAttributes.Add(key, value); } diff --git a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs index 977b9fbb606909..81e5b3de3c4bd1 100644 --- a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs +++ b/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); + } } } diff --git a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Tests.cs index e3bec8df31fd11..11d3784c08b58a 100644 --- a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Tests.cs +++ b/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("entry", () => writer.WriteEntry(entry)); } + + public static IEnumerable 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()); + } } } diff --git a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Tests.cs index be70a43bb25543..84ba2d8d83c2a1 100644 --- a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Tests.cs +++ b/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("entry", () => writer.WriteEntryAsync(entry)); } + + public static IEnumerable 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()); + } } } From d472c64de56e649d92c14cf085d92f9790096522 Mon Sep 17 00:00:00 2001 From: David Cantu Date: Fri, 30 Sep 2022 16:48:21 -0500 Subject: [PATCH 2/3] Add roundtrip tests --- ...Writer.WriteEntry.Entry.Roundtrip.Tests.cs | 98 +++++++++++++++++++ ...r.WriteEntryAsync.Entry.Roundtrip.Tests.cs | 98 +++++++++++++++++++ 2 files changed, 196 insertions(+) diff --git a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Entry.Roundtrip.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Entry.Roundtrip.Tests.cs index ae1a3b82163acc..b13863d0b0858e 100644 --- a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Entry.Roundtrip.Tests.cs +++ b/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 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(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 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(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); + } } } diff --git a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.Roundtrip.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.Roundtrip.Tests.cs index 030ffe5d57db3c..727474e50b1259 100644 --- a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.Roundtrip.Tests.cs +++ b/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 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(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 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(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); + } } } From 9d4d7f30c462499c98ef441d62e107f0a37a4bfb Mon Sep 17 00:00:00 2001 From: David Cantu Date: Fri, 30 Sep 2022 16:49:03 -0500 Subject: [PATCH 3/3] Fix TryAddStringField and always set mtime --- .../src/System/Formats/Tar/TarHeader.Write.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs index 30d0f3dfe47f39..c63e19bd5cd0bf 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs @@ -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); ExtendedAttributes[PaxEaLinkName] = _linkName; } @@ -740,12 +737,16 @@ private void CollectExtendedAttributesFromStandardFieldsIfNeeded() ExtendedAttributes[PaxEaSize] = _size.ToString(); } - // 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 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; } } }