Fix PAX extended attribute reading logic to treat '=' character as valid in the value...

* Move PaxExtendedAttribute_Roundtrips test to correct source code file. It is not handling any filesystem entries.

* Bug fix: Do not fail when reading an extended attribute when the value contains an '=' character.,

* Add unit tests that verify extended attribute and global extended attribute roundtripping when the value contains an '=' character.
Also add a null check for a subsequent GetNextEntry.

* Convert duplicate InlineData to single shared MemberData method.

* Apply suggestion

---------

Co-authored-by: carlossanlop <carlossanlop@users.noreply.github.com>

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 6231acd..5036a59 100644 (file)
--- 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
@@ -733,12 +733,6 @@ namespace System.Formats.Tar
             ReadOnlySpan<byte> keySlice = line.Slice(0, equalPos);
             ReadOnlySpan<byte> valueSlice = line.Slice(equalPos + 1);
 
-            // If the value contains an =, it's malformed.
-            if (valueSlice.IndexOf((byte)'=') >= 0)
-            {
-                return false;
-            }
-
             // Return the parsed key and value.
             key = Encoding.UTF8.GetString(keySlice);
             value = Encoding.UTF8.GetString(valueSlice);

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 755f337..7c25ba6 100644 (file)
--- 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
@@ -84,12 +84,7 @@ namespace System.Formats.Tar.Tests
         }
 
         [Theory]
-        [InlineData("key", "value")]
-        [InlineData("key    ", "value    ")]
-        [InlineData("    key", "    value")]
-        [InlineData("    key   ", "    value    ")]
-        [InlineData("    key spaced   ", "    value spaced    ")]
-        [InlineData("many sla/s\\hes", "/////////////\\\\\\///////////")]
+        [MemberData(nameof(GetPaxExtendedAttributesRoundtripTestData))]
         public void GlobalExtendedAttribute_Roundtrips(string key, string value)
         {
             var stream = new MemoryStream();
@@ -104,6 +99,7 @@ namespace System.Formats.Tar.Tests
                 PaxGlobalExtendedAttributesTarEntry entry = Assert.IsType<PaxGlobalExtendedAttributesTarEntry>(reader.GetNextEntry());
                 Assert.Equal(1, entry.GlobalExtendedAttributes.Count);
                 Assert.Equal(KeyValuePair.Create(key, value), entry.GlobalExtendedAttributes.First());
+                Assert.Null(reader.GetNextEntry());
             }
         }
     }

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 f1eaa42..6b5dae9 100644 (file)
--- a/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.Tests.cs
+++ b/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.Tests.cs
@@ -333,30 +333,6 @@ namespace System.Formats.Tar.Tests
             Assert.Throws<ArgumentOutOfRangeException>(() => reader.GetNextEntry());
         }
 
-        [Theory]
-        [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)
-        {
-            var stream = new MemoryStream();
-            using (var writer = new TarWriter(stream, leaveOpen: true))
-            {
-                writer.WriteEntry(new PaxTarEntry(TarEntryType.Directory, "entryName", new Dictionary<string, string>() { { key, value } }));
-            }
-
-            stream.Position = 0;
-            using (var reader = new TarReader(stream))
-            {
-                PaxTarEntry entry = Assert.IsType<PaxTarEntry>(reader.GetNextEntry());
-                Assert.Equal(5, entry.ExtendedAttributes.Count);
-                Assert.Contains(KeyValuePair.Create(key, value), entry.ExtendedAttributes);
-            }
-        }
-
         private static void VerifyDataStreamOfTarUncompressedInternal(string testFolderName, string testCaseName, bool copyData)
         {
             using MemoryStream archiveStream = GetTarMemoryStream(CompressionMethod.Uncompressed, testFolderName, testCaseName);

diff --git a/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.Tests.cs
index 479b433..f95a223 100644 (file)
--- a/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.Tests.cs
+++ b/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.Tests.cs
@@ -98,5 +98,25 @@ namespace System.Formats.Tar.Tests
                 ds.Dispose();
             }
         }
+
+        [Theory]
+        [MemberData(nameof(GetPaxExtendedAttributesRoundtripTestData))]
+        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<string, string>() { { key, value } }));
+            }
+
+            stream.Position = 0;
+            using (var reader = new TarReader(stream))
+            {
+                PaxTarEntry entry = Assert.IsType<PaxTarEntry>(reader.GetNextEntry());
+                Assert.Equal(5, entry.ExtendedAttributes.Count);
+                Assert.Contains(KeyValuePair.Create(key, value), entry.ExtendedAttributes);
+                Assert.Null(reader.GetNextEntry());
+            }
+        }
     }
 }

diff --git a/src/libraries/System.Formats.Tar/tests/TarTestsBase.Pax.cs b/src/libraries/System.Formats.Tar/tests/TarTestsBase.Pax.cs
index dd71b15..c177675 100644 (file)
--- a/src/libraries/System.Formats.Tar/tests/TarTestsBase.Pax.cs
+++ b/src/libraries/System.Formats.Tar/tests/TarTestsBase.Pax.cs
@@ -124,5 +124,21 @@ namespace System.Formats.Tar.Tests
             VerifyExtendedAttributeTimestamp(pax, PaxEaATime, MinimumTime);
             VerifyExtendedAttributeTimestamp(pax, PaxEaCTime, MinimumTime);
         }
+
+        public static IEnumerable<object[]> GetPaxExtendedAttributesRoundtripTestData()
+        {
+            yield return new object[] { "key", "value" };
+            yield return new object[] { "key    ", "value    " };
+            yield return new object[] { "    key", "    value" };
+            yield return new object[] { "    key   ", "    value    " };
+            yield return new object[] { "    key spaced   ", "    value spaced    " };
+            yield return new object[] { "many sla/s\\hes", "/////////////\\\\\\///////////" };
+            yield return new object[] { "key", "=" };
+            yield return new object[] { "key", "=value" };
+            yield return new object[] { "key", "va=lue" };
+            yield return new object[] { "key", "value=" };
+            // real world scenario
+            yield return new object[] { "MSWINDOWS.rawsd", "AQAAgBQAAAAkAAAAAAAAAAAAAAABAgAAAAAABSAAAAAhAgAAAQIAAAAAAAUgAAAAIQIAAA==" };
+        }
     }
 }