[Draft][release/7.0-rc1] Backport tar bug fixes and improvements

eng/Version.Details.xml

@@ -138,9 +138,9 @@
       <Uri>https://github.com/dotnet/runtime-assets</Uri>
       <Sha>555080fde81d34b38dfab27115c52f0a620803a2</Sha>
     </Dependency>
-    <Dependency Name="System.Formats.Tar.TestData" Version="7.0.0-beta.22415.3">
+    <Dependency Name="System.Formats.Tar.TestData" Version="7.0.0-beta.22421.2">
       <Uri>https://github.com/dotnet/runtime-assets</Uri>
-      <Sha>555080fde81d34b38dfab27115c52f0a620803a2</Sha>
+      <Sha>9d8fad5f0614bee808083308a3729084b681f7e7</Sha>
     </Dependency>
     <Dependency Name="System.IO.Compression.TestData" Version="7.0.0-beta.22415.3">
       <Uri>https://github.com/dotnet/runtime-assets</Uri>

eng/Versions.props

@@ -125,7 +125,7 @@
     <SystemRuntimeNumericsTestDataVersion>7.0.0-beta.22415.3</SystemRuntimeNumericsTestDataVersion>
     <SystemComponentModelTypeConverterTestDataVersion>7.0.0-beta.22415.3</SystemComponentModelTypeConverterTestDataVersion>
     <SystemDrawingCommonTestDataVersion>7.0.0-beta.22415.3</SystemDrawingCommonTestDataVersion>
-    <SystemFormatsTarTestDataVersion>7.0.0-beta.22415.3</SystemFormatsTarTestDataVersion>
+    <SystemFormatsTarTestDataVersion>7.0.0-beta.22421.2</SystemFormatsTarTestDataVersion>
     <SystemIOCompressionTestDataVersion>7.0.0-beta.22415.3</SystemIOCompressionTestDataVersion>
     <SystemIOPackagingTestDataVersion>7.0.0-beta.22415.3</SystemIOPackagingTestDataVersion>
     <SystemNetTestDataVersion>7.0.0-beta.22415.3</SystemNetTestDataVersion>

src/libraries/Common/src/System/IO/Archiving.Utils.Unix.cs

@@ -6,5 +6,25 @@ namespace System.IO
     internal static partial class ArchivingUtils
     {
         internal static string SanitizeEntryFilePath(string entryPath) => entryPath.Replace('\0', '_');
+
+        public static unsafe string EntryFromPath(ReadOnlySpan<char> path, bool appendPathSeparator = false)
+        {
+            // Remove leading separators.
+            int nonSlash = path.IndexOfAnyExcept('/');
+            if (nonSlash == -1)
+            {
+                nonSlash = path.Length;
+            }
+            path = path.Slice(nonSlash);
+
+            // Append a separator if necessary.
+            return (path.IsEmpty, appendPathSeparator) switch
+            {
+                (false, false) => path.ToString(),
+                (false, true) => string.Concat(path, "/"),
+                (true, false) => string.Empty,
+                (true, true) => "/",
+            };
+        }
     }
 }

src/libraries/Common/src/System/IO/Archiving.Utils.Windows.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Runtime.InteropServices;
 using System.Text;
 
 namespace System.IO
@@ -42,5 +43,47 @@ internal static string SanitizeEntryFilePath(string entryPath)
             // There weren't any characters to sanitize.  Just return the original string.
             return entryPath;
         }
+
+        public static unsafe string EntryFromPath(ReadOnlySpan<char> path, bool appendPathSeparator = false)
+        {
+            // Remove leading separators.
+            int nonSlash = path.IndexOfAnyExcept('/', '\\');
+            if (nonSlash == -1)
+            {
+                nonSlash = path.Length;
+            }
+            path = path.Slice(nonSlash);
+
+            // Replace \ with /, and append a separator if necessary.
+
+            if (path.IsEmpty)
+            {
+                return appendPathSeparator ?
+                    "/" :
+                    string.Empty;
+            }
+
+            fixed (char* pathPtr = &MemoryMarshal.GetReference(path))
+            {
+                return string.Create(appendPathSeparator ? path.Length + 1 : path.Length, (appendPathSeparator, (IntPtr)pathPtr, path.Length), static (dest, state) =>
+                {
+                    ReadOnlySpan<char> path = new ReadOnlySpan<char>((char*)state.Item2, state.Length);
+                    path.CopyTo(dest);
+                    if (state.appendPathSeparator)
+                    {
+                        dest[^1] = '/';
+                    }
+
+                    // To ensure tar files remain compatible with Unix, and per the ZIP File Format Specification 4.4.17.1,
+                    // all slashes should be forward slashes.
+                    int pos;
+                    while ((pos = dest.IndexOf('\\')) >= 0)
+                    {
+                        dest[pos] = '/';
+                        dest = dest.Slice(pos + 1);
+                    }
+                });
+            }
+        }
     }
 }

