Fix File.Copy onto a symlink on macOS

src/libraries/Common/src/Interop/Unix/System.Native/Interop.OpenFlags.cs

@@ -11,16 +11,17 @@ internal static partial class Sys
         internal enum OpenFlags
         {
             // Access modes (mutually exclusive)
-            O_RDONLY = 0x0000,
-            O_WRONLY = 0x0001,
-            O_RDWR   = 0x0002,
+            O_RDONLY  = 0x0000,
+            O_WRONLY  = 0x0001,
+            O_RDWR    = 0x0002,
 
             // Flags (combinable)
-            O_CLOEXEC = 0x0010,
-            O_CREAT   = 0x0020,
-            O_EXCL    = 0x0040,
-            O_TRUNC   = 0x0080,
-            O_SYNC    = 0x0100,
+            O_CLOEXEC  = 0x0010,
+            O_CREAT    = 0x0020,
+            O_EXCL     = 0x0040,
+            O_TRUNC    = 0x0080,
+            O_SYNC     = 0x0100,
+            O_NOFOLLOW = 0x0200,
         }
     }
 }

src/libraries/System.IO.FileSystem/tests/File/Copy.cs

@@ -253,6 +253,24 @@ public void Linux_CopyFromProcfsToFile(string path)
             Assert.Equal(File.ReadAllText(path), File.ReadAllText(testFile)); // assumes chosen files won't change between reads
         }
         #endregion
+
+        [ConditionalFact(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))]
+        public void OverwriteCopyOntoLink()
+        {
+            string file1 = GetTestFilePath();
+            string file2 = GetTestFilePath();
+            string link = GetTestFilePath();
+
+            File.Create(file1).Dispose();
+            File.Create(file2).Dispose();
+            File.CreateSymbolicLink(link, file2);
+
+            File.WriteAllText(file1, "abc");
+            File.WriteAllText(file2, "def");
+
+            File.Copy(file1, link, true);
+            Assert.Equal("abc", File.ReadAllText(file2));
+        }
     }
 
     public class File_Copy_str_str_b : File_Copy_str_str

src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs

@@ -88,9 +88,10 @@ internal bool TryGetCachedLength(out long cachedLength)
         }
 #pragma warning restore CA1822
 
