Skip to content

Commit a7a74a0

Browse files
committed
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
1 parent 7f21a4c commit a7a74a0

File tree

6 files changed

+78
-133
lines changed

6 files changed

+78
-133
lines changed

src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GenericOperations.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ internal static partial class Kernel32
77
{
88
internal static partial class GenericOperations
99
{
10+
internal const int DELETE = 0x00010000;
1011
internal const int GENERIC_READ = unchecked((int)0x80000000);
1112
internal const int GENERIC_WRITE = 0x40000000;
1213
}

src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetFinalPathNameByHandle.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,19 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System;
5+
using System.Diagnostics;
6+
using System.Diagnostics.CodeAnalysis;
57
using System.IO;
68
using System.Runtime.InteropServices;
9+
using System.Text;
710
using Microsoft.Win32.SafeHandles;
811

912
internal static partial class Interop
1013
{
1114
internal static partial class Kernel32
1215
{
1316
internal const uint FILE_NAME_NORMALIZED = 0x0;
17+
internal const uint VOLUME_NAME_DOS = 0x0;
1418

1519
// https://docs.microsoft.com/windows/desktop/api/fileapi/nf-fileapi-getfinalpathnamebyhandlew (kernel32)
1620
[LibraryImport(Libraries.Kernel32, EntryPoint = "GetFinalPathNameByHandleW", SetLastError = true)]
@@ -19,5 +23,31 @@ internal static unsafe partial uint GetFinalPathNameByHandle(
1923
char* lpszFilePath,
2024
uint cchFilePath,
2125
uint dwFlags);
26+
27+
internal static unsafe bool GetFinalPathNameByHandle(SafeFileHandle hFile, uint dwFlags, [MaybeNullWhen(false)] out string result)
28+
{
29+
// Default value
30+
result = null;
31+
32+
// Determine the required buffer size
33+
uint bufferSize = GetFinalPathNameByHandle(hFile, null, 0, dwFlags);
34+
if (bufferSize == 0) return false;
35+
36+
// Allocate the buffer
37+
ValueStringBuilder vsb = new(bufferSize <= Interop.Kernel32.MAX_PATH ? stackalloc char[Interop.Kernel32.MAX_PATH] : default);
38+
vsb.EnsureCapacity((int)bufferSize);
39+
40+
// Call the API
41+
fixed (char* lpszFilePath = vsb.RawChars)
42+
{
43+
int length = (int)GetFinalPathNameByHandle(hFile, lpszFilePath, bufferSize, dwFlags);
44+
if (length == 0) return false;
45+
46+
// Return our string
47+
vsb.Length = length;
48+
result = vsb.ToString();
49+
return true;
50+
}
51+
}
2252
}
2353
}

src/libraries/Common/src/Interop/Windows/Kernel32/Interop.GetVolumePathName.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,14 @@ internal static partial class Kernel32
1616
[return: MarshalAs(UnmanagedType.Bool)]
1717
private static unsafe partial bool GetVolumePathName(char* lpszFileName, char* lpszVolumePathName, int cchBufferLength);
1818

19-
public static unsafe string GetVolumePathName(string fileName)
19+
internal static unsafe string GetVolumePathName(string fileName)
2020
{
2121
// Ensure we have the prefix
2222
fileName = PathInternal.EnsureExtendedPrefixIfNeeded(fileName);
2323

2424
// Ensure our output buffer will be long enough (see https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getvolumepathnamew#remarks)
2525
int requiredBufferLength = (int)GetFullPathNameW(ref MemoryMarshal.GetReference<char>(fileName), 0, ref Unsafe.NullRef<char>(), 0);
26-
if (requiredBufferLength == 0) Win32Marshal.GetExceptionForWin32Error(Marshal.GetLastWin32Error(), fileName);
26+
if (requiredBufferLength == 0) throw Win32Marshal.GetExceptionForWin32Error(Marshal.GetLastWin32Error(), fileName);
2727

2828
// Allocate a value string builder
2929
// note: MAX_PATH is not a hard limit, but would only be exceeded by a long path

src/libraries/Common/tests/TestUtilities/TestUtilities.csproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@
6060
Link="Common\Interop\Windows\Kernel32\Interop.GetCurrentProcess.cs" />
6161
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetFinalPathNameByHandle.cs"
6262
Link="Common\Interop\Windows\Kernel32\Interop.GetFinalPathNameByHandle.cs" />
63+
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.MAX_PATH.cs"
64+
Link="Common\Interop\Windows\Kernel32\Interop.MAX_PATH.cs" />
6365
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.SECURITY_ATTRIBUTES.cs"
6466
Link="Common\Interop\Windows\Kernel32\Interop.SECURITY_ATTRIBUTES.cs" />
6567
<Compile Include="$(CommonPath)Interop\Windows\NtDll\Interop.RtlGetVersion.cs"

src/libraries/System.IO.FileSystem/tests/System.IO.FileSystem.Tests.csproj

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,6 @@
101101
<Compile Include="RandomAccess\SectorAlignedMemory.Windows.cs" />
102102
<Compile Include="VirtualDriveSymbolicLinks.Windows.cs" />
103103
<Compile Include="$(CommonPath)Interop\Windows\Interop.BOOL.cs" Link="Common\Interop\Windows\Interop.BOOL.cs" />
104-
<Compile Include="$(CommonPath)Interop\Windows\Interop.BOOLEAN.cs" Link="Common\Interop\Windows\Interop.BOOLEAN.cs" />
105104
<Compile Include="$(CommonPath)Interop\Windows\Interop.Libraries.cs" Link="Common\Interop\Windows\Interop.Libraries.cs" />
106105
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.CreateFile.cs" Link="Common\Interop\Windows\Interop.CreateFile.cs" />
107106
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.DeviceIoControl.cs" Link="Common\Interop\Windows\Interop.DeviceIoControl.cs" />
@@ -110,6 +109,7 @@
110109
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.FILE_TIME.cs" Link="Common\Interop\Windows\Interop.FILE_TIME.cs" />
111110
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.BY_HANDLE_FILE_INFORMATION.cs" Link="Common\Interop\Windows\Interop.BY_HANDLE_FILE_INFORMATION.cs" />
112111
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GetFinalPathNameByHandle.cs" Link="Common\Interop\Windows\Interop.GetFinalPathNameByHandle.cs" />
112+
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.MAX_PATH.cs" Link="Common\Interop\Windows\Interop.MAX_PATH.cs" />
113113
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.MemOptions.cs" Link="Common\Interop\Windows\Interop.MemOptions.cs" />
114114
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.SECURITY_ATTRIBUTES.cs" Link="Common\Interop\Windows\Interop.SECURITY_ATTRIBUTES.cs" />
115115
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.VirtualAlloc_Ptr.cs" Link="Common\Interop\Windows\Interop.VirtualAlloc_Ptr.cs" />
@@ -229,7 +229,6 @@
229229
<Compile Include="FileInfo\AppendText.cs" />
230230
<Compile Include="FileInfo\CopyTo.cs" />
231231
<!-- Helpers -->
232-
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.FILE_SET_SPARSE_BUFFER.cs" Link="Interop\Windows\Interop.FILE_SET_SPARSE_BUFFER.cs" />
233232
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.GenericOperations.cs" Link="Interop\Windows\Interop.GenericOperations.cs" />
234233
<Compile Include="$(CommonTestPath)System\Buffers\NativeMemoryManager.cs" Link="Common\System\Buffers\NativeMemoryManager.cs" />
235234
<Compile Include="$(CommonTestPath)System\IO\TempFile.cs" Link="Common\System\IO\TempFile.cs" />

src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs

Lines changed: 42 additions & 129 deletions
Original file line numberDiff line numberDiff line change
@@ -56,101 +56,6 @@ private static unsafe void ThrowExceptionEncryptDecryptFail(string fullPath)
5656
throw Win32Marshal.GetExceptionForWin32Error(errorCode, fullPath);
5757
}
5858

59-
private static SafeFileHandle? TryOpenTargetNoAlternativeDataStream(
60-
string lpFileName,
61-
int dwDesiredAccess,
62-
FileShare dwShareMode,
63-
FileMode dwCreationDisposition,
64-
int dwFlagsAndAttributes,
65-
ref Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA fileInformation,
66-
out string openedFileName)
67-
{
68-
// Default value for openedFileName.
69-
openedFileName = lpFileName;
70-
71-
// Check the file isn't an alternative data stream
72-
if (Path.GetFileName(lpFileName.AsSpan()).Contains(':'))
73-
{
74-
return null;
75-
}
76-
77-
// Try to open it in a way where we can detect symlinks.
78-
SafeFileHandle result = Interop.Kernel32.CreateFile(
79-
lpFileName,
80-
dwDesiredAccess,
81-
dwShareMode,
82-
dwCreationDisposition,
83-
dwFlagsAndAttributes | Interop.Kernel32.FileOperations.FILE_FLAG_OPEN_REPARSE_POINT
84-
);
85-
86-
// Check we successfully opened it.
87-
if (result.IsInvalid)
88-
{
89-
return null;
90-
}
91-
92-
// Read the file attributes.
93-
if (!Interop.Kernel32.GetFileAttributesEx(lpFileName, Interop.Kernel32.GET_FILEEX_INFO_LEVELS.GetFileExInfoStandard, ref fileInformation))
94-
{
95-
result.Dispose();
96-
return null;
97-
}
98-
99-
// Deal with the case of a reparse point.
100-
if ((fileInformation.dwFileAttributes & Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_REPARSE_POINT) != 0)
101-
{
102-
// Get the path of the target.
103-
result.Dispose();
104-
//try
105-
//{
106-
/* */string? target = GetFinalLinkTarget(lpFileName, false);
107-
//}
108-
//catch
109-
//{
110-
// return false;
111-
//}
112-
113-
// Deal with no target
114-
if (target == null)
115-
{
116-
return null;
117-
}
118-
119-
// Check the target's file name for alternative data streams.
120-
if (Path.GetFileName(target.AsSpan()).Contains(':'))
121-
{
122-
return null;
123-
}
124-
125-
// Open the target.
126-
result = Interop.Kernel32.CreateFile(
127-
target,
128-
dwDesiredAccess,
129-
dwShareMode,
130-
dwCreationDisposition,
131-
dwFlagsAndAttributes | Interop.Kernel32.FileOperations.FILE_FLAG_OPEN_REPARSE_POINT
132-
);
133-
openedFileName = target;
134-
135-
// Read the file attributes.
136-
if (!Interop.Kernel32.GetFileAttributesEx(lpFileName, Interop.Kernel32.GET_FILEEX_INFO_LEVELS.GetFileExInfoStandard, ref fileInformation))
137-
{
138-
result.Dispose();
139-
return null;
140-
}
141-
142-
// Check we haven't somehow ended up with another symlink due to things changing very quickly.
143-
if ((fileInformation.dwFileAttributes & Interop.Kernel32.FileAttributes.FILE_ATTRIBUTE_REPARSE_POINT) != 0)
144-
{
145-
result.Dispose();
146-
return null;
147-
}
148-
}
149-
150-
// Return our result.
151-
return result;
152-
}
153-
15459
private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPath, bool overwrite)
15560
{
15661
// 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
16368
// Open the source file.
16469
// We use FILE_FLAGS_NO_BUFFERING since we're not using unaligned writes during cloning and can skip buffering overhead.
16570
const int FILE_FLAG_NO_BUFFERING = 0x20000000;
166-
Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA sourceFileInformation = default;
167-
SafeFileHandle? _sourceHandle = TryOpenTargetNoAlternativeDataStream(
71+
using (SafeFileHandle sourceHandle = Interop.Kernel32.CreateFile(
16872
sourceFullPath,
16973
Interop.Kernel32.GenericOperations.GENERIC_READ,
17074
FileShare.Read,
17175
FileMode.Open,
172-
FILE_FLAG_NO_BUFFERING,
173-
ref sourceFileInformation,
174-
out string openedSourceFullPath);
175-
if (_sourceHandle == null)
176-
{
177-
return false;
178-
}
179-
using (SafeFileHandle sourceHandle = _sourceHandle)
76+
FILE_FLAG_NO_BUFFERING))
18077
{
18178
// Return false if we failed to open the source file.
18279
if (sourceHandle.IsInvalid)
@@ -200,9 +97,6 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa
20097
//return false;
20198
}
20299

203-
// Get file size.
204-
long sourceSize = (long)(((ulong)sourceFileInformation.nFileSizeHigh << 32) | sourceFileInformation.nFileSizeLow);
205-
206100
// Helper variables.
207101
SafeFileHandle? destinationHandle = null;
208102
bool madeNew = false;
@@ -214,19 +108,12 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa
214108
if (overwrite)
215109
{
216110
// Try to open the existing file.
217-
destinationHandle = TryOpenTargetNoAlternativeDataStream(
111+
destinationHandle = Interop.Kernel32.CreateFile(
218112
destFullPath,
219113
Interop.Kernel32.GenericOperations.GENERIC_READ | Interop.Kernel32.GenericOperations.GENERIC_WRITE,
220114
FileShare.None,
221115
FileMode.Open,
222-
0,
223-
ref destFileInformation,
224-
out _);
225-
if (destinationHandle == null)
226-
{
227-
// Alternative data stream.
228-
return false;
229-
}
116+
0);
230117
if (destinationHandle.IsInvalid)
231118
{
232119
// Try opening as a new file.
@@ -238,19 +125,12 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa
238125
if (destinationHandle == null)
239126
{
240127
// Try to create the destination file.
241-
destinationHandle = TryOpenTargetNoAlternativeDataStream(
128+
destinationHandle = Interop.Kernel32.CreateFile(
242129
destFullPath,
243-
Interop.Kernel32.GenericOperations.GENERIC_READ | Interop.Kernel32.GenericOperations.GENERIC_WRITE,
130+
Interop.Kernel32.GenericOperations.GENERIC_READ | Interop.Kernel32.GenericOperations.GENERIC_WRITE | Interop.Kernel32.GenericOperations.DELETE,
244131
FileShare.None,
245132
FileMode.CreateNew,
246-
0,
247-
ref destFileInformation,
248-
out _);
249-
if (destinationHandle == null)
250-
{
251-
// Alternative data stream.
252-
return false;
253-
}
133+
0);
254134
if (destinationHandle.IsInvalid)
255135
{
256136
// Failure to open.
@@ -283,9 +163,34 @@ private static unsafe bool TryCloneFile(string sourceFullPath, string destFullPa
283163
//return false;
284164
}
285165

286-
// Get the source volume path. Note: we need the real path here for symlinks also, hence openedSourceFullPath.
166+
// 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.
167+
if (!Interop.Kernel32.GetFinalPathNameByHandle(sourceHandle, Interop.Kernel32.FILE_NAME_NORMALIZED | Interop.Kernel32.VOLUME_NAME_DOS, out string? sourceName))
168+
{
169+
// This may fail as a result of it not being on a drive with a DOS path, we shouldn't throw here
170+
throw new Exception("I2");
171+
//return false;
172+
}
173+
174+
// 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.
175+
if (Path.GetFileName(sourceName.AsSpan()).Contains(':'))
176+
{
177+
return false;
178+
}
179+
180+
// Also check the destination file
181+
if (!Interop.Kernel32.GetFinalPathNameByHandle(destinationHandle, Interop.Kernel32.FILE_NAME_NORMALIZED | Interop.Kernel32.VOLUME_NAME_DOS, out string? destName))
182+
{
183+
throw new Exception("I3");
184+
//return false;
185+
}
186+
if (Path.GetFileName(destName.AsSpan()).Contains(':'))
187+
{
188+
return false;
189+
}
190+
191+
// Get the source volume path. Note: we need the real path here for symlinks also, hence sourceName.
287192
// Todo: do we care about not propogating an error from this?
288-
string volumePath = Interop.Kernel32.GetVolumePathName(openedSourceFullPath);
193+
string volumePath = Interop.Kernel32.GetVolumePathName(sourceName);
289194

290195
// Read the source volume's cluster size.
291196
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
364269
}
365270
}
366271

272+
// Read the file attributes.
273+
Interop.Kernel32.WIN32_FILE_ATTRIBUTE_DATA sourceFileInformation = default;
274+
int error = FillAttributeInfo(sourceHandle, ref sourceFileInformation);
275+
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
276+
277+
// Get file size.
278+
long sourceSize = (long)(((ulong)sourceFileInformation.nFileSizeHigh << 32) | sourceFileInformation.nFileSizeLow);
279+
367280
// Set length of destination to same as source.
368281
Interop.Kernel32.FILE_END_OF_FILE_INFO eofInfo;
369282
eofInfo.EndOfFile = sourceSize;

0 commit comments

Comments
 (0)