src/libraries/Common/src/System/IO/Archiving.Utils.cs

@@ -9,51 +9,6 @@ namespace System.IO
 {
     internal static partial class ArchivingUtils
     {
-        // To ensure tar files remain compatible with Unix,
-        // and per the ZIP File Format Specification 4.4.17.1,
-        // all slashes should be forward slashes.
-        private const char PathSeparatorChar = '/';
-        private const string PathSeparatorString = "/";
-
-        public static string EntryFromPath(string entry, int offset, int length, ref char[] buffer, bool appendPathSeparator = false)
-        {
-            Debug.Assert(length <= entry.Length - offset);
-            Debug.Assert(buffer != null);
-
-            // Remove any leading slashes from the entry name:
-            while (length > 0)
-            {
-                if (entry[offset] != Path.DirectorySeparatorChar &&
-                    entry[offset] != Path.AltDirectorySeparatorChar)
-                    break;
-
-                offset++;
-                length--;
-            }
-
-            if (length == 0)
-                return appendPathSeparator ? PathSeparatorString : string.Empty;
-
-            int resultLength = appendPathSeparator ? length + 1 : length;
-            EnsureCapacity(ref buffer, resultLength);
-            entry.CopyTo(offset, buffer, 0, length);
-
-            // '/' is a more broadly recognized directory separator on all platforms (eg: mac, linux)
-            // We don't use Path.DirectorySeparatorChar or AltDirectorySeparatorChar because this is
-            // explicitly trying to standardize to '/'
-            for (int i = 0; i < length; i++)
-            {
-                char ch = buffer[i];
-                if (ch == Path.DirectorySeparatorChar || ch == Path.AltDirectorySeparatorChar)
-                    buffer[i] = PathSeparatorChar;
-            }
-
-            if (appendPathSeparator)
-                buffer[length] = PathSeparatorChar;
-
-            return new string(buffer, 0, resultLength);
-        }
-
         public static void EnsureCapacity(ref char[] buffer, int min)
         {
             Debug.Assert(buffer != null);

src/libraries/System.Formats.Tar/src/Resources/Strings.resx

@@ -205,6 +205,9 @@
     <value>The entry is a symbolic link or a hard link but the LinkName field is null or empty.</value>
   </data>
   <data name="TarEntryTypeNotSupported" xml:space="preserve">
+    <value>Entry type '{0}' not supported.</value>
+  </data>
+  <data name="TarEntryTypeNotSupportedInFormat" xml:space="preserve">
     <value>Entry type '{0}' not supported in format '{1}'.</value>
   </data>
   <data name="TarEntryTypeNotSupportedForExtracting" xml:space="preserve">
@@ -255,4 +258,7 @@
   <data name="IO_SeekBeforeBegin" xml:space="preserve">
     <value>An attempt was made to move the position before the beginning of the stream.</value>
   </data>
+  <data name="TarInvalidNumber" xml:space="preserve">
+    <value>Unable to parse number.</value>
+  </data>
 </root>

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

@@ -187,11 +187,8 @@ public override Task FlushAsync(CancellationToken cancellationToken) =>
         // Close the stream for reading.  Note that this does NOT close the superStream (since
         // the substream is just 'a chunk' of the super-stream
         protected override void Dispose(bool disposing)
-        {
-            if (disposing && !_isDisposed)
         {
             _isDisposed = true;
-            }
             base.Dispose(disposing);
         }
     }

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

@@ -465,7 +465,7 @@ private void VerifyPathsForEntryType(string filePath, string? linkTargetPath, bo
             // If the destination contains a directory segment, need to check that it exists
             if (!string.IsNullOrEmpty(directoryPath) && !Path.Exists(directoryPath))
             {
-                throw new IOException(string.Format(SR.IO_PathNotFound_NoPathName, filePath));
+                throw new IOException(string.Format(SR.IO_PathNotFound_Path, filePath));
             }
 
             if (!Path.Exists(filePath))
@@ -529,7 +529,7 @@ private void ExtractAsRegularFile(string destinationFileName)
                 DataStream?.CopyTo(fs);
             }
 
-            ArchivingUtils.AttemptSetLastWriteTime(destinationFileName, ModificationTime);
+            AttemptSetLastWriteTime(destinationFileName, ModificationTime);
         }
 
         // Asynchronously extracts the current entry as a regular file into the specified destination.
@@ -551,7 +551,19 @@ private async Task ExtractAsRegularFileAsync(string destinationFileName, Cancell
                 }
             }
 