-        private static SafeFileHandle Open(string path, Interop.Sys.OpenFlags flags, int mode,
+        private static SafeFileHandle Open(string path, Interop.Sys.OpenFlags flags, int mode, bool failForSymlink, out bool wasSymlink,
                                            Func<Interop.ErrorInfo, Interop.Sys.OpenFlags, string, Exception?>? createOpenException)
         {
+            wasSymlink = false;
             Debug.Assert(path != null);
             SafeFileHandle handle = Interop.Sys.Open(path, flags, mode);
             handle._path = path;
@@ -100,6 +101,12 @@ private static SafeFileHandle Open(string path, Interop.Sys.OpenFlags flags, int
                 Interop.ErrorInfo error = Interop.Sys.GetLastErrorInfo();
                 handle.Dispose();
 
+                if (failForSymlink && error.Error == Interop.Error.ELOOP)
+                {
+                    wasSymlink = true;
+                    return handle;
+                }
+
                 if (createOpenException?.Invoke(error, flags, path) is Exception ex)
                 {
                     throw ex;
@@ -169,30 +176,43 @@ public override bool IsInvalid
         // This information is retrieved from the 'stat' syscall that must be performed to ensure the path is not a directory.
         internal static SafeFileHandle OpenReadOnly(string fullPath, FileOptions options, out long fileLength, out UnixFileMode filePermissions)
         {
-            SafeFileHandle handle = Open(fullPath, FileMode.Open, FileAccess.Read, FileShare.Read, options, preallocationSize: 0, DefaultCreateMode, out fileLength, out filePermissions, null);
+            SafeFileHandle handle = Open(fullPath, FileMode.Open, FileAccess.Read, FileShare.Read, options, preallocationSize: 0, DefaultCreateMode, out fileLength, out filePermissions, false, out _, null);
             Debug.Assert(fileLength >= 0);
             return handle;
         }
 
         internal static SafeFileHandle Open(string fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, long preallocationSize, UnixFileMode? unixCreateMode = null,
                                             Func<Interop.ErrorInfo, Interop.Sys.OpenFlags, string, Exception?>? createOpenException = null)
         {
-            return Open(fullPath, mode, access, share, options, preallocationSize, unixCreateMode ?? DefaultCreateMode, out _, out _, createOpenException);
+            return Open(fullPath, mode, access, share, options, preallocationSize, unixCreateMode ?? DefaultCreateMode, out _, out _, false, out _, createOpenException);
+        }
+
+        internal static SafeFileHandle? OpenNoFollowSymlink(string fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, long preallocationSize, out bool wasSymlink, UnixFileMode? unixCreateMode = null,
+                                            Func<Interop.ErrorInfo, Interop.Sys.OpenFlags, string, Exception?>? createOpenException = null)
+        {
+            return Open(fullPath, mode, access, share, options, preallocationSize, unixCreateMode ?? DefaultCreateMode, out _, out _, true, out wasSymlink, createOpenException);
         }
 
         private static SafeFileHandle Open(string fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, long preallocationSize, UnixFileMode openPermissions,
-                                            out long fileLength, out UnixFileMode filePermissions,
+                                            out long fileLength, out UnixFileMode filePermissions, bool failForSymlink, out bool wasSymlink,
                                             Func<Interop.ErrorInfo, Interop.Sys.OpenFlags, string, Exception?>? createOpenException = null)
         {
             // Translate the arguments into arguments for an open call.
-            Interop.Sys.OpenFlags openFlags = PreOpenConfigurationFromOptions(mode, access, share, options);
+            Interop.Sys.OpenFlags openFlags = PreOpenConfigurationFromOptions(mode, access, share, options, failForSymlink);
 
             SafeFileHandle? safeFileHandle = null;
             try
             {
                 while (true)
                 {
-                    safeFileHandle = Open(fullPath, openFlags, (int)openPermissions, createOpenException);
+                    safeFileHandle = Open(fullPath, openFlags, (int)openPermissions, failForSymlink, out wasSymlink, createOpenException);
+
+                    if (failForSymlink && wasSymlink)
+                    {
+                        fileLength = default;
+                        filePermissions = default;
+                        return safeFileHandle;
+                    }
 
                     // When Init return false, the path has changed to another file entry, and
                     // we need to re-open the path to reflect that.
@@ -219,11 +239,16 @@ private static SafeFileHandle Open(string fullPath, FileMode mode, FileAccess ac
         /// <param name="access">The FileAccess provided to the stream's constructor</param>
         /// <param name="share">The FileShare provided to the stream's constructor</param>
         /// <param name="options">The FileOptions provided to the stream's constructor</param>
+        /// <param name="failForSymlink">Whether to cause ELOOP error when opening a symlink</param>
         /// <returns>The flags value to be passed to the open system call.</returns>
-        private static Interop.Sys.OpenFlags PreOpenConfigurationFromOptions(FileMode mode, FileAccess access, FileShare share, FileOptions options)
+        private static Interop.Sys.OpenFlags PreOpenConfigurationFromOptions(FileMode mode, FileAccess access, FileShare share, FileOptions options, bool failForSymlink)
         {
             // Translate FileMode.  Most of the values map cleanly to one or more options for open.
             Interop.Sys.OpenFlags flags = default;
+            if (failForSymlink)
+            {
+                flags |= Interop.Sys.OpenFlags.O_NOFOLLOW;
+            }
             switch (mode)
             {
                 default:

src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.TryCloneFile.OSX.cs

@@ -53,14 +53,22 @@ static bool TryCloneFile(string sourceFullPath, string destFullPath, int flags,
                 // it's locked by something else, and then delete it. It should also fail if destination == source since it's already locked.
                 try
                 {
-                    using SafeFileHandle? dstHandle = SafeFileHandle.Open(destFullPath, FileMode.Open, FileAccess.ReadWrite,
-                        FileShare.None, FileOptions.None, preallocationSize: 0, createOpenException: CreateOpenExceptionForCopyFile);
-                    if (Interop.Sys.Unlink(destFullPath) < 0 &&
-                        Interop.Sys.GetLastError() != Interop.Error.ENOENT)
+                    using SafeFileHandle? dstHandle = SafeFileHandle.OpenNoFollowSymlink(destFullPath, FileMode.Open, FileAccess.ReadWrite,
+                        FileShare.None, FileOptions.None, preallocationSize: 0, out bool wasSymlink, createOpenException: CreateOpenExceptionForCopyFile);
+                    if (wasSymlink)
                     {
-                        // Fall back to standard copy as an unexpected error has occurred.
+                        // Don't try if it's a symlink.
                         return;
                     }
+                    else
+                    {
+                        if (Interop.Sys.Unlink(destFullPath) < 0 &&
+                            Interop.Sys.GetLastError() != Interop.Error.ENOENT)
+                        {
+                            // Fall back to standard copy as an unexpected error has occurred.
+                            return;
+                        }
+                    }
                 }
                 catch (FileNotFoundException)
                 {

src/native/libs/System.Native/pal_io.c

@@ -289,7 +289,7 @@ static int32_t ConvertOpenFlags(int32_t flags)
             return -1;
     }
 
-    if (flags & ~(PAL_O_ACCESS_MODE_MASK | PAL_O_CLOEXEC | PAL_O_CREAT | PAL_O_EXCL | PAL_O_TRUNC | PAL_O_SYNC))
+    if (flags & ~(PAL_O_ACCESS_MODE_MASK | PAL_O_CLOEXEC | PAL_O_CREAT | PAL_O_EXCL | PAL_O_TRUNC | PAL_O_SYNC | PAL_O_NOFOLLOW))
     {
         assert_msg(false, "Unknown Open flag", (int)flags);
         return -1;
@@ -307,6 +307,8 @@ static int32_t ConvertOpenFlags(int32_t flags)
         ret |= O_TRUNC;
     if (flags & PAL_O_SYNC)
         ret |= O_SYNC;
+    if (flags & PAL_O_NOFOLLOW)
+        ret |= O_NOFOLLOW;
 
     assert(ret != -1);
     return ret;

src/native/libs/System.Native/pal_io.h

@@ -151,11 +151,12 @@ enum
 
     // Flags (combinable)
     // These numeric values are not defined by POSIX and vary across targets.
-    PAL_O_CLOEXEC = 0x0010, // Close-on-exec
-    PAL_O_CREAT = 0x0020,   // Create file if it doesn't already exist
-    PAL_O_EXCL = 0x0040,    // When combined with CREAT, fails if file already exists
-    PAL_O_TRUNC = 0x0080,   // Truncate file to length 0 if it already exists
-    PAL_O_SYNC = 0x0100,    // Block writes call will block until physically written
+    PAL_O_CLOEXEC = 0x0010,  // Close-on-exec
+    PAL_O_CREAT = 0x0020,    // Create file if it doesn't already exist
+    PAL_O_EXCL = 0x0040,     // When combined with CREAT, fails if file already exists
+    PAL_O_TRUNC = 0x0080,    // Truncate file to length 0 if it already exists
+    PAL_O_SYNC = 0x0100,     // Block writes call will block until physically written
+    PAL_O_NOFOLLOW = 0x0200, // Fails to open the target if it's a symlink, parent symlinks are allowed
 };
 
 /**