[release/7.0] Fix PAX extended attribute reading logic to treat '=' c…

…haracter as valid in the value strings. (#83177) * 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> Co-authored-by: carlossanlop <1175054+carlossanlop@users.noreply.github.com>
dotnet · Mar 10, 2023 · 5b74781 · 5b74781
1 parent 170dd10
commit 5b74781
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 36 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
@@ -735,12 +735,6 @@ private static bool TryGetNextExtendedAttribute(
             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/...aries/System.Formats.Tar/tests/TarReader/TarReader.File.GlobalExtendedAttributes.Tests.cs b/...aries/System.Formats.Tar/tests/TarReader/TarReader.File.GlobalExtendedAttributes.Tests.cs
@@ -84,12 +84,7 @@ public void ExtractGlobalExtendedAttributesEntry_Throws()
         }
 
         [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 @@ public void GlobalExtendedAttribute_Roundtrips(string key, string value)
                 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
@@ -325,30 +325,6 @@ public void PaxSizeLargerThanMaxAllowedByStream()
             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
@@ -98,5 +98,25 @@ public void TarReader_LeaveOpen_False_CopiedDataNotDisposed()
                 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
@@ -124,5 +124,21 @@ protected void VerifyExtendedAttributeTimestamps(PaxTarEntry pax)
             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==" };
+        }
     }
 }