From 547a013321f72b577b9c15cba43e76ed66c0b8b0 Mon Sep 17 00:00:00 2001 From: Hugh Bellamy Date: Tue, 16 Apr 2019 19:34:23 +0100 Subject: [PATCH] Fix FolderNameEditor memory leak on cancel (#769) * Cleanup FolderNameEditor * Match FolderBrowserDialog code in FolderNameEditor Fix leaks * PR feedback, more cleanup and fixes * PR feedback --- src/Common/src/Interop/Interop.HRESULT.cs | 11 + src/Common/src/Interop/Interop.Libraries.cs | 14 ++ .../src/Interop/Kernel32/Interop.MAX_PATH.cs | 11 + .../Kernel32/Interop.MAX_UNICODESTRING_LEN.cs | 11 + .../Shell32/Interop.SHBrowseForFolder.cs | 53 +++++ .../Shell32/Interop.SHGetKnownFolderPath.cs | 15 ++ .../Interop.SHGetPathFromIDListLongPath.cs | 79 +++++++ .../Interop.SHGetSpecialFolderLocation.cs | 16 ++ .../Win32/SafeHandles/CoTaskMemSafeHandle.cs | 25 +++ src/Common/src/NativeMethods.cs | 28 +-- src/Common/src/UnsafeNativeMethods.cs | 171 +-------------- .../src/Misc/NativeMethods.cs | 15 +- .../src/Misc/UnsafeNativeMethods.cs | 96 --------- ...System.Windows.Forms.Design.Editors.csproj | 9 + .../Windows/Forms/Design/FolderNameEditor.cs | 196 ++++++++---------- ....Windows.Forms.Design.Editors.Tests.csproj | 2 + .../Forms/Design/FolderNameEditorTests.cs | 95 +++++++++ .../src/System.Windows.Forms.Design.csproj | 2 + .../src/System.Windows.Forms.csproj | 9 + .../src/System/Windows/Forms/DataObject.cs | 2 +- .../Windows/Forms/FileDialogCustomPlace.cs | 53 ++--- .../Windows/Forms/FolderBrowserDialog.cs | 137 +++++------- .../System/Windows/Forms/FxCopSuppression.cs | 1 - .../src/System/Windows/Forms/RichTextBox.cs | 2 +- 24 files changed, 510 insertions(+), 543 deletions(-) create mode 100644 src/Common/src/Interop/Interop.HRESULT.cs create mode 100644 src/Common/src/Interop/Interop.Libraries.cs create mode 100644 src/Common/src/Interop/Kernel32/Interop.MAX_PATH.cs create mode 100644 src/Common/src/Interop/Kernel32/Interop.MAX_UNICODESTRING_LEN.cs create mode 100644 src/Common/src/Interop/Shell32/Interop.SHBrowseForFolder.cs create mode 100644 src/Common/src/Interop/Shell32/Interop.SHGetKnownFolderPath.cs create mode 100644 src/Common/src/Interop/Shell32/Interop.SHGetPathFromIDListLongPath.cs create mode 100644 src/Common/src/Interop/Shell32/Interop.SHGetSpecialFolderLocation.cs create mode 100644 src/Common/src/Microsoft/Win32/SafeHandles/CoTaskMemSafeHandle.cs create mode 100644 src/System.Windows.Forms.Design.Editors/tests/UnitTests/System/Windows/Forms/Design/FolderNameEditorTests.cs diff --git a/src/Common/src/Interop/Interop.HRESULT.cs b/src/Common/src/Interop/Interop.HRESULT.cs new file mode 100644 index 00000000000..addf249e199 --- /dev/null +++ b/src/Common/src/Interop/Interop.HRESULT.cs @@ -0,0 +1,11 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +internal static partial class Interop +{ + public static class HRESULT + { + public const int S_OK = 0; + } +} diff --git a/src/Common/src/Interop/Interop.Libraries.cs b/src/Common/src/Interop/Interop.Libraries.cs new file mode 100644 index 00000000000..bb9384e9a4b --- /dev/null +++ b/src/Common/src/Interop/Interop.Libraries.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. +// See the LICENSE file in the project root for more information. + +internal static partial class Interop +{ + public static partial class Libraries + { + internal const string Kernel32 = "kernel32.dll"; + internal const string Gdi32 = "gdi32.dll"; + internal const string User32 = "user32.dll"; + internal const string Shell32 = "shell32.dll"; + } +} diff --git a/src/Common/src/Interop/Kernel32/Interop.MAX_PATH.cs b/src/Common/src/Interop/Kernel32/Interop.MAX_PATH.cs new file mode 100644 index 00000000000..f7fa32669ba --- /dev/null +++ b/src/Common/src/Interop/Kernel32/Interop.MAX_PATH.cs @@ -0,0 +1,11 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +internal partial class Interop +{ + internal partial class Kernel32 + { + internal const int MAX_PATH = 260; + } +} diff --git a/src/Common/src/Interop/Kernel32/Interop.MAX_UNICODESTRING_LEN.cs b/src/Common/src/Interop/Kernel32/Interop.MAX_UNICODESTRING_LEN.cs new file mode 100644 index 00000000000..f79886c337d --- /dev/null +++ b/src/Common/src/Interop/Kernel32/Interop.MAX_UNICODESTRING_LEN.cs @@ -0,0 +1,11 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +internal partial class Interop +{ + internal partial class Kernel32 + { + internal const int MAX_UNICODESTRING_LEN = short.MaxValue; + } +} diff --git a/src/Common/src/Interop/Shell32/Interop.SHBrowseForFolder.cs b/src/Common/src/Interop/Shell32/Interop.SHBrowseForFolder.cs new file mode 100644 index 00000000000..19af10c12b0 --- /dev/null +++ b/src/Common/src/Interop/Shell32/Interop.SHBrowseForFolder.cs @@ -0,0 +1,53 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.InteropServices; +using Microsoft.Win32.SafeHandles; + +internal static partial class Interop +{ + public static partial class Shell32 + { + [DllImport(Libraries.Shell32)] + public static extern CoTaskMemSafeHandle SHBrowseForFolderW(ref BROWSEINFO lpbi); + + public delegate int BrowseCallbackProc(IntPtr hwnd, int msg, IntPtr lParam, IntPtr lpData); + + public static class BrowseInfoFlags + { + public const uint BIF_RETURNONLYFSDIRS = 0x00000001; + public const uint BIF_DONTGOBELOWDOMAIN = 0x00000002; + public const uint BIF_RETURNFSANCESTORS = 0x00000008; + public const uint BIF_EDITBOX = 0x00000010; + public const uint BIF_NEWDIALOGSTYLE = 0x00000040; + public const uint BIF_NONEWFOLDERBUTTON = 0x00000200; + + public const uint BIF_BROWSEFORCOMPUTER = 0x00001000; + public const uint BIF_BROWSEFORPRINTER = 0x00002000; + public const uint BIF_BROWSEFOREVERYTHING = 0x00004000; + } + + [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Auto)] + public unsafe struct BROWSEINFO + { + public IntPtr hwndOwner; + + public CoTaskMemSafeHandle pidlRoot; + + public char *pszDisplayName; + + public string lpszTitle; + + public uint ulFlags; + + public BrowseCallbackProc lpfn; + + public IntPtr lParam; + + public int iImage; + } + } +} diff --git a/src/Common/src/Interop/Shell32/Interop.SHGetKnownFolderPath.cs b/src/Common/src/Interop/Shell32/Interop.SHGetKnownFolderPath.cs new file mode 100644 index 00000000000..0947eb7fc54 --- /dev/null +++ b/src/Common/src/Interop/Shell32/Interop.SHGetKnownFolderPath.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. +// See the LICENSE file in the project root for more information. + +using System; +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + public static partial class Shell32 + { + [DllImport(Libraries.Shell32, CharSet = CharSet.Unicode, SetLastError = false, BestFitMapping = false, ExactSpelling = true)] + public static extern int SHGetKnownFolderPath(ref Guid rfid, uint dwFlags, IntPtr hToken, out string pszPath); + } +} diff --git a/src/Common/src/Interop/Shell32/Interop.SHGetPathFromIDListLongPath.cs b/src/Common/src/Interop/Shell32/Interop.SHGetPathFromIDListLongPath.cs new file mode 100644 index 00000000000..f3d91119478 --- /dev/null +++ b/src/Common/src/Interop/Shell32/Interop.SHGetPathFromIDListLongPath.cs @@ -0,0 +1,79 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.InteropServices; +using Microsoft.Win32.SafeHandles; + +internal static partial class Interop +{ + public static partial class Shell32 + { + [DllImport(ExternDll.Shell32, EntryPoint = "SHGetPathFromIDListEx", ExactSpelling = true)] + private static extern bool SHGetPathFromIDListEx(IntPtr pidl, IntPtr pszPath, int cchPath, int flags); + + public static bool SHGetPathFromIDListLongPath(IntPtr pidl, out string path) + { + IntPtr pszPath = Marshal.AllocHGlobal((Interop.Kernel32.MAX_PATH + 1) * sizeof(char)); + int length = Interop.Kernel32.MAX_PATH; + try + { + if (!SHGetPathFromIDListLongPath(pidl, ref pszPath, length)) + { + path = null; + return false; + } + + path = Marshal.PtrToStringAuto(pszPath); + return true; + } + finally + { + Marshal.FreeHGlobal(pszPath); + } + } + + public unsafe static bool SHGetPathFromIDListLongPath(IntPtr pidl, ref IntPtr pszPath, int length) + { + // SHGetPathFromIDListEx is basically a helper to get IShellFolder.DisplayNameOf() with some + // extra functionally built in if the various flags are set. + + // SHGetPathFromIDListEx copies into the ouput buffer using StringCchCopyW, which truncates + // when there isn't enough space (with a terminating null) and fails. Long paths can be + // extracted by simply increasing the buffer size whenever the buffer is full. + + // To get the equivalent functionality we could call SHBindToParent on the PIDL to get IShellFolder + // and then invoke IShellFolder.DisplayNameOf directly. This would avoid long path contortions as + // we could directly convert from STRRET, calling CoTaskMemFree manually. (Presuming the type is + // STRRET_WSTR, of course. Otherwise we can just fall back to StrRetToBufW and give up for > MAX_PATH. + // Presumption is that we shouldn't be getting back ANSI results, and if we are they are likely + // some very old component that won't have a > MAX_PATH string.) + + // While we could avoid contortions and avoid intermediate buffers by invoking IShellFolder directly, + // it isn't without cost as we'd be initializing a COM wrapper (RCW) for IShellFolder. Presumably + // this is much less overhead then looping and copying to intermediate buffers before creating a string. + // Additionally, implementing this would allow us to short circuit the one caller (FolderBrowserDialog) + // who doesn't care about the path, but just wants to know that we have an IShellFolder. + while (SHGetPathFromIDListEx(pidl, pszPath, length, 0) == false) + { + if (length >= Kernel32.MAX_UNICODESTRING_LEN + || *(char*)pszPath.ToPointer() == '\0') + { + // Already at the maximum size string, or no data was copied in. Fail. + return false; + } + + // Try giving the API a larger buffer + length = length * 2; + if (length > Kernel32.MAX_UNICODESTRING_LEN) + length = Kernel32.MAX_UNICODESTRING_LEN; + + pszPath = Marshal.ReAllocHGlobal(pszPath, (IntPtr)(length * sizeof(char))); + } + + return true; + } + } +} diff --git a/src/Common/src/Interop/Shell32/Interop.SHGetSpecialFolderLocation.cs b/src/Common/src/Interop/Shell32/Interop.SHGetSpecialFolderLocation.cs new file mode 100644 index 00000000000..f79c1bbe073 --- /dev/null +++ b/src/Common/src/Interop/Shell32/Interop.SHGetSpecialFolderLocation.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. +// See the LICENSE file in the project root for more information. + +using System; +using System.Runtime.InteropServices; +using Microsoft.Win32.SafeHandles; + +internal static partial class Interop +{ + public static partial class Shell32 + { + [DllImport(Libraries.Shell32, EntryPoint = "SHGetSpecialFolderLocation", ExactSpelling = true)] + public static extern int SHGetSpecialFolderLocation(IntPtr hwnd, int csidl, out CoTaskMemSafeHandle ppidl); + } +} diff --git a/src/Common/src/Microsoft/Win32/SafeHandles/CoTaskMemSafeHandle.cs b/src/Common/src/Microsoft/Win32/SafeHandles/CoTaskMemSafeHandle.cs new file mode 100644 index 00000000000..1f7ac1d4019 --- /dev/null +++ b/src/Common/src/Microsoft/Win32/SafeHandles/CoTaskMemSafeHandle.cs @@ -0,0 +1,25 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; +using System.Runtime.InteropServices; + +namespace Microsoft.Win32.SafeHandles +{ + internal sealed class CoTaskMemSafeHandle : SafeHandle + { + internal CoTaskMemSafeHandle() : base(IntPtr.Zero, true) + { + } + + public override bool IsInvalid => IsClosed || handle == IntPtr.Zero; + + protected override bool ReleaseHandle() + { + Marshal.FreeCoTaskMem(handle); + handle = IntPtr.Zero; + return true; + } + } +} diff --git a/src/Common/src/NativeMethods.cs b/src/Common/src/NativeMethods.cs index 01467f31169..d40e9c6a0a6 100644 --- a/src/Common/src/NativeMethods.cs +++ b/src/Common/src/NativeMethods.cs @@ -361,27 +361,7 @@ public const int CONNECT_E_NOCONNECTION = unchecked((int)0x80040200), CONNECT_E_CANNOTCONNECT = unchecked((int)0x80040202), CTRLINFO_EATS_RETURN = 1, - CTRLINFO_EATS_ESCAPE = 2, - CSIDL_DESKTOP = 0x0000, // - CSIDL_INTERNET = 0x0001, // Internet Explorer (icon on desktop) - CSIDL_PROGRAMS = 0x0002, // Start Menu\Programs - CSIDL_PERSONAL = 0x0005, // My Documents - CSIDL_FAVORITES = 0x0006, // \Favorites - CSIDL_STARTUP = 0x0007, // Start Menu\Programs\Startup - CSIDL_RECENT = 0x0008, // \Recent - CSIDL_SENDTO = 0x0009, // \SendTo - CSIDL_STARTMENU = 0x000b, // \Start Menu - CSIDL_DESKTOPDIRECTORY = 0x0010, // \Desktop - CSIDL_TEMPLATES = 0x0015, - CSIDL_APPDATA = 0x001a, // \Application Data - CSIDL_LOCAL_APPDATA = 0x001c, // \Local Settings\Applicaiton Data (non roaming) - CSIDL_INTERNET_CACHE = 0x0020, - CSIDL_COOKIES = 0x0021, - CSIDL_HISTORY = 0x0022, - CSIDL_COMMON_APPDATA = 0x0023, // All Users\Application Data - CSIDL_SYSTEM = 0x0025, // GetSystemDirectory() - CSIDL_PROGRAM_FILES = 0x0026, // C:\Program Files - CSIDL_PROGRAM_FILES_COMMON = 0x002b; // C:\Program Files\Common + CTRLINFO_EATS_ESCAPE = 2; public const int DUPLICATE = 0x06, DISPID_UNKNOWN = (-1), @@ -1007,8 +987,6 @@ public static int MAKELCID(int lgid, int sort) { public const int MEMBERID_NIL = (-1), - MAX_PATH = 260, - MAX_UNICODESTRING_LEN = short.MaxValue, // maximum unicode string length ERROR_INSUFFICIENT_BUFFER = 122, //https://msdn.microsoft.com/en-us/library/windows/desktop/ms681382(v=vs.85).aspx MA_ACTIVATE = 0x0001, MA_ACTIVATEANDEAT = 0x0002, @@ -3270,9 +3248,9 @@ public class OPENFILENAME_I public int nMaxCustFilter = 0; public int nFilterIndex; public IntPtr lpstrFile; - public int nMaxFile = NativeMethods.MAX_PATH; + public int nMaxFile = Interop.Kernel32.MAX_PATH; public IntPtr lpstrFileTitle = IntPtr.Zero; - public int nMaxFileTitle = NativeMethods.MAX_PATH; + public int nMaxFileTitle = Interop.Kernel32.MAX_PATH; public string lpstrInitialDir; public string lpstrTitle; public int Flags; diff --git a/src/Common/src/UnsafeNativeMethods.cs b/src/Common/src/UnsafeNativeMethods.cs index a3ae53791eb..1cabdf0605b 100644 --- a/src/Common/src/UnsafeNativeMethods.cs +++ b/src/Common/src/UnsafeNativeMethods.cs @@ -95,9 +95,6 @@ internal static class UnsafeNativeMethods { [DllImport(ExternDll.User32, ExactSpelling = true, CharSet = CharSet.Auto)] public static extern int GetMessageTime(); - [DllImport(ExternDll.Ole32, SetLastError = true, CharSet = CharSet.Auto, ExactSpelling = true)] - internal extern static void CoTaskMemFree(IntPtr pv); - [DllImport(ExternDll.User32)] public static extern int GetClassName(HandleRef hwnd, StringBuilder lpClassName, int nMaxCount); @@ -197,10 +194,6 @@ internal static bool IsVista public static extern int OleSaveToStream(IPersistStream pPersistStream, IStream pStream); - [DllImport(ExternDll.Ole32)] - - public static extern int CoGetMalloc(int dwReserved, out IMalloc pMalloc); - [DllImport(ExternDll.User32, ExactSpelling = true, CharSet = System.Runtime.InteropServices.CharSet.Auto)] [ResourceExposure(ResourceScope.Process)] public static extern int GetWindowThreadProcessId(HandleRef hWnd, out int lpdwProcessId); @@ -285,7 +278,7 @@ public static int DragQueryFileLongPath(HandleRef hDrop, int iFile, StringBuilde // passing null for buffer will return actual number of charectors in the file name. // So, one extra call would be suffice to avoid while loop in case of long path. int capacity = DragQueryFile(hDrop, iFile, null, 0); - if (capacity < NativeMethods.MAX_UNICODESTRING_LEN) + if (capacity < Interop.Kernel32.MAX_UNICODESTRING_LEN) { lpszFile.EnsureCapacity(capacity); resultValue = DragQueryFile(hDrop, iFile, lpszFile, capacity); @@ -432,17 +425,17 @@ public static IntPtr DuplicateHandle(HandleRef processSource, HandleRef handleSo public static extern int GetModuleFileName(HandleRef hModule, StringBuilder buffer, int length); public static StringBuilder GetModuleFileNameLongPath(HandleRef hModule) { - StringBuilder buffer = new StringBuilder(NativeMethods.MAX_PATH); + StringBuilder buffer = new StringBuilder(Interop.Kernel32.MAX_PATH); int noOfTimes = 1; int length = 0; // Iterating by allocating chunk of memory each time we find the length is not sufficient. // Performance should not be an issue for current MAX_PATH length due to this change. while (((length = GetModuleFileName(hModule, buffer, buffer.Capacity)) == buffer.Capacity) && Marshal.GetLastWin32Error() == NativeMethods.ERROR_INSUFFICIENT_BUFFER - && buffer.Capacity < NativeMethods.MAX_UNICODESTRING_LEN) + && buffer.Capacity < Interop.Kernel32.MAX_UNICODESTRING_LEN) { noOfTimes += 2; // Increasing buffer size by 520 in each iteration. - int capacity = noOfTimes * NativeMethods.MAX_PATH < NativeMethods.MAX_UNICODESTRING_LEN ? noOfTimes * NativeMethods.MAX_PATH : NativeMethods.MAX_UNICODESTRING_LEN; + int capacity = noOfTimes * Interop.Kernel32.MAX_PATH < Interop.Kernel32.MAX_UNICODESTRING_LEN ? noOfTimes * Interop.Kernel32.MAX_PATH : Interop.Kernel32.MAX_UNICODESTRING_LEN; buffer.EnsureCapacity(capacity); } buffer.Length = length; @@ -7397,172 +7390,16 @@ internal static void ThrowExceptionForHR(int errorCode) Marshal.ThrowExceptionForHR(errorCode); } - - public delegate int BrowseCallbackProc( - IntPtr hwnd, - int msg, - IntPtr lParam, - IntPtr lpData); - - [Flags] - public enum BrowseInfos - { - NewDialogStyle = 0x0040, // Use the new dialog layout with the ability to resize - HideNewFolderButton = 0x0200 // Don't display the 'New Folder' button - } - - [StructLayout(LayoutKind.Sequential, CharSet=CharSet.Auto)] - public class BROWSEINFO - { - [SuppressMessage("Microsoft.Reliability", "CA2006:UseSafeHandleToEncapsulateNativeResources")] - public IntPtr hwndOwner; //HWND hwndOwner; // HWND of the owner for the dialog - [SuppressMessage("Microsoft.Reliability", "CA2006:UseSafeHandleToEncapsulateNativeResources")] - public IntPtr pidlRoot; //LPCITEMIDLIST pidlRoot; // Root ITEMIDLIST - - // For interop purposes, send over a buffer of MAX_PATH size. - [SuppressMessage("Microsoft.Reliability", "CA2006:UseSafeHandleToEncapsulateNativeResources")] - public IntPtr pszDisplayName; //LPWSTR pszDisplayName; // Return display name of item selected. - - public string lpszTitle; //LPCWSTR lpszTitle; // text to go in the banner over the tree. - public int ulFlags; //UINT ulFlags; // Flags that control the return stuff - public BrowseCallbackProc lpfn; //BFFCALLBACK lpfn; // Call back pointer - [SuppressMessage("Microsoft.Reliability", "CA2006:UseSafeHandleToEncapsulateNativeResources")] - public IntPtr lParam; //LPARAM lParam; // extra info that's passed back in callbacks - public int iImage; //int iImage; // output var: where to return the Image index. - } - [SuppressMessage("Microsoft.Performance", "CA1812:AvoidUninstantiatedInternalClasses")] internal class Shell32 { - [DllImport(ExternDll.Shell32)] - - public static extern int SHGetSpecialFolderLocation(IntPtr hwnd, int csidl, ref IntPtr ppidl); - //SHSTDAPI SHGetSpecialFolderLocation(HWND hwnd, int csidl, LPITEMIDLIST *ppidl); - - [DllImport(ExternDll.Shell32, CharSet = CharSet.Auto)] - - private static extern bool SHGetPathFromIDListEx(IntPtr pidl, IntPtr pszPath, int cchPath, int flags); - //SHSTDAPI_(BOOL) SHGetPathFromIDListW(LPCITEMIDLIST pidl, LPWSTR pszPath); - - public static bool SHGetPathFromIDListLongPath(IntPtr pidl, ref IntPtr pszPath) - { - int noOfTimes = 1; - int length = NativeMethods.MAX_PATH; - bool result = false; - - // SHGetPathFromIDListEx returns false in case of insufficient buffer. - // This method does not distinguish between insufficient memory and an error. Until we get a proper solution, - // this logic would work. In the worst case scenario, loop exits when length reaches unicode string length. - while ((result = SHGetPathFromIDListEx(pidl, pszPath, length, 0)) == false - && length < NativeMethods.MAX_UNICODESTRING_LEN) - { - string path = Marshal.PtrToStringAuto(pszPath); - - if (path.Length != 0 && path.Length < length) - break; - - noOfTimes += 2; //520 chars capacity increase in each iteration. - length = noOfTimes * length >= NativeMethods.MAX_UNICODESTRING_LEN - ? NativeMethods.MAX_UNICODESTRING_LEN : noOfTimes * length; - pszPath = Marshal.ReAllocHGlobal(pszPath, (IntPtr)((length + 1) * sizeof(char))); - } - - return result; - } - - [DllImport(ExternDll.Shell32, CharSet=CharSet.Auto)] - - public static extern IntPtr SHBrowseForFolder([In] BROWSEINFO lpbi); - //SHSTDAPI_(LPITEMIDLIST) SHBrowseForFolderW(LPBROWSEINFOW lpbi); - - [DllImport(ExternDll.Shell32)] - - public static extern int SHGetMalloc([Out, MarshalAs(UnmanagedType.LPArray)] UnsafeNativeMethods.IMalloc[] ppMalloc); - //SHSTDAPI SHGetMalloc(LPMALLOC * ppMalloc); - - [SuppressMessage("Microsoft.Interoperability", "CA1400:PInvokeEntryPointsShouldExist")] [DllImport(ExternDll.Shell32, PreserveSig = true)] - - private static extern int SHGetKnownFolderPath(ref Guid rfid, uint dwFlags, IntPtr hToken, out IntPtr pszPath); - - public static int SHGetFolderPathEx(ref Guid rfid, uint dwFlags, IntPtr hToken, StringBuilder pszPath) - { - if (IsVista) - { - IntPtr path = IntPtr.Zero; - int result = -1; - if ((result = SHGetKnownFolderPath(ref rfid, dwFlags, hToken, out path)) == NativeMethods.S_OK) - { - pszPath.Append(Marshal.PtrToStringAuto(path)); - CoTaskMemFree(path); - } - return result; - } - throw new NotSupportedException(); - } - - [DllImport(ExternDll.Shell32, PreserveSig = true)] - public static extern int SHCreateShellItem(IntPtr pidlParent, IntPtr psfParent, IntPtr pidl, out FileDialogNative.IShellItem ppsi); [DllImport(ExternDll.Shell32, PreserveSig = true)] - public static extern int SHILCreateFromPath([MarshalAs(UnmanagedType.LPWStr)]string pszPath, out IntPtr ppIdl, ref uint rgflnOut); } - [ - ComImport(), - Guid("00000002-0000-0000-c000-000000000046"), - System.Runtime.InteropServices.InterfaceTypeAttribute(System.Runtime.InteropServices.ComInterfaceType.InterfaceIsIUnknown)] - public interface IMalloc - { - [PreserveSig] - IntPtr Alloc(int cb); - - [PreserveSig] - IntPtr Realloc(IntPtr pv, int cb); - - [PreserveSig] - void Free(IntPtr pv); - - [PreserveSig] - int GetSize(IntPtr pv); - - [PreserveSig] - int DidAlloc(IntPtr pv); - - [PreserveSig] - void HeapMinimize(); - } - - [ - ComImport, - Guid("00000126-0000-0000-C000-000000000046"), - InterfaceTypeAttribute(ComInterfaceType.InterfaceIsIUnknown) - ] - public interface IRunnableObject - { - void GetRunningClass(out Guid guid); - - [PreserveSig] - int Run(IntPtr lpBindContext); - bool IsRunning(); - void LockRunning(bool fLock, bool fLastUnlockCloses); - void SetContainedObject(bool fContained); - } - - [ComVisible(true), ComImport(), Guid("B722BCC7-4E68-101B-A2BC-00AA00404770"), InterfaceTypeAttribute(ComInterfaceType.InterfaceIsIUnknown)] - public interface IOleDocumentSite - { - - [return: MarshalAs(UnmanagedType.I4)] - [PreserveSig] - int ActivateMe( - [In, MarshalAs(UnmanagedType.Interface)] - IOleDocumentView pViewToActivate); - - } - [ComVisible(true), Guid("B722BCC6-4E68-101B-A2BC-00AA00404770"), InterfaceTypeAttribute(ComInterfaceType.InterfaceIsIUnknown)] public interface IOleDocumentView { diff --git a/src/System.Windows.Forms.Design.Editors/src/Misc/NativeMethods.cs b/src/System.Windows.Forms.Design.Editors/src/Misc/NativeMethods.cs index 8546f505ae5..e7d54008a61 100644 --- a/src/System.Windows.Forms.Design.Editors/src/Misc/NativeMethods.cs +++ b/src/System.Windows.Forms.Design.Editors/src/Misc/NativeMethods.cs @@ -764,19 +764,6 @@ void OnShowWindow( void RequestNewObjectLayout(); } - [ComVisible(true)] - [ComImport] - [Guid("B722BCC7-4E68-101B-A2BC-00AA00404770")] - [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] - public interface IOleDocumentSite - { - [return: MarshalAs(UnmanagedType.I4)] - [PreserveSig] - int ActivateMe( - [In] [MarshalAs(UnmanagedType.Interface)] - IOleDocumentView pViewToActivate); - } - [ComVisible(true)] [ComImport] [Guid("B722BCC6-4E68-101B-A2BC-00AA00404770")] @@ -4567,5 +4554,7 @@ public class NONCLIENTMETRICS //SendMessage(IntPtr, int, bool, IntPtr), SendMessage(IntPtr, int, IntPtr, ListViewCompareCallback), //SendMessageW, SendMessageA, ValidateRect(IntPtr, ref RECT), ValidateRgn(IntPtr, IntPtr) //COMRECT.FromXYWH, RECT.FromXYWH + + public const int MAX_PATH = 260; } } diff --git a/src/System.Windows.Forms.Design.Editors/src/Misc/UnsafeNativeMethods.cs b/src/System.Windows.Forms.Design.Editors/src/Misc/UnsafeNativeMethods.cs index 35be9d218dc..6a460e95af4 100644 --- a/src/System.Windows.Forms.Design.Editors/src/Misc/UnsafeNativeMethods.cs +++ b/src/System.Windows.Forms.Design.Editors/src/Misc/UnsafeNativeMethods.cs @@ -158,102 +158,6 @@ public static IntPtr SetWindowLong(HandleRef hWnd, int nIndex, HandleRef dwNewLo [DllImport(ExternDll.Ole32, PreserveSig = false)] public static extern IStorage StgCreateDocfileOnILockBytes(ILockBytes iLockBytes, int grfMode, int reserved); - [Flags] - public enum BrowseInfos - { - // Browsing for directory. - ReturnOnlyFSDirs = 0x0001, // For finding a folder to start document searching - DontGoBelowDomain = 0x0002, // For starting the Find Computer - StatusText = 0x0004, // Top of the dialog has 2 lines of text for BROWSEINFO.lpszTitle and one line if - - // this flag is set. Passing the message BFFM_SETSTATUSTEXTA to the hwnd can set the - // rest of the text. This is not used with USENEWUI and BROWSEINFO.lpszTitle gets - // all three lines of text. - ReturnFSAncestors = 0x0008, - EditBox = 0x0010, // Add an editbox to the dialog - Validate = 0x0020, // insist on valid result (or CANCEL) - - NewDialogStyle = 0x0040, // Use the new dialog layout with the ability to resize - // Caller needs to call OleInitialize() before using this API - - UseNewUI = NewDialogStyle | EditBox, - - AllowUrls = 0x0080, // Allow URLs to be displayed or entered. (Requires USENEWUI) - - BrowseForComputer = 0x1000, // Browsing for Computers. - BrowseForPrinter = 0x2000, // Browsing for Printers - BrowseForEverything = 0x4000, // Browsing for Everything - ShowShares = 0x8000 // sharable resources displayed (remote shares, requires USENEWUI) - } - - [SuppressMessage("Microsoft.Design", "CA1049:TypesThatOwnNativeResourcesShouldBeDisposable")] - [SuppressMessage("Microsoft.Reliability", "CA2006:UseSafeHandleToEncapsulateNativeResources")] - [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Auto)] - public class BROWSEINFO - { - public IntPtr hwndOwner; //HWND hwndOwner; // HWND of the owner for the dialog - - public int - iImage; //int iImage; // output var: where to return the Image index. - - public IntPtr - lParam; //LPARAM lParam; // extra info that's passed back in callbacks - - public IntPtr lpfn; //BFFCALLBACK lpfn; // Call back pointer - - public string lpszTitle; //LPCWSTR lpszTitle; // text to go in the banner over the tree. - public IntPtr pidlRoot; //LPCITEMIDLIST pidlRoot; // Root ITEMIDLIST - - // For interop purposes, send over a buffer of MAX_PATH size. - public IntPtr pszDisplayName; //LPWSTR pszDisplayName; // Return display name of item selected. - public int ulFlags; //UINT ulFlags; // Flags that control the return stuff - } - - public class Shell32 - { - [DllImport(ExternDll.Shell32)] - public static extern int SHGetSpecialFolderLocation(IntPtr hwnd, int csidl, ref IntPtr ppidl); - //SHSTDAPI SHGetSpecialFolderLocation(HWND hwnd, int csidl, LPITEMIDLIST *ppidl); - - [DllImport(ExternDll.Shell32, CharSet = CharSet.Auto)] - public static extern bool SHGetPathFromIDList(IntPtr pidl, IntPtr pszPath); - //SHSTDAPI_(BOOL) SHGetPathFromIDListW(LPCITEMIDLIST pidl, LPWSTR pszPath); - - [DllImport(ExternDll.Shell32, CharSet = CharSet.Auto)] - public static extern IntPtr SHBrowseForFolder([In] BROWSEINFO lpbi); - //SHSTDAPI_(LPITEMIDLIST) SHBrowseForFolderW(LPBROWSEINFOW lpbi); - - [DllImport(ExternDll.Shell32)] - public static extern int SHGetMalloc([Out] [MarshalAs(UnmanagedType.LPArray)] - IMalloc[] ppMalloc); - - //SHSTDAPI SHGetMalloc(LPMALLOC * ppMalloc); - } - - [ComImport] - [Guid("00000002-0000-0000-c000-000000000046")] - [InterfaceType(ComInterfaceType.InterfaceIsIUnknown)] - public interface IMalloc - { - [PreserveSig] - IntPtr Alloc(int cb); - - [PreserveSig] - IntPtr Realloc(IntPtr pv, int cb); - - [PreserveSig] - void Free(IntPtr pv); - - [PreserveSig] - int GetSize(IntPtr pv); - - [PreserveSig] - int DidAlloc(IntPtr pv); - - [PreserveSig] - void HeapMinimize(); - } - [SuppressMessage("Microsoft.Design", "CA1049:TypesThatOwnNativeResourcesShouldBeDisposable")] [StructLayout(LayoutKind.Sequential)] public struct PAINTSTRUCT diff --git a/src/System.Windows.Forms.Design.Editors/src/System.Windows.Forms.Design.Editors.csproj b/src/System.Windows.Forms.Design.Editors/src/System.Windows.Forms.Design.Editors.csproj index b88f35ba5d7..7a08576593e 100644 --- a/src/System.Windows.Forms.Design.Editors/src/System.Windows.Forms.Design.Editors.csproj +++ b/src/System.Windows.Forms.Design.Editors/src/System.Windows.Forms.Design.Editors.csproj @@ -6,6 +6,7 @@ true true true + true @@ -29,6 +30,14 @@ + + + + + + + + diff --git a/src/System.Windows.Forms.Design.Editors/src/System/Windows/Forms/Design/FolderNameEditor.cs b/src/System.Windows.Forms.Design.Editors/src/System/Windows/Forms/Design/FolderNameEditor.cs index 536ddeef48e..0604d42fe56 100644 --- a/src/System.Windows.Forms.Design.Editors/src/System/Windows/Forms/Design/FolderNameEditor.cs +++ b/src/System.Windows.Forms.Design.Editors/src/System/Windows/Forms/Design/FolderNameEditor.cs @@ -2,51 +2,49 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Buffers; using System.ComponentModel; using System.Diagnostics.CodeAnalysis; using System.Drawing.Design; using System.Runtime.InteropServices; +using Microsoft.Win32.SafeHandles; namespace System.Windows.Forms.Design { /// > - /// Provides an editor - /// for choosing a folder from the filesystem. + /// Provides an editor for choosing a folder from the filesystem. /// [CLSCompliant(false)] public class FolderNameEditor : UITypeEditor { - private FolderBrowser folderBrowser; + private FolderBrowser _folderBrowser; - [SuppressMessage("Microsoft.Security", "CA2123:OverrideLinkDemandsShouldBeIdenticalToBase")] public override object EditValue(ITypeDescriptorContext context, IServiceProvider provider, object value) { - if (folderBrowser == null) + if (_folderBrowser == null) { - folderBrowser = new FolderBrowser(); - InitializeDialog(folderBrowser); + _folderBrowser = new FolderBrowser(); + InitializeDialog(_folderBrowser); } - if (folderBrowser.ShowDialog() != DialogResult.OK) return value; + if (_folderBrowser.ShowDialog() == DialogResult.OK) + { + return _folderBrowser.DirectoryPath; + } - return folderBrowser.DirectoryPath; + return value; } /// - /// Retrieves the editing style of the Edit method. If the method - /// is not supported, this will return None. - /// - [SuppressMessage("Microsoft.Security", "CA2123:OverrideLinkDemandsShouldBeIdenticalToBase")] - // everything in this assembly is full trust. + /// Retrieves the editing style of the Edit method. public override UITypeEditorEditStyle GetEditStyle(ITypeDescriptorContext context) { return UITypeEditorEditStyle.Modal; } /// - /// Initializes the folder browser dialog when it is created. This gives you - /// an opportunity to configure the dialog as you please. The default - /// implementation provides a generic folder browser. + /// Initializes the folder browser dialog when it is created. This gives you an opportunity + /// to configure the dialog as you please. The default implementation provides a generic folder browser. /// protected virtual void InitializeDialog(FolderBrowser folderBrowser) { @@ -54,128 +52,100 @@ protected virtual void InitializeDialog(FolderBrowser folderBrowser) protected sealed class FolderBrowser : Component { - private static readonly int MAX_PATH = 260; - // Description text to show. - private string descriptionText = string.Empty; - - // Folder picked by the user. - private readonly UnsafeNativeMethods.BrowseInfos privateOptions = - UnsafeNativeMethods.BrowseInfos.NewDialogStyle; + private string _descriptionText = string.Empty; /// - /// The styles the folder browser will use when browsing - /// folders. This should be a combination of flags from - /// the FolderBrowserStyles enum. + /// The styles the folder browser will use when browsing + /// folders. This should be a combination of flags from + /// the FolderBrowserStyles enum. /// public FolderBrowserStyles Style { get; set; } = FolderBrowserStyles.RestrictToFilesystem; /// - /// Gets the directory path of the folder the user picked. + /// Gets the directory path of the folder the user picked. /// public string DirectoryPath { get; private set; } = string.Empty; /// - /// Gets/sets the start location of the root node. + /// Gets/sets the start location of the root node. /// public FolderBrowserFolder StartLocation { get; set; } = FolderBrowserFolder.Desktop; /// > - /// Gets or sets a description to show above the folders. Here you can provide instructions for - /// selecting a folder. + /// Gets or sets a description to show above the folders. Here you can provide instructions for + /// selecting a folder. /// public string Description { - get => descriptionText; - set => descriptionText = value == null ? string.Empty : value; - } - - /// - /// Helper function that returns the IMalloc interface used by the shell. - /// - private static UnsafeNativeMethods.IMalloc GetSHMalloc() - { - UnsafeNativeMethods.IMalloc[] malloc = new UnsafeNativeMethods.IMalloc[1]; - - UnsafeNativeMethods.Shell32.SHGetMalloc(malloc); - - return malloc[0]; + get => _descriptionText; + set => _descriptionText = value ?? string.Empty; } /// - /// Shows the folder browser dialog. + /// Shows the folder browser dialog. /// - public DialogResult ShowDialog() - { - return ShowDialog(null); - } + public DialogResult ShowDialog() => ShowDialog(null); /// - /// Shows the folder browser dialog with the specified owner. + /// Shows the folder browser dialog with the specified owner. /// - public DialogResult ShowDialog(IWin32Window owner) + public unsafe DialogResult ShowDialog(IWin32Window owner) { - IntPtr pidlRoot = IntPtr.Zero; - // Get/find an owner HWND for this dialog - IntPtr hWndOwner; - - if (owner != null) - hWndOwner = owner.Handle; - else - hWndOwner = UnsafeNativeMethods.GetActiveWindow(); + IntPtr hWndOwner = owner == null ? owner.Handle : UnsafeNativeMethods.GetActiveWindow(); // Get the IDL for the specific startLocation - UnsafeNativeMethods.Shell32.SHGetSpecialFolderLocation(hWndOwner, (int)StartLocation, ref pidlRoot); - - if (pidlRoot == IntPtr.Zero) return DialogResult.Cancel; - - int mergedOptions = (int)Style | (int)privateOptions; - - if ((mergedOptions & (int)UnsafeNativeMethods.BrowseInfos.NewDialogStyle) != 0) - Application.OleRequired(); - - IntPtr pidlRet = IntPtr.Zero; - - try + CoTaskMemSafeHandle listHandle; + Interop.Shell32.SHGetSpecialFolderLocation(hWndOwner, (int)StartLocation, out listHandle); + if (listHandle.IsInvalid) { - // Construct a BROWSEINFO - UnsafeNativeMethods.BROWSEINFO bi = new UnsafeNativeMethods.BROWSEINFO(); - - IntPtr buffer = Marshal.AllocHGlobal(MAX_PATH); - - bi.pidlRoot = pidlRoot; - bi.hwndOwner = hWndOwner; - bi.pszDisplayName = buffer; - bi.lpszTitle = descriptionText; - bi.ulFlags = mergedOptions; - bi.lpfn = IntPtr.Zero; - bi.lParam = IntPtr.Zero; - bi.iImage = 0; - - // And show the dialog - pidlRet = UnsafeNativeMethods.Shell32.SHBrowseForFolder(bi); - - if (pidlRet == IntPtr.Zero) return DialogResult.Cancel; - - // Then retrieve the path from the IDList - UnsafeNativeMethods.Shell32.SHGetPathFromIDList(pidlRet, buffer); - - // Convert to a string - DirectoryPath = Marshal.PtrToStringAuto(buffer); - - // Then free all the stuff we've allocated or the SH API gave us - Marshal.FreeHGlobal(buffer); + return DialogResult.Cancel; } - finally - { - UnsafeNativeMethods.IMalloc malloc = GetSHMalloc(); - malloc.Free(pidlRoot); - if (pidlRet != IntPtr.Zero) malloc.Free(pidlRet); + using (listHandle) + { + uint mergedOptions = (uint)Style | Interop.Shell32.BrowseInfoFlags.BIF_NEWDIALOGSTYLE; + if ((mergedOptions & (int)Interop.Shell32.BrowseInfoFlags.BIF_NEWDIALOGSTYLE) != 0) + { + Application.OleRequired(); + } + + char[] displayName = ArrayPool.Shared.Rent(Interop.Kernel32.MAX_PATH + 1); + try + { + fixed (char *pDisplayName = displayName) + { + var bi = new Interop.Shell32.BROWSEINFO(); + bi.pidlRoot = listHandle; + bi.hwndOwner = hWndOwner; + bi.pszDisplayName = pDisplayName; + bi.lpszTitle = _descriptionText; + bi.ulFlags = mergedOptions; + bi.lpfn = null; + bi.lParam = IntPtr.Zero; + bi.iImage = 0; + + // Show the dialog. + using (CoTaskMemSafeHandle browseHandle = Interop.Shell32.SHBrowseForFolderW(ref bi)) + { + if (browseHandle.IsInvalid) + { + return DialogResult.Cancel; + } + + // Retrieve the path from the IDList. + Interop.Shell32.SHGetPathFromIDListLongPath(browseHandle.DangerousGetHandle(), out string selectedPath); + DirectoryPath = selectedPath; + return DialogResult.OK; + } + } + } + finally + { + ArrayPool.Shared.Return(displayName); + } } - - return DialogResult.OK; } } @@ -209,19 +179,19 @@ protected enum FolderBrowserFolder [Flags] protected enum FolderBrowserStyles { - BrowseForComputer = UnsafeNativeMethods.BrowseInfos.BrowseForComputer, + BrowseForComputer = unchecked((int)Interop.Shell32.BrowseInfoFlags.BIF_BROWSEFORCOMPUTER), - BrowseForEverything = UnsafeNativeMethods.BrowseInfos.BrowseForEverything, + BrowseForEverything = unchecked((int)Interop.Shell32.BrowseInfoFlags.BIF_BROWSEFOREVERYTHING), - BrowseForPrinter = UnsafeNativeMethods.BrowseInfos.BrowseForPrinter, + BrowseForPrinter = unchecked((int)Interop.Shell32.BrowseInfoFlags.BIF_BROWSEFORPRINTER), - RestrictToDomain = UnsafeNativeMethods.BrowseInfos.DontGoBelowDomain, + RestrictToDomain = unchecked((int)Interop.Shell32.BrowseInfoFlags.BIF_DONTGOBELOWDOMAIN), - RestrictToFilesystem = UnsafeNativeMethods.BrowseInfos.ReturnOnlyFSDirs, + RestrictToFilesystem = unchecked((int)Interop.Shell32.BrowseInfoFlags.BIF_RETURNONLYFSDIRS), - RestrictToSubfolders = UnsafeNativeMethods.BrowseInfos.ReturnFSAncestors, + RestrictToSubfolders = unchecked((int)Interop.Shell32.BrowseInfoFlags.BIF_RETURNFSANCESTORS), - ShowTextBox = UnsafeNativeMethods.BrowseInfos.EditBox + ShowTextBox = unchecked((int)Interop.Shell32.BrowseInfoFlags.BIF_EDITBOX) } } } diff --git a/src/System.Windows.Forms.Design.Editors/tests/UnitTests/System.Windows.Forms.Design.Editors.Tests.csproj b/src/System.Windows.Forms.Design.Editors/tests/UnitTests/System.Windows.Forms.Design.Editors.Tests.csproj index 1a2ad4a14fe..8ace057cbed 100644 --- a/src/System.Windows.Forms.Design.Editors/tests/UnitTests/System.Windows.Forms.Design.Editors.Tests.csproj +++ b/src/System.Windows.Forms.Design.Editors/tests/UnitTests/System.Windows.Forms.Design.Editors.Tests.csproj @@ -13,6 +13,8 @@ + + diff --git a/src/System.Windows.Forms.Design.Editors/tests/UnitTests/System/Windows/Forms/Design/FolderNameEditorTests.cs b/src/System.Windows.Forms.Design.Editors/tests/UnitTests/System/Windows/Forms/Design/FolderNameEditorTests.cs new file mode 100644 index 00000000000..cbd3d0c71e3 --- /dev/null +++ b/src/System.Windows.Forms.Design.Editors/tests/UnitTests/System/Windows/Forms/Design/FolderNameEditorTests.cs @@ -0,0 +1,95 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.ComponentModel; +using System.Drawing.Design; +using System.Globalization; +using System.Reflection; +using WinForms.Common.Tests; +using Xunit; + +namespace System.Windows.Forms.Design.Tests +{ + public class FolderNameEditorTests + { + [Fact] + public void GetEditStyle_Invoke_ReturnsModal() + { + var editor = new FolderNameEditor(); + Assert.Equal(UITypeEditorEditStyle.Modal, editor.GetEditStyle(null)); + } + + [Fact] + public void InitializeDialog_Invoke_Nop() + { + var editor = new SubFolderNameEditor(); + editor.InitializeDialog(); + } + + public class FolderBrowserTests : FolderNameEditor + { + [Fact] + public void Ctor_Default() + { + var browser = new FolderBrowser(); + Assert.Empty(browser.DirectoryPath); + Assert.Empty(browser.Description); + Assert.Equal(FolderBrowserStyles.RestrictToFilesystem, browser.Style); + Assert.Equal(FolderBrowserFolder.Desktop, browser.StartLocation); + } + + [Theory] + [CommonMemberData(nameof(CommonTestHelper.GetStringTheoryData))] + public void Description_Set_GetReturnsExpected(string value) + { + var browser = new FolderBrowser + { + Description = value + }; + Assert.Equal(value ?? string.Empty, browser.Description); + + // Set same. + browser.Description = value; + Assert.Equal(value ?? string.Empty, browser.Description); + } + + [Theory] + [CommonMemberData(nameof(CommonTestHelper.GetEnumTypeTheoryData), typeof(FolderBrowserFolder))] + [CommonMemberData(nameof(CommonTestHelper.GetEnumTypeTheoryDataInvalid), typeof(FolderBrowserFolder))] + protected void StartLocation_Set_GetReturnsExpected(FolderBrowserFolder value) + { + var browser = new FolderBrowser + { + StartLocation = value + }; + Assert.Equal(value, browser.StartLocation); + + // Set same. + browser.StartLocation = value; + Assert.Equal(value, browser.StartLocation); + } + + [Theory] + [CommonMemberData(nameof(CommonTestHelper.GetEnumTypeTheoryData), typeof(FolderBrowserStyles))] + [CommonMemberData(nameof(CommonTestHelper.GetEnumTypeTheoryDataInvalid), typeof(FolderBrowserStyles))] + protected void Style_Set_GetReturnsExpected(FolderBrowserStyles value) + { + var browser = new FolderBrowser + { + Style = value + }; + Assert.Equal(value, browser.Style); + + // Set same. + browser.Style = value; + Assert.Equal(value, browser.Style); + } + } + + private class SubFolderNameEditor : FolderNameEditor + { + public void InitializeDialog() => base.InitializeDialog(null); + } + } +} diff --git a/src/System.Windows.Forms.Design/src/System.Windows.Forms.Design.csproj b/src/System.Windows.Forms.Design/src/System.Windows.Forms.Design.csproj index dfc3fc009a4..583455780d5 100644 --- a/src/System.Windows.Forms.Design/src/System.Windows.Forms.Design.csproj +++ b/src/System.Windows.Forms.Design/src/System.Windows.Forms.Design.csproj @@ -43,6 +43,8 @@ + + diff --git a/src/System.Windows.Forms/src/System.Windows.Forms.csproj b/src/System.Windows.Forms/src/System.Windows.Forms.csproj index 1f12ea7c3c0..df0b1105802 100644 --- a/src/System.Windows.Forms/src/System.Windows.Forms.csproj +++ b/src/System.Windows.Forms/src/System.Windows.Forms.csproj @@ -42,6 +42,15 @@ + + + + + + + + + diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/DataObject.cs b/src/System.Windows.Forms/src/System/Windows/Forms/DataObject.cs index 054c8151f54..9d1d8671c38 100644 --- a/src/System.Windows.Forms/src/System/Windows/Forms/DataObject.cs +++ b/src/System.Windows.Forms/src/System/Windows/Forms/DataObject.cs @@ -1776,7 +1776,7 @@ private string[] ReadFileListFromHandle(IntPtr hdrop) { string[] files = null; - StringBuilder sb = new StringBuilder(NativeMethods.MAX_PATH); + StringBuilder sb = new StringBuilder(Interop.Kernel32.MAX_PATH); int count = UnsafeNativeMethods.DragQueryFile(new HandleRef(null, hdrop), unchecked((int)0xFFFFFFFF), null, 0); if (count > 0) diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/FileDialogCustomPlace.cs b/src/System.Windows.Forms/src/System/Windows/Forms/FileDialogCustomPlace.cs index 50afcf3ea15..49000bc28f5 100644 --- a/src/System.Windows.Forms/src/System/Windows/Forms/FileDialogCustomPlace.cs +++ b/src/System.Windows.Forms/src/System/Windows/Forms/FileDialogCustomPlace.cs @@ -68,53 +68,24 @@ public override string ToString() internal FileDialogNative.IShellItem GetNativePath() { - //This can throw in a multitude of ways if the path or Guid doesn't correspond - //to an actual filesystem directory. Caller is responsible for handling these situations. - string filePathString = ""; - if (!string.IsNullOrEmpty(this._path)) + // This can throw in a multitude of ways if the path or Guid doesn't correspond + // to an actual filesystem directory. + // The Caller is responsible for handling these situations. + string filePathString; + if (!string.IsNullOrEmpty(_path)) { - filePathString = this._path; + filePathString = _path; } else { - filePathString = GetFolderLocation(this._knownFolderGuid); - } - - if (string.IsNullOrEmpty(filePathString)) - { - return null; - } - else - { - return FileDialog.GetShellItemForPath(filePathString); - } - } - - private static string GetFolderLocation(Guid folderGuid) - { - //returns a null string if the path can't be found - - //SECURITY: This exposes the filesystem path of the GUID. The returned value - // must not be made available to user code. - - if (!UnsafeNativeMethods.IsVista) - { - return null; + int result = Interop.Shell32.SHGetKnownFolderPath(ref _knownFolderGuid, 0, IntPtr.Zero, out filePathString); + if (result == 0) + { + return null; + } } - StringBuilder path = new StringBuilder(); - - int result = UnsafeNativeMethods.Shell32.SHGetFolderPathEx(ref folderGuid, 0, IntPtr.Zero, path); - if (NativeMethods.S_OK == result) - { - string ret = path.ToString(); - return ret; - } - else - { - // 0x80070002 is an explicit FileNotFound error. - return null; - } + return FileDialog.GetShellItemForPath(filePathString); } } } \ No newline at end of file diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/FolderBrowserDialog.cs b/src/System.Windows.Forms/src/System/Windows/Forms/FolderBrowserDialog.cs index 9abebc87b62..e0fa1be03ad 100644 --- a/src/System.Windows.Forms/src/System/Windows/Forms/FolderBrowserDialog.cs +++ b/src/System.Windows.Forms/src/System/Windows/Forms/FolderBrowserDialog.cs @@ -8,6 +8,7 @@ namespace System.Windows.Forms { using System; using System.IO; + using System.Buffers; using System.Collections; using System.ComponentModel; using System.Drawing; @@ -16,7 +17,8 @@ namespace System.Windows.Forms using System.Runtime.InteropServices; using System.Security.Permissions; using System.Diagnostics.CodeAnalysis; - + using Microsoft.Win32.SafeHandles; + /// /// /// @@ -44,9 +46,6 @@ public sealed class FolderBrowserDialog : CommonDialog // Show the 'New Folder' button? private bool showNewFolderButton; - // Callback function for the folder browser dialog - private UnsafeNativeMethods.BrowseCallbackProc callback; - /// /// /// @@ -216,16 +215,6 @@ internal bool UseVistaDialogInternal } } - /// - /// Helper function that returns the IMalloc interface used by the shell. - /// - private static UnsafeNativeMethods.IMalloc GetSHMalloc() - { - UnsafeNativeMethods.IMalloc[] malloc = new UnsafeNativeMethods.IMalloc[1]; - UnsafeNativeMethods.Shell32.SHGetMalloc(malloc); - return malloc[0]; - } - /// /// /// @@ -314,93 +303,73 @@ private void GetResult(FileDialogNative.IFileDialog dialog) item.GetDisplayName(FileDialogNative.SIGDN.SIGDN_FILESYSPATH, out selectedPath); } - private bool RunDialogOld(IntPtr hWndOwner) + private unsafe bool RunDialogOld(IntPtr hWndOwner) { - IntPtr pidlRoot = IntPtr.Zero; - bool returnValue = false; + CoTaskMemSafeHandle listHandle; - UnsafeNativeMethods.Shell32.SHGetSpecialFolderLocation(hWndOwner, (int)rootFolder, ref pidlRoot); - if (pidlRoot == IntPtr.Zero) + Interop.Shell32.SHGetSpecialFolderLocation(hWndOwner, (int)rootFolder, out listHandle); + if (listHandle.IsInvalid) { - UnsafeNativeMethods.Shell32.SHGetSpecialFolderLocation(hWndOwner, NativeMethods.CSIDL_DESKTOP, ref pidlRoot); - if (pidlRoot == IntPtr.Zero) + Interop.Shell32.SHGetSpecialFolderLocation(hWndOwner, (int)Environment.SpecialFolder.Desktop, out listHandle); + if (listHandle.IsInvalid) { throw new InvalidOperationException(SR.FolderBrowserDialogNoRootFolder); } } - int mergedOptions = unchecked((int)(long)UnsafeNativeMethods.BrowseInfos.NewDialogStyle); - if (!showNewFolderButton) + using (listHandle) { - mergedOptions += unchecked((int)(long)UnsafeNativeMethods.BrowseInfos.HideNewFolderButton); - } - - // The SHBrowserForFolder dialog is OLE/COM based, and documented as only being safe to use under the STA - // threading model if the BIF_NEWDIALOGSTYLE flag has been requested (which we always do in mergedOptions - // above). So make sure OLE is initialized, and throw an exception if caller attempts to invoke dialog - // under the MTA threading model (...dialog does appear under MTA, but is totally non-functional). - if (Control.CheckForIllegalCrossThreadCalls && Application.OleRequired() != System.Threading.ApartmentState.STA) - { - throw new System.Threading.ThreadStateException(string.Format(SR.DebuggingExceptionOnly, SR.ThreadMustBeSTA)); - } - - IntPtr pidlRet = IntPtr.Zero; - IntPtr pszDisplayName = IntPtr.Zero; - IntPtr pszSelectedPath = IntPtr.Zero; - - try - { - // Construct a BROWSEINFO - UnsafeNativeMethods.BROWSEINFO bi = new UnsafeNativeMethods.BROWSEINFO(); - - pszDisplayName = Marshal.AllocHGlobal(NativeMethods.MAX_PATH * sizeof(char)); - pszSelectedPath = Marshal.AllocHGlobal((NativeMethods.MAX_PATH + 1) * sizeof(char)); - this.callback = new UnsafeNativeMethods.BrowseCallbackProc(this.FolderBrowserDialog_BrowseCallbackProc); - - bi.pidlRoot = pidlRoot; - bi.hwndOwner = hWndOwner; - bi.pszDisplayName = pszDisplayName; - bi.lpszTitle = descriptionText; - bi.ulFlags = mergedOptions; - bi.lpfn = callback; - bi.lParam = IntPtr.Zero; - bi.iImage = 0; - - // And show the dialog - pidlRet = UnsafeNativeMethods.Shell32.SHBrowseForFolder(bi); - - if (pidlRet != IntPtr.Zero) + uint mergedOptions = Interop.Shell32.BrowseInfoFlags.BIF_NEWDIALOGSTYLE; + if (!showNewFolderButton) { - // Then retrieve the path from the IDList - UnsafeNativeMethods.Shell32.SHGetPathFromIDListLongPath(pidlRet, ref pszSelectedPath); - - // Convert to a string - selectedPath = Marshal.PtrToStringAuto(pszSelectedPath); - - returnValue = true; + mergedOptions |= Interop.Shell32.BrowseInfoFlags.BIF_NONEWFOLDERBUTTON; } - } - finally - { - UnsafeNativeMethods.CoTaskMemFree(pidlRoot); - if (pidlRet != IntPtr.Zero) + + // The SHBrowserForFolder dialog is OLE/COM based, and documented as only being safe to use under the STA + // threading model if the BIF_NEWDIALOGSTYLE flag has been requested (which we always do in mergedOptions + // above). So make sure OLE is initialized, and throw an exception if caller attempts to invoke dialog + // under the MTA threading model (...dialog does appear under MTA, but is totally non-functional). + if (Control.CheckForIllegalCrossThreadCalls && Application.OleRequired() != System.Threading.ApartmentState.STA) { - UnsafeNativeMethods.CoTaskMemFree(pidlRet); + throw new System.Threading.ThreadStateException(string.Format(SR.DebuggingExceptionOnly, SR.ThreadMustBeSTA)); } - // Then free all the stuff we've allocated or the SH API gave us - if (pszSelectedPath != IntPtr.Zero) + var callback = new Interop.Shell32.BrowseCallbackProc(FolderBrowserDialog_BrowseCallbackProc); + char[] displayName = ArrayPool.Shared.Rent(Interop.Kernel32.MAX_PATH + 1); + try { - Marshal.FreeHGlobal(pszSelectedPath); + fixed (char *pDisplayName = displayName) + { + var bi = new Interop.Shell32.BROWSEINFO(); + bi.pidlRoot = listHandle; + bi.hwndOwner = hWndOwner; + bi.pszDisplayName = pDisplayName; + bi.lpszTitle = descriptionText; + bi.ulFlags = mergedOptions; + bi.lpfn = callback; + bi.lParam = IntPtr.Zero; + bi.iImage = 0; + + // Show the dialog + using (CoTaskMemSafeHandle browseHandle = Interop.Shell32.SHBrowseForFolderW(ref bi)) + { + if (browseHandle.IsInvalid) + { + return false; + } + + // Retrieve the path from the IDList. + Interop.Shell32.SHGetPathFromIDListLongPath(browseHandle.DangerousGetHandle(), out selectedPath); + GC.KeepAlive(callback); + return true; + } + } } - if (pszDisplayName != IntPtr.Zero) + finally { - Marshal.FreeHGlobal(pszDisplayName); + ArrayPool.Shared.Return(displayName); } - - this.callback = null; } - return returnValue; } /// @@ -427,10 +396,8 @@ private int FolderBrowserDialog_BrowseCallbackProc(IntPtr hwnd, IntPtr selectedPidl = lParam; if (selectedPidl != IntPtr.Zero) { - IntPtr pszSelectedPath = Marshal.AllocHGlobal((NativeMethods.MAX_PATH + 1) * sizeof(char)); // Try to retrieve the path from the IDList - bool isFileSystemFolder = UnsafeNativeMethods.Shell32.SHGetPathFromIDListLongPath(selectedPidl, ref pszSelectedPath); - Marshal.FreeHGlobal(pszSelectedPath); + bool isFileSystemFolder = Interop.Shell32.SHGetPathFromIDListLongPath(selectedPidl, out _); UnsafeNativeMethods.SendMessage(new HandleRef(null, hwnd), (int) NativeMethods.BFFM_ENABLEOK, 0, isFileSystemFolder ? 1 : 0); } break; diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/FxCopSuppression.cs b/src/System.Windows.Forms/src/System/Windows/Forms/FxCopSuppression.cs index 8d2620de0c5..0bbf683d4b1 100644 --- a/src/System.Windows.Forms/src/System/Windows/Forms/FxCopSuppression.cs +++ b/src/System.Windows.Forms/src/System/Windows/Forms/FxCopSuppression.cs @@ -197,7 +197,6 @@ [assembly: SuppressMessage("Microsoft.Security", "CA2101:SpecifyMarshalingForPInvokeStringArguments", Scope="member", Target="System.Windows.Forms.UnsafeNativeMethods.SystemParametersInfo(System.Int32,System.Int32,System.Windows.Forms.NativeMethods+LOGFONT,System.Int32):System.Boolean")] [assembly: SuppressMessage("Microsoft.Security", "CA2101:SpecifyMarshalingForPInvokeStringArguments", Scope="member", Target="System.Windows.Forms.UnsafeNativeMethods.SystemParametersInfo(System.Int32,System.Int32,System.Windows.Forms.NativeMethods+NONCLIENTMETRICS,System.Int32):System.Boolean")] [assembly: SuppressMessage("Microsoft.Security", "CA2101:SpecifyMarshalingForPInvokeStringArguments", Scope="member", Target="System.Windows.Forms.UnsafeNativeMethods.UnregisterClass(System.String,System.Runtime.InteropServices.HandleRef):System.Boolean")] -[assembly: SuppressMessage("Microsoft.Security", "CA2101:SpecifyMarshalingForPInvokeStringArguments", Scope="member", Target="System.Windows.Forms.UnsafeNativeMethods+Shell32.SHBrowseForFolder(System.Windows.Forms.UnsafeNativeMethods+BROWSEINFO):System.IntPtr")] [assembly: SuppressMessage("Microsoft.Security", "CA2101:SpecifyMarshalingForPInvokeStringArguments", Scope="member", Target="System.Windows.Forms.UnsafeNativeMethods.OleCreateFontIndirect(System.Windows.Forms.NativeMethods+tagFONTDESC,System.Guid&):System.Windows.Forms.UnsafeNativeMethods+IFont")] [assembly: SuppressMessage("Microsoft.Security", "CA2101:SpecifyMarshalingForPInvokeStringArguments", Scope="member", Target="System.Windows.Forms.UnsafeNativeMethods.OleCreateIFontIndirect(System.Windows.Forms.NativeMethods+FONTDESC,System.Guid&):System.Windows.Forms.UnsafeNativeMethods+IFont")] diff --git a/src/System.Windows.Forms/src/System/Windows/Forms/RichTextBox.cs b/src/System.Windows.Forms/src/System/Windows/Forms/RichTextBox.cs index cc2462700c4..542c9af1be2 100644 --- a/src/System.Windows.Forms/src/System/Windows/Forms/RichTextBox.cs +++ b/src/System.Windows.Forms/src/System/Windows/Forms/RichTextBox.cs @@ -3285,7 +3285,7 @@ internal void WmReflectNotify(ref Message m) { NativeMethods.ENDROPFILES endropfiles = (NativeMethods.ENDROPFILES)m.GetLParam(typeof(NativeMethods.ENDROPFILES)); // Only look at the first file. - StringBuilder path = new StringBuilder(NativeMethods.MAX_PATH); + StringBuilder path = new StringBuilder(Interop.Kernel32.MAX_PATH); if (UnsafeNativeMethods.DragQueryFileLongPath(new HandleRef(endropfiles, endropfiles.hDrop), 0, path) != 0) { // Try to load the file as an RTF