Bug fix: GNU can have two different metadata entries in a row if the …

…entry has a long link and a long path. Add logic to handle that and tests to verify this.

src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs

@@ -11,14 +11,11 @@ namespace System.Formats.Tar
     // Writes header attributes of a tar archive entry.
     internal partial struct TarHeader
     {
-        private const byte SpaceChar = 0x20;
-        private const byte EqualsChar = 0x3d;
-        private const byte NewLineChar = 0xa;
         private static readonly byte[] s_paxMagic = new byte[] { 0x75, 0x73, 0x74, 0x61, 0x72, 0x0 }; // "ustar\0"
-        private static readonly byte[] s_paxVersion = new byte[] { 0x30, 0x30 }; // "00"
+        private static readonly byte[] s_paxVersion = new byte[] { TarHelpers.ZeroChar, TarHelpers.ZeroChar }; // "00"
 
-        private static readonly byte[] s_gnuMagic = new byte[] { 0x75, 0x73, 0x74, 0x61, 0x72, 0x20 }; // "ustar "
-        private static readonly byte[] s_gnuVersion = new byte[] { 0x20, 0x0 }; // " \0"
+        private static readonly byte[] s_gnuMagic = new byte[] { 0x75, 0x73, 0x74, 0x61, 0x72, TarHelpers.SpaceChar }; // "ustar "
+        private static readonly byte[] s_gnuVersion = new byte[] { TarHelpers.SpaceChar, 0x0 }; // " \0"
 
         // Extended Attribute entries have a special format in the Name field:
         // "{dirName}/PaxHeaders.{processId}/{fileName}{trailingSeparator}"
@@ -28,6 +25,9 @@ internal partial struct TarHeader
         // "{tmpFolder}/GlobalHead.{processId}.1"
         private const string GlobalHeadFormat = "{0}/GlobalHead.{1}.1";
 
+        // 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";
+
         // Creates a PAX Global Extended Attributes header and writes it into the specified archive stream.
         internal static void WriteGlobalExtendedAttributesHeader(Stream archiveStream, IEnumerable<KeyValuePair<string, string>> globalExtendedAttributes)
         {
@@ -135,6 +135,7 @@ internal void WriteAsUstar(Stream archiveStream)
         }
 
         // Writes the current header as a PAX entry into the archive stream.
+        // Makes sure to add the preceding exteded attributes entry before the actual entry.
         internal void WriteAsPax(Stream archiveStream)
         {
             // First, we write the preceding extended attributes header
@@ -148,8 +149,52 @@ internal void WriteAsPax(Stream archiveStream)
             WriteAsPaxInternal(archiveStream);
         }
 
-        // Writes the current header as a GNU entry into the archive stream.
+        // Writes the current header as a Gnu entry into the archive stream.
+        // Makes sure to add the preceding LongLink and/or LongPath entries if necessary, before the actual entry.
         internal void WriteAsGnu(Stream archiveStream)
+        {
+            // First, we determine if we need a preceding LongLink, and write it if needed
+            if (_linkName.Length > FieldLengths.LinkName)
+            {
+                TarHeader longLinkHeader = GetGnuLongMetadataHeader(TarEntryType.LongLink, _linkName);
+                longLinkHeader.WriteAsGnuInternal(archiveStream);
+            }
+
+            // Second, we determine if we need a preceding LongPath, and write it if needed
+            if (_name.Length > FieldLengths.Name)
+            {
+                TarHeader longPathHeader = GetGnuLongMetadataHeader(TarEntryType.LongPath, _name);
+                longPathHeader.WriteAsGnuInternal(archiveStream);
+            }
+
+            // Third, we write this header as a normal one
+            WriteAsGnuInternal(archiveStream);
+        }
+
+        // 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));
+
+            TarHeader longMetadataHeader = default;
+
+            longMetadataHeader._name = GnuLongMetadataName; // Same name for both longpath or longlink
+            longMetadataHeader._mode = (int)TarHelpers.DefaultMode;
+            longMetadataHeader._uid = 0;
+            longMetadataHeader._gid = 0;
+            longMetadataHeader._mTime = DateTimeOffset.MinValue; // 0
+            longMetadataHeader._typeFlag = entryType;
+
+            longMetadataHeader._dataStream = new MemoryStream();
+            longMetadataHeader._dataStream.Write(Encoding.UTF8.GetBytes(longText));
+            longMetadataHeader._dataStream.Seek(0, SeekOrigin.Begin); // Ensure it gets written into the archive from the beginning
+
+            return longMetadataHeader;
+        }
+
+        // Writes the current header as a GNU entry into the archive stream.
+        internal void WriteAsGnuInternal(Stream archiveStream)
         {
             byte[] nameBytes = new byte[FieldLengths.Name];
             byte[] modeBytes = new byte[FieldLengths.Mode];
@@ -524,11 +569,11 @@ private static byte[] GenerateExtendedAttributeKeyValuePairAsByteArray(byte[] ke
             List<byte> bytesList = new();
 
             bytesList.AddRange(finalTotalCharCountBytes);
-            bytesList.Add(SpaceChar);
+            bytesList.Add(TarHelpers.SpaceChar);
             bytesList.AddRange(keyBytes);
-            bytesList.Add(EqualsChar);
+            bytesList.Add(TarHelpers.EqualsChar);
             bytesList.AddRange(valueBytes);
-            bytesList.Add(NewLineChar);
+            bytesList.Add(TarHelpers.NewLineChar);
 
             Debug.Assert(bytesList.Count == (realTotalCharCount + suffixByteCount));
 
@@ -542,12 +587,12 @@ internal static int SaveChecksumBytes(int checksum, Span<byte> destination)
         {
             // The checksum field is also counted towards the total sum
             // but as an array filled with spaces
-            checksum += SpaceChar * 8;
+            checksum += TarHelpers.SpaceChar * 8;
 
             byte[] converted = TarHelpers.GetAsciiBytes(TarHelpers.ConvertDecimalToOctal(checksum));
 
             // Checksum field ends with a null and a space
-            destination[^1] = SpaceChar; // ' '
+            destination[^1] = TarHelpers.SpaceChar; // ' '
             destination[^2] = 0; // '\0'
 
             int i = destination.Length - 3;
@@ -562,7 +607,7 @@ internal static int SaveChecksumBytes(int checksum, Span<byte> destination)
                 }
                 else
                 {
-                    destination[i] = 0x30; // Leading zero chars '0'
+                    destination[i] = TarHelpers.ZeroChar; // Leading zero chars '0'
                 }
                 i--;
             }
@@ -616,7 +661,7 @@ private static int WriteRightAlignedBytesAndGetChecksum(ReadOnlySpan<byte> bytes
                 }
                 else
                 {
-                    destination[i] = ZeroChar; // leading zeros
+                    destination[i] = TarHelpers.ZeroChar; // leading zeros
                 }
                 checksum += destination[i];
                 i--;

src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.cs

@@ -15,8 +15,11 @@ namespace System.Formats.Tar
     // Documentation: https://www.freebsd.org/cgi/man.cgi?query=tar&sektion=5
     internal partial struct TarHeader
     {
+        // POSIX fields (shared by Ustar and PAX)
         private const string UstarMagic = "ustar\0";
         private const string UstarVersion = "00";
+
+        // GNU-specific fields
         private const string GnuMagic = "ustar ";
         private const string GnuVersion = " \0";
 
@@ -35,9 +38,6 @@ internal partial struct TarHeader
         private const string PaxEaDevMajor = "devmajor";
         private const string PaxEaDevMinor = "devminor";
 
-        private const int ZeroChar = 0x30;
-
-        //private TarBlocks _blocks;
         internal Stream? _dataStream;
 
         // Position in the stream where the data ends in this header.

src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.cs

@@ -13,6 +13,11 @@ internal static class TarHelpers
         internal const short RecordSize = 512;
         internal const int MaxBufferLength = 4096;
 
+        internal const int ZeroChar = 0x30;
+        internal const byte SpaceChar = 0x20;
+        internal const byte EqualsChar = 0x3d;
+        internal const byte NewLineChar = 0xa;
+
         internal const TarFileMode DefaultMode = // 644 in octal
             TarFileMode.UserRead | TarFileMode.UserWrite | TarFileMode.GroupRead | TarFileMode.OtherRead;
 

src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarReader.cs

@@ -260,60 +260,24 @@ private bool TryGetNextEntryHeader(out TarHeader header, bool copyData)
             }
 
             // If a metadata typeflag entry is retrieved, handle it here, then read the next entry
-            if (header._typeFlag is TarEntryType.ExtendedAttributes or TarEntryType.LongLink or TarEntryType.LongPath)
-            {
-                TarHeader actualEntryHeader = default;
-
-                // We should know by now the format of the archive based on the first retrieved entry
-                actualEntryHeader._format = header._format;
 
-                // Now get the actual entry
-                if (!actualEntryHeader.TryGetNextHeader(_archiveStream, copyData))
+            // PAX metadata
+            if (header._typeFlag is TarEntryType.ExtendedAttributes)
+            {
+                if (!TryProcessExtendedAttributesHeader(header, copyData, out TarHeader mainHeader))
                 {
                     return false;
                 }
-
-                // Should never read a GEA entry at this point
-                if (header._typeFlag == TarEntryType.GlobalExtendedAttributes)
-                {
-                    throw new FormatException(SR.TarTooManyGlobalExtendedAttributesEntries);
-                }
-
-                // Can't have two metadata entries in a row, no matter the archive format
-                if (actualEntryHeader._typeFlag is TarEntryType.ExtendedAttributes or TarEntryType.LongLink or TarEntryType.LongPath)
-                {
-                    throw new FormatException(string.Format(SR.TarUnexpectedMetadataEntry, actualEntryHeader._typeFlag, header._typeFlag));
-                }
-
-                // Handle metadata entry types
-                switch (header._typeFlag)
+                header = mainHeader;
+            }
+            // GNU metadata
+            else if (header._typeFlag is TarEntryType.LongLink or TarEntryType.LongPath)
+            {
+                if (!TryProcessGnuMetadataHeader(header, copyData, out TarHeader mainHeader))
                 {
-                    case TarEntryType.ExtendedAttributes: // pax
-                        Debug.Assert(header._extendedAttributes != null);
-                        if (GlobalExtendedAttributes != null)
-                        {
-                            // First, replace some of the entry's standard attributes with the global ones
-                            actualEntryHeader.ReplaceNormalAttributesWithGlobalExtended(GlobalExtendedAttributes);
-                        }
-                        // Then replace all the standard attributes with the extended attributes ones,
-                        // overwriting the previous global replacements if needed
-                        actualEntryHeader.ReplaceNormalAttributesWithExtended(header._extendedAttributes);
-                        break;
-
-                    case TarEntryType.LongLink: // gnu
-                        Debug.Assert(header._linkName != null);
-                        // Replace with longer, complete path
-                        actualEntryHeader._linkName = header._linkName;
-                        break;
-
-                    case TarEntryType.LongPath: // gnu
-                        Debug.Assert(header._name != null);
-                        // Replace with longer, complete path
-                        actualEntryHeader._name = header._name;
-                        break;
+                    return false;
                 }
-
-                header = actualEntryHeader;
+                header = mainHeader;
             }
 
             // Common fields should always acquire a value
@@ -330,6 +294,118 @@ private bool TryGetNextEntryHeader(out TarHeader header, bool copyData)
             return true;
         }
 
+        private bool TryProcessExtendedAttributesHeader(TarHeader firstHeader, bool copyData, out TarHeader secondHeader)
+        {
+            secondHeader = default;
+            secondHeader._format = TarFormat.Pax;
+
+            // Now get the actual entry
+            if (!secondHeader.TryGetNextHeader(_archiveStream, copyData))
+            {
+                return false;
+            }
+
+            // Should never read a GEA entry at this point
+            if (secondHeader._typeFlag == TarEntryType.GlobalExtendedAttributes)
+            {
+                throw new FormatException(SR.TarTooManyGlobalExtendedAttributesEntries);
+            }
+
+            // Can't have two metadata entries in a row, no matter the archive format
+            if (secondHeader._typeFlag is TarEntryType.ExtendedAttributes)
+            {
+                throw new FormatException(string.Format(SR.TarUnexpectedMetadataEntry, TarEntryType.ExtendedAttributes, TarEntryType.ExtendedAttributes));
+            }
+
+            Debug.Assert(firstHeader._extendedAttributes != null);
+            if (GlobalExtendedAttributes != null)
+            {
+                // First, replace some of the entry's standard attributes with the global ones
+                secondHeader.ReplaceNormalAttributesWithGlobalExtended(GlobalExtendedAttributes);
+            }
+            // Then replace all the standard attributes with the extended attributes ones,
+            // overwriting the previous global replacements if needed
+            secondHeader.ReplaceNormalAttributesWithExtended(firstHeader._extendedAttributes);
+
+            return true;
+        }
+
+        private bool TryProcessGnuMetadataHeader(TarHeader header, bool copyData, out TarHeader finalHeader)
+        {
+            finalHeader = default;
+
+            TarHeader secondHeader = default;
+            secondHeader._format = TarFormat.Gnu;
+
+            // Get the second entry, which is the actual entry
+            if (!secondHeader.TryGetNextHeader(_archiveStream, copyData))
+            {
+                return false;
+            }
+
+            // Can't have two identical metadata entries in a row
+            if (secondHeader._typeFlag == header._typeFlag)
+            {
+                throw new FormatException(string.Format(SR.TarUnexpectedMetadataEntry, secondHeader._typeFlag, header._typeFlag));
+            }
+
+            // It's possible to have the two different metadata entries in a row
+            if ((header._typeFlag is TarEntryType.LongLink && secondHeader._typeFlag is TarEntryType.LongPath) ||
+                (header._typeFlag is TarEntryType.LongPath && secondHeader._typeFlag is TarEntryType.LongLink))
+            {
+                TarHeader thirdHeader = default;
+                thirdHeader._format = TarFormat.Gnu;
+
+                // Get the third entry, which is the actual entry
+                if (!thirdHeader.TryGetNextHeader(_archiveStream, copyData))
+                {
+                    return false;
+                }
+
+                // Can't have three GNU metadata entries in a row
+                if (thirdHeader._typeFlag is TarEntryType.LongLink or TarEntryType.LongPath)
+                {
+                    throw new FormatException(string.Format(SR.TarUnexpectedMetadataEntry, thirdHeader._typeFlag, secondHeader._typeFlag));
+                }
+
+                if (header._typeFlag is TarEntryType.LongLink)
+                {
+                    Debug.Assert(header._linkName != null);
+                    Debug.Assert(secondHeader._name != null);
+
+                    thirdHeader._linkName = header._linkName;
+                    thirdHeader._name = secondHeader._name;
+                }
+                else if (header._typeFlag is TarEntryType.LongPath)
+                {
+                    Debug.Assert(header._name != null);
+                    Debug.Assert(secondHeader._linkName != null);
+                    thirdHeader._name = header._name;
+                    thirdHeader._linkName = secondHeader._linkName;
+                }
+
+                finalHeader = thirdHeader;
+            }
+            // Only one metadata entry was found
+            else
+            {
+                if (header._typeFlag is TarEntryType.LongLink)
+                {
+                    Debug.Assert(header._linkName != null);
+                    secondHeader._linkName = header._linkName;
+                }
+                else if (header._typeFlag is TarEntryType.LongPath)
+                {
+                    Debug.Assert(header._name != null);
+                    secondHeader._name = header._name;
+                }
+
+                finalHeader = secondHeader;
+            }
+
+            return true;
+        }
+
         // If the current entry contains a non-null DataStream, that stream gets added to an internal
         // list of streams that need to be disposed when this TarReader instance gets disposed.
         private void PreserveDataStreamForDisposalIfNeeded(TarEntry entry)