-            ArchivingUtils.AttemptSetLastWriteTime(destinationFileName, ModificationTime);
+            AttemptSetLastWriteTime(destinationFileName, ModificationTime);
+        }
+
+        private static void AttemptSetLastWriteTime(string destinationFileName, DateTimeOffset lastWriteTime)
+        {
+            try
+            {
+                File.SetLastWriteTime(destinationFileName, lastWriteTime.LocalDateTime); // SetLastWriteTime expects local time
+            }
+            catch
+            {
+                // Some OSes like Android might not support setting the last write time, the extraction should not fail because of that
+            }
         }
 
         private FileStreamOptions CreateFileStreamOptions(bool isAsync)

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

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
+using System.IO.Enumeration;
 using System.Threading;
 using System.Threading.Tasks;
 
@@ -15,11 +16,6 @@ namespace System.Formats.Tar
     /// </summary>
     public static class TarFile
     {
-        // Windows' MaxPath (260) is used as an arbitrary default capacity, as it is likely
-        // to be greater than the length of typical entry names from the file system, even
-        // on non-Windows platforms. The capacity will be increased, if needed.
-        private const int DefaultCapacity = 260;
-
         /// <summary>
         /// Creates a tar stream that contains all the filesystem entries from the specified directory.
         /// </summary>
@@ -222,7 +218,7 @@ public static void ExtractToDirectory(string sourceFileName, string destinationD
 
             if (!File.Exists(sourceFileName))
             {
-                throw new FileNotFoundException(string.Format(SR.IO_FileNotFound, sourceFileName));
+                throw new FileNotFoundException(string.Format(SR.IO_FileNotFound_FileName, sourceFileName));
             }
 
             if (!Directory.Exists(destinationDirectoryName))
@@ -261,7 +257,7 @@ public static Task ExtractToDirectoryAsync(string sourceFileName, string destina
 
             if (!File.Exists(sourceFileName))
             {
-                return Task.FromException(new FileNotFoundException(string.Format(SR.IO_FileNotFound, sourceFileName)));
+                return Task.FromException(new FileNotFoundException(string.Format(SR.IO_FileNotFound_FileName, sourceFileName)));
             }
 
             if (!Directory.Exists(destinationDirectoryName))
@@ -283,23 +279,22 @@ private static void CreateFromDirectoryInternal(string sourceDirectoryName, Stre
                 DirectoryInfo di = new(sourceDirectoryName);
                 string basePath = GetBasePathForCreateFromDirectory(di, includeBaseDirectory);
 
-                char[] entryNameBuffer = ArrayPool<char>.Shared.Rent(DefaultCapacity);
-
-                try
-                {
+                bool skipBaseDirRecursion = false;
                 if (includeBaseDirectory)
                 {
-                        writer.WriteEntry(di.FullName, GetEntryNameForBaseDirectory(di.Name, ref entryNameBuffer));
+                    writer.WriteEntry(di.FullName, GetEntryNameForBaseDirectory(di.Name));
+                    skipBaseDirRecursion = (di.Attributes & FileAttributes.ReparsePoint) != 0;
                 }
 
-                    foreach (FileSystemInfo file in di.EnumerateFileSystemInfos("*", SearchOption.AllDirectories))
+                if (skipBaseDirRecursion)
                 {
-                        writer.WriteEntry(file.FullName, GetEntryNameForFileSystemInfo(file, basePath.Length, ref entryNameBuffer));
+                    // The base directory is a symlink, do not recurse into it
+                    return;
                 }
-                }
-                finally
+
+                foreach (FileSystemInfo file in GetFileSystemEnumerationForCreation(sourceDirectoryName))
                 {
-                    ArrayPool<char>.Shared.Return(entryNameBuffer);
+                    writer.WriteEntry(file.FullName, GetEntryNameForFileSystemInfo(file, basePath.Length));
                 }
             }
         }
@@ -339,44 +334,58 @@ private static async Task CreateFromDirectoryInternalAsync(string sourceDirector
                 DirectoryInfo di = new(sourceDirectoryName);
                 string basePath = GetBasePathForCreateFromDirectory(di, includeBaseDirectory);
 
-                char[] entryNameBuffer = ArrayPool<char>.Shared.Rent(DefaultCapacity);
-
-                try
-                {
+                bool skipBaseDirRecursion = false;
                 if (includeBaseDirectory)
                 {
-                        await writer.WriteEntryAsync(di.FullName, GetEntryNameForBaseDirectory(di.Name, ref entryNameBuffer), cancellationToken).ConfigureAwait(false);
+                    await writer.WriteEntryAsync(di.FullName, GetEntryNameForBaseDirectory(di.Name), cancellationToken).ConfigureAwait(false);
+                    skipBaseDirRecursion = (di.Attributes & FileAttributes.ReparsePoint) != 0;
                 }
 
-                    foreach (FileSystemInfo file in di.EnumerateFileSystemInfos("*", SearchOption.AllDirectories))
+                if (skipBaseDirRecursion)
                 {
-                        await writer.WriteEntryAsync(file.FullName, GetEntryNameForFileSystemInfo(file, basePath.Length, ref entryNameBuffer), cancellationToken).ConfigureAwait(false);
+                    // The base directory is a symlink, do not recurse into it
+                    return;
                 }
-                }
-                finally
+
+                foreach (FileSystemInfo file in GetFileSystemEnumerationForCreation(sourceDirectoryName))
                 {
-                    ArrayPool<char>.Shared.Return(entryNameBuffer);
+                    await writer.WriteEntryAsync(file.FullName, GetEntryNameForFileSystemInfo(file, basePath.Length), cancellationToken).ConfigureAwait(false);
                 }
             }
         }
 
+        // Generates a recursive enumeration of the filesystem entries inside the specified source directory, while
+        // making sure that directory symlinks do not get recursed.
+        private static IEnumerable<FileSystemInfo> GetFileSystemEnumerationForCreation(string sourceDirectoryName)
+        {
+            return new FileSystemEnumerable<FileSystemInfo>(
+                directory: sourceDirectoryName,
+                transform: (ref FileSystemEntry entry) => entry.ToFileSystemInfo(),
+                options: new EnumerationOptions()
+                {
+                    RecurseSubdirectories = true
+                })
+            {
+                ShouldRecursePredicate = IsNotADirectorySymlink
+            };
+
+            static bool IsNotADirectorySymlink(ref FileSystemEntry entry) => entry.IsDirectory && (entry.Attributes & FileAttributes.ReparsePoint) == 0;
+        }
+
         // Determines what should be the base path for all the entries when creating an archive.
         private static string GetBasePathForCreateFromDirectory(DirectoryInfo di, bool includeBaseDirectory) =>
             includeBaseDirectory && di.Parent != null ? di.Parent.FullName : di.FullName;
 
         // Constructs the entry name used for a filesystem entry when creating an archive.
-        private static string GetEntryNameForFileSystemInfo(FileSystemInfo file, int basePathLength, ref char[] entryNameBuffer)
+        private static string GetEntryNameForFileSystemInfo(FileSystemInfo file, int basePathLength)
         {
-            int entryNameLength = file.FullName.Length - basePathLength;
-            Debug.Assert(entryNameLength > 0);
-
-            bool isDirectory = file.Attributes.HasFlag(FileAttributes.Directory);
-            return ArchivingUtils.EntryFromPath(file.FullName, basePathLength, entryNameLength, ref entryNameBuffer, appendPathSeparator: isDirectory);
+            bool isDirectory = (file.Attributes & FileAttributes.Directory) != 0;
+            return ArchivingUtils.EntryFromPath(file.FullName.AsSpan(basePathLength), appendPathSeparator: isDirectory);
         }
 
-        private static string GetEntryNameForBaseDirectory(string name, ref char[] entryNameBuffer)
+        private static string GetEntryNameForBaseDirectory(string name)
         {
-            return ArchivingUtils.EntryFromPath(name, 0, name.Length, ref entryNameBuffer, appendPathSeparator: true);
+            return ArchivingUtils.EntryFromPath(name, appendPathSeparator: true);
         }
 
         // Extracts an archive into the specified directory.