From 622f6a03560fbfa2f75cc1be4480144a8b0f38bd Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Fri, 18 Nov 2022 20:29:52 -0500 Subject: [PATCH 1/3] Remove invalidation of whitespace in TryGetNextExtendedAttribute (#78465) * Remove invalidation of whitespace in TryGetNextExtendedAttribute * Add requested test --- .../src/System/Formats/Tar/TarHeader.Read.cs | 6 ----- ...der.File.GlobalExtendedAttributes.Tests.cs | 22 +++++++++++++++++++ .../tests/TarReader/TarReader.File.Tests.cs | 22 +++++++++++++++++++ 3 files changed, 44 insertions(+), 6 deletions(-) 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 0d5ec998497f9d..ff91a8b2ed1e5a 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 @@ -725,12 +725,6 @@ private static bool TryGetNextExtendedAttribute( } line = line.Slice(spacePos + 1).TrimStart((byte)' '); - // If there are any more spaces, it's malformed. - if (line.IndexOf((byte)' ') >= 0) - { - return false; - } - // Find the equal separator. int equalPos = line.IndexOf((byte)'='); if (equalPos < 0) diff --git a/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.GlobalExtendedAttributes.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.GlobalExtendedAttributes.Tests.cs index 2661c542ae4c0e..9874835fdda231 100644 --- a/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.GlobalExtendedAttributes.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.GlobalExtendedAttributes.Tests.cs @@ -82,5 +82,27 @@ public void ExtractGlobalExtendedAttributesEntry_Throws() Assert.Throws(() => entry.ExtractToFile(Path.Join(root.Path, "file"), overwrite: true)); } } + + [Theory] + [InlineData("key", "value")] + [InlineData("key ", " value ")] + [InlineData(" key ", " value ")] + [InlineData("many sla/s\\hes", "/////////////\\\\\\///////////")] + public void GlobalExtendedAttribute_Roundtrips(string key, string value) + { + var stream = new MemoryStream(); + using (var writer = new TarWriter(stream, leaveOpen: true)) + { + writer.WriteEntry(new PaxGlobalExtendedAttributesTarEntry(new Dictionary() { { key, value } })); + } + + stream.Position = 0; + using (var reader = new TarReader(stream)) + { + PaxGlobalExtendedAttributesTarEntry entry = Assert.IsType(reader.GetNextEntry()); + Assert.Equal(1, entry.GlobalExtendedAttributes.Count); + Assert.Equal(KeyValuePair.Create(key.TrimStart(), value), entry.GlobalExtendedAttributes.First()); + } + } } } diff --git a/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.Tests.cs index 17c67423c390b2..d46bf6c829c299 100644 --- a/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.Tests.cs @@ -325,6 +325,28 @@ public void PaxSizeLargerThanMaxAllowedByStream() Assert.Throws(() => reader.GetNextEntry()); } + [Theory] + [InlineData("key", "value")] + [InlineData("key ", " value ")] + [InlineData(" key ", " value ")] + [InlineData("many sla/s\\hes", "/////////////\\\\\\///////////")] + public void PaxExtendedAttribute_Roundtrips(string key, string value) + { + var stream = new MemoryStream(); + using (var writer = new TarWriter(stream, leaveOpen: true)) + { + writer.WriteEntry(new PaxTarEntry(TarEntryType.Directory, "entryName", new Dictionary() { { key, value } })); + } + + stream.Position = 0; + using (var reader = new TarReader(stream)) + { + PaxTarEntry entry = Assert.IsType(reader.GetNextEntry()); + Assert.Equal(5, entry.ExtendedAttributes.Count); + Assert.Contains(KeyValuePair.Create(key.TrimStart(), value), entry.ExtendedAttributes); + } + } + private static void VerifyDataStreamOfTarUncompressedInternal(string testFolderName, string testCaseName, bool copyData) { using MemoryStream archiveStream = GetTarMemoryStream(CompressionMethod.Uncompressed, testFolderName, testCaseName); From 087eccdacf11360902d53a912ae5aa6790e7e44b Mon Sep 17 00:00:00 2001 From: Carlos Sanchez <1175054+carlossanlop@users.noreply.github.com> Date: Tue, 22 Nov 2022 12:41:13 -0800 Subject: [PATCH 2/3] Remove TrimStart in PAX extended attributes (#78707) * Remove TrimStart in PAX extended attributes * Adjust existing tests with two extra inline datas. Co-authored-by: carlossanlop --- .../src/System/Formats/Tar/TarHeader.Read.cs | 2 +- .../TarReader.File.GlobalExtendedAttributes.Tests.cs | 8 +++++--- .../tests/TarReader/TarReader.File.Tests.cs | 8 +++++--- 3 files changed, 11 insertions(+), 7 deletions(-) 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 ff91a8b2ed1e5a..fd07150b3508af 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 @@ -723,7 +723,7 @@ private static bool TryGetNextExtendedAttribute( { return false; } - line = line.Slice(spacePos + 1).TrimStart((byte)' '); + line = line.Slice(spacePos + 1); // Find the equal separator. int equalPos = line.IndexOf((byte)'='); diff --git a/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.GlobalExtendedAttributes.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.GlobalExtendedAttributes.Tests.cs index 9874835fdda231..755f3377703fc6 100644 --- a/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.GlobalExtendedAttributes.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.GlobalExtendedAttributes.Tests.cs @@ -85,8 +85,10 @@ public void ExtractGlobalExtendedAttributesEntry_Throws() [Theory] [InlineData("key", "value")] - [InlineData("key ", " value ")] - [InlineData(" key ", " value ")] + [InlineData("key ", "value ")] + [InlineData(" key", " value")] + [InlineData(" key ", " value ")] + [InlineData(" key spaced ", " value spaced ")] [InlineData("many sla/s\\hes", "/////////////\\\\\\///////////")] public void GlobalExtendedAttribute_Roundtrips(string key, string value) { @@ -101,7 +103,7 @@ public void GlobalExtendedAttribute_Roundtrips(string key, string value) { PaxGlobalExtendedAttributesTarEntry entry = Assert.IsType(reader.GetNextEntry()); Assert.Equal(1, entry.GlobalExtendedAttributes.Count); - Assert.Equal(KeyValuePair.Create(key.TrimStart(), value), entry.GlobalExtendedAttributes.First()); + Assert.Equal(KeyValuePair.Create(key, value), entry.GlobalExtendedAttributes.First()); } } } diff --git a/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.Tests.cs index d46bf6c829c299..0e1b325ed1197d 100644 --- a/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.Tests.cs @@ -327,8 +327,10 @@ public void PaxSizeLargerThanMaxAllowedByStream() [Theory] [InlineData("key", "value")] - [InlineData("key ", " value ")] - [InlineData(" key ", " value ")] + [InlineData("key ", "value ")] + [InlineData(" key", " value")] + [InlineData(" key ", " value ")] + [InlineData(" key spaced ", " value spaced ")] [InlineData("many sla/s\\hes", "/////////////\\\\\\///////////")] public void PaxExtendedAttribute_Roundtrips(string key, string value) { @@ -343,7 +345,7 @@ public void PaxExtendedAttribute_Roundtrips(string key, string value) { PaxTarEntry entry = Assert.IsType(reader.GetNextEntry()); Assert.Equal(5, entry.ExtendedAttributes.Count); - Assert.Contains(KeyValuePair.Create(key.TrimStart(), value), entry.ExtendedAttributes); + Assert.Contains(KeyValuePair.Create(key, value), entry.ExtendedAttributes); } } From 542d5aef4bb26c00ad16ed9a7b311881fd97146d Mon Sep 17 00:00:00 2001 From: Carlos Sanchez <1175054+carlossanlop@users.noreply.github.com> Date: Wed, 23 Nov 2022 11:09:27 -0800 Subject: [PATCH 3/3] Tar: Extra tests to confirm extra long paths are not treated as duplicate entries when the full path is in the extended attributes. (#78744) Co-authored-by: carlossanlop --- ...TarFile.ExtractToDirectory.Stream.Tests.cs | 27 +++++++++++++++++++ ...le.ExtractToDirectoryAsync.Stream.Tests.cs | 27 +++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.Stream.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.Stream.Tests.cs index b62196bb5c5d6f..ab90f0043bc7d9 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.Stream.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.Stream.Tests.cs @@ -177,5 +177,32 @@ public void Extract_UnseekableStream_BlockAlignmentPadding_DoesNotAffectNextEntr Assert.Equal(2, Directory.GetFileSystemEntries(destination.Path, "*", SearchOption.AllDirectories).Count()); } + + [Fact] + public void PaxNameCollision_DedupInExtendedAttributes() + { + using TempDirectory root = new(); + + string sharedRootFolders = Path.Join(root.Path, "folder with spaces", new string('a', 100)); + string path1 = Path.Join(sharedRootFolders, "entry 1 with spaces.txt"); + string path2 = Path.Join(sharedRootFolders, "entry 2 with spaces.txt"); + + using MemoryStream stream = new(); + using (TarWriter writer = new(stream, TarEntryFormat.Pax, leaveOpen: true)) + { + // Paths don't fit in the standard 'name' field, but they differ in the filename, + // which is fully stored as an extended attribute + PaxTarEntry entry1 = new(TarEntryType.RegularFile, path1); + writer.WriteEntry(entry1); + PaxTarEntry entry2 = new(TarEntryType.RegularFile, path2); + writer.WriteEntry(entry2); + } + stream.Position = 0; + + TarFile.ExtractToDirectory(stream, root.Path, overwriteFiles: true); + + Assert.True(File.Exists(path1)); + Assert.True(Path.Exists(path2)); + } } } diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.Stream.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.Stream.Tests.cs index ec32ab1ab8136f..ed36eecf1f2b1e 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.Stream.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectoryAsync.Stream.Tests.cs @@ -219,5 +219,32 @@ public async Task Extract_UnseekableStream_BlockAlignmentPadding_DoesNotAffectNe Assert.Equal(2, Directory.GetFileSystemEntries(destination.Path, "*", SearchOption.AllDirectories).Count()); } + + [Fact] + public async Task PaxNameCollision_DedupInExtendedAttributesAsync() + { + using TempDirectory root = new(); + + string sharedRootFolders = Path.Join(root.Path, "folder with spaces", new string('a', 100)); + string path1 = Path.Join(sharedRootFolders, "entry 1 with spaces.txt"); + string path2 = Path.Join(sharedRootFolders, "entry 2 with spaces.txt"); + + await using MemoryStream stream = new(); + await using (TarWriter writer = new(stream, TarEntryFormat.Pax, leaveOpen: true)) + { + // Paths don't fit in the standard 'name' field, but they differ in the filename, + // which is fully stored as an extended attribute + PaxTarEntry entry1 = new(TarEntryType.RegularFile, path1); + await writer.WriteEntryAsync(entry1); + PaxTarEntry entry2 = new(TarEntryType.RegularFile, path2); + await writer.WriteEntryAsync(entry2); + } + stream.Position = 0; + + await TarFile.ExtractToDirectoryAsync(stream, root.Path, overwriteFiles: true); + + Assert.True(File.Exists(path1)); + Assert.True(Path.Exists(path2)); + } } }