From 4711fde2d5055854f9e67278c2e38b5a95a5a31b Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Wed, 12 Jul 2023 05:23:17 +1000 Subject: [PATCH 01/17] WIP commit - Initial code for ReFS block copy operation, currently throws exceptions whenever it fails to support checking if it's actually working (obviously needs to be changed before merging) - Contains a number of TODO items to be resolved - Works locally --- .../Kernel32/Interop.DeviceIoControl.cs | 38 ++ .../Kernel32/Interop.FILE_DISPOSITION_INFO.cs | 17 + .../Kernel32/Interop.FileAttributes.cs | 1 + .../Kernel32/Interop.GetDiskFreeSpace.cs | 14 + .../Kernel32/Interop.GetVolumeInformation.cs | 1 + .../Interop.GetVolumeInformationByHandle.cs | 76 ++++ .../Kernel32/Interop.GetVolumePathName.cs | 59 +++ .../Windows/Kernel32/Interop.SetEndOfFile.cs | 15 + .../System.IO.FileSystem/tests/File/Copy.cs | 88 ++++ .../System.Private.CoreLib.Shared.projitems | 15 + .../src/System/IO/FileSystem.Windows.cs | 409 ++++++++++++++++++ 11 files changed, 733 insertions(+) create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_DISPOSITION_INFO.cs create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetDiskFreeSpace.cs create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformationByHandle.cs create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.SetEndOfFile.cs diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs index aae40b9bb7fdd..1062fa0cfef39 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs @@ -15,6 +15,44 @@ internal static partial class Kernel32 // https://docs.microsoft.com/windows-hardware/drivers/ddi/ntddstor/ni-ntddstor-ioctl_storage_read_capacity internal const int IOCTL_STORAGE_READ_CAPACITY = 0x002D5140; + // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_set_sparse + internal const int FSCTL_SET_SPARSE = 0x000900c4; + internal struct FILE_SET_SPARSE_BUFFER + { + internal int SetSparse; + } + + // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_get_integrity_information + internal const int FSCTL_GET_INTEGRITY_INFORMATION = 0x0009027C; + internal struct FSCTL_GET_INTEGRITY_INFORMATION_BUFFER + { + internal ushort ChecksumAlgorithm; + internal ushort Reserved; + internal uint Flags; + internal uint ChecksumChunkSizeInBytes; + internal uint ClusterSizeInBytes; + } + + // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_set_integrity_information + internal const int FSCTL_SET_INTEGRITY_INFORMATION = 0x0009C280; + internal struct FSCTL_SET_INTEGRITY_INFORMATION_BUFFER + { + internal ushort ChecksumAlgorithm; + internal ushort Reserved; + internal uint Flags; + } + + // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_duplicate_extents_to_file + internal const int FSCTL_DUPLICATE_EXTENTS_TO_FILE = 0x00098344; + [StructLayout(LayoutKind.Sequential, Pack = 4)] //the longs are aligned to 4 bytes on 32-bit, and 8 on 64-bit + internal struct DUPLICATE_EXTENTS_DATA + { + internal IntPtr FileHandle; + internal long SourceFileOffset; + internal long TargetFileOffset; + internal long ByteCount; + } + [LibraryImport(Libraries.Kernel32, EntryPoint = "DeviceIoControl", SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] internal static unsafe partial bool DeviceIoControl( diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_DISPOSITION_INFO.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_DISPOSITION_INFO.cs new file mode 100644 index 0000000000000..c659b3f8a01fa --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_DISPOSITION_INFO.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + // From FILE_INFO_BY_HANDLE_CLASS + // Use for SetFileInformationByHandle + internal const int FileDispositionInfo = 4; + + internal struct FILE_DISPOSITION_INFO + { + internal int DeleteFile; + } + } +} diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FileAttributes.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FileAttributes.cs index c2350c35ec604..70ae7847537c9 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FileAttributes.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FileAttributes.cs @@ -10,6 +10,7 @@ internal static partial class FileAttributes internal const int FILE_ATTRIBUTE_NORMAL = 0x00000080; internal const int FILE_ATTRIBUTE_READONLY = 0x00000001; internal const int FILE_ATTRIBUTE_DIRECTORY = 0x00000010; + internal const int FILE_ATTRIBUTE_SPARSE_FILE = 0x00000200; internal const int FILE_ATTRIBUTE_REPARSE_POINT = 0x00000400; } } diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetDiskFreeSpace.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetDiskFreeSpace.cs new file mode 100644 index 0000000000000..31df630391ea4 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetDiskFreeSpace.cs @@ -0,0 +1,14 @@ +// 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; + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + [LibraryImport(Libraries.Kernel32, EntryPoint = "GetDiskFreeSpaceW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool GetDiskFreeSpace(string drive, out int sectorsPerCluster, out int bytesPerSector, out int numberOfFreeClusters, out int totalNumberOfClusters); + } +} diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformation.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformation.cs index 4082bc4567776..e020179508083 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformation.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformation.cs @@ -21,5 +21,6 @@ internal static unsafe partial bool GetVolumeInformation( int fileSystemNameBufLen); internal const uint FILE_SUPPORTS_ENCRYPTION = 0x00020000; + internal const uint FILE_SUPPORTS_BLOCK_REFCOUNTING = 0x08000000; } } diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformationByHandle.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformationByHandle.cs new file mode 100644 index 0000000000000..140c12e520515 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformationByHandle.cs @@ -0,0 +1,76 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.IO; +using System.Runtime.InteropServices; +using System.Text; +using Microsoft.Win32.SafeHandles; + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + [LibraryImport(Libraries.Kernel32, EntryPoint = "GetVolumeInformationByHandleW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] + [return: MarshalAs(UnmanagedType.Bool)] + private static unsafe partial bool GetVolumeInformationByHandleW( + SafeFileHandle hFile, + char* volumeName, + int volumeNameBufLen, + int* volSerialNumber, + int* maxFileNameLen, + int* fileSystemFlags, + char* fileSystemName, + int fileSystemNameBufLen); + + public static unsafe int GetVolumeInformationByHandle( + SafeFileHandle hFile, + out string? volumePath, + bool wantsVolumePath, + int* volSerialNumber, + int* maxFileNameLen, + out int fileSystemFlags, + char* fileSystemName, + int fileSystemNameBufLen) + { + // Allocate output buffer on the stack initially. + const int stackAllocation = 512; + Span volumePathBuffer = wantsVolumePath ? stackalloc char[stackAllocation + 1] : stackalloc char[0]; // +1 to ensure a \0 at the end, todo: is this necessary + int bufferSize = stackAllocation; + + // Loop until the buffer's big enough. + while (true) + { + fixed (char* lpszVolumePathName = volumePathBuffer) + { + // Try to get the volume name, will succeed if the buffer's big enough. + fixed (int* pFileSystemFlags = &fileSystemFlags) + { + if (GetVolumeInformationByHandleW(hFile, lpszVolumePathName, Math.Max(volumePathBuffer.Length - 1, 0), volSerialNumber, maxFileNameLen, pFileSystemFlags, fileSystemName, fileSystemNameBufLen)) + { + if (wantsVolumePath) volumePath = new string(lpszVolumePathName); + else volumePath = null; + return 0; + } + } + + // Check if the error was that the buffer is not large enough. + int error = Marshal.GetLastWin32Error(); + if (wantsVolumePath && error == Interop.Errors.ERROR_INSUFFICIENT_BUFFER) //todo: check this is the correct error + { + // Create a new buffer and try again. + // todo: use array pool and check for overflow + volumePathBuffer = new char[bufferSize *= 2]; + continue; + } + else + { + // Return our error. + volumePath = null; + return error; + } + } + } + } + } +} diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs new file mode 100644 index 0000000000000..d4036b2e18416 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs @@ -0,0 +1,59 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.IO; +using System.Runtime.InteropServices; +using System.Text; + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + [LibraryImport(Libraries.Kernel32, EntryPoint = "GetVolumePathNameW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] + [return: MarshalAs(UnmanagedType.Bool)] + private static unsafe partial bool _GetVolumePathName(char* lpszFileName, char* lpszVolumePathName, int cchBufferLength); + + public static unsafe int GetVolumePathName(string fileName, out string? volumePath) + { + fileName = PathInternal.EnsureExtendedPrefixIfNeeded(fileName); //todo: unsure if this is needed for this API + fixed (char* lpszFileName = fileName) + { + // Allocate output buffer on the stack initially. + const int stackAllocation = 512; + Span volumePathBuffer = stackalloc char[stackAllocation + 1]; // +1 to ensure a \0 at the end, todo: is this necessary + int bufferSize = stackAllocation; + + // Loop until the buffer's big enough. + while (true) + { + fixed (char* lpszVolumePathName = volumePathBuffer) + { + // Try to get the volume name, will succeed if the buffer's big enough. + if (_GetVolumePathName(lpszFileName, lpszVolumePathName, volumePathBuffer.Length - 1)) + { + volumePath = new string(lpszVolumePathName); + return 0; + } + + // Check if the error was that the buffer is not large enough. + int error = Marshal.GetLastWin32Error(); + if (error == Interop.Errors.ERROR_INSUFFICIENT_BUFFER) //todo: check this is the correct error + { + // Create a new buffer and try again. + // todo: use array pool and check for overflow + volumePathBuffer = new char[bufferSize *= 2]; + continue; + } + else + { + // Return our error. + volumePath = null; + return error; + } + } + } + } + } + } +} diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.SetEndOfFile.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.SetEndOfFile.cs new file mode 100644 index 0000000000000..ea27fa2c357d4 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.SetEndOfFile.cs @@ -0,0 +1,15 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Win32.SafeHandles; +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + [LibraryImport(Libraries.Kernel32, SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + internal static partial bool SetEndOfFile(SafeFileHandle hFile); + } +} diff --git a/src/libraries/System.IO.FileSystem/tests/File/Copy.cs b/src/libraries/System.IO.FileSystem/tests/File/Copy.cs index 4ab26c3748877..70d01f195b898 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/Copy.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/Copy.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; +using System.Runtime.InteropServices; using System.Security.Cryptography; using Xunit; @@ -400,5 +401,92 @@ public void EnsureThrowWhenCopyToNonSharedFile() using var stream = new FileStream(file1, FileMode.Open, FileAccess.Read, FileShare.None); Assert.Throws(() => File.Copy(file2, file1, overwrite: true)); } + + [ConditionalTheory(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks)), + InlineData("", ""), + /*InlineData(":a", ""),*/ + InlineData("", ":a")/*, + InlineData(":a", ":a")*/] + //todo: is copying from an ADS meant to fail? + [PlatformSpecific(TestPlatforms.Windows)] + public void WindowsAlternateDataStreamSymlinkTest(string stream1, string stream2) + { + // This test checks copying all combinations of alternate data streams with all combinations of symlinks referencing them. + // This test exists to check we don't cause a BSOD when using ReFS block copy operation on alternative data streams (pending? rolled out fix from Windows team), and that it has the correct behaviour. + + string sourceFile = GetTestFilePath(); + string destFile = GetTestFilePath(); + + void Test(string src, string dst) + { + try + { + File.WriteAllText(sourceFile, "abc"); + File.WriteAllText(destFile, "def"); + File.WriteAllText(sourceFile + stream1, "ghi"); + File.WriteAllText(destFile + stream2, "jkl"); + + File.Copy(src, dst, true); + + if (stream1 != "") Assert.Equal("abc", File.ReadAllText(sourceFile)); + if (stream2 != "") Assert.Equal("def", File.ReadAllText(destFile)); + Assert.Equal("ghi", File.ReadAllText(sourceFile + stream1)); + Assert.Equal("ghi", File.ReadAllText(destFile + stream2)); + } + catch (Exception ex) + { + throw new Exception($"Failed with src={src}, dst={dst}.", ex); + } + } + + File.CreateSymbolicLink(sourceFile + ".link", sourceFile + stream1); + File.CreateSymbolicLink(destFile + ".link", destFile + stream2); + + Test(sourceFile + stream1, destFile + stream2); + Test(sourceFile + stream1, destFile + ".link"); + Test(sourceFile + ".link", destFile + stream2); + Test(sourceFile + ".link", destFile + ".link"); + } + + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public unsafe void WindowsCheckSparseness() + { + string sourceFile = GetTestFilePath(); + string destFile = GetTestFilePath(); + + File.WriteAllText(sourceFile, "abc"); + File.WriteAllText(destFile, "def"); + + Assert.True((File.GetAttributes(sourceFile) & FileAttributes.SparseFile) == 0); + File.Copy(sourceFile, destFile, true); + Assert.True((File.GetAttributes(destFile) & FileAttributes.SparseFile) == 0); + Assert.Equal("abc", File.ReadAllText(sourceFile)); + + using (FileStream file = File.Open(sourceFile, FileMode.Open)) + { + DeviceIoControl(file.SafeFileHandle.DangerousGetHandle(), /*FSCTL_SET_SPARSE*/ 0x000900c4, null, 0, null, 0, out _, 0); + } + File.WriteAllText(destFile, "def"); + + Assert.True((File.GetAttributes(sourceFile) & FileAttributes.SparseFile) != 0); + File.Copy(sourceFile, destFile, true); + Assert.True((File.GetAttributes(destFile) & FileAttributes.SparseFile) != 0); + Assert.Equal("abc", File.ReadAllText(sourceFile)); + + [DllImport("kernel32.dll", EntryPoint = "DeviceIoControl", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + static unsafe extern bool DeviceIoControl( + IntPtr hDevice, + uint dwIoControlCode, + void* lpInBuffer, + uint nInBufferSize, + void* lpOutBuffer, + uint nOutBufferSize, + out uint lpBytesReturned, + IntPtr lpOverlapped); + } + + // Todo: add a way to run all these on ReFS, and a test to check we actually cloned the reference, not just the data on ReFS. } } diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index c7a6eed48f322..9f84162210259 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -1638,6 +1638,9 @@ Common\Interop\Windows\Kernel32\Interop.FILE_ALLOCATION_INFO.cs + + Common\Interop\Windows\Kernel32\Interop.FILE_DISPOSITION_INFO.cs + Common\Interop\Windows\Kernel32\Interop.FILE_END_OF_FILE_INFO.cs @@ -1704,6 +1707,9 @@ Common\Interop\Windows\Kernel32\Interop.GetCurrentProcessId.cs + + Common\Interop\Windows\Kernel32\Interop.GetDiskFreeSpace.cs + Common\Interop\Windows\Kernel32\Interop.GetFileAttributesEx.cs @@ -1761,6 +1767,12 @@ Common\Interop\Windows\Kernel32\Interop.GetVolumeInformation.cs + + Common\Interop\Windows\Kernel32\Interop.GetVolumeInformationByHandle.cs + + + Common\Interop\Windows\Kernel32\Interop.GetVolumePathName.cs + Common\Interop\Windows\Kernel32\Interop.GlobalMemoryStatusEx.cs @@ -1860,6 +1872,9 @@ Common\Interop\Windows\Kernel32\Interop.SetCurrentDirectory.cs + + Common\Interop\Windows\Kernel32\Interop.SetEndOfFile.cs + Common\Interop\Windows\Kernel32\Interop.SetFileAttributes.cs diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index fc063209e36fc..2a1fbcb0b8509 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -56,8 +56,417 @@ private static unsafe void ThrowExceptionEncryptDecryptFail(string fullPath) throw Win32Marshal.GetExceptionForWin32Error(errorCode, fullPath); } + private static SafeFileHandle? TryOpenTargetNoAlternativeDataStream( + string lpFileName, + int dwDesiredAccess, + FileShare dwShareMode, + FileMode dwCreationDisposition, + int dwFlagsAndAttributes, + ref Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA fileInformation, + out string openedFileName) + { + // Default value for openedFileName. + openedFileName = lpFileName; + + // Check the file isn't an alternative data stream + if (Path.GetFileName(lpFileName.AsSpan()).Contains(':')) + { + return null; + } + + // Try to open it in a way where we can detect symlinks. + SafeFileHandle result = Interop.Kernel32.CreateFile( + lpFileName, + dwDesiredAccess, + dwShareMode, + dwCreationDisposition, + dwFlagsAndAttributes | Interop.Kernel32.FileOperations.FILE_FLAG_OPEN_REPARSE_POINT + ); + + // Check we successfully opened it. + if (result.IsInvalid) + { + return null; + } + + // Read the file attributes. + if (!Interop.Kernel32.GetFileAttributesEx(lpFileName, Interop.Kernel32.GET_FILEEX_INFO_LEVELS.GetFileExInfoStandard, ref fileInformation)) + { + result.Dispose(); + return null; + } + + // Deal with the case of a reparse point. + if ((fileInformation.dwFileAttributes & Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_REPARSE_POINT) != 0) + { + // Get the path of the target. + result.Dispose(); + //try + //{ + /* */string? target = GetFinalLinkTarget(lpFileName, false); + //} + //catch + //{ + // return false; + //} + + // Deal with no target + if (target == null) + { + return null; + } + + // Check the target's file name for alternative data streams. + if (Path.GetFileName(target.AsSpan()).Contains(':')) + { + return null; + } + + // Open the target. + result = Interop.Kernel32.CreateFile( + target, + dwDesiredAccess, + dwShareMode, + dwCreationDisposition, + dwFlagsAndAttributes | Interop.Kernel32.FileOperations.FILE_FLAG_OPEN_REPARSE_POINT + ); + openedFileName = target; + + // Read the file attributes. + if (!Interop.Kernel32.GetFileAttributesEx(lpFileName, Interop.Kernel32.GET_FILEEX_INFO_LEVELS.GetFileExInfoStandard, ref fileInformation)) + { + result.Dispose(); + return null; + } + + // Check we haven't somehow ended up with another symlink due to things changing very quickly. + if ((fileInformation.dwFileAttributes & Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_REPARSE_POINT) != 0) + { + result.Dispose(); + return null; + } + } + + // Return our result. + return result; + } + + private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPath, bool overwrite) + { + // Check the destination file isn't an alternative data stream (unsupported, and crashes on up to some versions of Windows 11). + // Todo: make some of these conditional based on Windows version. + if (Path.GetFileName(destFullPath.AsSpan()).Contains(':')) + { + return false; + } + + // Open the source file. + // We use FILE_FLAGS_NO_BUFFERING since we're not using unaligned writes during cloning and can skip buffering overhead. + const int FILE_FLAG_NO_BUFFERING = 0x20000000; + Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA sourceFileInformation = default; + SafeFileHandle? _sourceHandle = TryOpenTargetNoAlternativeDataStream( + sourceFullPath, + Interop.Kernel32.GenericOperations.GENERIC_READ, + FileShare.Read, + FileMode.Open, + FILE_FLAG_NO_BUFFERING, + ref sourceFileInformation, + out string openedSourceFullPath); + if (_sourceHandle == null) + { + return false; + } + using (SafeFileHandle sourceHandle = _sourceHandle) + { + // Return false if we failed to open the source file. + if (sourceHandle.IsInvalid) + { + return false; + } + + // Read the source's volume's info. + // todo: do we want GetVolumePathName + GetVolumeInformationByHandle instead? + int sourceSerialNumber; + if (Interop.Kernel32.GetVolumeInformationByHandle(sourceHandle, out _, false, &sourceSerialNumber, null, out int sourceVolumeFlags, null, 0) != 0) + { + throw new Exception("C"); + //return false; + } + + // Check if it supports the block copy operation. + if ((sourceVolumeFlags & Interop.Kernel32.FILE_SUPPORTS_BLOCK_REFCOUNTING) == 0) + { + throw new Exception("D"); + //return false; + } + + // Get file size. + long sourceSize = (long)(((ulong)sourceFileInformation.nFileSizeHigh << 32) | sourceFileInformation.nFileSizeLow); + + // Helper variables. + SafeFileHandle? destinationHandle = null; + bool madeNew = false; + + try + { + // Open the destination, keeping track of whether we made a file. + Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA destFileInformation = default; + if (overwrite) + { + // Try to open the existing file. + destinationHandle = TryOpenTargetNoAlternativeDataStream( + destFullPath, + Interop.Kernel32.GenericOperations.GENERIC_READ | Interop.Kernel32.GenericOperations.GENERIC_WRITE, + FileShare.None, + FileMode.Open, + 0, + ref destFileInformation, + out _); + if (destinationHandle == null) + { + // Alternative data stream. + return false; + } + if (destinationHandle.IsInvalid) + { + // Try opening as a new file. + destinationHandle = null; + } + } + + // If we still haven't opened the file. + if (destinationHandle == null) + { + // Try to create the destination file. + destinationHandle = TryOpenTargetNoAlternativeDataStream( + destFullPath, + Interop.Kernel32.GenericOperations.GENERIC_READ | Interop.Kernel32.GenericOperations.GENERIC_WRITE, + FileShare.None, + FileMode.CreateNew, + 0, + ref destFileInformation, + out _); + if (destinationHandle == null) + { + // Alternative data stream. + return false; + } + if (destinationHandle.IsInvalid) + { + // Failure to open. + return false; + } + madeNew = true; + } + + // Read the destination file's volume's serial number. + int destSerialNumber; + if (Interop.Kernel32.GetVolumeInformationByHandle(sourceHandle, out _, false, &destSerialNumber, null, out int destVolumeFlags, null, 0) != 0) + { + throw new Exception("G"); + //return false; + } + + // Check the destination file is on the same volume. + // Note: this doesn't guarantee they're on the same volume for sure, so we have to take that into consideration later: https://devblogs.microsoft.com/oldnewthing/20170707-00/?p=96555. + if (sourceSerialNumber != destSerialNumber) + { + throw new Exception("H"); + //return false; + } + + // Quick sanity check to see if the destination supports the block copy operation, since it's basically free to check anyway. + if ((destVolumeFlags & Interop.Kernel32.FILE_SUPPORTS_BLOCK_REFCOUNTING) == 0) + { + throw new Exception("I"); + //return false; + } + + // Get the source volume path. Note: we need the real path here for symlinks also, hence openedSourceFullPath. + if (Interop.Kernel32.GetVolumePathName(openedSourceFullPath, out var volumePath) != 0) + { + throw new Exception("K1"); + //return false; + } + + // Read the source volume's cluster size. + if (!Interop.Kernel32.GetDiskFreeSpace(volumePath!, out int sectorsPerCluster, out int bytesPerCluster, out _, out _)) + { + throw new Exception("K"); + //return false; + } + long clusterSize = sectorsPerCluster * (long)bytesPerCluster; + + // Set file length to 0. + if (!madeNew) + { + if (!Interop.Kernel32.SetEndOfFile(destinationHandle)) + { + throw new Exception("O"); + //return false; + } + } + + // Ensure the destination file is not readonly. + // Todo: are there other flags we need to exclude here? + FileAttributes newAttributes = (FileAttributes)destFileInformation.dwFileAttributes & ~(FileAttributes.ReadOnly | FileAttributes.Normal); + if (newAttributes == 0) newAttributes = FileAttributes.Normal; + Interop.Kernel32.FILE_BASIC_INFO basicInfo = new() + { + FileAttributes = (uint)newAttributes, + }; + if (!Interop.Kernel32.SetFileInformationByHandle( + destinationHandle, + Interop.Kernel32.FileBasicInfo, + &basicInfo, + (uint)sizeof(Interop.Kernel32.FILE_BASIC_INFO))) + { + throw Win32Marshal.GetExceptionForLastWin32Error(destinationHandle.Path); + } + + // Make the destination sparse. + if (!Interop.Kernel32.DeviceIoControl(destinationHandle, Interop.Kernel32.FSCTL_SET_SPARSE, null, 0, null, 0, out _, 0)) + { + throw new Exception("Q3"); + } + + // Clone file integrity settings from source to destination. Must be done while file is zero size. + Interop.Kernel32.FSCTL_GET_INTEGRITY_INFORMATION_BUFFER getIntegrityInfo = default; + if (!Interop.Kernel32.DeviceIoControl( + sourceHandle, + Interop.Kernel32.FSCTL_GET_INTEGRITY_INFORMATION, + null, + 0, + &getIntegrityInfo, + (uint)sizeof(Interop.Kernel32.FSCTL_GET_INTEGRITY_INFORMATION_BUFFER), + out _, + IntPtr.Zero)) + { + throw new Exception("Q"); + //return false; + } + if (!madeNew || getIntegrityInfo.ChecksumAlgorithm != 0 || getIntegrityInfo.Flags != 0) + { + Interop.Kernel32.FSCTL_SET_INTEGRITY_INFORMATION_BUFFER setIntegrityInfo = default; + setIntegrityInfo.ChecksumAlgorithm = getIntegrityInfo.ChecksumAlgorithm; + setIntegrityInfo.Flags = getIntegrityInfo.Flags; + setIntegrityInfo.Reserved = getIntegrityInfo.Reserved; + if (!Interop.Kernel32.DeviceIoControl( + destinationHandle, + Interop.Kernel32.FSCTL_SET_INTEGRITY_INFORMATION, + &setIntegrityInfo, + (uint)sizeof(Interop.Kernel32.FSCTL_SET_INTEGRITY_INFORMATION_BUFFER), + null, + 0, + out _, + IntPtr.Zero)) + { + throw new Exception("R"); + //return false; + } + } + + // Set length of destination to same as source. + Interop.Kernel32.FILE_END_OF_FILE_INFO eofInfo = default; + eofInfo.EndOfFile = sourceSize; + if (!Interop.Kernel32.SetFileInformationByHandle(destinationHandle, Interop.Kernel32.FileEndOfFileInfo, &eofInfo, (uint)sizeof(Interop.Kernel32.FILE_END_OF_FILE_INFO))) + { + throw new Exception("S"); + //return false; + } + + // Copy all of the blocks. + Interop.Kernel32.DUPLICATE_EXTENTS_DATA duplicateExtentsData = default; + duplicateExtentsData.FileHandle = sourceHandle.DangerousGetHandle(); + // ReFS requires that cloned regions reside on a disk cluster boundary. + long clusterSizeMask = clusterSize - 1; + long fileSizeRoundedUpToClusterBoundary = (sourceSize + clusterSizeMask) & ~clusterSizeMask; + long sourceOffset = 0; + while (sourceOffset < sourceSize) + { + duplicateExtentsData.SourceFileOffset = sourceOffset; + duplicateExtentsData.TargetFileOffset = sourceOffset; + const long MaxChunkSize = 1L << 31; // Each cloned region must be < 4GiB in length. Use a smaller default (2GiB). + long thisChunkSize = Math.Min(fileSizeRoundedUpToClusterBoundary - sourceOffset, MaxChunkSize); + duplicateExtentsData.ByteCount = thisChunkSize; + + if (!Interop.Kernel32.DeviceIoControl( + destinationHandle, + Interop.Kernel32.FSCTL_DUPLICATE_EXTENTS_TO_FILE, + &duplicateExtentsData, + (uint)sizeof(Interop.Kernel32.DUPLICATE_EXTENTS_DATA), + null, + 0, + out _, + IntPtr.Zero)) + { + throw new Exception("U1"); + //return false; + } + + sourceOffset += thisChunkSize; + } + + // Match source sparseness. + if ((sourceFileInformation.dwFileAttributes & Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_SPARSE_FILE) == 0) + { + Interop.Kernel32.FILE_SET_SPARSE_BUFFER sparseBuffer; + sparseBuffer.SetSparse = 0; + Interop.Kernel32.DeviceIoControl(destinationHandle, Interop.Kernel32.FSCTL_SET_SPARSE, &sparseBuffer, (uint)sizeof(Interop.Kernel32.FILE_SET_SPARSE_BUFFER), null, 0, out _, 0); + } + + // Update the attributes and times. + basicInfo = new Interop.Kernel32.FILE_BASIC_INFO + { + CreationTime = sourceFileInformation.ftCreationTime.ToTicks(), + LastAccessTime = sourceFileInformation.ftLastAccessTime.ToTicks(), + LastWriteTime = sourceFileInformation.ftLastWriteTime.ToTicks(), + FileAttributes = (uint)sourceFileInformation.dwFileAttributes, + }; + if (!Interop.Kernel32.SetFileInformationByHandle( + destinationHandle, + Interop.Kernel32.FileBasicInfo, + &basicInfo, + (uint)sizeof(Interop.Kernel32.FILE_BASIC_INFO))) + { + throw Win32Marshal.GetExceptionForLastWin32Error(destinationHandle.Path); + } + + // We have now succeeded, and don't want to delete the destination. + madeNew = false; + return true; + } + finally + { + if (madeNew) + { + // Mark the file for deletion. + Interop.Kernel32.FILE_DISPOSITION_INFO dispositionInfo = new Interop.Kernel32.FILE_DISPOSITION_INFO() + { + DeleteFile = 1, + }; + if (!Interop.Kernel32.SetFileInformationByHandle( + destinationHandle!, + Interop.Kernel32.FileDispositionInfo, + &dispositionInfo, + (uint)sizeof(Interop.Kernel32.FILE_DISPOSITION_INFO))) + { + throw Win32Marshal.GetExceptionForLastWin32Error(destinationHandle!.Path); + } + } + + destinationHandle?.Dispose(); + } + } + } + public static void CopyFile(string sourceFullPath, string destFullPath, bool overwrite) { + if (TryCloneFile(sourceFullPath, destFullPath, overwrite)) + { + return; + } + int errorCode = Interop.Kernel32.CopyFile(sourceFullPath, destFullPath, !overwrite); if (errorCode != Interop.Errors.ERROR_SUCCESS) From 306ca0021d6cbed726c96599a782d7e2f2f08aff Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Wed, 12 Jul 2023 16:47:32 +1000 Subject: [PATCH 02/17] Fix `LARGE_INTEGER` alignment/packing - As per review --- .../src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs index 1062fa0cfef39..b9a289a26b8ba 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs @@ -44,7 +44,6 @@ internal struct FSCTL_SET_INTEGRITY_INFORMATION_BUFFER // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_duplicate_extents_to_file internal const int FSCTL_DUPLICATE_EXTENTS_TO_FILE = 0x00098344; - [StructLayout(LayoutKind.Sequential, Pack = 4)] //the longs are aligned to 4 bytes on 32-bit, and 8 on 64-bit internal struct DUPLICATE_EXTENTS_DATA { internal IntPtr FileHandle; From e1be061bee911600a3c889e618bf3ad715c2f477 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 13 Jul 2023 08:58:08 +1000 Subject: [PATCH 03/17] Fix some p/invoke issues - as per feedback - also simplify the GetVolumeInformationByHandle code, as we don't need the volumePath currently --- .../Kernel32/Interop.FILE_DISPOSITION_INFO.cs | 2 +- .../Interop.GetVolumeInformationByHandle.cs | 64 ++++++------------- .../src/System/IO/FileSystem.Windows.cs | 8 +-- 3 files changed, 24 insertions(+), 50 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_DISPOSITION_INFO.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_DISPOSITION_INFO.cs index c659b3f8a01fa..344221b985315 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_DISPOSITION_INFO.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_DISPOSITION_INFO.cs @@ -11,7 +11,7 @@ internal static partial class Kernel32 internal struct FILE_DISPOSITION_INFO { - internal int DeleteFile; + internal byte DeleteFile; } } } diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformationByHandle.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformationByHandle.cs index 140c12e520515..b756e6fe1040b 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformationByHandle.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformationByHandle.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Diagnostics; using System.IO; using System.Runtime.InteropServices; using System.Text; @@ -16,61 +17,34 @@ internal static partial class Kernel32 private static unsafe partial bool GetVolumeInformationByHandleW( SafeFileHandle hFile, char* volumeName, - int volumeNameBufLen, - int* volSerialNumber, - int* maxFileNameLen, - int* fileSystemFlags, + uint volumeNameBufLen, + uint* volSerialNumber, + uint* maxFileNameLen, + uint* fileSystemFlags, char* fileSystemName, - int fileSystemNameBufLen); + uint fileSystemNameBufLen); public static unsafe int GetVolumeInformationByHandle( SafeFileHandle hFile, - out string? volumePath, - bool wantsVolumePath, - int* volSerialNumber, - int* maxFileNameLen, - out int fileSystemFlags, + uint* volSerialNumber, + uint* maxFileNameLen, + out uint fileSystemFlags, char* fileSystemName, - int fileSystemNameBufLen) + uint fileSystemNameBufLen) { - // Allocate output buffer on the stack initially. - const int stackAllocation = 512; - Span volumePathBuffer = wantsVolumePath ? stackalloc char[stackAllocation + 1] : stackalloc char[0]; // +1 to ensure a \0 at the end, todo: is this necessary - int bufferSize = stackAllocation; - - // Loop until the buffer's big enough. - while (true) + // Try to get the volume information. + fixed (uint* pFileSystemFlags = &fileSystemFlags) { - fixed (char* lpszVolumePathName = volumePathBuffer) + if (GetVolumeInformationByHandleW(hFile, null, 0, volSerialNumber, maxFileNameLen, pFileSystemFlags, fileSystemName, fileSystemNameBufLen)) { - // Try to get the volume name, will succeed if the buffer's big enough. - fixed (int* pFileSystemFlags = &fileSystemFlags) - { - if (GetVolumeInformationByHandleW(hFile, lpszVolumePathName, Math.Max(volumePathBuffer.Length - 1, 0), volSerialNumber, maxFileNameLen, pFileSystemFlags, fileSystemName, fileSystemNameBufLen)) - { - if (wantsVolumePath) volumePath = new string(lpszVolumePathName); - else volumePath = null; - return 0; - } - } - - // Check if the error was that the buffer is not large enough. - int error = Marshal.GetLastWin32Error(); - if (wantsVolumePath && error == Interop.Errors.ERROR_INSUFFICIENT_BUFFER) //todo: check this is the correct error - { - // Create a new buffer and try again. - // todo: use array pool and check for overflow - volumePathBuffer = new char[bufferSize *= 2]; - continue; - } - else - { - // Return our error. - volumePath = null; - return error; - } + return 0; } } + + // Return the error. + int error = Marshal.GetLastWin32Error(); + Debug.Assert(error != Errors.ERROR_INSUFFICIENT_BUFFER); + return error; } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index 2a1fbcb0b8509..5baca30f61621 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -186,8 +186,8 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa // Read the source's volume's info. // todo: do we want GetVolumePathName + GetVolumeInformationByHandle instead? - int sourceSerialNumber; - if (Interop.Kernel32.GetVolumeInformationByHandle(sourceHandle, out _, false, &sourceSerialNumber, null, out int sourceVolumeFlags, null, 0) != 0) + uint sourceSerialNumber; + if (Interop.Kernel32.GetVolumeInformationByHandle(sourceHandle, &sourceSerialNumber, null, out uint sourceVolumeFlags, null, 0) != 0) { throw new Exception("C"); //return false; @@ -260,8 +260,8 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa } // Read the destination file's volume's serial number. - int destSerialNumber; - if (Interop.Kernel32.GetVolumeInformationByHandle(sourceHandle, out _, false, &destSerialNumber, null, out int destVolumeFlags, null, 0) != 0) + uint destSerialNumber; + if (Interop.Kernel32.GetVolumeInformationByHandle(sourceHandle, &destSerialNumber, null, out uint destVolumeFlags, null, 0) != 0) { throw new Exception("G"); //return false; From 0b9b36a80e6d53fc97eec11cbf65cb747754f353 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 13 Jul 2023 09:28:18 +1000 Subject: [PATCH 04/17] Implement more feedback - Fix int->byte for FILE_SET_SPARSE_BUFFER - Implement GetVolumePathName properly, using GetFullPathName and ValueStringBuilder --- .../Kernel32/Interop.DeviceIoControl.cs | 2 +- .../Kernel32/Interop.GetVolumePathName.cs | 53 +++++++------------ .../src/System/IO/FileSystem.Windows.cs | 7 +-- .../src/System/IO/PathHelper.Windows.cs | 2 +- 4 files changed, 24 insertions(+), 40 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs index b9a289a26b8ba..bb812be3f27d3 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs @@ -19,7 +19,7 @@ internal static partial class Kernel32 internal const int FSCTL_SET_SPARSE = 0x000900c4; internal struct FILE_SET_SPARSE_BUFFER { - internal int SetSparse; + internal byte SetSparse; } // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_get_integrity_information diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs index d4036b2e18416..e9127cd93075d 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Diagnostics; using System.IO; using System.Runtime.InteropServices; using System.Text; @@ -12,48 +13,34 @@ internal static partial class Kernel32 { [LibraryImport(Libraries.Kernel32, EntryPoint = "GetVolumePathNameW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] [return: MarshalAs(UnmanagedType.Bool)] - private static unsafe partial bool _GetVolumePathName(char* lpszFileName, char* lpszVolumePathName, int cchBufferLength); + private static unsafe partial bool GetVolumePathName(char* lpszFileName, char* lpszVolumePathName, int cchBufferLength); - public static unsafe int GetVolumePathName(string fileName, out string? volumePath) + public static unsafe string GetVolumePathName(string fileName) { - fileName = PathInternal.EnsureExtendedPrefixIfNeeded(fileName); //todo: unsure if this is needed for this API + // Ensure we have the prefix + fileName = PathInternal.EnsureExtendedPrefixIfNeeded(fileName); + + // Ensure our output buffer will be long enough (see https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getvolumepathnamew#remarks) + ValueStringBuilder vsb = new(stackalloc char[MAX_PATH + 1]); //note: MAX_PATH is not a hard limit, but would only be exceeded by a long path + PathHelper.GetFullPathName(fileName, ref vsb); + + // Call the actual API fixed (char* lpszFileName = fileName) { - // Allocate output buffer on the stack initially. - const int stackAllocation = 512; - Span volumePathBuffer = stackalloc char[stackAllocation + 1]; // +1 to ensure a \0 at the end, todo: is this necessary - int bufferSize = stackAllocation; - - // Loop until the buffer's big enough. - while (true) + fixed (char* lpszVolumePathName = vsb.RawChars) { - fixed (char* lpszVolumePathName = volumePathBuffer) + // + 1 because \0 is not included in Length from GetFullPathName, but should exist + if (_GetVolumePathName(lpszFileName, lpszVolumePathName, vsb.RawChars.Length + 1)) { - // Try to get the volume name, will succeed if the buffer's big enough. - if (_GetVolumePathName(lpszFileName, lpszVolumePathName, volumePathBuffer.Length - 1)) - { - volumePath = new string(lpszVolumePathName); - return 0; - } - - // Check if the error was that the buffer is not large enough. - int error = Marshal.GetLastWin32Error(); - if (error == Interop.Errors.ERROR_INSUFFICIENT_BUFFER) //todo: check this is the correct error - { - // Create a new buffer and try again. - // todo: use array pool and check for overflow - volumePathBuffer = new char[bufferSize *= 2]; - continue; - } - else - { - // Return our error. - volumePath = null; - return error; - } + return new string(vsb.RawChars[..vsb.RawChars.IndexOf('\0')]); } } } + + // Deal with error + int error = Marshal.GetLastWin32Error(); + Debug.Assert(error != Errors.ERROR_INSUFFICIENT_BUFFER); + throw Win32Marshal.GetExceptionForWin32Error(error, fileName); } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index 5baca30f61621..a15a8fb5cb846 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -283,11 +283,8 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa } // Get the source volume path. Note: we need the real path here for symlinks also, hence openedSourceFullPath. - if (Interop.Kernel32.GetVolumePathName(openedSourceFullPath, out var volumePath) != 0) - { - throw new Exception("K1"); - //return false; - } + // Todo: do we care about not propogating an error from this? + string volumePath = Interop.Kernel32.GetVolumePathName(openedSourceFullPath); // Read the source volume's cluster size. if (!Interop.Kernel32.GetDiskFreeSpace(volumePath!, out int sectorsPerCluster, out int bytesPerCluster, out _, out _)) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/PathHelper.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/PathHelper.Windows.cs index ff479ce957f2c..ac713da49001c 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/PathHelper.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/PathHelper.Windows.cs @@ -67,7 +67,7 @@ internal static string Normalize(ref ValueStringBuilder path) /// /// The path name. MUST be null terminated after the span. /// Builder that will store the result. - private static void GetFullPathName(ReadOnlySpan path, ref ValueStringBuilder builder) + internal static void GetFullPathName(ReadOnlySpan path, ref ValueStringBuilder builder) { // If the string starts with an extended prefix we would need to remove it from the path before we call GetFullPathName as // it doesn't root extended paths correctly. We don't currently resolve extended paths, so we'll just assert here. From 2a8f65036eb5888c328047f0ba2ce4430edca480 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 13 Jul 2023 11:20:55 +1000 Subject: [PATCH 05/17] Fix usage of ValueStringBuilder --- .../Kernel32/Interop.GetVolumeInformationByHandle.cs | 2 +- .../Windows/Kernel32/Interop.GetVolumePathName.cs | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformationByHandle.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformationByHandle.cs index b756e6fe1040b..16cbcdc01092a 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformationByHandle.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformationByHandle.cs @@ -43,7 +43,7 @@ public static unsafe int GetVolumeInformationByHandle( // Return the error. int error = Marshal.GetLastWin32Error(); - Debug.Assert(error != Errors.ERROR_INSUFFICIENT_BUFFER); + Debug.Assert(error != Interop.Errors.ERROR_INSUFFICIENT_BUFFER); return error; } } diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs index e9127cd93075d..59c47f6d7ac0f 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs @@ -21,7 +21,7 @@ public static unsafe string GetVolumePathName(string fileName) fileName = PathInternal.EnsureExtendedPrefixIfNeeded(fileName); // Ensure our output buffer will be long enough (see https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getvolumepathnamew#remarks) - ValueStringBuilder vsb = new(stackalloc char[MAX_PATH + 1]); //note: MAX_PATH is not a hard limit, but would only be exceeded by a long path + ValueStringBuilder vsb = new(stackalloc char[Interop.Kernel32.MAX_PATH]); //note: MAX_PATH is not a hard limit, but would only be exceeded by a long path PathHelper.GetFullPathName(fileName, ref vsb); // Call the actual API @@ -30,16 +30,18 @@ public static unsafe string GetVolumePathName(string fileName) fixed (char* lpszVolumePathName = vsb.RawChars) { // + 1 because \0 is not included in Length from GetFullPathName, but should exist - if (_GetVolumePathName(lpszFileName, lpszVolumePathName, vsb.RawChars.Length + 1)) + Debug.Assert(vsb.Length + 1 <= vsb.Capacity); + if (GetVolumePathName(lpszFileName, lpszVolumePathName, vsb.Length + 1)) { - return new string(vsb.RawChars[..vsb.RawChars.IndexOf('\0')]); + vsb.Length = vsb.RawChars.IndexOf('\0'); + return vsb.ToString(); } } } // Deal with error int error = Marshal.GetLastWin32Error(); - Debug.Assert(error != Errors.ERROR_INSUFFICIENT_BUFFER); + Debug.Assert(error != Interop.Errors.ERROR_INSUFFICIENT_BUFFER); throw Win32Marshal.GetExceptionForWin32Error(error, fileName); } } From ee32befb860e7489e2ba6229490ece464b2bc68b Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 13 Jul 2023 11:34:04 +1000 Subject: [PATCH 06/17] Readonly structs where possible --- .../Kernel32/Interop.DeviceIoControl.cs | 26 ++++++++++--------- .../Kernel32/Interop.FILE_DISPOSITION_INFO.cs | 5 ++-- .../src/System/IO/FileSystem.Windows.cs | 14 +++------- 3 files changed, 21 insertions(+), 24 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs index bb812be3f27d3..e95c9231ac22f 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs @@ -17,29 +17,31 @@ internal static partial class Kernel32 // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_set_sparse internal const int FSCTL_SET_SPARSE = 0x000900c4; - internal struct FILE_SET_SPARSE_BUFFER + internal readonly struct FILE_SET_SPARSE_BUFFER { - internal byte SetSparse; + internal FILE_SET_SPARSE_BUFFER(byte setSparse) => SetSparse = setSparse; + internal readonly byte SetSparse; } // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_get_integrity_information internal const int FSCTL_GET_INTEGRITY_INFORMATION = 0x0009027C; - internal struct FSCTL_GET_INTEGRITY_INFORMATION_BUFFER + internal readonly struct FSCTL_GET_INTEGRITY_INFORMATION_BUFFER { - internal ushort ChecksumAlgorithm; - internal ushort Reserved; - internal uint Flags; - internal uint ChecksumChunkSizeInBytes; - internal uint ClusterSizeInBytes; + internal readonly ushort ChecksumAlgorithm; + internal readonly ushort Reserved; + internal readonly uint Flags; + internal readonly uint ChecksumChunkSizeInBytes; + internal readonly uint ClusterSizeInBytes; } // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_set_integrity_information internal const int FSCTL_SET_INTEGRITY_INFORMATION = 0x0009C280; - internal struct FSCTL_SET_INTEGRITY_INFORMATION_BUFFER + internal readonly struct FSCTL_SET_INTEGRITY_INFORMATION_BUFFER { - internal ushort ChecksumAlgorithm; - internal ushort Reserved; - internal uint Flags; + internal FSCTL_SET_INTEGRITY_INFORMATION_BUFFER(ushort checksumAlgorithm, ushort reserved, uint flags) => (ChecksumAlgorithm, Reserved, Flags) = (checksumAlgorithm, reserved, flags); + internal readonly ushort ChecksumAlgorithm; + internal readonly ushort Reserved; + internal readonly uint Flags; } // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_duplicate_extents_to_file diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_DISPOSITION_INFO.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_DISPOSITION_INFO.cs index 344221b985315..fc14990449f69 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_DISPOSITION_INFO.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_DISPOSITION_INFO.cs @@ -9,9 +9,10 @@ internal static partial class Kernel32 // Use for SetFileInformationByHandle internal const int FileDispositionInfo = 4; - internal struct FILE_DISPOSITION_INFO + internal readonly struct FILE_DISPOSITION_INFO { - internal byte DeleteFile; + internal FILE_DISPOSITION_INFO(byte deleteFile) => DeleteFile = deleteFile; + internal readonly byte DeleteFile; } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index a15a8fb5cb846..4ade63fb624e1 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -344,10 +344,8 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa } if (!madeNew || getIntegrityInfo.ChecksumAlgorithm != 0 || getIntegrityInfo.Flags != 0) { - Interop.Kernel32.FSCTL_SET_INTEGRITY_INFORMATION_BUFFER setIntegrityInfo = default; - setIntegrityInfo.ChecksumAlgorithm = getIntegrityInfo.ChecksumAlgorithm; - setIntegrityInfo.Flags = getIntegrityInfo.Flags; - setIntegrityInfo.Reserved = getIntegrityInfo.Reserved; + Interop.Kernel32.FSCTL_SET_INTEGRITY_INFORMATION_BUFFER setIntegrityInfo = + new(checksumAlgorithm: getIntegrityInfo.ChecksumAlgorithm, reserved: getIntegrityInfo.Reserved, flags: getIntegrityInfo.Flags); if (!Interop.Kernel32.DeviceIoControl( destinationHandle, Interop.Kernel32.FSCTL_SET_INTEGRITY_INFORMATION, @@ -407,8 +405,7 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa // Match source sparseness. if ((sourceFileInformation.dwFileAttributes & Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_SPARSE_FILE) == 0) { - Interop.Kernel32.FILE_SET_SPARSE_BUFFER sparseBuffer; - sparseBuffer.SetSparse = 0; + Interop.Kernel32.FILE_SET_SPARSE_BUFFER sparseBuffer = new(setSparse: 0); Interop.Kernel32.DeviceIoControl(destinationHandle, Interop.Kernel32.FSCTL_SET_SPARSE, &sparseBuffer, (uint)sizeof(Interop.Kernel32.FILE_SET_SPARSE_BUFFER), null, 0, out _, 0); } @@ -438,10 +435,7 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa if (madeNew) { // Mark the file for deletion. - Interop.Kernel32.FILE_DISPOSITION_INFO dispositionInfo = new Interop.Kernel32.FILE_DISPOSITION_INFO() - { - DeleteFile = 1, - }; + Interop.Kernel32.FILE_DISPOSITION_INFO dispositionInfo = new(deleteFile: 1); if (!Interop.Kernel32.SetFileInformationByHandle( destinationHandle!, Interop.Kernel32.FileDispositionInfo, From 33b60904e3be001271d83fca95251aef1048cb65 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 13 Jul 2023 13:15:07 +1000 Subject: [PATCH 07/17] Implement feedback for test project --- .../System.IO.FileSystem/tests/File/Copy.cs | 14 +------------- .../tests/System.IO.FileSystem.Tests.csproj | 1 + 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/File/Copy.cs b/src/libraries/System.IO.FileSystem/tests/File/Copy.cs index 8bf54b0448348..56bce428d6601 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/Copy.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/Copy.cs @@ -483,7 +483,7 @@ public unsafe void WindowsCheckSparseness() using (FileStream file = File.Open(sourceFile, FileMode.Open)) { - DeviceIoControl(file.SafeFileHandle.DangerousGetHandle(), /*FSCTL_SET_SPARSE*/ 0x000900c4, null, 0, null, 0, out _, 0); + Assert.True(Interop.Kernel32.DeviceIoControl(file.SafeFileHandle, Interop.Kernel32.FSCTL_SET_SPARSE, null, 0, null, 0, out _, 0)); } File.WriteAllText(destFile, "def"); @@ -491,18 +491,6 @@ public unsafe void WindowsCheckSparseness() File.Copy(sourceFile, destFile, true); Assert.True((File.GetAttributes(destFile) & FileAttributes.SparseFile) != 0); Assert.Equal("abc", File.ReadAllText(sourceFile)); - - [DllImport("kernel32.dll", EntryPoint = "DeviceIoControl", SetLastError = true)] - [return: MarshalAs(UnmanagedType.Bool)] - static unsafe extern bool DeviceIoControl( - IntPtr hDevice, - uint dwIoControlCode, - void* lpInBuffer, - uint nInBufferSize, - void* lpOutBuffer, - uint nOutBufferSize, - out uint lpBytesReturned, - IntPtr lpOverlapped); } // Todo: add a way to run all these on ReFS, and a test to check we actually cloned the reference, not just the data on ReFS. diff --git a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj index fa31f6979a533..536308dbe0cd2 100644 --- a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj +++ b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj @@ -226,6 +226,7 @@ + From ddb816b452338536f4c313be909213d9cac5f8cc Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 13 Jul 2023 16:06:47 +1000 Subject: [PATCH 08/17] Implement feedback - Make the structs not readonly again - Use the BOOLEAN type, instead of byte directly - And make usage of structs used for native methods consistent --- .../Kernel32/Interop.DeviceIoControl.cs | 26 +++++++++---------- .../Kernel32/Interop.FILE_DISPOSITION_INFO.cs | 5 ++-- .../src/System/IO/FileSystem.Windows.cs | 18 ++++++++----- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs index e95c9231ac22f..dabcbeee1b7a6 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs @@ -17,31 +17,29 @@ internal static partial class Kernel32 // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_set_sparse internal const int FSCTL_SET_SPARSE = 0x000900c4; - internal readonly struct FILE_SET_SPARSE_BUFFER + internal struct FILE_SET_SPARSE_BUFFER { - internal FILE_SET_SPARSE_BUFFER(byte setSparse) => SetSparse = setSparse; - internal readonly byte SetSparse; + internal BOOLEAN SetSparse; } // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_get_integrity_information internal const int FSCTL_GET_INTEGRITY_INFORMATION = 0x0009027C; - internal readonly struct FSCTL_GET_INTEGRITY_INFORMATION_BUFFER + internal struct FSCTL_GET_INTEGRITY_INFORMATION_BUFFER { - internal readonly ushort ChecksumAlgorithm; - internal readonly ushort Reserved; - internal readonly uint Flags; - internal readonly uint ChecksumChunkSizeInBytes; - internal readonly uint ClusterSizeInBytes; + internal ushort ChecksumAlgorithm; + internal ushort Reserved; + internal uint Flags; + internal uint ChecksumChunkSizeInBytes; + internal uint ClusterSizeInBytes; } // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_set_integrity_information internal const int FSCTL_SET_INTEGRITY_INFORMATION = 0x0009C280; - internal readonly struct FSCTL_SET_INTEGRITY_INFORMATION_BUFFER + internal struct FSCTL_SET_INTEGRITY_INFORMATION_BUFFER { - internal FSCTL_SET_INTEGRITY_INFORMATION_BUFFER(ushort checksumAlgorithm, ushort reserved, uint flags) => (ChecksumAlgorithm, Reserved, Flags) = (checksumAlgorithm, reserved, flags); - internal readonly ushort ChecksumAlgorithm; - internal readonly ushort Reserved; - internal readonly uint Flags; + internal ushort ChecksumAlgorithm; + internal ushort Reserved; + internal uint Flags; } // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_duplicate_extents_to_file diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_DISPOSITION_INFO.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_DISPOSITION_INFO.cs index fc14990449f69..5f3e5c678d711 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_DISPOSITION_INFO.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_DISPOSITION_INFO.cs @@ -9,10 +9,9 @@ internal static partial class Kernel32 // Use for SetFileInformationByHandle internal const int FileDispositionInfo = 4; - internal readonly struct FILE_DISPOSITION_INFO + internal struct FILE_DISPOSITION_INFO { - internal FILE_DISPOSITION_INFO(byte deleteFile) => DeleteFile = deleteFile; - internal readonly byte DeleteFile; + internal BOOLEAN DeleteFile; } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index 4ade63fb624e1..4fcd8f7cd1071 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -328,7 +328,7 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa } // Clone file integrity settings from source to destination. Must be done while file is zero size. - Interop.Kernel32.FSCTL_GET_INTEGRITY_INFORMATION_BUFFER getIntegrityInfo = default; + Interop.Kernel32.FSCTL_GET_INTEGRITY_INFORMATION_BUFFER getIntegrityInfo; if (!Interop.Kernel32.DeviceIoControl( sourceHandle, Interop.Kernel32.FSCTL_GET_INTEGRITY_INFORMATION, @@ -344,8 +344,10 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa } if (!madeNew || getIntegrityInfo.ChecksumAlgorithm != 0 || getIntegrityInfo.Flags != 0) { - Interop.Kernel32.FSCTL_SET_INTEGRITY_INFORMATION_BUFFER setIntegrityInfo = - new(checksumAlgorithm: getIntegrityInfo.ChecksumAlgorithm, reserved: getIntegrityInfo.Reserved, flags: getIntegrityInfo.Flags); + Interop.Kernel32.FSCTL_SET_INTEGRITY_INFORMATION_BUFFER setIntegrityInfo; + setIntegrityInfo.ChecksumAlgorithm = getIntegrityInfo.ChecksumAlgorithm; + setIntegrityInfo.Reserved = getIntegrityInfo.Reserved; + setIntegrityInfo.Flags = getIntegrityInfo.Flags; if (!Interop.Kernel32.DeviceIoControl( destinationHandle, Interop.Kernel32.FSCTL_SET_INTEGRITY_INFORMATION, @@ -362,7 +364,7 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa } // Set length of destination to same as source. - Interop.Kernel32.FILE_END_OF_FILE_INFO eofInfo = default; + Interop.Kernel32.FILE_END_OF_FILE_INFO eofInfo; eofInfo.EndOfFile = sourceSize; if (!Interop.Kernel32.SetFileInformationByHandle(destinationHandle, Interop.Kernel32.FileEndOfFileInfo, &eofInfo, (uint)sizeof(Interop.Kernel32.FILE_END_OF_FILE_INFO))) { @@ -371,7 +373,7 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa } // Copy all of the blocks. - Interop.Kernel32.DUPLICATE_EXTENTS_DATA duplicateExtentsData = default; + Interop.Kernel32.DUPLICATE_EXTENTS_DATA duplicateExtentsData; duplicateExtentsData.FileHandle = sourceHandle.DangerousGetHandle(); // ReFS requires that cloned regions reside on a disk cluster boundary. long clusterSizeMask = clusterSize - 1; @@ -405,7 +407,8 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa // Match source sparseness. if ((sourceFileInformation.dwFileAttributes & Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_SPARSE_FILE) == 0) { - Interop.Kernel32.FILE_SET_SPARSE_BUFFER sparseBuffer = new(setSparse: 0); + Interop.Kernel32.FILE_SET_SPARSE_BUFFER sparseBuffer; + sparseBuffer.SetSparse = Interop.BOOLEAN.FALSE; Interop.Kernel32.DeviceIoControl(destinationHandle, Interop.Kernel32.FSCTL_SET_SPARSE, &sparseBuffer, (uint)sizeof(Interop.Kernel32.FILE_SET_SPARSE_BUFFER), null, 0, out _, 0); } @@ -435,7 +438,8 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa if (madeNew) { // Mark the file for deletion. - Interop.Kernel32.FILE_DISPOSITION_INFO dispositionInfo = new(deleteFile: 1); + Interop.Kernel32.FILE_DISPOSITION_INFO dispositionInfo; + dispositionInfo.DeleteFile = Interop.BOOLEAN.TRUE; if (!Interop.Kernel32.SetFileInformationByHandle( destinationHandle!, Interop.Kernel32.FileDispositionInfo, From cf8fbf673f9c0fb25ddc7f968f54d3dd8622fc6c Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 13 Jul 2023 16:14:09 +1000 Subject: [PATCH 09/17] Fix file imports for test project --- .../tests/System.IO.FileSystem.Tests.csproj | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj index 536308dbe0cd2..92f4fdaf4168c 100644 --- a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj +++ b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj @@ -226,7 +226,9 @@ - + + + From 29ef3d3e786c22046b58f3c29e2f3d7962951bf6 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 13 Jul 2023 16:33:51 +1000 Subject: [PATCH 10/17] Move structs in to their own files --- .../Interop.DUPLICATE_EXTENTS_DATA.cs | 19 ++++++++++++++ .../Kernel32/Interop.DeviceIoControl.cs | 25 ------------------- .../Interop.FILE_SET_SPARSE_BUFFER.cs | 14 +++++++++++ ....FSCTL_GET_INTEGRITY_INFORMATION_BUFFER.cs | 18 +++++++++++++ ....FSCTL_SET_INTEGRITY_INFORMATION_BUFFER.cs | 16 ++++++++++++ .../tests/System.IO.FileSystem.Tests.csproj | 1 + .../System.Private.CoreLib.Shared.projitems | 12 +++++++++ 7 files changed, 80 insertions(+), 25 deletions(-) create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DUPLICATE_EXTENTS_DATA.cs create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_SET_SPARSE_BUFFER.cs create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FSCTL_GET_INTEGRITY_INFORMATION_BUFFER.cs create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FSCTL_SET_INTEGRITY_INFORMATION_BUFFER.cs diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DUPLICATE_EXTENTS_DATA.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DUPLICATE_EXTENTS_DATA.cs new file mode 100644 index 0000000000000..0423415b0b5b9 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DUPLICATE_EXTENTS_DATA.cs @@ -0,0 +1,19 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + // https://learn.microsoft.com/windows/win32/api/winioctl/ns-winioctl-duplicate_extents_data + internal struct DUPLICATE_EXTENTS_DATA + { + internal IntPtr FileHandle; + internal long SourceFileOffset; + internal long TargetFileOffset; + internal long ByteCount; + } + } +} diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs index dabcbeee1b7a6..979758bbe7e90 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DeviceIoControl.cs @@ -17,40 +17,15 @@ internal static partial class Kernel32 // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_set_sparse internal const int FSCTL_SET_SPARSE = 0x000900c4; - internal struct FILE_SET_SPARSE_BUFFER - { - internal BOOLEAN SetSparse; - } // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_get_integrity_information internal const int FSCTL_GET_INTEGRITY_INFORMATION = 0x0009027C; - internal struct FSCTL_GET_INTEGRITY_INFORMATION_BUFFER - { - internal ushort ChecksumAlgorithm; - internal ushort Reserved; - internal uint Flags; - internal uint ChecksumChunkSizeInBytes; - internal uint ClusterSizeInBytes; - } // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_set_integrity_information internal const int FSCTL_SET_INTEGRITY_INFORMATION = 0x0009C280; - internal struct FSCTL_SET_INTEGRITY_INFORMATION_BUFFER - { - internal ushort ChecksumAlgorithm; - internal ushort Reserved; - internal uint Flags; - } // https://learn.microsoft.com/windows/win32/api/winioctl/ni-winioctl-fsctl_duplicate_extents_to_file internal const int FSCTL_DUPLICATE_EXTENTS_TO_FILE = 0x00098344; - internal struct DUPLICATE_EXTENTS_DATA - { - internal IntPtr FileHandle; - internal long SourceFileOffset; - internal long TargetFileOffset; - internal long ByteCount; - } [LibraryImport(Libraries.Kernel32, EntryPoint = "DeviceIoControl", SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_SET_SPARSE_BUFFER.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_SET_SPARSE_BUFFER.cs new file mode 100644 index 0000000000000..9b22e7076c9b2 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_SET_SPARSE_BUFFER.cs @@ -0,0 +1,14 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + // https://learn.microsoft.com/windows/win32/api/winioctl/ns-winioctl-file_set_sparse_buffer + internal struct FILE_SET_SPARSE_BUFFER + { + internal BOOLEAN SetSparse; + } + } +} diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FSCTL_GET_INTEGRITY_INFORMATION_BUFFER.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FSCTL_GET_INTEGRITY_INFORMATION_BUFFER.cs new file mode 100644 index 0000000000000..04d89fab7e396 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FSCTL_GET_INTEGRITY_INFORMATION_BUFFER.cs @@ -0,0 +1,18 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + // https://learn.microsoft.com/windows/win32/api/winioctl/ns-winioctl-fsctl_get_integrity_information_buffer + internal struct FSCTL_GET_INTEGRITY_INFORMATION_BUFFER + { + internal ushort ChecksumAlgorithm; + internal ushort Reserved; + internal uint Flags; + internal uint ChecksumChunkSizeInBytes; + internal uint ClusterSizeInBytes; + } + } +} diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FSCTL_SET_INTEGRITY_INFORMATION_BUFFER.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FSCTL_SET_INTEGRITY_INFORMATION_BUFFER.cs new file mode 100644 index 0000000000000..c83b440f95660 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FSCTL_SET_INTEGRITY_INFORMATION_BUFFER.cs @@ -0,0 +1,16 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + // https://learn.microsoft.com/windows/win32/api/winioctl/ns-winioctl-fsctl_set_integrity_information_buffer + internal struct FSCTL_SET_INTEGRITY_INFORMATION_BUFFER + { + internal ushort ChecksumAlgorithm; + internal ushort Reserved; + internal uint Flags; + } + } +} diff --git a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj index 92f4fdaf4168c..97563dff22f3c 100644 --- a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj +++ b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj @@ -229,6 +229,7 @@ + diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 5586584c27dc7..c7c2da65a1fb0 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -1657,6 +1657,18 @@ Common\Interop\Windows\Kernel32\Interop.DeviceIoControl.cs + + Common\Interop\Windows\Kernel32\Interop.FILE_SET_SPARSE_BUFFER.cs + + + Common\Interop\Windows\Kernel32\Interop.FSCTL_GET_INTEGRITY_INFORMATION_BUFFER.cs + + + Common\Interop\Windows\Kernel32\Interop.FSCTL_SET_INTEGRITY_INFORMATION_BUFFER.cs + + + Common\Interop\Windows\Kernel32\Interop.DUPLICATE_EXTENTS_DATA.cs + Common\Interop\Windows\Kernel32\Interop.ExpandEnvironmentStrings.cs From fba6f0fcc7890d75a745380f4e789170d3f51bda Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Thu, 13 Jul 2023 16:34:29 +1000 Subject: [PATCH 11/17] Implement feedback for GetVolumeInformationByHandle --- .../Interop.GetVolumeInformationByHandle.cs | 25 +------------------ .../src/System/IO/FileSystem.Windows.cs | 7 +++--- 2 files changed, 5 insertions(+), 27 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformationByHandle.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformationByHandle.cs index 16cbcdc01092a..104b82bd91e45 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformationByHandle.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumeInformationByHandle.cs @@ -14,7 +14,7 @@ internal static partial class Kernel32 { [LibraryImport(Libraries.Kernel32, EntryPoint = "GetVolumeInformationByHandleW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] [return: MarshalAs(UnmanagedType.Bool)] - private static unsafe partial bool GetVolumeInformationByHandleW( + internal static unsafe partial bool GetVolumeInformationByHandle( SafeFileHandle hFile, char* volumeName, uint volumeNameBufLen, @@ -23,28 +23,5 @@ private static unsafe partial bool GetVolumeInformationByHandleW( uint* fileSystemFlags, char* fileSystemName, uint fileSystemNameBufLen); - - public static unsafe int GetVolumeInformationByHandle( - SafeFileHandle hFile, - uint* volSerialNumber, - uint* maxFileNameLen, - out uint fileSystemFlags, - char* fileSystemName, - uint fileSystemNameBufLen) - { - // Try to get the volume information. - fixed (uint* pFileSystemFlags = &fileSystemFlags) - { - if (GetVolumeInformationByHandleW(hFile, null, 0, volSerialNumber, maxFileNameLen, pFileSystemFlags, fileSystemName, fileSystemNameBufLen)) - { - return 0; - } - } - - // Return the error. - int error = Marshal.GetLastWin32Error(); - Debug.Assert(error != Interop.Errors.ERROR_INSUFFICIENT_BUFFER); - return error; - } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index 4fcd8f7cd1071..1f56265b96b20 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -185,9 +185,9 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa } // Read the source's volume's info. - // todo: do we want GetVolumePathName + GetVolumeInformationByHandle instead? uint sourceSerialNumber; - if (Interop.Kernel32.GetVolumeInformationByHandle(sourceHandle, &sourceSerialNumber, null, out uint sourceVolumeFlags, null, 0) != 0) + uint sourceVolumeFlags; + if (!Interop.Kernel32.GetVolumeInformationByHandle(sourceHandle, null, 0, &sourceSerialNumber, null, &sourceVolumeFlags, null, 0)) { throw new Exception("C"); //return false; @@ -261,7 +261,8 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa // Read the destination file's volume's serial number. uint destSerialNumber; - if (Interop.Kernel32.GetVolumeInformationByHandle(sourceHandle, &destSerialNumber, null, out uint destVolumeFlags, null, 0) != 0) + uint destVolumeFlags; + if (!Interop.Kernel32.GetVolumeInformationByHandle(sourceHandle, null, 0, &destSerialNumber, null, &destVolumeFlags, null, 0)) { throw new Exception("G"); //return false; From 5b1180d3be472026c569736cde3c1bacca5ba950 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Fri, 14 Jul 2023 09:00:48 +1000 Subject: [PATCH 12/17] Improve impl of GetVolumePathName --- .../Windows/Kernel32/Interop.GetVolumePathName.cs | 14 +++++++++----- .../src/System/IO/PathHelper.Windows.cs | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs index 59c47f6d7ac0f..67b83724411f7 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics; using System.IO; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Text; @@ -21,17 +22,20 @@ public static unsafe string GetVolumePathName(string fileName) fileName = PathInternal.EnsureExtendedPrefixIfNeeded(fileName); // Ensure our output buffer will be long enough (see https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getvolumepathnamew#remarks) - ValueStringBuilder vsb = new(stackalloc char[Interop.Kernel32.MAX_PATH]); //note: MAX_PATH is not a hard limit, but would only be exceeded by a long path - PathHelper.GetFullPathName(fileName, ref vsb); + int requiredBufferLength = (int)GetFullPathNameW(ref MemoryMarshal.GetReference(fileName), 0, ref Unsafe.NullRef(), 0); + if (requiredBufferLength == 0) Win32Marshal.GetExceptionForWin32Error(Marshal.GetLastWin32Error(), fileName); + + // Allocate a value string builder + // note: MAX_PATH is not a hard limit, but would only be exceeded by a long path + ValueStringBuilder vsb = new(requiredBufferLength <= Interop.Kernel32.MAX_PATH ? stackalloc char[Interop.Kernel32.MAX_PATH] : default); + vsb.EnsureCapacity(requiredBufferLength); // Call the actual API fixed (char* lpszFileName = fileName) { fixed (char* lpszVolumePathName = vsb.RawChars) { - // + 1 because \0 is not included in Length from GetFullPathName, but should exist - Debug.Assert(vsb.Length + 1 <= vsb.Capacity); - if (GetVolumePathName(lpszFileName, lpszVolumePathName, vsb.Length + 1)) + if (GetVolumePathName(lpszFileName, lpszVolumePathName, requiredBufferLength)) { vsb.Length = vsb.RawChars.IndexOf('\0'); return vsb.ToString(); diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/PathHelper.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/PathHelper.Windows.cs index ac713da49001c..ff479ce957f2c 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/PathHelper.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/PathHelper.Windows.cs @@ -67,7 +67,7 @@ internal static string Normalize(ref ValueStringBuilder path) /// /// The path name. MUST be null terminated after the span. /// Builder that will store the result. - internal static void GetFullPathName(ReadOnlySpan path, ref ValueStringBuilder builder) + private static void GetFullPathName(ReadOnlySpan path, ref ValueStringBuilder builder) { // If the string starts with an extended prefix we would need to remove it from the path before we call GetFullPathName as // it doesn't root extended paths correctly. We don't currently resolve extended paths, so we'll just assert here. From 7f21a4c074ee387b66d1f9cb06b452bd91369c05 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Fri, 14 Jul 2023 09:07:37 +1000 Subject: [PATCH 13/17] Split WindowsCheckSparseness to a new file - Split the WindowsCheckSparseness test to a new file, since it's windows specific, and relies on Windows Interop APIs --- .../tests/File/Copy.Windows.cs | 43 +++++++++++++++++++ .../System.IO.FileSystem/tests/File/Copy.cs | 31 +------------ .../tests/System.IO.FileSystem.Tests.csproj | 6 +-- 3 files changed, 47 insertions(+), 33 deletions(-) create mode 100644 src/libraries/System.IO.FileSystem/tests/File/Copy.Windows.cs diff --git a/src/libraries/System.IO.FileSystem/tests/File/Copy.Windows.cs b/src/libraries/System.IO.FileSystem/tests/File/Copy.Windows.cs new file mode 100644 index 0000000000000..c2b651decec5b --- /dev/null +++ b/src/libraries/System.IO.FileSystem/tests/File/Copy.Windows.cs @@ -0,0 +1,43 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Linq; +using System.Runtime.InteropServices; +using System.Security.Cryptography; +using Xunit; + +namespace System.IO.Tests +{ + partial class File_Copy_Single + { + [Fact] + [PlatformSpecific(TestPlatforms.Windows)] + public unsafe void WindowsCheckSparseness() + { + string sourceFile = GetTestFilePath(); + string destFile = GetTestFilePath(); + + File.WriteAllText(sourceFile, "abc"); + File.WriteAllText(destFile, "def"); + + Assert.True((File.GetAttributes(sourceFile) & FileAttributes.SparseFile) == 0); + File.Copy(sourceFile, destFile, true); + Assert.True((File.GetAttributes(destFile) & FileAttributes.SparseFile) == 0); + Assert.Equal("abc", File.ReadAllText(sourceFile)); + + using (FileStream file = File.Open(sourceFile, FileMode.Open)) + { + Assert.True(Interop.Kernel32.DeviceIoControl(file.SafeFileHandle, Interop.Kernel32.FSCTL_SET_SPARSE, null, 0, null, 0, out _, 0)); + } + File.WriteAllText(destFile, "def"); + + Assert.True((File.GetAttributes(sourceFile) & FileAttributes.SparseFile) != 0); + File.Copy(sourceFile, destFile, true); + Assert.True((File.GetAttributes(destFile) & FileAttributes.SparseFile) != 0); + Assert.Equal("abc", File.ReadAllText(sourceFile)); + } + + // Todo: add a way to run all these on ReFS, and a test to check we actually cloned the reference, not just the data on ReFS. + } +} diff --git a/src/libraries/System.IO.FileSystem/tests/File/Copy.cs b/src/libraries/System.IO.FileSystem/tests/File/Copy.cs index 56bce428d6601..290affa21f4ad 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/Copy.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/Copy.cs @@ -404,7 +404,7 @@ public void DestinationFileIsTruncatedWhenItsLargerThanSourceFile() /// Single tests that shouldn't be duplicated by inheritance. /// [ConditionalClass(typeof(PlatformDetection), nameof(PlatformDetection.IsFileLockingEnabled))] - public sealed class File_Copy_Single : FileSystemTest + public sealed partial class File_Copy_Single : FileSystemTest { [Fact] public void EnsureThrowWhenCopyToNonSharedFile() @@ -465,34 +465,5 @@ void Test(string src, string dst) Test(sourceFile + ".link", destFile + stream2); Test(sourceFile + ".link", destFile + ".link"); } - - [Fact] - [PlatformSpecific(TestPlatforms.Windows)] - public unsafe void WindowsCheckSparseness() - { - string sourceFile = GetTestFilePath(); - string destFile = GetTestFilePath(); - - File.WriteAllText(sourceFile, "abc"); - File.WriteAllText(destFile, "def"); - - Assert.True((File.GetAttributes(sourceFile) & FileAttributes.SparseFile) == 0); - File.Copy(sourceFile, destFile, true); - Assert.True((File.GetAttributes(destFile) & FileAttributes.SparseFile) == 0); - Assert.Equal("abc", File.ReadAllText(sourceFile)); - - using (FileStream file = File.Open(sourceFile, FileMode.Open)) - { - Assert.True(Interop.Kernel32.DeviceIoControl(file.SafeFileHandle, Interop.Kernel32.FSCTL_SET_SPARSE, null, 0, null, 0, out _, 0)); - } - File.WriteAllText(destFile, "def"); - - Assert.True((File.GetAttributes(sourceFile) & FileAttributes.SparseFile) != 0); - File.Copy(sourceFile, destFile, true); - Assert.True((File.GetAttributes(destFile) & FileAttributes.SparseFile) != 0); - Assert.Equal("abc", File.ReadAllText(sourceFile)); - } - - // Todo: add a way to run all these on ReFS, and a test to check we actually cloned the reference, not just the data on ReFS. } } diff --git a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj index 97563dff22f3c..cdf138e16d7aa 100644 --- a/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj +++ b/src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj @@ -90,6 +90,7 @@ + @@ -100,8 +101,10 @@ + + @@ -226,9 +229,6 @@ - - - From a7a74a0e02e42195ebb046a6f9f337c8ba3d54c7 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sun, 16 Jul 2023 21:56:25 +1000 Subject: [PATCH 14/17] Implement improved logic & other changes - Implement new and improved logic, which saves 1 syscall in the ideal case (described in a comment on #88695) - Fix missing DELETE access when creating the destination file, in case it needs to be deleted - Add overload of GetFinalPathNameByHandle, which does creates a string and returns it - Fix a missing throw statement in GetVolumePathName's impl - Changes to make tests compile - Remove unneeded files for tests --- .../Kernel32/Interop.GenericOperations.cs | 1 + .../Interop.GetFinalPathNameByHandle.cs | 30 +++ .../Kernel32/Interop.GetVolumePathName.cs | 4 +- .../tests/TestUtilities/TestUtilities.csproj | 2 + .../tests/System.IO.FileSystem.Tests.csproj | 3 +- .../src/System/IO/FileSystem.Windows.cs | 171 +++++------------- 6 files changed, 78 insertions(+), 133 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GenericOperations.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GenericOperations.cs index f346fa89f8910..82c780384bf14 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GenericOperations.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GenericOperations.cs @@ -7,6 +7,7 @@ internal static partial class Kernel32 { internal static partial class GenericOperations { + internal const int DELETE = 0x00010000; internal const int GENERIC_READ = unchecked((int)0x80000000); internal const int GENERIC_WRITE = 0x40000000; } diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFinalPathNameByHandle.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFinalPathNameByHandle.cs index cdc0fcc3e5319..aa978e149729f 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFinalPathNameByHandle.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFinalPathNameByHandle.cs @@ -2,8 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Runtime.InteropServices; +using System.Text; using Microsoft.Win32.SafeHandles; internal static partial class Interop @@ -11,6 +14,7 @@ internal static partial class Interop internal static partial class Kernel32 { internal const uint FILE_NAME_NORMALIZED = 0x0; + internal const uint VOLUME_NAME_DOS = 0x0; // https://docs.microsoft.com/windows/desktop/api/fileapi/nf-fileapi-getfinalpathnamebyhandlew (kernel32) [LibraryImport(Libraries.Kernel32, EntryPoint = "GetFinalPathNameByHandleW", SetLastError = true)] @@ -19,5 +23,31 @@ internal static unsafe partial uint GetFinalPathNameByHandle( char* lpszFilePath, uint cchFilePath, uint dwFlags); + + internal static unsafe bool GetFinalPathNameByHandle(SafeFileHandle hFile, uint dwFlags, [MaybeNullWhen(false)] out string result) + { + // Default value + result = null; + + // Determine the required buffer size + uint bufferSize = GetFinalPathNameByHandle(hFile, null, 0, dwFlags); + if (bufferSize == 0) return false; + + // Allocate the buffer + ValueStringBuilder vsb = new(bufferSize <= Interop.Kernel32.MAX_PATH ? stackalloc char[Interop.Kernel32.MAX_PATH] : default); + vsb.EnsureCapacity((int)bufferSize); + + // Call the API + fixed (char* lpszFilePath = vsb.RawChars) + { + int length = (int)GetFinalPathNameByHandle(hFile, lpszFilePath, bufferSize, dwFlags); + if (length == 0) return false; + + // Return our string + vsb.Length = length; + result = vsb.ToString(); + return true; + } + } } } diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs index 67b83724411f7..811cd2b3ab421 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs @@ -16,14 +16,14 @@ internal static partial class Kernel32 [return: MarshalAs(UnmanagedType.Bool)] private static unsafe partial bool GetVolumePathName(char* lpszFileName, char* lpszVolumePathName, int cchBufferLength); - public static unsafe string GetVolumePathName(string fileName) + internal static unsafe string GetVolumePathName(string fileName) { // Ensure we have the prefix fileName = PathInternal.EnsureExtendedPrefixIfNeeded(fileName); // Ensure our output buffer will be long enough (see https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getvolumepathnamew#remarks) int requiredBufferLength = (int)GetFullPathNameW(ref MemoryMarshal.GetReference(fileName), 0, ref Unsafe.NullRef(), 0); - if (requiredBufferLength == 0) Win32Marshal.GetExceptionForWin32Error(Marshal.GetLastWin32Error(), fileName); + if (requiredBufferLength == 0) throw Win32Marshal.GetExceptionForWin32Error(Marshal.GetLastWin32Error(), fileName); // Allocate a value string builder // note: MAX_PATH is not a hard limit, but would only be exceeded by a long path diff --git a/src/libraries/Common/tests/TestUtilities/TestUtilities.csproj b/src/libraries/Common/tests/TestUtilities/TestUtilities.csproj index cbb1f4ed69be9..8141ab2c16d57 100644 --- a/src/libraries/Common/tests/TestUtilities/TestUtilities.csproj +++ b/src/libraries/Common/tests/TestUtilities/TestUtilities.csproj @@ -60,6 +60,8 @@ Link="Common\Interop\Windows\Kernel32\Interop.GetCurrentProcess.cs" /> + - @@ -110,6 +109,7 @@ + @@ -229,7 +229,6 @@ - diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index 1f56265b96b20..12872b9e0a606 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -56,101 +56,6 @@ private static unsafe void ThrowExceptionEncryptDecryptFail(string fullPath) throw Win32Marshal.GetExceptionForWin32Error(errorCode, fullPath); } - private static SafeFileHandle? TryOpenTargetNoAlternativeDataStream( - string lpFileName, - int dwDesiredAccess, - FileShare dwShareMode, - FileMode dwCreationDisposition, - int dwFlagsAndAttributes, - ref Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA fileInformation, - out string openedFileName) - { - // Default value for openedFileName. - openedFileName = lpFileName; - - // Check the file isn't an alternative data stream - if (Path.GetFileName(lpFileName.AsSpan()).Contains(':')) - { - return null; - } - - // Try to open it in a way where we can detect symlinks. - SafeFileHandle result = Interop.Kernel32.CreateFile( - lpFileName, - dwDesiredAccess, - dwShareMode, - dwCreationDisposition, - dwFlagsAndAttributes | Interop.Kernel32.FileOperations.FILE_FLAG_OPEN_REPARSE_POINT - ); - - // Check we successfully opened it. - if (result.IsInvalid) - { - return null; - } - - // Read the file attributes. - if (!Interop.Kernel32.GetFileAttributesEx(lpFileName, Interop.Kernel32.GET_FILEEX_INFO_LEVELS.GetFileExInfoStandard, ref fileInformation)) - { - result.Dispose(); - return null; - } - - // Deal with the case of a reparse point. - if ((fileInformation.dwFileAttributes & Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_REPARSE_POINT) != 0) - { - // Get the path of the target. - result.Dispose(); - //try - //{ - /* */string? target = GetFinalLinkTarget(lpFileName, false); - //} - //catch - //{ - // return false; - //} - - // Deal with no target - if (target == null) - { - return null; - } - - // Check the target's file name for alternative data streams. - if (Path.GetFileName(target.AsSpan()).Contains(':')) - { - return null; - } - - // Open the target. - result = Interop.Kernel32.CreateFile( - target, - dwDesiredAccess, - dwShareMode, - dwCreationDisposition, - dwFlagsAndAttributes | Interop.Kernel32.FileOperations.FILE_FLAG_OPEN_REPARSE_POINT - ); - openedFileName = target; - - // Read the file attributes. - if (!Interop.Kernel32.GetFileAttributesEx(lpFileName, Interop.Kernel32.GET_FILEEX_INFO_LEVELS.GetFileExInfoStandard, ref fileInformation)) - { - result.Dispose(); - return null; - } - - // Check we haven't somehow ended up with another symlink due to things changing very quickly. - if ((fileInformation.dwFileAttributes & Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_REPARSE_POINT) != 0) - { - result.Dispose(); - return null; - } - } - - // Return our result. - return result; - } - private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPath, bool overwrite) { // Check the destination file isn't an alternative data stream (unsupported, and crashes on up to some versions of Windows 11). @@ -163,20 +68,12 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa // Open the source file. // We use FILE_FLAGS_NO_BUFFERING since we're not using unaligned writes during cloning and can skip buffering overhead. const int FILE_FLAG_NO_BUFFERING = 0x20000000; - Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA sourceFileInformation = default; - SafeFileHandle? _sourceHandle = TryOpenTargetNoAlternativeDataStream( + using (SafeFileHandle sourceHandle = Interop.Kernel32.CreateFile( sourceFullPath, Interop.Kernel32.GenericOperations.GENERIC_READ, FileShare.Read, FileMode.Open, - FILE_FLAG_NO_BUFFERING, - ref sourceFileInformation, - out string openedSourceFullPath); - if (_sourceHandle == null) - { - return false; - } - using (SafeFileHandle sourceHandle = _sourceHandle) + FILE_FLAG_NO_BUFFERING)) { // Return false if we failed to open the source file. if (sourceHandle.IsInvalid) @@ -200,9 +97,6 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa //return false; } - // Get file size. - long sourceSize = (long)(((ulong)sourceFileInformation.nFileSizeHigh << 32) | sourceFileInformation.nFileSizeLow); - // Helper variables. SafeFileHandle? destinationHandle = null; bool madeNew = false; @@ -214,19 +108,12 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa if (overwrite) { // Try to open the existing file. - destinationHandle = TryOpenTargetNoAlternativeDataStream( + destinationHandle = Interop.Kernel32.CreateFile( destFullPath, Interop.Kernel32.GenericOperations.GENERIC_READ | Interop.Kernel32.GenericOperations.GENERIC_WRITE, FileShare.None, FileMode.Open, - 0, - ref destFileInformation, - out _); - if (destinationHandle == null) - { - // Alternative data stream. - return false; - } + 0); if (destinationHandle.IsInvalid) { // Try opening as a new file. @@ -238,19 +125,12 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa if (destinationHandle == null) { // Try to create the destination file. - destinationHandle = TryOpenTargetNoAlternativeDataStream( + destinationHandle = Interop.Kernel32.CreateFile( destFullPath, - Interop.Kernel32.GenericOperations.GENERIC_READ | Interop.Kernel32.GenericOperations.GENERIC_WRITE, + Interop.Kernel32.GenericOperations.GENERIC_READ | Interop.Kernel32.GenericOperations.GENERIC_WRITE | Interop.Kernel32.GenericOperations.DELETE, FileShare.None, FileMode.CreateNew, - 0, - ref destFileInformation, - out _); - if (destinationHandle == null) - { - // Alternative data stream. - return false; - } + 0); if (destinationHandle.IsInvalid) { // Failure to open. @@ -283,9 +163,34 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa //return false; } - // Get the source volume path. Note: we need the real path here for symlinks also, hence openedSourceFullPath. + // Get the source file path. We need to do this, because we may have opened a symlink - this returns the file we actually have open. + if (!Interop.Kernel32.GetFinalPathNameByHandle(sourceHandle, Interop.Kernel32.FILE_NAME_NORMALIZED | Interop.Kernel32.VOLUME_NAME_DOS, out string? sourceName)) + { + // This may fail as a result of it not being on a drive with a DOS path, we shouldn't throw here + throw new Exception("I2"); + //return false; + } + + // Check we haven't opened an alternate data stream - this may cause a BSOD on Windows versions lower than TBD (todo) if we don't exit before block copy. + if (Path.GetFileName(sourceName.AsSpan()).Contains(':')) + { + return false; + } + + // Also check the destination file + if (!Interop.Kernel32.GetFinalPathNameByHandle(destinationHandle, Interop.Kernel32.FILE_NAME_NORMALIZED | Interop.Kernel32.VOLUME_NAME_DOS, out string? destName)) + { + throw new Exception("I3"); + //return false; + } + if (Path.GetFileName(destName.AsSpan()).Contains(':')) + { + return false; + } + + // Get the source volume path. Note: we need the real path here for symlinks also, hence sourceName. // Todo: do we care about not propogating an error from this? - string volumePath = Interop.Kernel32.GetVolumePathName(openedSourceFullPath); + string volumePath = Interop.Kernel32.GetVolumePathName(sourceName); // Read the source volume's cluster size. if (!Interop.Kernel32.GetDiskFreeSpace(volumePath!, out int sectorsPerCluster, out int bytesPerCluster, out _, out _)) @@ -364,6 +269,14 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa } } + // Read the file attributes. + Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA sourceFileInformation = default; + int error = FillAttributeInfo(sourceHandle, ref sourceFileInformation); + Debug.Assert(error == 0); //todo: is there a good reason for this to fail? if so we should handle it properly, not by a Debug.Assert + + // Get file size. + long sourceSize = (long)(((ulong)sourceFileInformation.nFileSizeHigh << 32) | sourceFileInformation.nFileSizeLow); + // Set length of destination to same as source. Interop.Kernel32.FILE_END_OF_FILE_INFO eofInfo; eofInfo.EndOfFile = sourceSize; From b6ff8b9af912441ac5b5cfbcd0b4424d8924ccb8 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sun, 16 Jul 2023 22:03:26 +1000 Subject: [PATCH 15/17] Fix signature of GetDiskFreeSpace --- .../src/Interop/Windows/Kernel32/Interop.GetDiskFreeSpace.cs | 2 +- .../System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetDiskFreeSpace.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetDiskFreeSpace.cs index 31df630391ea4..05595173dd0a9 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetDiskFreeSpace.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetDiskFreeSpace.cs @@ -9,6 +9,6 @@ internal static partial class Kernel32 { [LibraryImport(Libraries.Kernel32, EntryPoint = "GetDiskFreeSpaceW", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] [return: MarshalAs(UnmanagedType.Bool)] - internal static partial bool GetDiskFreeSpace(string drive, out int sectorsPerCluster, out int bytesPerSector, out int numberOfFreeClusters, out int totalNumberOfClusters); + internal static unsafe partial bool GetDiskFreeSpace(string drive, uint* sectorsPerCluster, uint* bytesPerSector, uint* numberOfFreeClusters, uint* totalNumberOfClusters); } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index 12872b9e0a606..51d4808d14cd4 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -193,7 +193,8 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa string volumePath = Interop.Kernel32.GetVolumePathName(sourceName); // Read the source volume's cluster size. - if (!Interop.Kernel32.GetDiskFreeSpace(volumePath!, out int sectorsPerCluster, out int bytesPerCluster, out _, out _)) + uint sectorsPerCluster, bytesPerCluster; + if (!Interop.Kernel32.GetDiskFreeSpace(volumePath!, §orsPerCluster, &bytesPerCluster, null, null)) { throw new Exception("K"); //return false; From f6df0151895a0c3961ea76024e45b794aded73e2 Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sun, 16 Jul 2023 22:05:16 +1000 Subject: [PATCH 16/17] Remove unnecessary check (which only pessimises a particular case) --- .../src/System/IO/FileSystem.Windows.cs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index 51d4808d14cd4..a0a9b1b428371 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -58,13 +58,6 @@ private static unsafe void ThrowExceptionEncryptDecryptFail(string fullPath) private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPath, bool overwrite) { - // Check the destination file isn't an alternative data stream (unsupported, and crashes on up to some versions of Windows 11). - // Todo: make some of these conditional based on Windows version. - if (Path.GetFileName(destFullPath.AsSpan()).Contains(':')) - { - return false; - } - // Open the source file. // We use FILE_FLAGS_NO_BUFFERING since we're not using unaligned writes during cloning and can skip buffering overhead. const int FILE_FLAG_NO_BUFFERING = 0x20000000; From ce80ab205b9e559ae66de9a56423be893bf330ad Mon Sep 17 00:00:00 2001 From: Hamish Arblaster Date: Sat, 26 Aug 2023 21:43:02 +1000 Subject: [PATCH 17/17] Fix name of `bytesPerSector` local --- .../src/System/IO/FileSystem.Windows.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index a0a9b1b428371..404136198110e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -186,13 +186,13 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa string volumePath = Interop.Kernel32.GetVolumePathName(sourceName); // Read the source volume's cluster size. - uint sectorsPerCluster, bytesPerCluster; - if (!Interop.Kernel32.GetDiskFreeSpace(volumePath!, §orsPerCluster, &bytesPerCluster, null, null)) + uint sectorsPerCluster, bytesPerSector; + if (!Interop.Kernel32.GetDiskFreeSpace(volumePath!, §orsPerCluster, &bytesPerSector, null, null)) { throw new Exception("K"); //return false; } - long clusterSize = sectorsPerCluster * (long)bytesPerCluster; + long clusterSize = sectorsPerCluster * (long)bytesPerSector; // Set file length to 0. if (!madeNew)