From 2f57bf848b03828ee6c343b55f7ce80df2e5a23e Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 5 Oct 2021 10:29:16 -0700 Subject: [PATCH] Change `Target` from a `CodeProperty` to be an `AliasProperty` that points to `FileSystemInfo.LinkTarget` (#16165) --- .../host/msh/ConsoleHost.cs | 2 +- .../CoreCLR/CorePsPlatform.cs | 5 - .../engine/NativeCommandProcessor.cs | 22 +- .../engine/TypeTable_Types_Ps1Xml.cs | 10 +- .../namespaces/FileSystemProvider.cs | 249 +++++------------- .../FileSystem.Tests.ps1 | 36 +++ 6 files changed, 120 insertions(+), 204 deletions(-) diff --git a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs index b0ae47350..c2206ac7d 100644 --- a/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs +++ b/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHost.cs @@ -432,7 +432,7 @@ namespace Microsoft.PowerShell host.ShouldEndSession = shouldEndSession; } - // Creation of the tread and starting it should be an atomic operation. + // Creation of the thread and starting it should be an atomic operation. // otherwise the code in Run method can get instance of the breakhandlerThread // after it is created and before started and call join on it. This will result // in ThreadStateException. diff --git a/src/System.Management.Automation/CoreCLR/CorePsPlatform.cs b/src/System.Management.Automation/CoreCLR/CorePsPlatform.cs index 884e91e33..ce37ae12d 100644 --- a/src/System.Management.Automation/CoreCLR/CorePsPlatform.cs +++ b/src/System.Management.Automation/CoreCLR/CorePsPlatform.cs @@ -516,11 +516,6 @@ namespace System.Management.Automation return Unix.IsHardLink(fileInfo); } - internal static string NonWindowsInternalGetTarget(string path) - { - return Unix.NativeMethods.FollowSymLink(path); - } - internal static string NonWindowsGetUserFromPid(int path) { return Unix.NativeMethods.GetUserFromPid(path); diff --git a/src/System.Management.Automation/engine/NativeCommandProcessor.cs b/src/System.Management.Automation/engine/NativeCommandProcessor.cs index 7df3dd825..2c5efb356 100644 --- a/src/System.Management.Automation/engine/NativeCommandProcessor.cs +++ b/src/System.Management.Automation/engine/NativeCommandProcessor.cs @@ -1101,15 +1101,21 @@ namespace System.Management.Automation #if UNIX return false; #else - if (!Platform.IsWindowsDesktop) { return false; } - - // SHGetFileInfo() does not understand reparse points and returns 0 ("non exe or error") - // so we are trying to get a real path before. - // It is a workaround for Microsoft Store applications. - string realPath = Microsoft.PowerShell.Commands.InternalSymbolicLinkLinkCodeMethods.WinInternalGetTarget(fileName); - if (realPath is not null) + if (!Platform.IsWindowsDesktop) { - fileName = realPath; + return false; + } + + // The function 'SHGetFileInfo()' does not understand reparse points and returns 0 ("non exe or error") + // for a symbolic link file, so we try to get the immediate link target in that case. + // Why not get the final target (use 'returnFinalTarget: true')? Because: + // 1. When starting a process on Windows, if the 'FileName' is a symbolic link, the immediate link target will automatically be used, + // but the OS does not do recursive resolution when the immediate link target is also a symbolic link. + // 2. Keep the same behavior as before adopting the 'LinkTarget' and 'ResolveLinkTarget' APIs in .NET 6. + string linkTarget = File.ResolveLinkTarget(fileName, returnFinalTarget: false)?.FullName; + if (linkTarget is not null) + { + fileName = linkTarget; } SHFILEINFO shinfo = new SHFILEINFO(); diff --git a/src/System.Management.Automation/engine/TypeTable_Types_Ps1Xml.cs b/src/System.Management.Automation/engine/TypeTable_Types_Ps1Xml.cs index 7113c0953..648bafe15 100644 --- a/src/System.Management.Automation/engine/TypeTable_Types_Ps1Xml.cs +++ b/src/System.Management.Automation/engine/TypeTable_Types_Ps1Xml.cs @@ -680,10 +680,7 @@ namespace System.Management.Automation.Runspaces AddMember( errors, typeName, - new PSCodeProperty( - @"Target", - GetMethodInfo(typeof(Microsoft.PowerShell.Commands.InternalSymbolicLinkLinkCodeMethods), @"GetTarget"), - setterCodeReference: null), + new PSAliasProperty(@"Target", @"LinkTarget", conversionType: null), typeMembers, isOverride: false); @@ -808,10 +805,7 @@ namespace System.Management.Automation.Runspaces AddMember( errors, typeName, - new PSCodeProperty( - @"Target", - GetMethodInfo(typeof(Microsoft.PowerShell.Commands.InternalSymbolicLinkLinkCodeMethods), @"GetTarget"), - setterCodeReference: null), + new PSAliasProperty(@"Target", @"LinkTarget", conversionType: null), typeMembers, isOverride: false); diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index 34c399f7b..7e6d302b2 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -2069,7 +2069,7 @@ namespace Microsoft.PowerShell.Commands { if (InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(fileInfo)) { - return $"{PSStyle.Instance.FileInfo.SymbolicLink}{fileInfo.Name}{PSStyle.Instance.Reset} -> {InternalSymbolicLinkLinkCodeMethods.GetTarget(instance)}"; + return $"{PSStyle.Instance.FileInfo.SymbolicLink}{fileInfo.Name}{PSStyle.Instance.Reset} -> {fileInfo.LinkTarget}"; } else if (fileInfo.Attributes.HasFlag(FileAttributes.Directory)) { @@ -2096,7 +2096,7 @@ namespace Microsoft.PowerShell.Commands { return instance?.BaseObject is FileSystemInfo fileInfo ? InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(fileInfo) - ? $"{fileInfo.Name} -> {InternalSymbolicLinkLinkCodeMethods.GetTarget(instance)}" + ? $"{fileInfo.Name} -> {fileInfo.LinkTarget}" : fileInfo.Name : string.Empty; } @@ -8105,15 +8105,18 @@ namespace Microsoft.PowerShell.Commands /// /// The object of FileInfo or DirectoryInfo type. /// The target of the reparse point. + [Obsolete("This method is now obsolete. Please use the .NET API 'FileSystemInfo.LinkTarget'", error: true)] public static string GetTarget(PSObject instance) { if (instance.BaseObject is FileSystemInfo fileSysInfo) { -#if !UNIX - return WinInternalGetTarget(fileSysInfo.FullName); -#else - return UnixInternalGetTarget(fileSysInfo.FullName); -#endif + if (!fileSysInfo.Exists) + { + throw new ArgumentException( + StringUtil.Format(SessionStateStrings.PathNotFound, fileSysInfo.FullName)); + } + + return fileSysInfo.LinkTarget; } return null; @@ -8136,20 +8139,6 @@ namespace Microsoft.PowerShell.Commands return null; } -#if UNIX - private static string UnixInternalGetTarget(string filePath) - { - string link = Platform.NonWindowsInternalGetTarget(filePath); - - if (string.IsNullOrEmpty(link)) - { - throw new Win32Exception(Marshal.GetLastWin32Error()); - } - - return link; - } -#endif - private static string InternalGetLinkType(FileSystemInfo fileInfo) { if (Platform.IsWindows) @@ -8165,16 +8154,11 @@ namespace Microsoft.PowerShell.Commands [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2001:AvoidCallingProblematicMethods")] private static string WinInternalGetLinkType(string filePath) { - if (!Platform.IsWindows) - { - throw new PlatformNotSupportedException(); - } - // We set accessMode parameter to zero because documentation says: // If this parameter is zero, the application can query certain metadata // such as file, directory, or device attributes without accessing // that file or device, even if GENERIC_READ access would have been denied. - using (SafeFileHandle handle = OpenReparsePoint(filePath, FileDesiredAccess.GenericZero)) + using (SafeFileHandle handle = WinOpenReparsePoint(filePath, FileDesiredAccess.GenericZero)) { int outBufferSize = Marshal.SizeOf(); @@ -8439,176 +8423,77 @@ namespace Microsoft.PowerShell.Commands return succeeded && (handleInfo.NumberOfLinks > 1); } -#if !UNIX - internal static string WinInternalGetTarget(string path) - { - // We set accessMode parameter to zero because documentation says: - // If this parameter is zero, the application can query certain metadata - // such as file, directory, or device attributes without accessing - // that file or device, even if GENERIC_READ access would have been denied. - using (SafeFileHandle handle = OpenReparsePoint(path, FileDesiredAccess.GenericZero)) - { - return WinInternalGetTarget(handle); - } - } - - [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2001:AvoidCallingProblematicMethods")] - private static string WinInternalGetTarget(SafeFileHandle handle) - { - int outBufferSize = Marshal.SizeOf(); - - IntPtr outBuffer = Marshal.AllocHGlobal(outBufferSize); - bool success = false; - - try - { - int bytesReturned; - - // OACR warning 62001 about using DeviceIOControl has been disabled. - // According to MSDN guidance DangerousAddRef() and DangerousRelease() have been used. - handle.DangerousAddRef(ref success); - - bool result = DeviceIoControl( - handle.DangerousGetHandle(), - FSCTL_GET_REPARSE_POINT, - InBuffer: IntPtr.Zero, - nInBufferSize: 0, - outBuffer, - outBufferSize, - out bytesReturned, - lpOverlapped: IntPtr.Zero); - - if (!result) - { - // It's not a reparse point or the file system doesn't support reparse points. - return null; - } - - string targetDir = null; - - REPARSE_DATA_BUFFER_SYMBOLICLINK reparseDataBuffer = Marshal.PtrToStructure(outBuffer); - - switch (reparseDataBuffer.ReparseTag) - { - case IO_REPARSE_TAG_SYMLINK: - targetDir = Encoding.Unicode.GetString(reparseDataBuffer.PathBuffer, reparseDataBuffer.SubstituteNameOffset, reparseDataBuffer.SubstituteNameLength); - break; - - case IO_REPARSE_TAG_MOUNT_POINT: - REPARSE_DATA_BUFFER_MOUNTPOINT reparseMountPointDataBuffer = Marshal.PtrToStructure(outBuffer); - targetDir = Encoding.Unicode.GetString(reparseMountPointDataBuffer.PathBuffer, reparseMountPointDataBuffer.SubstituteNameOffset, reparseMountPointDataBuffer.SubstituteNameLength); - break; - - default: - return null; - } - - if (targetDir != null && targetDir.StartsWith(NonInterpretedPathPrefix, StringComparison.OrdinalIgnoreCase)) - { - targetDir = targetDir.Substring(NonInterpretedPathPrefix.Length); - } - - return targetDir; - } - finally - { - if (success) - { - handle.DangerousRelease(); - } - - Marshal.FreeHGlobal(outBuffer); - } - } -#endif - internal static bool CreateJunction(string path, string target) { - // this is a purely Windows specific feature, no feature flag - // used for that reason + // this is a purely Windows specific feature, no feature flag used for that reason. if (Platform.IsWindows) { return WinCreateJunction(path, target); } - else - { - return false; - } + + return false; } [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2001:AvoidCallingProblematicMethods")] private static bool WinCreateJunction(string path, string target) { - if (!string.IsNullOrEmpty(path)) - { - if (!string.IsNullOrEmpty(target)) - { - using (SafeHandle handle = OpenReparsePoint(path, FileDesiredAccess.GenericWrite)) - { - byte[] mountPointBytes = Encoding.Unicode.GetBytes(NonInterpretedPathPrefix + Path.GetFullPath(target)); - - REPARSE_DATA_BUFFER_MOUNTPOINT mountPoint = new REPARSE_DATA_BUFFER_MOUNTPOINT(); - mountPoint.ReparseTag = IO_REPARSE_TAG_MOUNT_POINT; - mountPoint.ReparseDataLength = (ushort)(mountPointBytes.Length + 12); // Added space for the header and null endo - mountPoint.SubstituteNameOffset = 0; - mountPoint.SubstituteNameLength = (ushort)mountPointBytes.Length; - mountPoint.PrintNameOffset = (ushort)(mountPointBytes.Length + 2); // 2 as unicode null take 2 bytes. - mountPoint.PrintNameLength = 0; - mountPoint.PathBuffer = new byte[0x3FF0]; // Buffer for max size. - Array.Copy(mountPointBytes, mountPoint.PathBuffer, mountPointBytes.Length); - - int nativeBufferSize = Marshal.SizeOf(mountPoint); - IntPtr nativeBuffer = Marshal.AllocHGlobal(nativeBufferSize); - bool success = false; - - try - { - Marshal.StructureToPtr(mountPoint, nativeBuffer, false); - - int bytesReturned = 0; - - // OACR warning 62001 about using DeviceIOControl has been disabled. - // According to MSDN guidance DangerousAddRef() and DangerousRelease() have been used. - handle.DangerousAddRef(ref success); - - bool result = DeviceIoControl(handle.DangerousGetHandle(), FSCTL_SET_REPARSE_POINT, nativeBuffer, mountPointBytes.Length + 20, IntPtr.Zero, 0, out bytesReturned, IntPtr.Zero); - - if (!result) - { - throw new Win32Exception(Marshal.GetLastWin32Error()); - } - - return result; - } - finally - { - Marshal.FreeHGlobal(nativeBuffer); - - if (success) - { - handle.DangerousRelease(); - } - } - } - } - else - { - throw new ArgumentNullException(nameof(target)); - } - } - else + if (string.IsNullOrEmpty(path)) { throw new ArgumentNullException(nameof(path)); } - } - private static SafeFileHandle OpenReparsePoint(string reparsePoint, FileDesiredAccess accessMode) - { -#if UNIX - throw new PlatformNotSupportedException(); -#else - return WinOpenReparsePoint(reparsePoint, accessMode); -#endif + if (string.IsNullOrEmpty(target)) + { + throw new ArgumentNullException(nameof(target)); + } + + using (SafeHandle handle = WinOpenReparsePoint(path, FileDesiredAccess.GenericWrite)) + { + byte[] mountPointBytes = Encoding.Unicode.GetBytes(NonInterpretedPathPrefix + Path.GetFullPath(target)); + + var mountPoint = new REPARSE_DATA_BUFFER_MOUNTPOINT(); + mountPoint.ReparseTag = IO_REPARSE_TAG_MOUNT_POINT; + mountPoint.ReparseDataLength = (ushort)(mountPointBytes.Length + 12); // Added space for the header and null endo + mountPoint.SubstituteNameOffset = 0; + mountPoint.SubstituteNameLength = (ushort)mountPointBytes.Length; + mountPoint.PrintNameOffset = (ushort)(mountPointBytes.Length + 2); // 2 as unicode null take 2 bytes. + mountPoint.PrintNameLength = 0; + mountPoint.PathBuffer = new byte[0x3FF0]; // Buffer for max size. + Array.Copy(mountPointBytes, mountPoint.PathBuffer, mountPointBytes.Length); + + int nativeBufferSize = Marshal.SizeOf(mountPoint); + IntPtr nativeBuffer = Marshal.AllocHGlobal(nativeBufferSize); + bool success = false; + + try + { + Marshal.StructureToPtr(mountPoint, nativeBuffer, false); + + int bytesReturned = 0; + + // OACR warning 62001 about using DeviceIOControl has been disabled. + // According to MSDN guidance DangerousAddRef() and DangerousRelease() have been used. + handle.DangerousAddRef(ref success); + + bool result = DeviceIoControl(handle.DangerousGetHandle(), FSCTL_SET_REPARSE_POINT, nativeBuffer, mountPointBytes.Length + 20, IntPtr.Zero, 0, out bytesReturned, IntPtr.Zero); + + if (!result) + { + throw new Win32Exception(Marshal.GetLastWin32Error()); + } + + return result; + } + finally + { + Marshal.FreeHGlobal(nativeBuffer); + + if (success) + { + handle.DangerousRelease(); + } + } + } } private static SafeFileHandle WinOpenReparsePoint(string reparsePoint, FileDesiredAccess accessMode) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 index e5d150013..10ec92b6b 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 @@ -773,6 +773,42 @@ Describe "Hard link and symbolic link tests" -Tags "CI", "RequireAdminOnWindows" $childB.Name | Should -BeExactly $childA.Name } } + + Context "Show immediate target" { + BeforeAll { + $testDir = Join-Path $TestDrive "immediate-target" + New-Item -ItemType Directory $testDir > $null + + $testFile = Join-Path $testDir "target" + Set-Content -Path $testFile -Value "Hello world" + + Push-Location $testDir + New-Item -ItemType SymbolicLink -Path 'firstLink' -Value 'target' > $null + New-Item -ItemType SymbolicLink -Path 'secondLink' -Value 'firstLink' > $null + Pop-Location + } + + AfterAll { + Remove-Item $testDir -Recurse -Force + } + + It "Property 'Target' should show the immediate target" { + $firstLink = Get-Item (Join-Path $testDir 'firstLink') + $firstLink.Target | Should -BeExactly 'target' + $str = [Microsoft.PowerShell.Commands.FileSystemProvider]::NameString($firstLink) + [System.Management.Automation.Internal.StringDecorated]::new($str).ToString([System.Management.Automation.OutputRendering]::PlainText) | Should -BeExactly 'firstLink -> target' + + $secondLink = Get-Item (Join-Path $testDir 'secondLink') + $secondLink.Target | Should -BeExactly 'firstLink' + $str = [Microsoft.PowerShell.Commands.FileSystemProvider]::NameString($secondLink) + [System.Management.Automation.Internal.StringDecorated]::new($str).ToString([System.Management.Automation.OutputRendering]::PlainText) | Should -BeExactly 'secondLink -> firstLink' + } + + It "Get-Content should be able to resolve the final target" { + Get-Content (Join-Path $testDir 'firstLink') | Should -BeExactly "Hello world" + Get-Content (Join-Path $testDir 'secondLink') | Should -BeExactly "Hello world" + } + } } Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI", "RequireAdminOnWindows" {