From abe6dafcc6ab03ea81928f436bb9e0ffa3b5e0ed Mon Sep 17 00:00:00 2001 From: David Cantu Date: Tue, 20 Sep 2022 10:55:13 -0500 Subject: [PATCH 01/12] Use UTF8 encoding on Tar string fields --- .../src/System/Formats/Tar/TarHeader.Read.cs | 4 +- .../src/System/Formats/Tar/TarHeader.Write.cs | 73 ++++++--- .../tests/System.Formats.Tar.Tests.csproj | 1 + .../System.Formats.Tar/tests/TarTestsBase.cs | 114 +++++++++++++- ...Writer.WriteEntry.Entry.Roundtrip.Tests.cs | 140 ++++++++++++++++++ 5 files changed, 310 insertions(+), 22 deletions(-) create mode 100644 src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Entry.Roundtrip.Tests.cs diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs index ce37a74c304c80..c1eacf8da7c727 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs @@ -517,8 +517,8 @@ private void ReadVersionAttribute(Span buffer) private void ReadPosixAndGnuSharedAttributes(Span buffer) { // Convert the byte arrays - _uName = TarHelpers.GetTrimmedAsciiString(buffer.Slice(FieldLocations.UName, FieldLengths.UName)); - _gName = TarHelpers.GetTrimmedAsciiString(buffer.Slice(FieldLocations.GName, FieldLengths.GName)); + _uName = TarHelpers.GetTrimmedUtf8String(buffer.Slice(FieldLocations.UName, FieldLengths.UName)); + _gName = TarHelpers.GetTrimmedUtf8String(buffer.Slice(FieldLocations.GName, FieldLengths.GName)); // DevMajor and DevMinor only have values with character devices and block devices. // For all other typeflags, the values in these fields are irrelevant. 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 43fe79ce7dc0a6..ae250eae15bb96 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 @@ -358,28 +358,41 @@ private void WriteAsPaxSharedInternal(Span buffer, out long actualLength) _checksum = WriteChecksum(tmpChecksum, buffer); } - // All formats save in the name byte array only the ASCII bytes that fit. + // All formats save in the name byte array only the UTF8 bytes that fit. private int WriteName(Span buffer) { - ReadOnlySpan src = _name.AsSpan(0, Math.Min(_name.Length, FieldLengths.Name)); - Span dest = buffer.Slice(FieldLocations.Name, FieldLengths.Name); - int encoded = Encoding.ASCII.GetBytes(src, dest); - return Checksum(dest.Slice(0, encoded)); + ReadOnlySpan name = _name; + int utf16NameTruncatedLength = GetUtf16TruncatedTextLength(name, FieldLengths.Name); + + ReadOnlySpan truncatedName = name.Slice(0, utf16NameTruncatedLength); + Span destination = buffer.Slice(FieldLocations.Name, FieldLengths.Name); + Encoding.UTF8.GetBytes(truncatedName, destination); + + return Checksum(destination); } - // Ustar and PAX save in the name byte array only the ASCII bytes that fit, and the rest of that string is saved in the prefix field. + // Ustar and PAX save in the name byte array only the UTF8 bytes that fit, and the rest of that string is saved in the prefix field. private int WritePosixName(Span buffer) { - int checksum = WriteName(buffer); + ReadOnlySpan name = _name; + int utf16NameTruncatedLength = GetUtf16TruncatedTextLength(name, FieldLengths.Name); - if (_name.Length > FieldLengths.Name) + ReadOnlySpan truncatedName = name.Slice(0, utf16NameTruncatedLength); + Span destination = buffer.Slice(FieldLocations.Name, FieldLengths.Name); + Encoding.UTF8.GetBytes(truncatedName, destination); + + int checksum = Checksum(destination); + + if (utf16NameTruncatedLength < name.Length) { - int prefixBytesLength = Math.Min(_name.Length - FieldLengths.Name, FieldLengths.Prefix); - Span remaining = stackalloc byte[prefixBytesLength]; - int encoded = Encoding.ASCII.GetBytes(_name.AsSpan(FieldLengths.Name, prefixBytesLength), remaining); - Debug.Assert(encoded == remaining.Length); + ReadOnlySpan prefix = name.Slice(utf16NameTruncatedLength); + int utf16PrefixTruncatedLength = GetUtf16TruncatedTextLength(prefix, FieldLengths.Prefix); + + destination = buffer.Slice(FieldLocations.Prefix, FieldLengths.Prefix); + ReadOnlySpan truncatedPrefix = prefix.Slice(0, utf16PrefixTruncatedLength); + Encoding.UTF8.GetBytes(truncatedPrefix, destination); - checksum += WriteLeftAlignedBytesAndGetChecksum(remaining, buffer.Slice(FieldLocations.Prefix, FieldLengths.Prefix)); + checksum += Checksum(destination); } return checksum; @@ -423,7 +436,7 @@ private int WriteCommonFields(Span buffer, long actualLength, TarEntryType if (!string.IsNullOrEmpty(_linkName)) { - checksum += WriteAsAsciiString(_linkName, buffer.Slice(FieldLocations.LinkName, FieldLengths.LinkName)); + checksum += WriteAsUtf8String(_linkName, buffer.Slice(FieldLocations.LinkName, FieldLengths.LinkName)); } return checksum; @@ -467,12 +480,12 @@ private int WritePosixAndGnuSharedFields(Span buffer) if (!string.IsNullOrEmpty(_uName)) { - checksum += WriteAsAsciiString(_uName, buffer.Slice(FieldLocations.UName, FieldLengths.UName)); + checksum += WriteAsUtf8String(_uName, buffer.Slice(FieldLocations.UName, FieldLengths.UName)); } if (!string.IsNullOrEmpty(_gName)) { - checksum += WriteAsAsciiString(_gName, buffer.Slice(FieldLocations.GName, FieldLengths.GName)); + checksum += WriteAsUtf8String(_gName, buffer.Slice(FieldLocations.GName, FieldLengths.GName)); } if (_devMajor > 0) @@ -766,10 +779,10 @@ private static int WriteAsTimestamp(DateTimeOffset timestamp, Span destina return FormatOctal(unixTimeSeconds, destination); } - // Writes the specified text as an ASCII string aligned to the left, and returns its checksum. - private static int WriteAsAsciiString(string str, Span buffer) + // Writes the specified text as an UTF8 string aligned to the left, and returns its checksum. + private static int WriteAsUtf8String(string str, Span buffer) { - byte[] bytes = Encoding.ASCII.GetBytes(str); + byte[] bytes = Encoding.UTF8.GetBytes(str); return WriteLeftAlignedBytesAndGetChecksum(bytes.AsSpan(), buffer); } @@ -819,5 +832,27 @@ private static string GenerateGlobalExtendedAttributeName(int globalExtendedAttr return result; } + + // Returns the text's utf16 length truncated at the specified max length. + private static int GetUtf16TruncatedTextLength(ReadOnlySpan text, int maxLength) + { + int utf8Length = 0; + int utf16TruncatedLength = 0; + + foreach (Rune rune in text.EnumerateRunes()) + { + utf8Length += rune.Utf8SequenceLength; + if (utf8Length <= maxLength) + { + utf16TruncatedLength += rune.Utf16SequenceLength; + } + else + { + break; + } + } + + return utf16TruncatedLength; + } } } diff --git a/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj b/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj index c43c8dff343f23..5f1c5b83ec2cca 100644 --- a/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj +++ b/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj @@ -47,6 +47,7 @@ + diff --git a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs index ae01821d62b6d6..0f1050937de390 100644 --- a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs +++ b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; @@ -89,6 +89,11 @@ public abstract partial class TarTestsBase : FileCleanupTestBase protected const string PaxEaSize = "size"; protected const string PaxEaDevMajor = "devmajor"; protected const string PaxEaDevMinor = "devminor"; + internal const char OneByteCharacter = 'a'; + internal const char TwoBytesCharacter = 'ö'; + internal const string FourBytesCharacter = "😒"; + internal const char Separator = '/'; + internal const int MaxPathComponent = 255; private static readonly string[] V7TestCaseNames = new[] { @@ -622,5 +627,112 @@ public static IEnumerable GetPaxAndGnuTestCaseNames() yield return new object[] { name }; } } + + private static List GetPrefixes() + { + List prefixes = new() { "", "/", "./", "../" }; + + if (OperatingSystem.IsWindows()) + prefixes.Add("C:/"); + + return prefixes; + } + + internal static IEnumerable GetNamesPrefixedTestData(NameCapabilities max) + { + Assert.True(Enum.IsDefined(max)); + List prefixes = GetPrefixes(); + + foreach (string prefix in prefixes) + { + int nameLength = 100 - prefix.Length; + yield return prefix + Repeat(OneByteCharacter, nameLength); + yield return prefix + Repeat(OneByteCharacter, nameLength - 2) + Repeat(TwoBytesCharacter, 1); + yield return prefix + Repeat(OneByteCharacter, nameLength - 4) + Repeat(FourBytesCharacter, 1); + } + + if (max == NameCapabilities.Name) + yield break; + + // test maxed out prefix. + foreach (string prefix in prefixes) + { + int directoryLength = 155 - prefix.Length; + yield return prefix + Repeat(OneByteCharacter, directoryLength) + Separator + Repeat(OneByteCharacter, 100); + yield return prefix + Repeat(OneByteCharacter, directoryLength - 2) + Repeat(TwoBytesCharacter, 1) + Separator + Repeat(OneByteCharacter, 100); + yield return prefix + Repeat(OneByteCharacter, directoryLength - 4) + Repeat(FourBytesCharacter, 1) + Separator + Repeat(OneByteCharacter, 100); + } + + if (max == NameCapabilities.NameAndPrefix) + yield break; + + foreach (string prefix in prefixes) + { + int directoryLength = MaxPathComponent - prefix.Length; + yield return prefix + Repeat(OneByteCharacter, directoryLength) + Separator + Repeat(OneByteCharacter, MaxPathComponent); + yield return prefix + Repeat(OneByteCharacter, directoryLength - 2) + Repeat(TwoBytesCharacter, 1) + Separator + Repeat(OneByteCharacter, MaxPathComponent); + yield return prefix + Repeat(OneByteCharacter, directoryLength - 4) + Repeat(FourBytesCharacter, 1) + Separator + Repeat(OneByteCharacter, MaxPathComponent); + } + } + + internal static IEnumerable GetNamesNonAsciiTestData(NameCapabilities max) + { + Assert.True(Enum.IsDefined(max)); + + yield return Repeat(OneByteCharacter, 100); + yield return Repeat(TwoBytesCharacter, 100 / 2); + yield return Repeat(OneByteCharacter, 2) + Repeat(TwoBytesCharacter, 49); + + yield return Repeat(FourBytesCharacter, 100 / 4); + yield return Repeat(OneByteCharacter, 4) + Repeat(FourBytesCharacter, 24); + + if (max == NameCapabilities.Name) + yield break; + + // prefix + name + // this is 256 but is supported because prefix is not required to end in separator. + yield return Repeat(OneByteCharacter, 155) + Separator + Repeat(OneByteCharacter, 100); + + // non-ascii prefix + name + yield return Repeat(TwoBytesCharacter, 155 / 2) + Separator + Repeat(OneByteCharacter, 100); + yield return Repeat(FourBytesCharacter, 155 / 4) + Separator + Repeat(OneByteCharacter, 100); + + // prefix + non-ascii name + yield return Repeat(OneByteCharacter, 155) + Separator + Repeat(TwoBytesCharacter, 100 / 2); + yield return Repeat(OneByteCharacter, 155) + Separator + Repeat(FourBytesCharacter, 100 / 4); + + // non-ascii prefix + non-ascii name + yield return Repeat(TwoBytesCharacter, 155 / 2) + Separator + Repeat(TwoBytesCharacter, 100 / 2); + yield return Repeat(FourBytesCharacter, 155 / 4) + Separator + Repeat(FourBytesCharacter, 100 / 4); + + if (max == NameCapabilities.NameAndPrefix) + yield break; + + // Pax and Gnu support unlimited paths. + yield return Repeat(OneByteCharacter, MaxPathComponent); + yield return Repeat(TwoBytesCharacter, MaxPathComponent / 2); + yield return Repeat(FourBytesCharacter, MaxPathComponent / 4); + + yield return Repeat(OneByteCharacter, MaxPathComponent) + Separator + Repeat(OneByteCharacter, MaxPathComponent); + yield return Repeat(TwoBytesCharacter, MaxPathComponent / 2) + Separator + Repeat(TwoBytesCharacter, MaxPathComponent / 2); + yield return Repeat(FourBytesCharacter, MaxPathComponent / 4) + Separator + Repeat(FourBytesCharacter, MaxPathComponent / 4); + } + + internal static string Repeat(char c, int count) + { + return new string(c, count); + } + + internal static string Repeat(string c, int count) + { + return string.Concat(Enumerable.Repeat(c, count)); + } + + internal enum NameCapabilities + { + Name, + NameAndPrefix, + Unlimited + } } } 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 new file mode 100644 index 00000000000000..3460a027b485f8 --- /dev/null +++ b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Entry.Roundtrip.Tests.cs @@ -0,0 +1,140 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Linq; +using Xunit; + +namespace System.Formats.Tar.Tests +{ + public class TarWriter_WriteEntry_Roundtrip_Tests : TarTestsBase + { + public static IEnumerable NameRoundtripsTheoryData() + { + foreach (TarEntryType entryType in new[] { TarEntryType.RegularFile, TarEntryType.Directory }) + { + TarEntryType v7EntryType = entryType is TarEntryType.RegularFile ? TarEntryType.V7RegularFile : entryType; + foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.Name).Concat(GetNamesPrefixedTestData(NameCapabilities.Name))) + { + yield return new object[] { TarEntryFormat.V7, v7EntryType, name }; + } + + // TODO: Use NameCapabilities.NameAndPrefix once https://github.com/dotnet/runtime/issues/75360 is fixed. + foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.Name).Concat(GetNamesPrefixedTestData(NameCapabilities.Name))) + { + yield return new object[] { TarEntryFormat.Ustar, entryType, name }; + } + + foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.Unlimited).Concat(GetNamesPrefixedTestData(NameCapabilities.Unlimited))) + { + yield return new object[] { TarEntryFormat.Pax, entryType, name }; + yield return new object[] { TarEntryFormat.Gnu, entryType, name }; + } + } + } + + [Theory] + [MemberData(nameof(NameRoundtripsTheoryData))] + public void NameRoundtrips(TarEntryFormat entryFormat, TarEntryType entryType, string name) + { + TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, name); + entry.Name = name; + + MemoryStream ms = new(); + using (TarWriter writer = new(ms, leaveOpen: true)) + { + writer.WriteEntry(entry); + } + + ms.Position = 0; + using TarReader reader = new(ms); + + entry = reader.GetNextEntry(); + Assert.Null(reader.GetNextEntry()); + Assert.Equal(name, entry.Name); + } + + public static IEnumerable LinkNameRoundtripsTheoryData() + { + foreach (TarEntryType entryType in new[] { TarEntryType.SymbolicLink, TarEntryType.HardLink }) + { + foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.Name).Concat(GetNamesPrefixedTestData(NameCapabilities.Name))) + { + yield return new object[] { TarEntryFormat.V7, entryType, name }; + // TODO: Use NameCapabilities.NameAndPrefix once https://github.com/dotnet/runtime/issues/75360 is fixed. + yield return new object[] { TarEntryFormat.Ustar, entryType, name }; + } + + foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.Unlimited).Concat(GetNamesPrefixedTestData(NameCapabilities.Unlimited))) + { + yield return new object[] { TarEntryFormat.Pax, entryType, name }; + yield return new object[] { TarEntryFormat.Gnu, entryType, name }; + } + } + } + + [Theory] + [MemberData(nameof(LinkNameRoundtripsTheoryData))] + public void LinkNameRoundtrips(TarEntryFormat entryFormat, TarEntryType entryType, string linkName) + { + string name = "foo"; + TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, name); + entry.LinkName = linkName; + + MemoryStream ms = new(); + using (TarWriter writer = new(ms, leaveOpen: true)) + { + writer.WriteEntry(entry); + } + + ms.Position = 0; + using TarReader reader = new(ms); + + entry = reader.GetNextEntry(); + Assert.Null(reader.GetNextEntry()); + Assert.Equal(name, entry.Name); + Assert.Equal(linkName, entry.LinkName); + } + + + public static IEnumerable UserNameGroupNameRoundtripsTheoryData() + { + foreach (TarEntryFormat entryFormat in new[] { TarEntryFormat.Ustar, TarEntryFormat.Pax, TarEntryFormat.Gnu }) + { + yield return new object[] { entryFormat, Repeat(OneByteCharacter, 32) }; + yield return new object[] { entryFormat, Repeat(TwoBytesCharacter, 32 / 2) }; + yield return new object[] { entryFormat, Repeat(FourBytesCharacter, 32 / 4) }; + } + } + + [Theory] + [MemberData(nameof(UserNameGroupNameRoundtripsTheoryData))] + public void UserNameGroupNameRoundtrips(TarEntryFormat entryFormat, string userGroupName) + { + string name = "foo"; + TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, TarEntryType.RegularFile, name); + PosixTarEntry posixEntry = Assert.IsAssignableFrom(entry); + posixEntry.UserName = userGroupName; + posixEntry.GroupName = userGroupName; + + MemoryStream ms = new(); + using (TarWriter writer = new(ms, leaveOpen: true)) + { + writer.WriteEntry(posixEntry); + } + + ms.Position = 0; + using TarReader reader = new(ms); + + entry = reader.GetNextEntry(); + posixEntry = Assert.IsAssignableFrom(entry); + Assert.Null(reader.GetNextEntry()); + + Assert.Equal(name, posixEntry.Name); + Assert.Equal(userGroupName, posixEntry.UserName); + Assert.Equal(userGroupName, posixEntry.GroupName); + } + } +} From 646a96b1026085995eab5e26e5adac59bd9088f5 Mon Sep 17 00:00:00 2001 From: David Cantu Date: Tue, 20 Sep 2022 11:52:04 -0500 Subject: [PATCH 02/12] Slice destination on Checksum --- .../src/System/Formats/Tar/TarHeader.Write.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 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 ae250eae15bb96..9654da7751d8e5 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 @@ -366,9 +366,9 @@ private int WriteName(Span buffer) ReadOnlySpan truncatedName = name.Slice(0, utf16NameTruncatedLength); Span destination = buffer.Slice(FieldLocations.Name, FieldLengths.Name); - Encoding.UTF8.GetBytes(truncatedName, destination); + int encoded = Encoding.UTF8.GetBytes(truncatedName, destination); - return Checksum(destination); + return Checksum(destination.Slice(0, encoded)); } // Ustar and PAX save in the name byte array only the UTF8 bytes that fit, and the rest of that string is saved in the prefix field. @@ -379,9 +379,9 @@ private int WritePosixName(Span buffer) ReadOnlySpan truncatedName = name.Slice(0, utf16NameTruncatedLength); Span destination = buffer.Slice(FieldLocations.Name, FieldLengths.Name); - Encoding.UTF8.GetBytes(truncatedName, destination); + int encoded = Encoding.UTF8.GetBytes(truncatedName, destination); - int checksum = Checksum(destination); + int checksum = Checksum(destination.Slice(0, encoded)); if (utf16NameTruncatedLength < name.Length) { @@ -390,9 +390,9 @@ private int WritePosixName(Span buffer) destination = buffer.Slice(FieldLocations.Prefix, FieldLengths.Prefix); ReadOnlySpan truncatedPrefix = prefix.Slice(0, utf16PrefixTruncatedLength); - Encoding.UTF8.GetBytes(truncatedPrefix, destination); + encoded = Encoding.UTF8.GetBytes(truncatedPrefix, destination); - checksum += Checksum(destination); + checksum += Checksum(destination.Slice(0, encoded)); } return checksum; From 90a1f51d91755079ae1a79bb3e4a58e39c55308b Mon Sep 17 00:00:00 2001 From: David Cantu Date: Tue, 20 Sep 2022 11:55:01 -0500 Subject: [PATCH 03/12] Use Encoding.GetByteCount as fast path --- .../src/System/Formats/Tar/TarHeader.Write.cs | 6 ++++++ 1 file changed, 6 insertions(+) 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 9654da7751d8e5..48e63c4b67a899 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 @@ -836,6 +836,12 @@ private static string GenerateGlobalExtendedAttributeName(int globalExtendedAttr // Returns the text's utf16 length truncated at the specified max length. private static int GetUtf16TruncatedTextLength(ReadOnlySpan text, int maxLength) { + // fast path, most entries will be smaller than maxLength. + if (Encoding.UTF8.GetByteCount(text) <= maxLength) + { + return text.Length; + } + int utf8Length = 0; int utf16TruncatedLength = 0; From e4b1c692cf85ac18d69dcfa9a282114c8634df50 Mon Sep 17 00:00:00 2001 From: David Cantu Date: Tue, 20 Sep 2022 12:11:30 -0500 Subject: [PATCH 04/12] Use escape sequences on hardcoded UTF8 characters --- src/libraries/System.Formats.Tar/tests/TarTestsBase.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs index 0f1050937de390..2182a4b0879d41 100644 --- a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs +++ b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs @@ -90,8 +90,8 @@ public abstract partial class TarTestsBase : FileCleanupTestBase protected const string PaxEaDevMajor = "devmajor"; protected const string PaxEaDevMinor = "devminor"; internal const char OneByteCharacter = 'a'; - internal const char TwoBytesCharacter = 'ö'; - internal const string FourBytesCharacter = "😒"; + internal const char TwoBytesCharacter = '\u00F6'; + internal const string FourBytesCharacter = "\uD83D\uDE12"; internal const char Separator = '/'; internal const int MaxPathComponent = 255; From 130bad1f293f9bd636cbb39c8177801a60bf611e Mon Sep 17 00:00:00 2001 From: David Cantu Date: Thu, 22 Sep 2022 10:27:47 -0500 Subject: [PATCH 05/12] Fix ustar prefix logic and throw if name would be truncated --- .../src/Resources/Strings.resx | 59 +++++----- .../src/System.Formats.Tar.csproj | 1 + .../src/System/Formats/Tar/TarHeader.Write.cs | 111 +++++++++++++----- .../System.Formats.Tar/tests/TarTestsBase.cs | 56 ++++++++- .../tests/TarWriter/TarWriter.Tests.cs | 20 ++-- .../TarWriter/TarWriter.WriteEntry.Tests.cs | 29 ++++- 6 files changed, 206 insertions(+), 70 deletions(-) diff --git a/src/libraries/System.Formats.Tar/src/Resources/Strings.resx b/src/libraries/System.Formats.Tar/src/Resources/Strings.resx index 5308e9153c9791..b79968d3f4d945 100644 --- a/src/libraries/System.Formats.Tar/src/Resources/Strings.resx +++ b/src/libraries/System.Formats.Tar/src/Resources/Strings.resx @@ -1,17 +1,17 @@  - @@ -261,4 +261,7 @@ Unable to parse number. - + + The name exceeds the maximum allowed length for this format. + + \ No newline at end of file diff --git a/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj b/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj index c0487caf28049e..33a9aa8c1dc0cb 100644 --- a/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj +++ b/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj @@ -65,6 +65,7 @@ + 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 48e63c4b67a899..53c4ada6edeaa6 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 @@ -22,9 +22,11 @@ internal sealed partial class TarHeader private static ReadOnlySpan GnuMagicBytes => "ustar "u8; private static ReadOnlySpan GnuVersionBytes => " \0"u8; + private static ReadOnlySpan SeparatorsAsBytes => Encoding.UTF8.GetBytes(PathInternal.DirectorySeparators); // Predefined text for the Name field of a GNU long metadata entry. Applies for both LongPath ('L') and LongLink ('K'). private const string GnuLongMetadataName = "././@LongLink"; + private const string ArgNameEntry = "entry"; // Writes the current header as a V7 entry into the archive stream. internal void WriteAsV7(Stream archiveStream, Span buffer) @@ -101,7 +103,7 @@ private long WriteUstarFieldsToBuffer(Span buffer) long actualLength = GetTotalDataBytesToWrite(); TarEntryType actualEntryType = TarHelpers.GetCorrectTypeFlagForFormat(TarEntryFormat.Ustar, _typeFlag); - int tmpChecksum = WritePosixName(buffer); + int tmpChecksum = WriteUstarName(buffer); tmpChecksum += WriteCommonFields(buffer, actualLength, actualEntryType); tmpChecksum += WritePosixMagicAndVersion(buffer); tmpChecksum += WritePosixAndGnuSharedFields(buffer); @@ -178,7 +180,7 @@ internal async Task WriteAsPaxAsync(Stream archiveStream, Memory buffer, C internal void WriteAsGnu(Stream archiveStream, Span buffer) { // First, we determine if we need a preceding LongLink, and write it if needed - if (_linkName?.Length > FieldLengths.LinkName) + if (_linkName != null && Encoding.UTF8.GetByteCount(_linkName) > FieldLengths.LinkName) { TarHeader longLinkHeader = GetGnuLongMetadataHeader(TarEntryType.LongLink, _linkName); longLinkHeader.WriteAsGnuInternal(archiveStream, buffer); @@ -186,7 +188,7 @@ internal void WriteAsGnu(Stream archiveStream, Span buffer) } // Second, we determine if we need a preceding LongPath, and write it if needed - if (_name.Length > FieldLengths.Name) + if (Encoding.UTF8.GetByteCount(_name) > FieldLengths.Name) { TarHeader longPathHeader = GetGnuLongMetadataHeader(TarEntryType.LongPath, _name); longPathHeader.WriteAsGnuInternal(archiveStream, buffer); @@ -204,7 +206,7 @@ internal async Task WriteAsGnuAsync(Stream archiveStream, Memory buffer, C cancellationToken.ThrowIfCancellationRequested(); // First, we determine if we need a preceding LongLink, and write it if needed - if (_linkName?.Length > FieldLengths.LinkName) + if (_linkName != null && Encoding.UTF8.GetByteCount(_linkName) > FieldLengths.LinkName) { TarHeader longLinkHeader = GetGnuLongMetadataHeader(TarEntryType.LongLink, _linkName); await longLinkHeader.WriteAsGnuInternalAsync(archiveStream, buffer, cancellationToken).ConfigureAwait(false); @@ -212,7 +214,7 @@ internal async Task WriteAsGnuAsync(Stream archiveStream, Memory buffer, C } // Second, we determine if we need a preceding LongPath, and write it if needed - if (_name.Length > FieldLengths.Name) + if (Encoding.UTF8.GetByteCount(_name) > FieldLengths.Name) { TarHeader longPathHeader = GetGnuLongMetadataHeader(TarEntryType.LongPath, _name); await longPathHeader.WriteAsGnuInternalAsync(archiveStream, buffer, cancellationToken).ConfigureAwait(false); @@ -226,8 +228,7 @@ internal async Task WriteAsGnuAsync(Stream archiveStream, Memory buffer, C // Creates and returns a GNU long metadata header, with the specified long text written into its data stream. private static TarHeader GetGnuLongMetadataHeader(TarEntryType entryType, string longText) { - Debug.Assert((entryType is TarEntryType.LongPath && longText.Length > FieldLengths.Name) || - (entryType is TarEntryType.LongLink && longText.Length > FieldLengths.LinkName)); + Debug.Assert(entryType is TarEntryType.LongPath or TarEntryType.LongLink); TarHeader longMetadataHeader = new(TarEntryFormat.Gnu); @@ -350,7 +351,7 @@ private void WriteAsPaxSharedInternal(Span buffer, out long actualLength) { actualLength = GetTotalDataBytesToWrite(); - int tmpChecksum = WritePosixName(buffer); + int tmpChecksum = WriteName(buffer); tmpChecksum += WriteCommonFields(buffer, actualLength, TarHelpers.GetCorrectTypeFlagForFormat(TarEntryFormat.Pax, _typeFlag)); tmpChecksum += WritePosixMagicAndVersion(buffer); tmpChecksum += WritePosixAndGnuSharedFields(buffer); @@ -358,12 +359,18 @@ private void WriteAsPaxSharedInternal(Span buffer, out long actualLength) _checksum = WriteChecksum(tmpChecksum, buffer); } - // All formats save in the name byte array only the UTF8 bytes that fit. + // Gnu and pax save in the name byte array only the UTF8 bytes that fit. + // V7 does not support more than 100 bytes so it throws. private int WriteName(Span buffer) { ReadOnlySpan name = _name; int utf16NameTruncatedLength = GetUtf16TruncatedTextLength(name, FieldLengths.Name); + if (_format is TarEntryFormat.V7 && name.Length != utf16NameTruncatedLength) + { + throw new ArgumentException(SR.TarEntryNameExceedsMaxLength, ArgNameEntry); + } + ReadOnlySpan truncatedName = name.Slice(0, utf16NameTruncatedLength); Span destination = buffer.Slice(FieldLocations.Name, FieldLengths.Name); int encoded = Encoding.UTF8.GetBytes(truncatedName, destination); @@ -371,31 +378,77 @@ private int WriteName(Span buffer) return Checksum(destination.Slice(0, encoded)); } - // Ustar and PAX save in the name byte array only the UTF8 bytes that fit, and the rest of that string is saved in the prefix field. - private int WritePosixName(Span buffer) + // 'https://www.freebsd.org/cgi/man.cgi?tar(5)' + // If the pathname is too long to fit in the 100 bytes provided by the standard format, + // it can be split at any / character with the first portion going into the prefix field. + private int WriteUstarName(Span buffer) { - ReadOnlySpan name = _name; - int utf16NameTruncatedLength = GetUtf16TruncatedTextLength(name, FieldLengths.Name); + const int MaxPathname = FieldLengths.Prefix + 1 + FieldLengths.Name; + if (Encoding.UTF8.GetByteCount(_name) > MaxPathname) + { + throw new ArgumentException(SR.TarEntryNameExceedsMaxLength, ArgNameEntry); + } - ReadOnlySpan truncatedName = name.Slice(0, utf16NameTruncatedLength); - Span destination = buffer.Slice(FieldLocations.Name, FieldLengths.Name); - int encoded = Encoding.UTF8.GetBytes(truncatedName, destination); + Span encodingBuffer = stackalloc byte[MaxPathname]; + int encoded = Encoding.UTF8.GetBytes(_name, encodingBuffer); + ReadOnlySpan nameAndPrefixBytes = encodingBuffer.Slice(0, encoded); + + if (nameAndPrefixBytes.Length <= FieldLengths.Name) + { + return WriteLeftAlignedBytesAndGetChecksum(nameAndPrefixBytes, buffer.Slice(FieldLocations.Name, FieldLengths.Name)); + } - int checksum = Checksum(destination.Slice(0, encoded)); + int lastIdx = nameAndPrefixBytes.LastIndexOfAny(SeparatorsAsBytes); + ReadOnlySpan name = stackalloc byte[0]; + ReadOnlySpan prefix = stackalloc byte[0]; - if (utf16NameTruncatedLength < name.Length) + if (lastIdx == -1) { - ReadOnlySpan prefix = name.Slice(utf16NameTruncatedLength); - int utf16PrefixTruncatedLength = GetUtf16TruncatedTextLength(prefix, FieldLengths.Prefix); + name = nameAndPrefixBytes; + prefix = default; + } + else + { + name = nameAndPrefixBytes.Slice(lastIdx + 1); - destination = buffer.Slice(FieldLocations.Prefix, FieldLengths.Prefix); - ReadOnlySpan truncatedPrefix = prefix.Slice(0, utf16PrefixTruncatedLength); - encoded = Encoding.UTF8.GetBytes(truncatedPrefix, destination); + if (lastIdx > 0) + { + prefix = nameAndPrefixBytes.Slice(0, lastIdx); + } + else + { + // We cannot neglect the separator if that's the only thing we can place in prefix. + prefix = nameAndPrefixBytes.Slice(0, 1); + } + } - checksum += Checksum(destination.Slice(0, encoded)); + while (prefix.Length - name.Length > FieldLengths.Prefix) + { + lastIdx = prefix.LastIndexOfAny(SeparatorsAsBytes); + if (lastIdx == -1) { break; } + + name = nameAndPrefixBytes.Slice(lastIdx + 1); + if (lastIdx > 0) + { + prefix = prefix.Slice(0, lastIdx); + } + else + { + prefix = prefix.Slice(0, 1); + } } - return checksum; + if (prefix.Length <= FieldLengths.Prefix && name.Length <= FieldLengths.Name) + { + int checksum = WriteLeftAlignedBytesAndGetChecksum(prefix, buffer.Slice(FieldLocations.Prefix, FieldLengths.Prefix)); + checksum += WriteLeftAlignedBytesAndGetChecksum(name, buffer.Slice(FieldLocations.Name, FieldLengths.Name)); + + return checksum; + } + else + { + throw new ArgumentException(SR.TarEntryNameExceedsMaxLength, ArgNameEntry); + } } // Writes all the common fields shared by all formats into the specified spans. @@ -833,11 +886,11 @@ private static string GenerateGlobalExtendedAttributeName(int globalExtendedAttr return result; } - // Returns the text's utf16 length truncated at the specified max length. - private static int GetUtf16TruncatedTextLength(ReadOnlySpan text, int maxLength) + // Returns the text's utf16 length truncated at the specified utf8 max length. + private static int GetUtf16TruncatedTextLength(ReadOnlySpan text, int utf8MaxLength) { // fast path, most entries will be smaller than maxLength. - if (Encoding.UTF8.GetByteCount(text) <= maxLength) + if (Encoding.UTF8.GetByteCount(text) <= utf8MaxLength) { return text.Length; } @@ -848,7 +901,7 @@ private static int GetUtf16TruncatedTextLength(ReadOnlySpan text, int maxL foreach (Rune rune in text.EnumerateRunes()) { utf8Length += rune.Utf8SequenceLength; - if (utf8Length <= maxLength) + if (utf8Length <= utf8MaxLength) { utf16TruncatedLength += rune.Utf16SequenceLength; } diff --git a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs index 2182a4b0879d41..0b997126074562 100644 --- a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs +++ b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs @@ -643,6 +643,7 @@ internal static IEnumerable GetNamesPrefixedTestData(NameCapabilities ma Assert.True(Enum.IsDefined(max)); List prefixes = GetPrefixes(); + // prefix + name of length 100 foreach (string prefix in prefixes) { int nameLength = 100 - prefix.Length; @@ -654,7 +655,15 @@ internal static IEnumerable GetNamesPrefixedTestData(NameCapabilities ma if (max == NameCapabilities.Name) yield break; - // test maxed out prefix. + // maxed out name. + foreach (string prefix in prefixes) + { + yield return prefix + Repeat(OneByteCharacter, 100); + yield return prefix + Repeat(OneByteCharacter, 100 - 2) + Repeat(TwoBytesCharacter, 1); + yield return prefix + Repeat(OneByteCharacter, 100 - 4) + Repeat(FourBytesCharacter, 1); + } + + // maxed out prefix and name. foreach (string prefix in prefixes) { int directoryLength = 155 - prefix.Length; @@ -718,6 +727,51 @@ internal static IEnumerable GetNamesNonAsciiTestData(NameCapabilities ma yield return Repeat(FourBytesCharacter, MaxPathComponent / 4) + Separator + Repeat(FourBytesCharacter, MaxPathComponent / 4); } + internal static IEnumerable GetTooLongNamesTestData(NameCapabilities max) + { + Assert.True(max is NameCapabilities.Name or NameCapabilities.NameAndPrefix); + List prefixes = GetPrefixes(); + + // 1. non-ascii last character doesn't fit in name. + foreach (string prefix in prefixes) + { + // 1.1. last character doesn't fit fully. + yield return prefix + Repeat(OneByteCharacter, 100 + 1); + yield return prefix + Repeat(OneByteCharacter, 100 - 2) + Repeat(TwoBytesCharacter, 2); + yield return prefix + Repeat(OneByteCharacter, 100 - 4) + Repeat(FourBytesCharacter, 2); + + // 1.2. last character doesn't fit by one byte. + yield return prefix + Repeat(OneByteCharacter, 100 - 2 + 1) + Repeat(TwoBytesCharacter, 1); + yield return prefix + Repeat(OneByteCharacter, 100 - 4 + 1) + Repeat(FourBytesCharacter, 1); + } + + // 2. non-ascii last character doesn't fit in prefix. + string maxedOutName = Repeat(OneByteCharacter, 100); + + // 2.1. last char doesn't fit fully. + yield return Repeat(OneByteCharacter, 155 + 1) + Separator + maxedOutName; + yield return Repeat(OneByteCharacter, 155 - 2) + Repeat(TwoBytesCharacter, 2) + Separator + maxedOutName; + yield return Repeat(OneByteCharacter, 155 - 4) + Repeat(FourBytesCharacter, 2) + Separator + maxedOutName; + + // 2.2 last char doesn't fit by one byte. + yield return Repeat(OneByteCharacter, 155 - 2 + 1) + Repeat(TwoBytesCharacter, 1) + Separator + maxedOutName; + yield return Repeat(OneByteCharacter, 155 - 4 + 1) + Repeat(FourBytesCharacter, 1) + Separator + maxedOutName; + + if (max is NameCapabilities.NameAndPrefix) + yield break; + + // Next cases only apply for V7 which only allows 100 length names. + foreach (string prefix in prefixes) + { + if (prefix.Length == 0) + continue; + + yield return prefix + Repeat(OneByteCharacter, 100); + yield return prefix + Repeat(TwoBytesCharacter, 100 / 2); + yield return prefix + Repeat(FourBytesCharacter, 100 / 4); + } + } + internal static string Repeat(char c, int count) { return new string(c, count); diff --git a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.Tests.cs index 8d482af0b1dff1..65b35689f5d802 100644 --- a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.Tests.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Collections.Generic; using System.IO; using Xunit; @@ -195,17 +194,18 @@ private int GetNameChecksum(TarEntryFormat format, bool longPath, out string ent } else { - entryName = new string('a', 150); - // 100 * 97 = 9700 (first 100 bytes go into 'name' field) - expectedChecksum += 9700; + entryName = new string('a', 100); + expectedChecksum += 9700; // 100 * 97 = 9700 (first 100 bytes go into 'name' field) + + // V7 does not support name fields larger than 100 + if (format is not TarEntryFormat.V7) + entryName += "/" + new string('a', 50); - // - V7 does not support name fields larger than 100, writes what it can - // - Gnu writes first 100 bytes in 'name' field, then the full name is written in a LonPath entry - // that precedes this one. - if (format is TarEntryFormat.Ustar or TarEntryFormat.Pax) + // Gnu and Pax writes first 100 bytes in 'name' field, then the full name is written in a metadata entry that precedes this one. + if (format is TarEntryFormat.Ustar) { - // 50 * 97 = 4850 (rest of bytes go into 'prefix' field) - expectedChecksum += 4850; + // Ustar can write the directory into prefix. + expectedChecksum += 4850; // 50 * 97 = 4850 } } return expectedChecksum; 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 2808914db4e52a..d70e5d924c1914 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 @@ -302,8 +302,6 @@ public void WriteTimestampsBeyondOctalLimit(TarEntryFormat format) } [Theory] - [InlineData(TarEntryFormat.V7)] - // [InlineData(TarEntryFormat.Ustar)] https://github.com/dotnet/runtime/issues/75360 [InlineData(TarEntryFormat.Pax)] [InlineData(TarEntryFormat.Gnu)] public void WriteLongName(TarEntryFormat format) @@ -352,5 +350,32 @@ string GetExpectedNameForFormat(TarEntryFormat format, string expectedName) return expectedName; } } + + public static IEnumerable WriteEntry_TooLongName_Throws_TheoryData() + { + foreach (TarEntryType entryType in new[] { TarEntryType.RegularFile, TarEntryType.Directory }) + { + foreach (string name in GetTooLongNamesTestData(NameCapabilities.Name)) + { + TarEntryType v7EntryType = entryType is TarEntryType.RegularFile ? TarEntryType.V7RegularFile : entryType; + yield return new object[] { TarEntryFormat.V7, v7EntryType, name }; + } + + foreach (string name in GetTooLongNamesTestData(NameCapabilities.NameAndPrefix)) + { + yield return new object[] { TarEntryFormat.Ustar, entryType, name }; + } + } + } + + [Theory] + [MemberData(nameof(WriteEntry_TooLongName_Throws_TheoryData))] + public void WriteEntry_TooLongName_Throws(TarEntryFormat entryFormat, TarEntryType entryType, string name) + { + using TarWriter writer = new(new MemoryStream()); + + TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, name); + Assert.Throws("entry", () => writer.WriteEntry(entry)); + } } } From 512eb0405c9748ca1321f9617636d3f88dfdd231 Mon Sep 17 00:00:00 2001 From: David Cantu Date: Thu, 22 Sep 2022 12:28:22 -0500 Subject: [PATCH 06/12] Address feedback --- .../Common/src/System/IO/PathInternal.Unix.cs | 1 + .../src/System/IO/PathInternal.Windows.cs | 1 + .../src/System/Formats/Tar/TarHeader.Write.cs | 36 +++++++------------ 3 files changed, 14 insertions(+), 24 deletions(-) diff --git a/src/libraries/Common/src/System/IO/PathInternal.Unix.cs b/src/libraries/Common/src/System/IO/PathInternal.Unix.cs index 5bb8ab93fd9b23..4914071a448717 100644 --- a/src/libraries/Common/src/System/IO/PathInternal.Unix.cs +++ b/src/libraries/Common/src/System/IO/PathInternal.Unix.cs @@ -17,6 +17,7 @@ internal static partial class PathInternal internal const string DirectorySeparatorCharAsString = "/"; internal const string ParentDirectoryPrefix = @"../"; internal const string DirectorySeparators = DirectorySeparatorCharAsString; + internal static ReadOnlySpan Utf8DirectorySeparators => "/"u8; internal static int GetRootLength(ReadOnlySpan path) { diff --git a/src/libraries/Common/src/System/IO/PathInternal.Windows.cs b/src/libraries/Common/src/System/IO/PathInternal.Windows.cs index 3bc5b493c37845..e0aff52855590c 100644 --- a/src/libraries/Common/src/System/IO/PathInternal.Windows.cs +++ b/src/libraries/Common/src/System/IO/PathInternal.Windows.cs @@ -55,6 +55,7 @@ internal static partial class PathInternal internal const string DevicePathPrefix = @"\\.\"; internal const string ParentDirectoryPrefix = @"..\"; internal const string DirectorySeparators = @"\/"; + internal static ReadOnlySpan Utf8DirectorySeparators => @"\/"u8; internal const int MaxShortPath = 260; internal const int MaxShortDirectoryPath = 248; 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 53c4ada6edeaa6..3ddbf7d9606d0d 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 @@ -22,7 +22,6 @@ internal sealed partial class TarHeader private static ReadOnlySpan GnuMagicBytes => "ustar "u8; private static ReadOnlySpan GnuVersionBytes => " \0"u8; - private static ReadOnlySpan SeparatorsAsBytes => Encoding.UTF8.GetBytes(PathInternal.DirectorySeparators); // Predefined text for the Name field of a GNU long metadata entry. Applies for both LongPath ('L') and LongLink ('K'). private const string GnuLongMetadataName = "././@LongLink"; @@ -398,9 +397,9 @@ private int WriteUstarName(Span buffer) return WriteLeftAlignedBytesAndGetChecksum(nameAndPrefixBytes, buffer.Slice(FieldLocations.Name, FieldLengths.Name)); } - int lastIdx = nameAndPrefixBytes.LastIndexOfAny(SeparatorsAsBytes); - ReadOnlySpan name = stackalloc byte[0]; - ReadOnlySpan prefix = stackalloc byte[0]; + int lastIdx = nameAndPrefixBytes.LastIndexOfAny(PathInternal.Utf8DirectorySeparators); + scoped ReadOnlySpan name; + scoped ReadOnlySpan prefix; if (lastIdx == -1) { @@ -410,32 +409,21 @@ private int WriteUstarName(Span buffer) else { name = nameAndPrefixBytes.Slice(lastIdx + 1); - - if (lastIdx > 0) - { - prefix = nameAndPrefixBytes.Slice(0, lastIdx); - } - else - { - // We cannot neglect the separator if that's the only thing we can place in prefix. - prefix = nameAndPrefixBytes.Slice(0, 1); - } + prefix = nameAndPrefixBytes.Slice(0, Math.Max(lastIdx, 1)); // need at least the separator } + // At this point nameAndPrefixBytes.Length > 100. + // Attempt to split it in a way it can use prefix. while (prefix.Length - name.Length > FieldLengths.Prefix) { - lastIdx = prefix.LastIndexOfAny(SeparatorsAsBytes); - if (lastIdx == -1) { break; } - - name = nameAndPrefixBytes.Slice(lastIdx + 1); - if (lastIdx > 0) - { - prefix = prefix.Slice(0, lastIdx); - } - else + lastIdx = prefix.LastIndexOfAny(PathInternal.Utf8DirectorySeparators); + if (lastIdx < 0) { - prefix = prefix.Slice(0, 1); + break; } + + name = nameAndPrefixBytes.Slice(lastIdx + 1); + prefix = nameAndPrefixBytes.Slice(0, Math.Max(lastIdx, 1)); // need at least the separator } if (prefix.Length <= FieldLengths.Prefix && name.Length <= FieldLengths.Name) From d774bf0d9ca18a94de6042bf4145826c86cf2a95 Mon Sep 17 00:00:00 2001 From: David Cantu Date: Mon, 26 Sep 2022 12:34:26 -0700 Subject: [PATCH 07/12] Fix truncation and prefix logic --- .../src/Resources/Strings.resx | 4 +- .../src/System/Formats/Tar/TarHeader.Write.cs | 117 ++++++++++++------ .../System.Formats.Tar/tests/TarTestsBase.cs | 32 +++-- .../tests/TarWriter/TarWriter.Tests.cs | 18 ++- ...Writer.WriteEntry.Entry.Roundtrip.Tests.cs | 7 +- .../TarWriter/TarWriter.WriteEntry.Tests.cs | 70 +++++++++++ 6 files changed, 185 insertions(+), 63 deletions(-) diff --git a/src/libraries/System.Formats.Tar/src/Resources/Strings.resx b/src/libraries/System.Formats.Tar/src/Resources/Strings.resx index b79968d3f4d945..90001cf8bcc603 100644 --- a/src/libraries/System.Formats.Tar/src/Resources/Strings.resx +++ b/src/libraries/System.Formats.Tar/src/Resources/Strings.resx @@ -261,7 +261,7 @@ Unable to parse number. - - The name exceeds the maximum allowed length for this format. + + The field '{0}' exceeds the maximum allowed length for this format. \ No newline at end of file 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 3ddbf7d9606d0d..ca208de9b09f86 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 @@ -5,9 +5,7 @@ using System.Buffers.Text; using System.Collections.Generic; using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; using System.IO; -using System.Numerics; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -363,18 +361,20 @@ private void WriteAsPaxSharedInternal(Span buffer, out long actualLength) private int WriteName(Span buffer) { ReadOnlySpan name = _name; - int utf16NameTruncatedLength = GetUtf16TruncatedTextLength(name, FieldLengths.Name); + int encodedLength = GetUtf8TextLength(name); - if (_format is TarEntryFormat.V7 && name.Length != utf16NameTruncatedLength) + if (encodedLength > FieldLengths.Name) { - throw new ArgumentException(SR.TarEntryNameExceedsMaxLength, ArgNameEntry); - } + if (_format is TarEntryFormat.V7) + { + throw new ArgumentException(SR.Format(SR.TarEntryFieldExceedsMaxLength, nameof(TarEntry.Name)), ArgNameEntry); + } - ReadOnlySpan truncatedName = name.Slice(0, utf16NameTruncatedLength); - Span destination = buffer.Slice(FieldLocations.Name, FieldLengths.Name); - int encoded = Encoding.UTF8.GetBytes(truncatedName, destination); + int utf16NameTruncatedLength = GetUtf16TruncatedTextLength(name, FieldLengths.Name); + name = name.Slice(0, utf16NameTruncatedLength); + } - return Checksum(destination.Slice(0, encoded)); + return WriteAsUtf8String(name, buffer.Slice(FieldLocations.Name, FieldLengths.Name)); } // 'https://www.freebsd.org/cgi/man.cgi?tar(5)' @@ -382,52 +382,58 @@ private int WriteName(Span buffer) // it can be split at any / character with the first portion going into the prefix field. private int WriteUstarName(Span buffer) { - const int MaxPathname = FieldLengths.Prefix + 1 + FieldLengths.Name; - if (Encoding.UTF8.GetByteCount(_name) > MaxPathname) + // We can have a pathname as big as 256, prefix + '/' + name, + // the separator in between can be neglected as the reader will append it when it joins both fields. + const int MaxPathname = FieldLengths.Prefix + FieldLengths.Name + 1; + + if (GetUtf8TextLength(_name) > MaxPathname) { - throw new ArgumentException(SR.TarEntryNameExceedsMaxLength, ArgNameEntry); + throw new ArgumentException(SR.TarEntryFieldExceedsMaxLength, ArgNameEntry); } Span encodingBuffer = stackalloc byte[MaxPathname]; int encoded = Encoding.UTF8.GetBytes(_name, encodingBuffer); - ReadOnlySpan nameAndPrefixBytes = encodingBuffer.Slice(0, encoded); + ReadOnlySpan pathnameBytes = encodingBuffer.Slice(0, encoded); - if (nameAndPrefixBytes.Length <= FieldLengths.Name) + // If the pathname is able to fit in Name, we can write it down there and avoid calculating Prefix. + if (pathnameBytes.Length <= FieldLengths.Name) { - return WriteLeftAlignedBytesAndGetChecksum(nameAndPrefixBytes, buffer.Slice(FieldLocations.Name, FieldLengths.Name)); + return WriteLeftAlignedBytesAndGetChecksum(pathnameBytes, buffer.Slice(FieldLocations.Name, FieldLengths.Name)); } - int lastIdx = nameAndPrefixBytes.LastIndexOfAny(PathInternal.Utf8DirectorySeparators); + int lastIdx = pathnameBytes.LastIndexOfAny(PathInternal.Utf8DirectorySeparators); scoped ReadOnlySpan name; scoped ReadOnlySpan prefix; - if (lastIdx == -1) + if (lastIdx < 1) // splitting at the root is not allowed. { - name = nameAndPrefixBytes; + name = pathnameBytes; prefix = default; } else { - name = nameAndPrefixBytes.Slice(lastIdx + 1); - prefix = nameAndPrefixBytes.Slice(0, Math.Max(lastIdx, 1)); // need at least the separator + name = pathnameBytes.Slice(lastIdx + 1); + prefix = pathnameBytes.Slice(0, lastIdx); } - // At this point nameAndPrefixBytes.Length > 100. + // At this point pathnameBytes.Length > 100. // Attempt to split it in a way it can use prefix. while (prefix.Length - name.Length > FieldLengths.Prefix) { lastIdx = prefix.LastIndexOfAny(PathInternal.Utf8DirectorySeparators); - if (lastIdx < 0) + if (lastIdx < 1) { break; } - name = nameAndPrefixBytes.Slice(lastIdx + 1); - prefix = nameAndPrefixBytes.Slice(0, Math.Max(lastIdx, 1)); // need at least the separator + name = pathnameBytes.Slice(lastIdx + 1); + prefix = pathnameBytes.Slice(0, lastIdx); } if (prefix.Length <= FieldLengths.Prefix && name.Length <= FieldLengths.Name) { + Debug.Assert(prefix.Length != 1 || !PathInternal.Utf8DirectorySeparators.Contains(prefix[0])); + int checksum = WriteLeftAlignedBytesAndGetChecksum(prefix, buffer.Slice(FieldLocations.Prefix, FieldLengths.Prefix)); checksum += WriteLeftAlignedBytesAndGetChecksum(name, buffer.Slice(FieldLocations.Name, FieldLengths.Name)); @@ -435,7 +441,7 @@ private int WriteUstarName(Span buffer) } else { - throw new ArgumentException(SR.TarEntryNameExceedsMaxLength, ArgNameEntry); + throw new ArgumentException(SR.Format(SR.TarEntryFieldExceedsMaxLength, nameof(TarEntry.Name)), ArgNameEntry); } } @@ -477,7 +483,20 @@ private int WriteCommonFields(Span buffer, long actualLength, TarEntryType if (!string.IsNullOrEmpty(_linkName)) { - checksum += WriteAsUtf8String(_linkName, buffer.Slice(FieldLocations.LinkName, FieldLengths.LinkName)); + ReadOnlySpan linkName = _linkName; + + if (GetUtf8TextLength(linkName) > FieldLengths.LinkName) + { + if (_format is not TarEntryFormat.Pax and not TarEntryFormat.Gnu) + { + throw new ArgumentException(SR.Format(SR.TarEntryFieldExceedsMaxLength, nameof(TarEntry.LinkName)), ArgNameEntry); + } + + int truncatedLength = GetUtf16TruncatedTextLength(linkName, FieldLengths.LinkName); + linkName = linkName.Slice(0, truncatedLength); + } + + checksum += WriteAsUtf8String(linkName, buffer.Slice(FieldLocations.LinkName, FieldLengths.LinkName)); } return checksum; @@ -521,12 +540,37 @@ private int WritePosixAndGnuSharedFields(Span buffer) if (!string.IsNullOrEmpty(_uName)) { - checksum += WriteAsUtf8String(_uName, buffer.Slice(FieldLocations.UName, FieldLengths.UName)); + ReadOnlySpan uName = _uName; + + if (GetUtf8TextLength(uName) > FieldLengths.UName) + { + if (_format is not TarEntryFormat.Pax) + { + throw new ArgumentException(SR.Format(SR.TarEntryFieldExceedsMaxLength, nameof(PaxTarEntry.UserName)), ArgNameEntry); + } + + int truncatedLength = GetUtf16TruncatedTextLength(uName, FieldLengths.UName); + uName = uName.Slice(0, truncatedLength); + } + + checksum += WriteAsUtf8String(uName, buffer.Slice(FieldLocations.UName, FieldLengths.UName)); } if (!string.IsNullOrEmpty(_gName)) { - checksum += WriteAsUtf8String(_gName, buffer.Slice(FieldLocations.GName, FieldLengths.GName)); + ReadOnlySpan gName = _gName; + if (GetUtf8TextLength(gName) > FieldLengths.GName) + { + if (_format is not TarEntryFormat.Pax) + { + throw new ArgumentException(SR.Format(SR.TarEntryFieldExceedsMaxLength, nameof(PaxTarEntry.GroupName)), ArgNameEntry); + } + + int truncatedLength = GetUtf16TruncatedTextLength(gName, FieldLengths.UName); + gName = gName.Slice(0, truncatedLength); + } + + checksum += WriteAsUtf8String(gName, buffer.Slice(FieldLocations.GName, FieldLengths.GName)); } if (_devMajor > 0) @@ -821,10 +865,10 @@ private static int WriteAsTimestamp(DateTimeOffset timestamp, Span destina } // Writes the specified text as an UTF8 string aligned to the left, and returns its checksum. - private static int WriteAsUtf8String(string str, Span buffer) + private static int WriteAsUtf8String(ReadOnlySpan text, Span buffer) { - byte[] bytes = Encoding.UTF8.GetBytes(str); - return WriteLeftAlignedBytesAndGetChecksum(bytes.AsSpan(), buffer); + int encoded = Encoding.UTF8.GetBytes(text, buffer); + return WriteLeftAlignedBytesAndGetChecksum(buffer.Slice(0, encoded), buffer); } // Gets the special name for the 'name' field in an extended attribute entry. @@ -874,15 +918,12 @@ private static string GenerateGlobalExtendedAttributeName(int globalExtendedAttr return result; } + private static int GetUtf8TextLength(ReadOnlySpan text) + => Encoding.UTF8.GetByteCount(text); + // Returns the text's utf16 length truncated at the specified utf8 max length. private static int GetUtf16TruncatedTextLength(ReadOnlySpan text, int utf8MaxLength) { - // fast path, most entries will be smaller than maxLength. - if (Encoding.UTF8.GetByteCount(text) <= utf8MaxLength) - { - return text.Length; - } - int utf8Length = 0; int utf16TruncatedLength = 0; diff --git a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs index 0b997126074562..977b9fbb606909 100644 --- a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs +++ b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs @@ -630,7 +630,7 @@ public static IEnumerable GetPaxAndGnuTestCaseNames() private static List GetPrefixes() { - List prefixes = new() { "", "/", "./", "../" }; + List prefixes = new() { "", "/a/", "./", "../" }; if (OperatingSystem.IsWindows()) prefixes.Add("C:/"); @@ -643,13 +643,17 @@ internal static IEnumerable GetNamesPrefixedTestData(NameCapabilities ma Assert.True(Enum.IsDefined(max)); List prefixes = GetPrefixes(); - // prefix + name of length 100 foreach (string prefix in prefixes) { + // prefix + name of length 100 int nameLength = 100 - prefix.Length; yield return prefix + Repeat(OneByteCharacter, nameLength); - yield return prefix + Repeat(OneByteCharacter, nameLength - 2) + Repeat(TwoBytesCharacter, 1); - yield return prefix + Repeat(OneByteCharacter, nameLength - 4) + Repeat(FourBytesCharacter, 1); + yield return prefix + Repeat(OneByteCharacter, nameLength - 2) + TwoBytesCharacter; + yield return prefix + Repeat(OneByteCharacter, nameLength - 4) + FourBytesCharacter; + + // prefix alone + if (prefix != string.Empty) + yield return prefix; } if (max == NameCapabilities.Name) @@ -659,8 +663,8 @@ internal static IEnumerable GetNamesPrefixedTestData(NameCapabilities ma foreach (string prefix in prefixes) { yield return prefix + Repeat(OneByteCharacter, 100); - yield return prefix + Repeat(OneByteCharacter, 100 - 2) + Repeat(TwoBytesCharacter, 1); - yield return prefix + Repeat(OneByteCharacter, 100 - 4) + Repeat(FourBytesCharacter, 1); + yield return prefix + Repeat(OneByteCharacter, 100 - 2) + TwoBytesCharacter; + yield return prefix + Repeat(OneByteCharacter, 100 - 4) + FourBytesCharacter; } // maxed out prefix and name. @@ -668,8 +672,8 @@ internal static IEnumerable GetNamesPrefixedTestData(NameCapabilities ma { int directoryLength = 155 - prefix.Length; yield return prefix + Repeat(OneByteCharacter, directoryLength) + Separator + Repeat(OneByteCharacter, 100); - yield return prefix + Repeat(OneByteCharacter, directoryLength - 2) + Repeat(TwoBytesCharacter, 1) + Separator + Repeat(OneByteCharacter, 100); - yield return prefix + Repeat(OneByteCharacter, directoryLength - 4) + Repeat(FourBytesCharacter, 1) + Separator + Repeat(OneByteCharacter, 100); + yield return prefix + Repeat(OneByteCharacter, directoryLength - 2) + TwoBytesCharacter + Separator + Repeat(OneByteCharacter, 100); + yield return prefix + Repeat(OneByteCharacter, directoryLength - 4) + FourBytesCharacter + Separator + Repeat(OneByteCharacter, 100); } if (max == NameCapabilities.NameAndPrefix) @@ -679,8 +683,8 @@ internal static IEnumerable GetNamesPrefixedTestData(NameCapabilities ma { int directoryLength = MaxPathComponent - prefix.Length; yield return prefix + Repeat(OneByteCharacter, directoryLength) + Separator + Repeat(OneByteCharacter, MaxPathComponent); - yield return prefix + Repeat(OneByteCharacter, directoryLength - 2) + Repeat(TwoBytesCharacter, 1) + Separator + Repeat(OneByteCharacter, MaxPathComponent); - yield return prefix + Repeat(OneByteCharacter, directoryLength - 4) + Repeat(FourBytesCharacter, 1) + Separator + Repeat(OneByteCharacter, MaxPathComponent); + yield return prefix + Repeat(OneByteCharacter, directoryLength - 2) + TwoBytesCharacter + Separator + Repeat(OneByteCharacter, MaxPathComponent); + yield return prefix + Repeat(OneByteCharacter, directoryLength - 4) + FourBytesCharacter + Separator + Repeat(OneByteCharacter, MaxPathComponent); } } @@ -730,6 +734,10 @@ internal static IEnumerable GetNamesNonAsciiTestData(NameCapabilities ma internal static IEnumerable GetTooLongNamesTestData(NameCapabilities max) { Assert.True(max is NameCapabilities.Name or NameCapabilities.NameAndPrefix); + + // root directory can't be saved as prefix + yield return "/" + Repeat(OneByteCharacter, 100); + List prefixes = GetPrefixes(); // 1. non-ascii last character doesn't fit in name. @@ -754,8 +762,8 @@ internal static IEnumerable GetTooLongNamesTestData(NameCapabilities max yield return Repeat(OneByteCharacter, 155 - 4) + Repeat(FourBytesCharacter, 2) + Separator + maxedOutName; // 2.2 last char doesn't fit by one byte. - yield return Repeat(OneByteCharacter, 155 - 2 + 1) + Repeat(TwoBytesCharacter, 1) + Separator + maxedOutName; - yield return Repeat(OneByteCharacter, 155 - 4 + 1) + Repeat(FourBytesCharacter, 1) + Separator + maxedOutName; + yield return Repeat(OneByteCharacter, 155 - 2 + 1) + TwoBytesCharacter + Separator + maxedOutName; + yield return Repeat(OneByteCharacter, 155 - 4 + 1) + FourBytesCharacter + Separator + maxedOutName; if (max is NameCapabilities.NameAndPrefix) yield break; diff --git a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.Tests.cs index 65b35689f5d802..b46816844b44cb 100644 --- a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.Tests.cs @@ -173,7 +173,7 @@ private TarEntry CreateTarEntryAndGetExpectedChecksum(TarEntryFormat format, Tar if (entryType is TarEntryType.SymbolicLink) { - expectedChecksum += GetLinkChecksum(longLink, out string linkName); + expectedChecksum += GetLinkChecksum(format, longLink, out string linkName); entry.LinkName = linkName; } @@ -199,7 +199,9 @@ private int GetNameChecksum(TarEntryFormat format, bool longPath, out string ent // V7 does not support name fields larger than 100 if (format is not TarEntryFormat.V7) + { entryName += "/" + new string('a', 50); + } // Gnu and Pax writes first 100 bytes in 'name' field, then the full name is written in a metadata entry that precedes this one. if (format is TarEntryFormat.Ustar) @@ -211,7 +213,7 @@ private int GetNameChecksum(TarEntryFormat format, bool longPath, out string ent return expectedChecksum; } - private int GetLinkChecksum(bool longLink, out string linkName) + private int GetLinkChecksum(TarEntryFormat format, bool longLink, out string linkName) { int expectedChecksum = 0; if (!longLink) @@ -222,12 +224,16 @@ private int GetLinkChecksum(bool longLink, out string linkName) } else { - linkName = new string('a', 150); - // 100 * 97 = 9700 (first 100 bytes go into 'linkName' field) + linkName = new string('a', 100); // 100 * 97 = 9700 (first 100 bytes go into 'linkName' field) expectedChecksum += 9700; - // - V7 and Ustar ignore the rest of the bytes - // - Pax and Gnu write first 100 bytes in 'linkName' field, then the full link name is written in the + + // V7 and Ustar does not support name fields larger than 100 + // Pax and Gnu write first 100 bytes in 'linkName' field, then the full link name is written in the // preceding metadata entry (extended attributes for PAX, LongLink for GNU). + if (format is not TarEntryFormat.V7 and not TarEntryFormat.Ustar) + { + linkName += "/" + new string('a', 50); + } } return expectedChecksum; 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 3460a027b485f8..5d4abcb753367a 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 @@ -15,14 +15,13 @@ public static IEnumerable NameRoundtripsTheoryData() { foreach (TarEntryType entryType in new[] { TarEntryType.RegularFile, TarEntryType.Directory }) { - TarEntryType v7EntryType = entryType is TarEntryType.RegularFile ? TarEntryType.V7RegularFile : entryType; foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.Name).Concat(GetNamesPrefixedTestData(NameCapabilities.Name))) { + TarEntryType v7EntryType = entryType is TarEntryType.RegularFile ? TarEntryType.V7RegularFile : entryType; yield return new object[] { TarEntryFormat.V7, v7EntryType, name }; } - // TODO: Use NameCapabilities.NameAndPrefix once https://github.com/dotnet/runtime/issues/75360 is fixed. - foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.Name).Concat(GetNamesPrefixedTestData(NameCapabilities.Name))) + foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.NameAndPrefix).Concat(GetNamesPrefixedTestData(NameCapabilities.NameAndPrefix))) { yield return new object[] { TarEntryFormat.Ustar, entryType, name }; } @@ -63,7 +62,6 @@ public static IEnumerable LinkNameRoundtripsTheoryData() foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.Name).Concat(GetNamesPrefixedTestData(NameCapabilities.Name))) { yield return new object[] { TarEntryFormat.V7, entryType, name }; - // TODO: Use NameCapabilities.NameAndPrefix once https://github.com/dotnet/runtime/issues/75360 is fixed. yield return new object[] { TarEntryFormat.Ustar, entryType, name }; } @@ -98,7 +96,6 @@ public void LinkNameRoundtrips(TarEntryFormat entryFormat, TarEntryType entryTyp Assert.Equal(linkName, entry.LinkName); } - public static IEnumerable UserNameGroupNameRoundtripsTheoryData() { foreach (TarEntryFormat entryFormat in new[] { TarEntryFormat.Ustar, TarEntryFormat.Pax, TarEntryFormat.Gnu }) 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 d70e5d924c1914..a6a7cb579d0a4b 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 @@ -377,5 +377,75 @@ public void WriteEntry_TooLongName_Throws(TarEntryFormat entryFormat, TarEntryTy TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, name); Assert.Throws("entry", () => writer.WriteEntry(entry)); } + + public static IEnumerable WriteEntry_TooLongLinkName_Throws_TheoryData() + { + foreach (TarEntryType entryType in new[] { TarEntryType.SymbolicLink, TarEntryType.HardLink }) + { + foreach (string name in GetTooLongNamesTestData(NameCapabilities.Name)) + { + yield return new object[] { TarEntryFormat.V7, entryType, name }; + } + + foreach (string name in GetTooLongNamesTestData(NameCapabilities.NameAndPrefix)) + { + yield return new object[] { TarEntryFormat.Ustar, entryType, name }; + } + } + } + + [Theory] + [MemberData(nameof(WriteEntry_TooLongLinkName_Throws_TheoryData))] // change usar, v7 + public void WriteEntry_TooLongLinkName_Throws(TarEntryFormat entryFormat, TarEntryType entryType, string linkName) + { + using TarWriter writer = new(new MemoryStream()); + + TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, "foo"); + entry.LinkName = linkName; + + Assert.Throws("entry", () => writer.WriteEntry(entry)); + } + + public static IEnumerable WriteEntry_TooLongUserGroupName_Throws_TheoryData() + { + // Not testing Pax as it supports unlimited size uname/gname. + foreach (TarEntryFormat entryFormat in new[] { TarEntryFormat.Ustar, TarEntryFormat.Gnu }) + { + // Last character doesn't fit fully. + yield return new object[] { entryFormat, Repeat(OneByteCharacter, 32 + 1) }; + yield return new object[] { entryFormat, Repeat(TwoBytesCharacter, 32 / 2 + 1) }; + yield return new object[] { entryFormat, Repeat(FourBytesCharacter, 32 / 4 + 1) }; + + // Last character doesn't fit by one byte. + yield return new object[] { entryFormat, Repeat(TwoBytesCharacter, 32 - 2 + 1) + TwoBytesCharacter }; + yield return new object[] { entryFormat, Repeat(FourBytesCharacter, 32 - 4 + 1) + FourBytesCharacter }; + } + } + + [Theory] + [MemberData(nameof(WriteEntry_TooLongUserGroupName_Throws_TheoryData))] + public void WriteEntry_TooLongUserName_Throws(TarEntryFormat entryFormat, string userName) + { + using TarWriter writer = new(new MemoryStream()); + + TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, TarEntryType.RegularFile, "foo"); + PosixTarEntry posixEntry = Assert.IsAssignableFrom(entry); + posixEntry.UserName = userName; + + Assert.Throws("entry", () => writer.WriteEntry(entry)); + } + + [Theory] + [MemberData(nameof(WriteEntry_TooLongUserGroupName_Throws_TheoryData))] + public void WriteEntry_TooLongGroupName_Throws(TarEntryFormat entryFormat, string groupName) + { + using TarWriter writer = new(new MemoryStream()); + + TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, TarEntryType.RegularFile, "foo"); + PosixTarEntry posixEntry = Assert.IsAssignableFrom(entry); + posixEntry.GroupName = groupName; + + Assert.Throws("entry", () => writer.WriteEntry(entry)); + } } } From 818ab422ffc9e81e5162b18f29428a051cdac73f Mon Sep 17 00:00:00 2001 From: David Cantu Date: Mon, 26 Sep 2022 20:57:06 -0700 Subject: [PATCH 08/12] Fix nits --- .../src/System/Formats/Tar/TarHeader.Write.cs | 5 ++++- .../tests/TarWriter/TarWriter.WriteEntry.Tests.cs | 2 +- 2 files changed, 5 insertions(+), 2 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 ca208de9b09f86..f9392a03e75016 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 @@ -559,6 +559,7 @@ private int WritePosixAndGnuSharedFields(Span buffer) if (!string.IsNullOrEmpty(_gName)) { ReadOnlySpan gName = _gName; + if (GetUtf8TextLength(gName) > FieldLengths.GName) { if (_format is not TarEntryFormat.Pax) @@ -566,7 +567,7 @@ private int WritePosixAndGnuSharedFields(Span buffer) throw new ArgumentException(SR.Format(SR.TarEntryFieldExceedsMaxLength, nameof(PaxTarEntry.GroupName)), ArgNameEntry); } - int truncatedLength = GetUtf16TruncatedTextLength(gName, FieldLengths.UName); + int truncatedLength = GetUtf16TruncatedTextLength(gName, FieldLengths.GName); gName = gName.Slice(0, truncatedLength); } @@ -924,6 +925,8 @@ private static int GetUtf8TextLength(ReadOnlySpan text) // Returns the text's utf16 length truncated at the specified utf8 max length. private static int GetUtf16TruncatedTextLength(ReadOnlySpan text, int utf8MaxLength) { + Debug.Assert(GetUtf8TextLength(text) > utf8MaxLength); + int utf8Length = 0; int utf16TruncatedLength = 0; 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 a6a7cb579d0a4b..17ecf4ef398cb9 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 @@ -395,7 +395,7 @@ public static IEnumerable WriteEntry_TooLongLinkName_Throws_TheoryData } [Theory] - [MemberData(nameof(WriteEntry_TooLongLinkName_Throws_TheoryData))] // change usar, v7 + [MemberData(nameof(WriteEntry_TooLongLinkName_Throws_TheoryData))] public void WriteEntry_TooLongLinkName_Throws(TarEntryFormat entryFormat, TarEntryType entryType, string linkName) { using TarWriter writer = new(new MemoryStream()); From ae3a6230be141c40af09aed8754854701b93596d Mon Sep 17 00:00:00 2001 From: David Cantu Date: Mon, 26 Sep 2022 21:14:38 -0700 Subject: [PATCH 09/12] Add async tests --- .../tests/System.Formats.Tar.Tests.csproj | 1 + ...Writer.WriteEntry.Entry.Roundtrip.Tests.cs | 1 - ...r.WriteEntryAsync.Entry.Roundtrip.Tests.cs | 94 +++++++++++++++++++ .../TarWriter.WriteEntryAsync.Tests.cs | 57 +++++++++++ 4 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.Roundtrip.Tests.cs diff --git a/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj b/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj index 5f1c5b83ec2cca..d4741850029265 100644 --- a/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj +++ b/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj @@ -51,6 +51,7 @@ + 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 5d4abcb753367a..cdf9521f4d2c85 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 @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; -using System.Diagnostics; using System.IO; using System.Linq; using Xunit; 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 new file mode 100644 index 00000000000000..132108108a5d54 --- /dev/null +++ b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.Entry.Roundtrip.Tests.cs @@ -0,0 +1,94 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.IO; +using System.Threading.Tasks; +using Xunit; + +namespace System.Formats.Tar.Tests +{ + public class TarWriter_WriteEntryAsync_Roundtrip_Tests : TarTestsBase + { + public static IEnumerable NameRoundtripsAsyncTheoryData() + => TarWriter_WriteEntry_Roundtrip_Tests.NameRoundtripsTheoryData(); + + [Theory] + [MemberData(nameof(NameRoundtripsAsyncTheoryData))] + public async Task NameRoundtripsAsync(TarEntryFormat entryFormat, TarEntryType entryType, string name) + { + TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, name); + entry.Name = name; + + MemoryStream ms = new(); + using (TarWriter writer = new(ms, leaveOpen: true)) + { + await writer.WriteEntryAsync(entry); + } + + ms.Position = 0; + using TarReader reader = new(ms); + + entry = await reader.GetNextEntryAsync(); + Assert.Null(await reader.GetNextEntryAsync()); + Assert.Equal(name, entry.Name); + } + + public static IEnumerable LinkNameRoundtripsAsyncTheoryData() + => TarWriter_WriteEntry_Roundtrip_Tests.LinkNameRoundtripsTheoryData(); + + [Theory] + [MemberData(nameof(LinkNameRoundtripsAsyncTheoryData))] + public async Task LinkNameRoundtrips(TarEntryFormat entryFormat, TarEntryType entryType, string linkName) + { + string name = "foo"; + TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, name); + entry.LinkName = linkName; + + MemoryStream ms = new(); + using (TarWriter writer = new(ms, leaveOpen: true)) + { + await writer.WriteEntryAsync(entry); + } + + ms.Position = 0; + using TarReader reader = new(ms); + + entry = await reader.GetNextEntryAsync(); + Assert.Null(await reader.GetNextEntryAsync()); + Assert.Equal(name, entry.Name); + Assert.Equal(linkName, entry.LinkName); + } + + public static IEnumerable UserNameGroupNameRoundtripsAsyncTheoryData() + => TarWriter_WriteEntry_Roundtrip_Tests.UserNameGroupNameRoundtripsTheoryData(); + + [Theory] + [MemberData(nameof(UserNameGroupNameRoundtripsAsyncTheoryData))] + public async Task UserNameGroupNameRoundtrips(TarEntryFormat entryFormat, string userGroupName) + { + string name = "foo"; + TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, TarEntryType.RegularFile, name); + PosixTarEntry posixEntry = Assert.IsAssignableFrom(entry); + posixEntry.UserName = userGroupName; + posixEntry.GroupName = userGroupName; + + MemoryStream ms = new(); + using (TarWriter writer = new(ms, leaveOpen: true)) + { + await writer.WriteEntryAsync(posixEntry); + } + + ms.Position = 0; + using TarReader reader = new(ms); + + entry = await reader.GetNextEntryAsync(); + posixEntry = Assert.IsAssignableFrom(entry); + Assert.Null(await reader.GetNextEntryAsync()); + + Assert.Equal(name, posixEntry.Name); + Assert.Equal(userGroupName, posixEntry.UserName); + Assert.Equal(userGroupName, posixEntry.GroupName); + } + } +} 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 c2eb58a7f1f24a..b1070ac96c92c9 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 @@ -322,5 +322,62 @@ public async Task WriteTimestampsBeyondOctalLimit_Async(TarEntryFormat format) } } } + + public static IEnumerable WriteEntry_TooLongName_Throws_Async_TheoryData() + => TarWriter_WriteEntry_Tests.WriteEntry_TooLongName_Throws_TheoryData(); + + [Theory] + [MemberData(nameof(WriteEntry_TooLongName_Throws_Async_TheoryData))] + public async Task WriteEntry_TooLongName_Throws_Async(TarEntryFormat entryFormat, TarEntryType entryType, string name) + { + using TarWriter writer = new(new MemoryStream()); + + TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, name); + await Assert.ThrowsAsync("entry", () => writer.WriteEntryAsync(entry)); + } + + public static IEnumerable WriteEntry_TooLongLinkName_Throws_Async_TheoryData() + => TarWriter_WriteEntry_Tests.WriteEntry_TooLongLinkName_Throws_TheoryData(); + + [Theory] + [MemberData(nameof(WriteEntry_TooLongLinkName_Throws_Async_TheoryData))] + public async Task WriteEntry_TooLongLinkName_Throws_Async(TarEntryFormat entryFormat, TarEntryType entryType, string linkName) + { + using TarWriter writer = new(new MemoryStream()); + + TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, "foo"); + entry.LinkName = linkName; + + await Assert.ThrowsAsync("entry", () => writer.WriteEntryAsync(entry)); + } + + public static IEnumerable WriteEntry_TooLongUserGroupName_Throws_Async_TheoryData() + => TarWriter_WriteEntry_Tests.WriteEntry_TooLongUserGroupName_Throws_TheoryData(); + + [Theory] + [MemberData(nameof(WriteEntry_TooLongUserGroupName_Throws_Async_TheoryData))] + public async Task WriteEntry_TooLongUserName_Throws(TarEntryFormat entryFormat, string userName) + { + using TarWriter writer = new(new MemoryStream()); + + TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, TarEntryType.RegularFile, "foo"); + PosixTarEntry posixEntry = Assert.IsAssignableFrom(entry); + posixEntry.UserName = userName; + + await Assert.ThrowsAsync("entry", () => writer.WriteEntryAsync(entry)); + } + + [Theory] + [MemberData(nameof(WriteEntry_TooLongUserGroupName_Throws_Async_TheoryData))] + public async Task WriteEntry_TooLongGroupName_Throws(TarEntryFormat entryFormat, string groupName) + { + using TarWriter writer = new(new MemoryStream()); + + TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, TarEntryType.RegularFile, "foo"); + PosixTarEntry posixEntry = Assert.IsAssignableFrom(entry); + posixEntry.GroupName = groupName; + + await Assert.ThrowsAsync("entry", () => writer.WriteEntryAsync(entry)); + } } } From 14571b03af2c71fcf42d46fdd596eb12c4696cf5 Mon Sep 17 00:00:00 2001 From: David Cantu Date: Mon, 26 Sep 2022 21:30:30 -0700 Subject: [PATCH 10/12] Add tests for unseekable streams --- ...Writer.WriteEntry.Entry.Roundtrip.Tests.cs | 91 +++++++++++-------- ...r.WriteEntryAsync.Entry.Roundtrip.Tests.cs | 24 +++-- 2 files changed, 68 insertions(+), 47 deletions(-) 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 cdf9521f4d2c85..ae1a3b82163acc 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 @@ -12,42 +12,47 @@ public class TarWriter_WriteEntry_Roundtrip_Tests : TarTestsBase { public static IEnumerable NameRoundtripsTheoryData() { - foreach (TarEntryType entryType in new[] { TarEntryType.RegularFile, TarEntryType.Directory }) + foreach (bool unseekableStream in new[] { false, true }) { - foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.Name).Concat(GetNamesPrefixedTestData(NameCapabilities.Name))) + foreach (TarEntryType entryType in new[] { TarEntryType.RegularFile, TarEntryType.Directory }) { - TarEntryType v7EntryType = entryType is TarEntryType.RegularFile ? TarEntryType.V7RegularFile : entryType; - yield return new object[] { TarEntryFormat.V7, v7EntryType, name }; - } - - foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.NameAndPrefix).Concat(GetNamesPrefixedTestData(NameCapabilities.NameAndPrefix))) - { - yield return new object[] { TarEntryFormat.Ustar, entryType, name }; - } - - foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.Unlimited).Concat(GetNamesPrefixedTestData(NameCapabilities.Unlimited))) - { - yield return new object[] { TarEntryFormat.Pax, entryType, name }; - yield return new object[] { TarEntryFormat.Gnu, entryType, name }; + foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.Name).Concat(GetNamesPrefixedTestData(NameCapabilities.Name))) + { + TarEntryType v7EntryType = entryType is TarEntryType.RegularFile ? TarEntryType.V7RegularFile : entryType; + yield return new object[] { TarEntryFormat.V7, v7EntryType, unseekableStream, name }; + } + + foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.NameAndPrefix).Concat(GetNamesPrefixedTestData(NameCapabilities.NameAndPrefix))) + { + yield return new object[] { TarEntryFormat.Ustar, entryType, unseekableStream, name }; + } + + foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.Unlimited).Concat(GetNamesPrefixedTestData(NameCapabilities.Unlimited))) + { + yield return new object[] { TarEntryFormat.Pax, entryType, unseekableStream, name }; + yield return new object[] { TarEntryFormat.Gnu, entryType, unseekableStream, name }; + } } } } [Theory] [MemberData(nameof(NameRoundtripsTheoryData))] - public void NameRoundtrips(TarEntryFormat entryFormat, TarEntryType entryType, string name) + public void NameRoundtrips(TarEntryFormat entryFormat, TarEntryType entryType, bool unseekableStream, string name) { TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, name); entry.Name = name; MemoryStream ms = new(); - using (TarWriter writer = new(ms, leaveOpen: true)) + Stream s = unseekableStream ? new WrappedStream(ms, ms.CanRead, ms.CanWrite, canSeek: false) : ms; + + using (TarWriter writer = new(s, leaveOpen: true)) { writer.WriteEntry(entry); } ms.Position = 0; - using TarReader reader = new(ms); + using TarReader reader = new(s); entry = reader.GetNextEntry(); Assert.Null(reader.GetNextEntry()); @@ -56,38 +61,43 @@ public void NameRoundtrips(TarEntryFormat entryFormat, TarEntryType entryType, s public static IEnumerable LinkNameRoundtripsTheoryData() { - foreach (TarEntryType entryType in new[] { TarEntryType.SymbolicLink, TarEntryType.HardLink }) + foreach (bool unseekableStream in new[] { false, true }) { - foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.Name).Concat(GetNamesPrefixedTestData(NameCapabilities.Name))) + foreach (TarEntryType entryType in new[] { TarEntryType.SymbolicLink, TarEntryType.HardLink }) { - yield return new object[] { TarEntryFormat.V7, entryType, name }; - yield return new object[] { TarEntryFormat.Ustar, entryType, name }; - } - - foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.Unlimited).Concat(GetNamesPrefixedTestData(NameCapabilities.Unlimited))) - { - yield return new object[] { TarEntryFormat.Pax, entryType, name }; - yield return new object[] { TarEntryFormat.Gnu, entryType, name }; + foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.Name).Concat(GetNamesPrefixedTestData(NameCapabilities.Name))) + { + yield return new object[] { TarEntryFormat.V7, entryType, unseekableStream, name }; + yield return new object[] { TarEntryFormat.Ustar, entryType, unseekableStream, name }; + } + + foreach (string name in GetNamesNonAsciiTestData(NameCapabilities.Unlimited).Concat(GetNamesPrefixedTestData(NameCapabilities.Unlimited))) + { + yield return new object[] { TarEntryFormat.Pax, entryType, unseekableStream, name }; + yield return new object[] { TarEntryFormat.Gnu, entryType, unseekableStream, name }; + } } } } [Theory] [MemberData(nameof(LinkNameRoundtripsTheoryData))] - public void LinkNameRoundtrips(TarEntryFormat entryFormat, TarEntryType entryType, string linkName) + public void LinkNameRoundtrips(TarEntryFormat entryFormat, TarEntryType entryType, bool unseekableStream, string linkName) { string name = "foo"; TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, name); entry.LinkName = linkName; MemoryStream ms = new(); - using (TarWriter writer = new(ms, leaveOpen: true)) + Stream s = unseekableStream ? new WrappedStream(ms, ms.CanRead, ms.CanWrite, canSeek: false) : ms; + + using (TarWriter writer = new(s, leaveOpen: true)) { writer.WriteEntry(entry); } ms.Position = 0; - using TarReader reader = new(ms); + using TarReader reader = new(s); entry = reader.GetNextEntry(); Assert.Null(reader.GetNextEntry()); @@ -97,17 +107,20 @@ public void LinkNameRoundtrips(TarEntryFormat entryFormat, TarEntryType entryTyp public static IEnumerable UserNameGroupNameRoundtripsTheoryData() { - foreach (TarEntryFormat entryFormat in new[] { TarEntryFormat.Ustar, TarEntryFormat.Pax, TarEntryFormat.Gnu }) + foreach (bool unseekableStream in new[] { false, true }) { - yield return new object[] { entryFormat, Repeat(OneByteCharacter, 32) }; - yield return new object[] { entryFormat, Repeat(TwoBytesCharacter, 32 / 2) }; - yield return new object[] { entryFormat, Repeat(FourBytesCharacter, 32 / 4) }; + foreach (TarEntryFormat entryFormat in new[] { TarEntryFormat.Ustar, TarEntryFormat.Pax, TarEntryFormat.Gnu }) + { + yield return new object[] { entryFormat, unseekableStream, Repeat(OneByteCharacter, 32) }; + yield return new object[] { entryFormat, unseekableStream, Repeat(TwoBytesCharacter, 32 / 2) }; + yield return new object[] { entryFormat, unseekableStream, Repeat(FourBytesCharacter, 32 / 4) }; + } } } [Theory] [MemberData(nameof(UserNameGroupNameRoundtripsTheoryData))] - public void UserNameGroupNameRoundtrips(TarEntryFormat entryFormat, string userGroupName) + public void UserNameGroupNameRoundtrips(TarEntryFormat entryFormat, bool unseekableStream, string userGroupName) { string name = "foo"; TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, TarEntryType.RegularFile, name); @@ -116,13 +129,15 @@ public void UserNameGroupNameRoundtrips(TarEntryFormat entryFormat, string userG posixEntry.GroupName = userGroupName; MemoryStream ms = new(); - using (TarWriter writer = new(ms, leaveOpen: true)) + Stream s = unseekableStream ? new WrappedStream(ms, ms.CanRead, ms.CanWrite, canSeek: false) : ms; + + using (TarWriter writer = new(s, leaveOpen: true)) { writer.WriteEntry(posixEntry); } ms.Position = 0; - using TarReader reader = new(ms); + using TarReader reader = new(s); entry = reader.GetNextEntry(); posixEntry = Assert.IsAssignableFrom(entry); 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 132108108a5d54..3c0348d02156a0 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 @@ -15,19 +15,21 @@ public static IEnumerable NameRoundtripsAsyncTheoryData() [Theory] [MemberData(nameof(NameRoundtripsAsyncTheoryData))] - public async Task NameRoundtripsAsync(TarEntryFormat entryFormat, TarEntryType entryType, string name) + public async Task NameRoundtripsAsync(TarEntryFormat entryFormat, TarEntryType entryType, bool unseekableStream, string name) { TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, name); entry.Name = name; MemoryStream ms = new(); - using (TarWriter writer = new(ms, leaveOpen: true)) + Stream s = unseekableStream ? new WrappedStream(ms, ms.CanRead, ms.CanWrite, canSeek: false) : ms; + + using (TarWriter writer = new(s, leaveOpen: true)) { await writer.WriteEntryAsync(entry); } ms.Position = 0; - using TarReader reader = new(ms); + using TarReader reader = new(s); entry = await reader.GetNextEntryAsync(); Assert.Null(await reader.GetNextEntryAsync()); @@ -39,20 +41,22 @@ public static IEnumerable LinkNameRoundtripsAsyncTheoryData() [Theory] [MemberData(nameof(LinkNameRoundtripsAsyncTheoryData))] - public async Task LinkNameRoundtrips(TarEntryFormat entryFormat, TarEntryType entryType, string linkName) + public async Task LinkNameRoundtrips(TarEntryFormat entryFormat, TarEntryType entryType, bool unseekableStream, string linkName) { string name = "foo"; TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, name); entry.LinkName = linkName; MemoryStream ms = new(); - using (TarWriter writer = new(ms, leaveOpen: true)) + Stream s = unseekableStream ? new WrappedStream(ms, ms.CanRead, ms.CanWrite, canSeek: false) : ms; + + using (TarWriter writer = new(s, leaveOpen: true)) { await writer.WriteEntryAsync(entry); } ms.Position = 0; - using TarReader reader = new(ms); + using TarReader reader = new(s); entry = await reader.GetNextEntryAsync(); Assert.Null(await reader.GetNextEntryAsync()); @@ -65,7 +69,7 @@ public static IEnumerable UserNameGroupNameRoundtripsAsyncTheoryData() [Theory] [MemberData(nameof(UserNameGroupNameRoundtripsAsyncTheoryData))] - public async Task UserNameGroupNameRoundtrips(TarEntryFormat entryFormat, string userGroupName) + public async Task UserNameGroupNameRoundtrips(TarEntryFormat entryFormat, bool unseekableStream, string userGroupName) { string name = "foo"; TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, TarEntryType.RegularFile, name); @@ -74,13 +78,15 @@ public async Task UserNameGroupNameRoundtrips(TarEntryFormat entryFormat, string posixEntry.GroupName = userGroupName; MemoryStream ms = new(); - using (TarWriter writer = new(ms, leaveOpen: true)) + Stream s = unseekableStream ? new WrappedStream(ms, ms.CanRead, ms.CanWrite, canSeek: false) : ms; + + using (TarWriter writer = new(s, leaveOpen: true)) { await writer.WriteEntryAsync(posixEntry); } ms.Position = 0; - using TarReader reader = new(ms); + using TarReader reader = new(s); entry = await reader.GetNextEntryAsync(); posixEntry = Assert.IsAssignableFrom(entry); From a255c73960e9411147a23b178d036dbae7d953a7 Mon Sep 17 00:00:00 2001 From: David Cantu Date: Wed, 28 Sep 2022 10:22:35 -0500 Subject: [PATCH 11/12] Address feedback --- .../src/System/Formats/Tar/TarHeader.Write.cs | 32 +++++++++---------- ...r.WriteEntryAsync.Entry.Roundtrip.Tests.cs | 16 +++++----- .../TarWriter.WriteEntryAsync.Tests.cs | 12 +++---- 3 files changed, 30 insertions(+), 30 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 f9392a03e75016..32fe09310ae521 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 @@ -378,45 +378,45 @@ private int WriteName(Span buffer) } // 'https://www.freebsd.org/cgi/man.cgi?tar(5)' - // If the pathname is too long to fit in the 100 bytes provided by the standard format, + // If the path name is too long to fit in the 100 bytes provided by the standard format, // it can be split at any / character with the first portion going into the prefix field. private int WriteUstarName(Span buffer) { - // We can have a pathname as big as 256, prefix + '/' + name, + // We can have a path name as big as 256, prefix + '/' + name, // the separator in between can be neglected as the reader will append it when it joins both fields. - const int MaxPathname = FieldLengths.Prefix + FieldLengths.Name + 1; + const int MaxPathName = FieldLengths.Prefix + 1 + FieldLengths.Name; - if (GetUtf8TextLength(_name) > MaxPathname) + if (GetUtf8TextLength(_name) > MaxPathName) { - throw new ArgumentException(SR.TarEntryFieldExceedsMaxLength, ArgNameEntry); + throw new ArgumentException(SR.Format(SR.TarEntryFieldExceedsMaxLength, nameof(TarEntry.Name)), ArgNameEntry); } - Span encodingBuffer = stackalloc byte[MaxPathname]; + Span encodingBuffer = stackalloc byte[MaxPathName]; int encoded = Encoding.UTF8.GetBytes(_name, encodingBuffer); - ReadOnlySpan pathnameBytes = encodingBuffer.Slice(0, encoded); + ReadOnlySpan pathNameBytes = encodingBuffer.Slice(0, encoded); // If the pathname is able to fit in Name, we can write it down there and avoid calculating Prefix. - if (pathnameBytes.Length <= FieldLengths.Name) + if (pathNameBytes.Length <= FieldLengths.Name) { - return WriteLeftAlignedBytesAndGetChecksum(pathnameBytes, buffer.Slice(FieldLocations.Name, FieldLengths.Name)); + return WriteLeftAlignedBytesAndGetChecksum(pathNameBytes, buffer.Slice(FieldLocations.Name, FieldLengths.Name)); } - int lastIdx = pathnameBytes.LastIndexOfAny(PathInternal.Utf8DirectorySeparators); + int lastIdx = pathNameBytes.LastIndexOfAny(PathInternal.Utf8DirectorySeparators); scoped ReadOnlySpan name; scoped ReadOnlySpan prefix; if (lastIdx < 1) // splitting at the root is not allowed. { - name = pathnameBytes; + name = pathNameBytes; prefix = default; } else { - name = pathnameBytes.Slice(lastIdx + 1); - prefix = pathnameBytes.Slice(0, lastIdx); + name = pathNameBytes.Slice(lastIdx + 1); + prefix = pathNameBytes.Slice(0, lastIdx); } - // At this point pathnameBytes.Length > 100. + // At this point path name is > 100. // Attempt to split it in a way it can use prefix. while (prefix.Length - name.Length > FieldLengths.Prefix) { @@ -426,8 +426,8 @@ private int WriteUstarName(Span buffer) break; } - name = pathnameBytes.Slice(lastIdx + 1); - prefix = pathnameBytes.Slice(0, lastIdx); + name = pathNameBytes.Slice(lastIdx + 1); + prefix = pathNameBytes.Slice(0, lastIdx); } if (prefix.Length <= FieldLengths.Prefix && name.Length <= FieldLengths.Name) 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 3c0348d02156a0..030ffe5d57db3c 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 @@ -23,13 +23,13 @@ public async Task NameRoundtripsAsync(TarEntryFormat entryFormat, TarEntryType e MemoryStream ms = new(); Stream s = unseekableStream ? new WrappedStream(ms, ms.CanRead, ms.CanWrite, canSeek: false) : ms; - using (TarWriter writer = new(s, leaveOpen: true)) + await using (TarWriter writer = new(s, leaveOpen: true)) { await writer.WriteEntryAsync(entry); } ms.Position = 0; - using TarReader reader = new(s); + await using TarReader reader = new(s); entry = await reader.GetNextEntryAsync(); Assert.Null(await reader.GetNextEntryAsync()); @@ -41,7 +41,7 @@ public static IEnumerable LinkNameRoundtripsAsyncTheoryData() [Theory] [MemberData(nameof(LinkNameRoundtripsAsyncTheoryData))] - public async Task LinkNameRoundtrips(TarEntryFormat entryFormat, TarEntryType entryType, bool unseekableStream, string linkName) + public async Task LinkNameRoundtripsAsync(TarEntryFormat entryFormat, TarEntryType entryType, bool unseekableStream, string linkName) { string name = "foo"; TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, name); @@ -50,13 +50,13 @@ public async Task LinkNameRoundtrips(TarEntryFormat entryFormat, TarEntryType en MemoryStream ms = new(); Stream s = unseekableStream ? new WrappedStream(ms, ms.CanRead, ms.CanWrite, canSeek: false) : ms; - using (TarWriter writer = new(s, leaveOpen: true)) + await using (TarWriter writer = new(s, leaveOpen: true)) { await writer.WriteEntryAsync(entry); } ms.Position = 0; - using TarReader reader = new(s); + await using TarReader reader = new(s); entry = await reader.GetNextEntryAsync(); Assert.Null(await reader.GetNextEntryAsync()); @@ -69,7 +69,7 @@ public static IEnumerable UserNameGroupNameRoundtripsAsyncTheoryData() [Theory] [MemberData(nameof(UserNameGroupNameRoundtripsAsyncTheoryData))] - public async Task UserNameGroupNameRoundtrips(TarEntryFormat entryFormat, bool unseekableStream, string userGroupName) + public async Task UserNameGroupNameRoundtripsAsync(TarEntryFormat entryFormat, bool unseekableStream, string userGroupName) { string name = "foo"; TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, TarEntryType.RegularFile, name); @@ -80,13 +80,13 @@ public async Task UserNameGroupNameRoundtrips(TarEntryFormat entryFormat, bool u MemoryStream ms = new(); Stream s = unseekableStream ? new WrappedStream(ms, ms.CanRead, ms.CanWrite, canSeek: false) : ms; - using (TarWriter writer = new(s, leaveOpen: true)) + await using (TarWriter writer = new(s, leaveOpen: true)) { await writer.WriteEntryAsync(posixEntry); } ms.Position = 0; - using TarReader reader = new(s); + await using TarReader reader = new(s); entry = await reader.GetNextEntryAsync(); posixEntry = Assert.IsAssignableFrom(entry); 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 b1070ac96c92c9..be70a43bb25543 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 @@ -330,7 +330,7 @@ public static IEnumerable WriteEntry_TooLongName_Throws_Async_TheoryDa [MemberData(nameof(WriteEntry_TooLongName_Throws_Async_TheoryData))] public async Task WriteEntry_TooLongName_Throws_Async(TarEntryFormat entryFormat, TarEntryType entryType, string name) { - using TarWriter writer = new(new MemoryStream()); + await using TarWriter writer = new(new MemoryStream()); TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, name); await Assert.ThrowsAsync("entry", () => writer.WriteEntryAsync(entry)); @@ -343,7 +343,7 @@ public static IEnumerable WriteEntry_TooLongLinkName_Throws_Async_Theo [MemberData(nameof(WriteEntry_TooLongLinkName_Throws_Async_TheoryData))] public async Task WriteEntry_TooLongLinkName_Throws_Async(TarEntryFormat entryFormat, TarEntryType entryType, string linkName) { - using TarWriter writer = new(new MemoryStream()); + await using TarWriter writer = new(new MemoryStream()); TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, entryType, "foo"); entry.LinkName = linkName; @@ -356,9 +356,9 @@ public static IEnumerable WriteEntry_TooLongUserGroupName_Throws_Async [Theory] [MemberData(nameof(WriteEntry_TooLongUserGroupName_Throws_Async_TheoryData))] - public async Task WriteEntry_TooLongUserName_Throws(TarEntryFormat entryFormat, string userName) + public async Task WriteEntry_TooLongUserName_Throws_Async(TarEntryFormat entryFormat, string userName) { - using TarWriter writer = new(new MemoryStream()); + await using TarWriter writer = new(new MemoryStream()); TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, TarEntryType.RegularFile, "foo"); PosixTarEntry posixEntry = Assert.IsAssignableFrom(entry); @@ -369,9 +369,9 @@ public async Task WriteEntry_TooLongUserName_Throws(TarEntryFormat entryFormat, [Theory] [MemberData(nameof(WriteEntry_TooLongUserGroupName_Throws_Async_TheoryData))] - public async Task WriteEntry_TooLongGroupName_Throws(TarEntryFormat entryFormat, string groupName) + public async Task WriteEntry_TooLongGroupName_Throws_Async(TarEntryFormat entryFormat, string groupName) { - using TarWriter writer = new(new MemoryStream()); + await using TarWriter writer = new(new MemoryStream()); TarEntry entry = InvokeTarEntryCreationConstructor(entryFormat, TarEntryType.RegularFile, "foo"); PosixTarEntry posixEntry = Assert.IsAssignableFrom(entry); From 0aa84d2ea1875018ca42e7e3ac5d94b454a74e92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Cant=C3=BA?= Date: Mon, 3 Oct 2022 15:45:37 -0500 Subject: [PATCH 12/12] 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 --- .../src/System/Formats/Tar/TarHeader.Write.cs | 36 +++---- .../System.Formats.Tar/tests/TarTestsBase.cs | 15 ++- ...Writer.WriteEntry.Entry.Roundtrip.Tests.cs | 98 +++++++++++++++++++ .../TarWriter/TarWriter.WriteEntry.Tests.cs | 40 ++++++++ ...r.WriteEntryAsync.Entry.Roundtrip.Tests.cs | 98 +++++++++++++++++++ .../TarWriter.WriteEntryAsync.Tests.cs | 29 ++++++ 6 files changed, 294 insertions(+), 22 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..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 @@ -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 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 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; } } } 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.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.WriteEntry.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.Tests.cs index 17ecf4ef398cb9..158c6fba96f4fd 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 @@ -447,5 +447,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.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); + } } } 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()); + } } }