From 8638919b29b236b014eda8a5bbcf8adf5cb794df Mon Sep 17 00:00:00 2001 From: tonythethompson Date: Sat, 25 Jul 2026 04:25:26 -0700 Subject: [PATCH 1/8] Drop WinForms so Release MSIX can trim, keep trust digests stable. Replace folder/file dialogs and clipboard with Win32/COM interop, use System.Drawing.Common for icons, and serialize digests through a dedicated JSON context that preserves the legacy compact payload bytes. Co-authored-by: Cursor --- .github/workflows/release-extension.yml | 2 +- .github/workflows/release-run-plugin.yml | 2 +- Directory.Packages.props | 1 + .../WorkspaceSecurityPolicyTests.cs | 72 ++++ .../Classification/ProjectAnalysisService.cs | 25 +- QuickShell.Core/Models/FormChoiceJson.cs | 15 + QuickShell.Core/Models/TaskTypeChoiceJson.cs | 16 + .../Models/WorkspaceDigestPayload.cs | 25 ++ QuickShell.Core/QuickShell.Core.csproj | 4 +- .../QuickShellDigestJsonContext.cs | 21 + QuickShell.Core/QuickShellJsonContext.cs | 4 + .../Services/CompanionAppCatalog.cs | 4 +- .../Services/WorkspaceSecurityPolicy.cs | 12 +- QuickShell.Run/Interop/ShellFileDialog.cs | 360 ++++++++++++++++++ QuickShell.Run/Interop/Win32Clipboard.cs | 160 ++++++++ QuickShell.Run/QuickShell.Run.csproj | 8 +- .../Services/FolderPickerService.cs | 102 +---- QuickShell.Run/Services/StaClipboard.cs | 4 +- QuickShell/ILLink.Descriptors.xml | 15 + QuickShell/Interop/ShellFileDialog.cs | 360 ++++++++++++++++++ QuickShell/Interop/Win32Clipboard.cs | 160 ++++++++ QuickShell/QuickShell.csproj | 26 +- QuickShell/Services/FolderPickerService.cs | 114 +----- .../Services/ShortcutFilePickerService.cs | 117 ++---- QuickShell/Services/StaClipboard.cs | 10 +- 25 files changed, 1338 insertions(+), 301 deletions(-) create mode 100644 QuickShell.Core/Models/FormChoiceJson.cs create mode 100644 QuickShell.Core/Models/TaskTypeChoiceJson.cs create mode 100644 QuickShell.Core/Models/WorkspaceDigestPayload.cs create mode 100644 QuickShell.Core/QuickShellDigestJsonContext.cs create mode 100644 QuickShell.Run/Interop/ShellFileDialog.cs create mode 100644 QuickShell.Run/Interop/Win32Clipboard.cs create mode 100644 QuickShell/ILLink.Descriptors.xml create mode 100644 QuickShell/Interop/ShellFileDialog.cs create mode 100644 QuickShell/Interop/Win32Clipboard.cs diff --git a/.github/workflows/release-extension.yml b/.github/workflows/release-extension.yml index b7142069..4b0648bb 100644 --- a/.github/workflows/release-extension.yml +++ b/.github/workflows/release-extension.yml @@ -106,7 +106,7 @@ jobs: ${{ github.event.inputs.release_notes }} - Package matrix: [docs/release-packages.md](https://github.com/tonythethompson/QuickShell/blob/main/docs/release-packages.md) + Package matrix: [docs/release-packages.md](https://github.com/tonythethompson/QuickShell/blob/master/docs/release-packages.md) ## Install diff --git a/.github/workflows/release-run-plugin.yml b/.github/workflows/release-run-plugin.yml index d65a11a1..1cda0571 100644 --- a/.github/workflows/release-run-plugin.yml +++ b/.github/workflows/release-run-plugin.yml @@ -92,7 +92,7 @@ jobs: Restart PowerToys after copying files. - Activate with the **qs** action keyword, or type **Quick Shell** / **quickshell** in global Run. - - See [docs/powertoys-run-plugin.md](https://github.com/tonythethompson/QuickShell/blob/main/docs/powertoys-run-plugin.md). + - See [docs/powertoys-run-plugin.md](https://github.com/tonythethompson/QuickShell/blob/master/docs/powertoys-run-plugin.md). files: | QuickShell.Run/bin/x64/Release/QuickShell.Run-x64.zip QuickShell.Run/bin/ARM64/Release/QuickShell.Run-ARM64.zip diff --git a/Directory.Packages.props b/Directory.Packages.props index 44b762c5..87a20217 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -14,6 +14,7 @@ + diff --git a/QuickShell.Core.Tests/WorkspaceSecurityPolicyTests.cs b/QuickShell.Core.Tests/WorkspaceSecurityPolicyTests.cs index 8a9d7598..0e361175 100644 --- a/QuickShell.Core.Tests/WorkspaceSecurityPolicyTests.cs +++ b/QuickShell.Core.Tests/WorkspaceSecurityPolicyTests.cs @@ -270,6 +270,78 @@ public void Open_directory_rejects_non_local_path_namespaces(string directory) Assert.Equal(WorkspaceIssueCode.DirectoryOpenNotAllowed, result.PrimaryIssueCode); } + [Fact] + public void ComputeDigest_matches_legacy_anonymous_Serialize_bytes() + { + // Pins trim-safe DTO serialization to the pre-refactor JsonSerializer.Serialize(anonymous) + // shape (compact, nulls included). Drift here silently invalidates WorkspaceReviewToken digests. + var workspace = new TerminalShortcut + { + Id = "digest-workspace", + Name = "Digest", + Directory = @"C:\repos\demo", + Command = null, + Terminal = "wt", + WtProfile = null, + RunAsAdmin = false, + Launches = + [ + new WorkspaceEntry + { + Id = "launch-1", + Label = "Launch", + Terminal = "default", + WtProfile = null, + Command = "echo hi", + RunAsAdmin = false, + IsEnabled = true, + Order = 0, + TaskType = "none", + }, + ], + DevServerUrl = null, + OpenDevServerOnLaunch = false, + RepoUrl = null, + CompanionApps = + [ + new CompanionAppEntry + { + Id = "companion-1", + Path = @"C:\Tools\editor.exe", + Arguments = null, + OpenOnLaunch = true, + Order = 0, + }, + ], + OpenCompanionAppOnLaunch = true, + CompanionAppPath = @"C:\Tools\editor.exe", + CompanionAppArguments = null, + }; + + var legacyPayload = System.Text.Json.JsonSerializer.Serialize(new + { + workspace.Id, + workspace.Name, + workspace.Directory, + workspace.Command, + workspace.Terminal, + workspace.WtProfile, + workspace.RunAsAdmin, + workspace.Launches, + workspace.DevServerUrl, + workspace.OpenDevServerOnLaunch, + workspace.RepoUrl, + workspace.CompanionApps, + workspace.OpenCompanionAppOnLaunch, + workspace.CompanionAppPath, + workspace.CompanionAppArguments, + }); + var expected = Convert.ToHexString( + System.Security.Cryptography.SHA256.HashData(System.Text.Encoding.UTF8.GetBytes(legacyPayload))); + + Assert.Equal(expected, WorkspaceSecurityPolicy.ComputeDigest(workspace)); + } + [Fact] public void Trust_review_token_is_invalidated_by_revision_change() { diff --git a/QuickShell.Core/Classification/ProjectAnalysisService.cs b/QuickShell.Core/Classification/ProjectAnalysisService.cs index 9482c0ee..244c08fb 100644 --- a/QuickShell.Core/Classification/ProjectAnalysisService.cs +++ b/QuickShell.Core/Classification/ProjectAnalysisService.cs @@ -1,4 +1,5 @@ using QuickShell.Abstractions.Classification; +using QuickShell.Models; using QuickShell.Services; namespace QuickShell.Classification; @@ -84,15 +85,13 @@ public string BuildTaskTypeChoicesJson( bool includePlaceholder = true) { pickContext ??= TaskTypePickContext.Empty; - var choices = new List(); + var choices = new List(); if (includePlaceholder) { - choices.Add(new - { - title = "Choose a command…", - value = TaskTypeCatalog.None, - tooltip = "Adds a new command row with a project-aware suggestion.", - }); + choices.Add(new TaskTypeChoiceJson( + "Choose a command…", + TaskTypeCatalog.None, + "Adds a new command row with a project-aware suggestion.")); } foreach (var def in TaskTypeCatalog.GetChoices()) @@ -102,15 +101,13 @@ public string BuildTaskTypeChoicesJson( continue; } - choices.Add(new - { - title = def.Title, - value = def.Id, - tooltip = GetTaskTypeChoiceTooltip(directory, def.Id, pickContext), - }); + choices.Add(new TaskTypeChoiceJson( + def.Title, + def.Id, + GetTaskTypeChoiceTooltip(directory, def.Id, pickContext))); } - return System.Text.Json.JsonSerializer.Serialize(choices); + return System.Text.Json.JsonSerializer.Serialize(choices, QuickShellJsonContext.Default.ListTaskTypeChoiceJson); } public CompanionAppSuggestion? TrySuggestCompanionApp(string directory) => diff --git a/QuickShell.Core/Models/FormChoiceJson.cs b/QuickShell.Core/Models/FormChoiceJson.cs new file mode 100644 index 00000000..d11bb4b5 --- /dev/null +++ b/QuickShell.Core/Models/FormChoiceJson.cs @@ -0,0 +1,15 @@ +using System.Text.Json.Serialization; + +namespace QuickShell.Models; + +/// +/// JSON shape for a CmdPal dynamic-choice-set entry (title/value pair). A named type so +/// can serialize it through the +/// source-generated instead of +/// System.Text.Json's reflection-based Serialize<T>, which trimming forbids. +/// preserves the exact lowercase JSON keys the +/// previous anonymous-type serialization produced (the CmdPal host expects this casing). +/// +internal sealed record FormChoiceJson( + [property: JsonPropertyName("title")] string Title, + [property: JsonPropertyName("value")] string Value); diff --git a/QuickShell.Core/Models/TaskTypeChoiceJson.cs b/QuickShell.Core/Models/TaskTypeChoiceJson.cs new file mode 100644 index 00000000..a3cf3f37 --- /dev/null +++ b/QuickShell.Core/Models/TaskTypeChoiceJson.cs @@ -0,0 +1,16 @@ +using System.Text.Json.Serialization; + +namespace QuickShell.Models; + +/// +/// JSON shape for a task-type picker choice (title/value/tooltip). A named type so +/// can serialize it through the +/// source-generated instead of +/// System.Text.Json's reflection-based Serialize<T>, which trimming forbids. +/// preserves the exact lowercase JSON keys the +/// previous anonymous-type serialization produced (the CmdPal host expects this casing). +/// +internal sealed record TaskTypeChoiceJson( + [property: JsonPropertyName("title")] string Title, + [property: JsonPropertyName("value")] string Value, + [property: JsonPropertyName("tooltip")] string Tooltip); diff --git a/QuickShell.Core/Models/WorkspaceDigestPayload.cs b/QuickShell.Core/Models/WorkspaceDigestPayload.cs new file mode 100644 index 00000000..60c2fb7c --- /dev/null +++ b/QuickShell.Core/Models/WorkspaceDigestPayload.cs @@ -0,0 +1,25 @@ +namespace QuickShell.Models; + +/// +/// Shape of the payload hashed by . +/// A named type (not an anonymous type) so it can go through the source-generated +/// instead of reflection-based serialization, +/// which trimming forbids. Field set and order are load-bearing: changing either changes the +/// digest and invalidates outstanding WorkspaceReviewToken values. +/// +internal sealed record WorkspaceDigestPayload( + string Id, + string Name, + string Directory, + string? Command, + string Terminal, + string? WtProfile, + bool RunAsAdmin, + List Launches, + string? DevServerUrl, + bool OpenDevServerOnLaunch, + string? RepoUrl, + List CompanionApps, + bool OpenCompanionAppOnLaunch, + string? CompanionAppPath, + string? CompanionAppArguments); diff --git a/QuickShell.Core/QuickShell.Core.csproj b/QuickShell.Core/QuickShell.Core.csproj index e84a47dc..b5b6f576 100644 --- a/QuickShell.Core/QuickShell.Core.csproj +++ b/QuickShell.Core/QuickShell.Core.csproj @@ -7,11 +7,13 @@ enable x64;ARM64 $(Platform) - true + + diff --git a/QuickShell.Core/QuickShellDigestJsonContext.cs b/QuickShell.Core/QuickShellDigestJsonContext.cs new file mode 100644 index 00000000..a6f58f69 --- /dev/null +++ b/QuickShell.Core/QuickShellDigestJsonContext.cs @@ -0,0 +1,21 @@ +using System.Text.Json.Serialization; +using QuickShell.Models; + +namespace QuickShell; + +/// +/// Source-generated serializer for only. +/// Options must match System.Text.Json's default Serialize(object) output (compact, +/// nulls included, PascalCase) so existing WorkspaceReviewToken digests keep matching. +/// Do not reuse here: that context uses WriteIndented and +/// WhenWritingNull for settings/form payloads and would invalidate outstanding trust tokens. +/// +[JsonSourceGenerationOptions( + WriteIndented = false, + DefaultIgnoreCondition = JsonIgnoreCondition.Never)] +[JsonSerializable(typeof(WorkspaceDigestPayload))] +[JsonSerializable(typeof(WorkspaceEntry))] +[JsonSerializable(typeof(List))] +[JsonSerializable(typeof(CompanionAppEntry))] +[JsonSerializable(typeof(List))] +internal sealed partial class QuickShellDigestJsonContext : JsonSerializerContext; diff --git a/QuickShell.Core/QuickShellJsonContext.cs b/QuickShell.Core/QuickShellJsonContext.cs index 040bfc21..8011cf30 100644 --- a/QuickShell.Core/QuickShellJsonContext.cs +++ b/QuickShell.Core/QuickShellJsonContext.cs @@ -24,4 +24,8 @@ namespace QuickShell; [JsonSerializable(typeof(string))] [JsonSerializable(typeof(Dictionary))] [JsonSerializable(typeof(WorktreeBranchTargetsDocument))] +[JsonSerializable(typeof(FormChoiceJson))] +[JsonSerializable(typeof(List))] +[JsonSerializable(typeof(TaskTypeChoiceJson))] +[JsonSerializable(typeof(List))] internal sealed partial class QuickShellJsonContext : JsonSerializerContext; diff --git a/QuickShell.Core/Services/CompanionAppCatalog.cs b/QuickShell.Core/Services/CompanionAppCatalog.cs index ff02ca5c..69d3fb75 100644 --- a/QuickShell.Core/Services/CompanionAppCatalog.cs +++ b/QuickShell.Core/Services/CompanionAppCatalog.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using QuickShell.Models; namespace QuickShell.Services; @@ -166,7 +167,8 @@ private static void EnsureFormChoicesCached() private static string SerializeFormChoices(IReadOnlyList<(string Id, string Title)> choices) => JsonSerializer.Serialize( - choices.Select(choice => new { title = choice.Title, value = choice.Id })); + choices.Select(choice => new FormChoiceJson(choice.Title, choice.Id)).ToList(), + QuickShellJsonContext.Default.ListFormChoiceJson); private static List<(string Id, string Title)> BuildInstalledFormChoicesUncached() { diff --git a/QuickShell.Core/Services/WorkspaceSecurityPolicy.cs b/QuickShell.Core/Services/WorkspaceSecurityPolicy.cs index 369894cd..42dfeaf7 100644 --- a/QuickShell.Core/Services/WorkspaceSecurityPolicy.cs +++ b/QuickShell.Core/Services/WorkspaceSecurityPolicy.cs @@ -409,8 +409,10 @@ public static bool MatchesReviewToken(StoredWorkspace workspace, WorkspaceReview public static string ComputeDigest(TerminalShortcut workspace) { - var payload = JsonSerializer.Serialize(new - { + // Named DTO + QuickShellDigestJsonContext (not QuickShellJsonContext): the digest + // context mirrors default Serialize(object) bytes so existing WorkspaceReviewToken + // values keep matching. Field set/order below is load-bearing. + var digestPayload = new WorkspaceDigestPayload( workspace.Id, workspace.Name, workspace.Directory, @@ -425,8 +427,10 @@ public static string ComputeDigest(TerminalShortcut workspace) workspace.CompanionApps, workspace.OpenCompanionAppOnLaunch, workspace.CompanionAppPath, - workspace.CompanionAppArguments, - }); + workspace.CompanionAppArguments); + var payload = JsonSerializer.Serialize( + digestPayload, + QuickShellDigestJsonContext.Default.WorkspaceDigestPayload); return Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(payload))); } diff --git a/QuickShell.Run/Interop/ShellFileDialog.cs b/QuickShell.Run/Interop/ShellFileDialog.cs new file mode 100644 index 00000000..b8dafed9 --- /dev/null +++ b/QuickShell.Run/Interop/ShellFileDialog.cs @@ -0,0 +1,360 @@ +using System.IO; +using System.Runtime.InteropServices; +using System.Runtime.InteropServices.Marshalling; + +namespace QuickShell.Interop; + +/// +/// Minimal wrapper over the Win32 shell Common Item Dialog (IFileOpenDialog / +/// IFileSaveDialog). Replaces the WinForms FolderBrowserDialog / OpenFileDialog / +/// SaveFileDialog so the packaged app no longer forces UseWindowsForms (which blocks +/// trimming). Calls are synchronous and modal — IFileDialog.Show blocks the calling +/// thread exactly like the old ShowDialog, so the existing background-STA-thread +/// callers keep working unchanged. +/// +/// Uses source-generated COM () so the +/// interop stays AOT/trim-safe (the project is IsAotCompatible=true). The full +/// IFileDialog vtable is declared in order; slots we never call are declared with +/// blittable [PreserveSig] signatures purely to preserve vtable layout. +/// +internal static partial class ShellFileDialog +{ + // A single StrategyBasedComWrappers marshals the raw IUnknown pointers returned by + // CoCreateInstance / SHCreateItemFromParsingName into our generated interfaces. + private static readonly StrategyBasedComWrappers ComWrappers = new(); + + private static readonly Guid CLSID_FileOpenDialog = new("DC1C5A9C-E88A-4dde-A5A1-60F82A20AEF7"); + private static readonly Guid CLSID_FileSaveDialog = new("C0B4E2F3-BA21-4773-8DBA-335EC946EB8B"); + private static readonly Guid IID_IFileOpenDialog = new("d57c7288-d4ad-4768-be02-9d969532d960"); + private static readonly Guid IID_IFileSaveDialog = new("84bccd23-5fde-4cdb-aea4-af64b83d78ab"); + private static readonly Guid IID_IShellItem = new("43826d1e-e718-42ee-bc55-a1e261c37bfe"); + + private const uint FOS_OVERWRITEPROMPT = 0x00000002; + private const uint FOS_NOCHANGEDIR = 0x00000008; + private const uint FOS_PICKFOLDERS = 0x00000020; + private const uint FOS_FORCEFILESYSTEM = 0x00000040; + private const uint FOS_PATHMUSTEXIST = 0x00000800; + private const uint FOS_FILEMUSTEXIST = 0x00001000; + + private const uint SIGDN_FILESYSPATH = 0x80058000; + + private const uint CLSCTX_INPROC_SERVER = 0x1; + + // HRESULT_FROM_WIN32(ERROR_CANCELLED) — returned by Show when the user cancels. + private const int ERROR_CANCELLED_HRESULT = unchecked((int)0x800704C7); + + /// Modern folder picker (IFileOpenDialog with FOS_PICKFOLDERS). + public static string? PickFolder(nint owner, string? initialDirectory) + { + var dialog = (IFileOpenDialog)CreateInstance(CLSID_FileOpenDialog, IID_IFileOpenDialog); + try + { + dialog.SetOptions(FOS_PICKFOLDERS | FOS_FORCEFILESYSTEM | FOS_PATHMUSTEXIST | FOS_NOCHANGEDIR); + ApplyInitialDirectory(dialog, initialDirectory); + return ShowAndGetPath(dialog, owner); + } + finally + { + Release(dialog); + } + } + + /// Open-file picker. pairs display name to a + /// semicolon-delimited pattern (e.g. "JSON files (*.json)" -> "*.json"). + public static string? PickOpenFile(nint owner, string title, (string Name, string Spec)[] filters, string? defaultExt, string? initialDirectory) + { + var dialog = (IFileOpenDialog)CreateInstance(CLSID_FileOpenDialog, IID_IFileOpenDialog); + try + { + dialog.SetOptions(FOS_FORCEFILESYSTEM | FOS_FILEMUSTEXIST | FOS_PATHMUSTEXIST | FOS_NOCHANGEDIR); + dialog.SetTitle(title); + ApplyFilters(dialog, filters, defaultExt); + ApplyInitialDirectory(dialog, initialDirectory); + return ShowAndGetPath(dialog, owner); + } + finally + { + Release(dialog); + } + } + + /// Save-file picker with overwrite prompt and a suggested file name. + public static string? PickSaveFile(nint owner, string title, (string Name, string Spec)[] filters, string? defaultExt, string? defaultFileName, string? initialDirectory) + { + var dialog = (IFileSaveDialog)CreateInstance(CLSID_FileSaveDialog, IID_IFileSaveDialog); + try + { + dialog.SetOptions(FOS_FORCEFILESYSTEM | FOS_OVERWRITEPROMPT | FOS_PATHMUSTEXIST | FOS_NOCHANGEDIR); + dialog.SetTitle(title); + ApplyFilters(dialog, filters, defaultExt); + if (!string.IsNullOrEmpty(defaultFileName)) + { + dialog.SetFileName(defaultFileName); + } + + ApplyInitialDirectory(dialog, initialDirectory); + return ShowAndGetPath(dialog, owner); + } + finally + { + Release(dialog); + } + } + + private static string? ShowAndGetPath(IFileDialog dialog, nint owner) + { + int hr = dialog.Show(owner); + if (hr == ERROR_CANCELLED_HRESULT) + { + return null; + } + + Marshal.ThrowExceptionForHR(hr); + + dialog.GetResult(out nint itemPtr); + if (itemPtr == 0) + { + return null; + } + + var item = (IShellItem)ComWrappers.GetOrCreateObjectForComInstance(itemPtr, CreateObjectFlags.UniqueInstance); + Marshal.Release(itemPtr); + try + { + item.GetDisplayName(SIGDN_FILESYSPATH, out nint pathPtr); + if (pathPtr == 0) + { + return null; + } + + try + { + return Marshal.PtrToStringUni(pathPtr); + } + finally + { + Marshal.FreeCoTaskMem(pathPtr); + } + } + finally + { + Release(item); + } + } + + private static void ApplyInitialDirectory(IFileDialog dialog, string? initialDirectory) + { + if (string.IsNullOrWhiteSpace(initialDirectory) || !Directory.Exists(initialDirectory)) + { + return; + } + + int hr = SHCreateItemFromParsingName(initialDirectory!, 0, in IID_IShellItem, out nint itemPtr); + if (hr < 0 || itemPtr == 0) + { + return; // Non-fatal: fall back to the shell's default folder. + } + + var item = (IShellItem)ComWrappers.GetOrCreateObjectForComInstance(itemPtr, CreateObjectFlags.UniqueInstance); + Marshal.Release(itemPtr); + try + { + dialog.SetFolder(item); + } + finally + { + Release(item); + } + } + + private static void ApplyFilters(IFileDialog dialog, (string Name, string Spec)[] filters, string? defaultExt) + { + if (filters.Length == 0) + { + return; + } + + // COMDLG_FILTERSPEC is { LPWSTR pszName; LPWSTR pszSpec; }. Build the native array + // by hand so the interface method stays blittable (no struct marshalling in the vtable). + nint block = Marshal.AllocCoTaskMem(filters.Length * nint.Size * 2); + var strings = new List(filters.Length * 2); + try + { + for (int i = 0; i < filters.Length; i++) + { + nint namePtr = Marshal.StringToCoTaskMemUni(filters[i].Name); + nint specPtr = Marshal.StringToCoTaskMemUni(filters[i].Spec); + strings.Add(namePtr); + strings.Add(specPtr); + Marshal.WriteIntPtr(block, i * nint.Size * 2, namePtr); + Marshal.WriteIntPtr(block, (i * nint.Size * 2) + nint.Size, specPtr); + } + + dialog.SetFileTypes((uint)filters.Length, block); + dialog.SetFileTypeIndex(1); + } + finally + { + foreach (nint s in strings) + { + Marshal.FreeCoTaskMem(s); + } + + Marshal.FreeCoTaskMem(block); + } + + if (!string.IsNullOrEmpty(defaultExt)) + { + dialog.SetDefaultExtension(defaultExt); + } + } + + private static object CreateInstance(Guid clsid, Guid iid) + { + int hr = CoCreateInstance(in clsid, 0, CLSCTX_INPROC_SERVER, in iid, out nint ptr); + Marshal.ThrowExceptionForHR(hr); + object instance = ComWrappers.GetOrCreateObjectForComInstance(ptr, CreateObjectFlags.UniqueInstance); + Marshal.Release(ptr); + return instance; + } + + // Source-generated COM wrappers (UniqueInstance) release their underlying COM reference + // on Dispose — Marshal.FinalReleaseComObject does not work with them (SYSLIB1099). + private static void Release(object comObject) => (comObject as IDisposable)?.Dispose(); + + [LibraryImport("ole32.dll")] + private static partial int CoCreateInstance(in Guid rclsid, nint pUnkOuter, uint dwClsContext, in Guid riid, out nint ppv); + + [LibraryImport("shell32.dll", StringMarshalling = StringMarshalling.Utf16)] + private static partial int SHCreateItemFromParsingName(string pszPath, nint pbc, in Guid riid, out nint ppv); + + // IModalWindow — base of IFileDialog. Only Show is called. + [GeneratedComInterface] + [Guid("b4db1657-70d7-485e-8e3e-6fcb5a5c1802")] + internal partial interface IModalWindow + { + [PreserveSig] + int Show(nint parent); + } + + // IFileDialog : IModalWindow. Declared in full vtable order; unused slots use blittable + // [PreserveSig] signatures so we never generate marshalling for methods we don't call. + [GeneratedComInterface] + [Guid("42f85136-db7e-439c-85f1-e4075d135fc8")] + internal partial interface IFileDialog : IModalWindow + { + void SetFileTypes(uint cFileTypes, nint rgFilterSpec); + + void SetFileTypeIndex(uint iFileType); + + [PreserveSig] + int GetFileTypeIndex(out uint piFileType); + + [PreserveSig] + int Advise(nint pfde, out uint pdwCookie); + + [PreserveSig] + int Unadvise(uint dwCookie); + + void SetOptions(uint fos); + + [PreserveSig] + int GetOptions(out uint pfos); + + [PreserveSig] + int SetDefaultFolder(nint psi); + + void SetFolder(IShellItem psi); + + [PreserveSig] + int GetFolder(out nint ppsi); + + [PreserveSig] + int GetCurrentSelection(out nint ppsi); + + void SetFileName([MarshalAs(UnmanagedType.LPWStr)] string pszName); + + [PreserveSig] + int GetFileName(out nint pszName); + + void SetTitle([MarshalAs(UnmanagedType.LPWStr)] string pszTitle); + + [PreserveSig] + int SetOkButtonLabel(nint pszText); + + [PreserveSig] + int SetFileNameLabel(nint pszLabel); + + void GetResult(out nint ppsi); + + [PreserveSig] + int AddPlace(nint psi, int fdap); + + void SetDefaultExtension([MarshalAs(UnmanagedType.LPWStr)] string pszDefaultExtension); + + [PreserveSig] + int Close(int hr); + + [PreserveSig] + int SetClientGuid(in Guid guid); + + [PreserveSig] + int ClearClientData(); + + [PreserveSig] + int SetFilter(nint pFilter); + } + + // IFileOpenDialog : IFileDialog. Adds GetResults / GetSelectedItems (unused; single-select + // uses IFileDialog.GetResult). Slots declared to keep the type usable if ever needed. + [GeneratedComInterface] + [Guid("d57c7288-d4ad-4768-be02-9d969532d960")] + internal partial interface IFileOpenDialog : IFileDialog + { + [PreserveSig] + int GetResults(out nint ppenum); + + [PreserveSig] + int GetSelectedItems(out nint ppsai); + } + + // IFileSaveDialog : IFileDialog. Extra slots unused (SetSaveAsItem/SetProperties/...). + [GeneratedComInterface] + [Guid("84bccd23-5fde-4cdb-aea4-af64b83d78ab")] + internal partial interface IFileSaveDialog : IFileDialog + { + [PreserveSig] + int SetSaveAsItem(nint psi); + + [PreserveSig] + int SetProperties(nint pStore); + + [PreserveSig] + int SetCollectedProperties(nint pList, int fAppendDefault); + + [PreserveSig] + int GetProperties(out nint ppStore); + + [PreserveSig] + int ApplyProperties(nint psi, nint pStore, nint hwnd, nint pSink); + } + + [GeneratedComInterface] + [Guid("43826d1e-e718-42ee-bc55-a1e261c37bfe")] + internal partial interface IShellItem + { + [PreserveSig] + int BindToHandler(nint pbc, in Guid bhid, in Guid riid, out nint ppv); + + [PreserveSig] + int GetParent(out nint ppsi); + + void GetDisplayName(uint sigdnName, out nint ppszName); + + [PreserveSig] + int GetAttributes(uint sfgaoMask, out uint psfgaoAttribs); + + [PreserveSig] + int Compare(nint psi, uint hint, out int piOrder); + } +} diff --git a/QuickShell.Run/Interop/Win32Clipboard.cs b/QuickShell.Run/Interop/Win32Clipboard.cs new file mode 100644 index 00000000..b86141bf --- /dev/null +++ b/QuickShell.Run/Interop/Win32Clipboard.cs @@ -0,0 +1,160 @@ +using System.Runtime.InteropServices; + +namespace QuickShell.Interop; + +/// +/// Win32 clipboard text read/write via user32/kernel32 P/Invoke. Replaces +/// System.Windows.Forms.Clipboard so the app no longer forces UseWindowsForms. +/// Behaves like WinForms Clipboard.SetText/GetText — the OS owns the CF_UNICODETEXT +/// data after SetClipboardData, so it persists after the calling thread exits (no +/// WinRT-style Flush needed). Callers already marshal to an STA thread, which the +/// clipboard API requires. +/// +internal static partial class Win32Clipboard +{ + private const uint CF_UNICODETEXT = 13; + private const uint GMEM_MOVEABLE = 0x0002; + + public static string? GetText() + { + if (!IsClipboardFormatAvailable(CF_UNICODETEXT)) + { + return null; + } + + if (!OpenClipboard(0)) + { + return null; + } + + try + { + nint handle = GetClipboardData(CF_UNICODETEXT); + if (handle == 0) + { + return null; + } + + nint locked = GlobalLock(handle); + if (locked == 0) + { + return null; + } + + try + { + return Marshal.PtrToStringUni(locked); + } + finally + { + GlobalUnlock(handle); + } + } + finally + { + CloseClipboard(); + } + } + + public static bool SetText(string text) + { + ArgumentNullException.ThrowIfNull(text); + + if (!OpenClipboard(0)) + { + return false; + } + + try + { + if (!EmptyClipboard()) + { + return false; + } + + // Include the terminating null. GlobalAlloc GMEM_MOVEABLE is required for + // clipboard data; ownership transfers to the system on SetClipboardData. + int bytes = (text.Length + 1) * sizeof(char); + nint hGlobal = GlobalAlloc(GMEM_MOVEABLE, bytes); + if (hGlobal == 0) + { + return false; + } + + bool ownershipTransferred = false; + try + { + nint target = GlobalLock(hGlobal); + if (target == 0) + { + return false; + } + + try + { + Marshal.Copy(text.ToCharArray(), 0, target, text.Length); + Marshal.WriteInt16(target, text.Length * sizeof(char), 0); + } + finally + { + GlobalUnlock(hGlobal); + } + + if (SetClipboardData(CF_UNICODETEXT, hGlobal) == 0) + { + return false; + } + + ownershipTransferred = true; + return true; + } + finally + { + // Only free on failure; on success the system owns the memory. + if (!ownershipTransferred) + { + GlobalFree(hGlobal); + } + } + } + finally + { + CloseClipboard(); + } + } + + [LibraryImport("user32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + private static partial bool OpenClipboard(nint hWndNewOwner); + + [LibraryImport("user32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + private static partial bool CloseClipboard(); + + [LibraryImport("user32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + private static partial bool EmptyClipboard(); + + [LibraryImport("user32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + private static partial bool IsClipboardFormatAvailable(uint format); + + [LibraryImport("user32.dll", SetLastError = true)] + private static partial nint GetClipboardData(uint uFormat); + + [LibraryImport("user32.dll", SetLastError = true)] + private static partial nint SetClipboardData(uint uFormat, nint hMem); + + [LibraryImport("kernel32.dll", SetLastError = true)] + private static partial nint GlobalAlloc(uint uFlags, nint dwBytes); + + [LibraryImport("kernel32.dll", SetLastError = true)] + private static partial nint GlobalFree(nint hMem); + + [LibraryImport("kernel32.dll", SetLastError = true)] + private static partial nint GlobalLock(nint hMem); + + [LibraryImport("kernel32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + private static partial bool GlobalUnlock(nint hMem); +} diff --git a/QuickShell.Run/QuickShell.Run.csproj b/QuickShell.Run/QuickShell.Run.csproj index b0fb9025..9108ba03 100644 --- a/QuickShell.Run/QuickShell.Run.csproj +++ b/QuickShell.Run/QuickShell.Run.csproj @@ -7,16 +7,12 @@ enable enable true - true + + true x64;ARM64 $(Platform) - - - - - true diff --git a/QuickShell.Run/Services/FolderPickerService.cs b/QuickShell.Run/Services/FolderPickerService.cs index 5072d83f..2e656a09 100644 --- a/QuickShell.Run/Services/FolderPickerService.cs +++ b/QuickShell.Run/Services/FolderPickerService.cs @@ -1,6 +1,7 @@ using System.IO; using System.Runtime.InteropServices; using System.Threading; +using QuickShell.Interop; namespace QuickShell.Services; @@ -15,98 +16,41 @@ internal static class FolderPickerService if (Thread.CurrentThread.GetApartmentState() == ApartmentState.STA) { - return PickFolderOnStaThread(initialDirectory, ownerHandle, dialogHwnd: null); + return PickFolderOnStaThread(initialDirectory, ownerHandle); } string? selected = null; - var dialogHwnd = new DialogHwndBox(); - var thread = new Thread(() => selected = PickFolderOnStaThread(initialDirectory, ownerHandle, dialogHwnd)) + var thread = new Thread(() => selected = PickFolderOnStaThread(initialDirectory, ownerHandle)) { IsBackground = true, }; thread.SetApartmentState(ApartmentState.STA); thread.Start(); - // Happy path: PickFolderOnStaThread auto-closes at DialogTimeout and the STA thread - // exits soon after. JoinGracePeriod covers dismiss + ShowDialog unwind. If the thread - // is still alive, force another WM_CLOSE to the captured dialog hwnd, wait briefly, - // then cancel so the caller never hangs and we do not leave a live orphan dialog behind. + // The modal IFileDialog blocks the STA thread until dismissed. DialogTimeout + + // JoinGracePeriod bounds a stuck dialog; on timeout, post WM_CLOSE to the foreground + // window (the modal dialog is topmost) so the thread unblocks and the caller never hangs. if (thread.Join(DialogTimeout + JoinGracePeriod)) { return selected; } - TryCloseDialog(dialogHwnd.Value, ownerHandle); + TryCloseForegroundDialog(ownerHandle); return thread.Join(JoinGracePeriod) ? selected : null; } - private static string? PickFolderOnStaThread(string? initialDirectory, nint ownerHandle, DialogHwndBox? dialogHwnd) + private static string? PickFolderOnStaThread(string? initialDirectory, nint ownerHandle) { - using var dialog = new System.Windows.Forms.FolderBrowserDialog - { - UseDescriptionForTitle = true, - Description = "Select a folder for this shortcut.", - ShowNewFolderButton = true, - AutoUpgradeEnabled = true, - OkRequiresInteraction = false, - }; - - if (!string.IsNullOrWhiteSpace(initialDirectory) && Directory.Exists(initialDirectory)) - { - dialog.InitialDirectory = initialDirectory; - dialog.SelectedPath = initialDirectory; - } - - // Capture the dialog hwnd once it becomes foreground (same STA message loop as - // ShowDialog) so auto-close and the caller-side safety net can PostMessage WM_CLOSE - // without EnumThreadWindows / GetCurrentThreadId. - using var captureHwnd = new System.Windows.Forms.Timer - { - Interval = 50, - }; - captureHwnd.Tick += (_, _) => - { - var hwnd = GetForegroundWindow(); - if (hwnd == 0 || hwnd == ownerHandle) - { - return; - } - - if (dialogHwnd is not null) - { - dialogHwnd.Value = hwnd; - } - - captureHwnd.Stop(); - }; - - // Auto-close the modal dialog if left open past the timeout so the STA thread exits - // and repeated calls do not stack orphaned dialogs. - using var autoClose = new System.Windows.Forms.Timer - { - Interval = (int)DialogTimeout.TotalMilliseconds, - }; - autoClose.Tick += (_, _) => - { - autoClose.Stop(); - var captured = dialogHwnd?.Value ?? 0; - TryCloseDialog(captured != 0 ? captured : GetForegroundWindow(), ownerHandle); - }; - - captureHwnd.Start(); - autoClose.Start(); - - var owner = ownerHandle != 0 ? new NativeWindowWrapper(ownerHandle) : null; - var result = dialog.ShowDialog(owner); - captureHwnd.Stop(); - autoClose.Stop(); - return result == System.Windows.Forms.DialogResult.OK - ? dialog.SelectedPath + var initial = !string.IsNullOrWhiteSpace(initialDirectory) && Directory.Exists(initialDirectory) + ? initialDirectory : null; + + return ShellFileDialog.PickFolder(ownerHandle, initial); } - private static void TryCloseDialog(nint hwnd, nint ownerHandle) + private static void TryCloseForegroundDialog(nint ownerHandle) { + var hwnd = GetForegroundWindow(); if (hwnd == 0 || hwnd == ownerHandle) { return; @@ -123,20 +67,4 @@ private static void TryCloseDialog(nint hwnd, nint ownerHandle) [DllImport("user32.dll")] [return: MarshalAs(UnmanagedType.Bool)] private static extern bool PostMessage(nint hWnd, int msg, nint wParam, nint lParam); - - private sealed class DialogHwndBox - { - private nint _value; - - public nint Value - { - get => Volatile.Read(ref _value); - set => Volatile.Write(ref _value, value); - } - } - - private sealed class NativeWindowWrapper(nint handle) : System.Windows.Forms.IWin32Window - { - public nint Handle { get; } = handle; - } -} \ No newline at end of file +} diff --git a/QuickShell.Run/Services/StaClipboard.cs b/QuickShell.Run/Services/StaClipboard.cs index 9abbca3f..b67ef696 100644 --- a/QuickShell.Run/Services/StaClipboard.cs +++ b/QuickShell.Run/Services/StaClipboard.cs @@ -1,5 +1,5 @@ using System.Threading; -using System.Windows.Forms; +using QuickShell.Interop; namespace QuickShell.Services; @@ -16,7 +16,7 @@ public static bool TrySetText(string text) bool SetText() { - try { Clipboard.SetText(text); return true; } + try { return Win32Clipboard.SetText(text); } catch { return false; } } } diff --git a/QuickShell/ILLink.Descriptors.xml b/QuickShell/ILLink.Descriptors.xml new file mode 100644 index 00000000..65cf232f --- /dev/null +++ b/QuickShell/ILLink.Descriptors.xml @@ -0,0 +1,15 @@ + + + + + + diff --git a/QuickShell/Interop/ShellFileDialog.cs b/QuickShell/Interop/ShellFileDialog.cs new file mode 100644 index 00000000..b8dafed9 --- /dev/null +++ b/QuickShell/Interop/ShellFileDialog.cs @@ -0,0 +1,360 @@ +using System.IO; +using System.Runtime.InteropServices; +using System.Runtime.InteropServices.Marshalling; + +namespace QuickShell.Interop; + +/// +/// Minimal wrapper over the Win32 shell Common Item Dialog (IFileOpenDialog / +/// IFileSaveDialog). Replaces the WinForms FolderBrowserDialog / OpenFileDialog / +/// SaveFileDialog so the packaged app no longer forces UseWindowsForms (which blocks +/// trimming). Calls are synchronous and modal — IFileDialog.Show blocks the calling +/// thread exactly like the old ShowDialog, so the existing background-STA-thread +/// callers keep working unchanged. +/// +/// Uses source-generated COM () so the +/// interop stays AOT/trim-safe (the project is IsAotCompatible=true). The full +/// IFileDialog vtable is declared in order; slots we never call are declared with +/// blittable [PreserveSig] signatures purely to preserve vtable layout. +/// +internal static partial class ShellFileDialog +{ + // A single StrategyBasedComWrappers marshals the raw IUnknown pointers returned by + // CoCreateInstance / SHCreateItemFromParsingName into our generated interfaces. + private static readonly StrategyBasedComWrappers ComWrappers = new(); + + private static readonly Guid CLSID_FileOpenDialog = new("DC1C5A9C-E88A-4dde-A5A1-60F82A20AEF7"); + private static readonly Guid CLSID_FileSaveDialog = new("C0B4E2F3-BA21-4773-8DBA-335EC946EB8B"); + private static readonly Guid IID_IFileOpenDialog = new("d57c7288-d4ad-4768-be02-9d969532d960"); + private static readonly Guid IID_IFileSaveDialog = new("84bccd23-5fde-4cdb-aea4-af64b83d78ab"); + private static readonly Guid IID_IShellItem = new("43826d1e-e718-42ee-bc55-a1e261c37bfe"); + + private const uint FOS_OVERWRITEPROMPT = 0x00000002; + private const uint FOS_NOCHANGEDIR = 0x00000008; + private const uint FOS_PICKFOLDERS = 0x00000020; + private const uint FOS_FORCEFILESYSTEM = 0x00000040; + private const uint FOS_PATHMUSTEXIST = 0x00000800; + private const uint FOS_FILEMUSTEXIST = 0x00001000; + + private const uint SIGDN_FILESYSPATH = 0x80058000; + + private const uint CLSCTX_INPROC_SERVER = 0x1; + + // HRESULT_FROM_WIN32(ERROR_CANCELLED) — returned by Show when the user cancels. + private const int ERROR_CANCELLED_HRESULT = unchecked((int)0x800704C7); + + /// Modern folder picker (IFileOpenDialog with FOS_PICKFOLDERS). + public static string? PickFolder(nint owner, string? initialDirectory) + { + var dialog = (IFileOpenDialog)CreateInstance(CLSID_FileOpenDialog, IID_IFileOpenDialog); + try + { + dialog.SetOptions(FOS_PICKFOLDERS | FOS_FORCEFILESYSTEM | FOS_PATHMUSTEXIST | FOS_NOCHANGEDIR); + ApplyInitialDirectory(dialog, initialDirectory); + return ShowAndGetPath(dialog, owner); + } + finally + { + Release(dialog); + } + } + + /// Open-file picker. pairs display name to a + /// semicolon-delimited pattern (e.g. "JSON files (*.json)" -> "*.json"). + public static string? PickOpenFile(nint owner, string title, (string Name, string Spec)[] filters, string? defaultExt, string? initialDirectory) + { + var dialog = (IFileOpenDialog)CreateInstance(CLSID_FileOpenDialog, IID_IFileOpenDialog); + try + { + dialog.SetOptions(FOS_FORCEFILESYSTEM | FOS_FILEMUSTEXIST | FOS_PATHMUSTEXIST | FOS_NOCHANGEDIR); + dialog.SetTitle(title); + ApplyFilters(dialog, filters, defaultExt); + ApplyInitialDirectory(dialog, initialDirectory); + return ShowAndGetPath(dialog, owner); + } + finally + { + Release(dialog); + } + } + + /// Save-file picker with overwrite prompt and a suggested file name. + public static string? PickSaveFile(nint owner, string title, (string Name, string Spec)[] filters, string? defaultExt, string? defaultFileName, string? initialDirectory) + { + var dialog = (IFileSaveDialog)CreateInstance(CLSID_FileSaveDialog, IID_IFileSaveDialog); + try + { + dialog.SetOptions(FOS_FORCEFILESYSTEM | FOS_OVERWRITEPROMPT | FOS_PATHMUSTEXIST | FOS_NOCHANGEDIR); + dialog.SetTitle(title); + ApplyFilters(dialog, filters, defaultExt); + if (!string.IsNullOrEmpty(defaultFileName)) + { + dialog.SetFileName(defaultFileName); + } + + ApplyInitialDirectory(dialog, initialDirectory); + return ShowAndGetPath(dialog, owner); + } + finally + { + Release(dialog); + } + } + + private static string? ShowAndGetPath(IFileDialog dialog, nint owner) + { + int hr = dialog.Show(owner); + if (hr == ERROR_CANCELLED_HRESULT) + { + return null; + } + + Marshal.ThrowExceptionForHR(hr); + + dialog.GetResult(out nint itemPtr); + if (itemPtr == 0) + { + return null; + } + + var item = (IShellItem)ComWrappers.GetOrCreateObjectForComInstance(itemPtr, CreateObjectFlags.UniqueInstance); + Marshal.Release(itemPtr); + try + { + item.GetDisplayName(SIGDN_FILESYSPATH, out nint pathPtr); + if (pathPtr == 0) + { + return null; + } + + try + { + return Marshal.PtrToStringUni(pathPtr); + } + finally + { + Marshal.FreeCoTaskMem(pathPtr); + } + } + finally + { + Release(item); + } + } + + private static void ApplyInitialDirectory(IFileDialog dialog, string? initialDirectory) + { + if (string.IsNullOrWhiteSpace(initialDirectory) || !Directory.Exists(initialDirectory)) + { + return; + } + + int hr = SHCreateItemFromParsingName(initialDirectory!, 0, in IID_IShellItem, out nint itemPtr); + if (hr < 0 || itemPtr == 0) + { + return; // Non-fatal: fall back to the shell's default folder. + } + + var item = (IShellItem)ComWrappers.GetOrCreateObjectForComInstance(itemPtr, CreateObjectFlags.UniqueInstance); + Marshal.Release(itemPtr); + try + { + dialog.SetFolder(item); + } + finally + { + Release(item); + } + } + + private static void ApplyFilters(IFileDialog dialog, (string Name, string Spec)[] filters, string? defaultExt) + { + if (filters.Length == 0) + { + return; + } + + // COMDLG_FILTERSPEC is { LPWSTR pszName; LPWSTR pszSpec; }. Build the native array + // by hand so the interface method stays blittable (no struct marshalling in the vtable). + nint block = Marshal.AllocCoTaskMem(filters.Length * nint.Size * 2); + var strings = new List(filters.Length * 2); + try + { + for (int i = 0; i < filters.Length; i++) + { + nint namePtr = Marshal.StringToCoTaskMemUni(filters[i].Name); + nint specPtr = Marshal.StringToCoTaskMemUni(filters[i].Spec); + strings.Add(namePtr); + strings.Add(specPtr); + Marshal.WriteIntPtr(block, i * nint.Size * 2, namePtr); + Marshal.WriteIntPtr(block, (i * nint.Size * 2) + nint.Size, specPtr); + } + + dialog.SetFileTypes((uint)filters.Length, block); + dialog.SetFileTypeIndex(1); + } + finally + { + foreach (nint s in strings) + { + Marshal.FreeCoTaskMem(s); + } + + Marshal.FreeCoTaskMem(block); + } + + if (!string.IsNullOrEmpty(defaultExt)) + { + dialog.SetDefaultExtension(defaultExt); + } + } + + private static object CreateInstance(Guid clsid, Guid iid) + { + int hr = CoCreateInstance(in clsid, 0, CLSCTX_INPROC_SERVER, in iid, out nint ptr); + Marshal.ThrowExceptionForHR(hr); + object instance = ComWrappers.GetOrCreateObjectForComInstance(ptr, CreateObjectFlags.UniqueInstance); + Marshal.Release(ptr); + return instance; + } + + // Source-generated COM wrappers (UniqueInstance) release their underlying COM reference + // on Dispose — Marshal.FinalReleaseComObject does not work with them (SYSLIB1099). + private static void Release(object comObject) => (comObject as IDisposable)?.Dispose(); + + [LibraryImport("ole32.dll")] + private static partial int CoCreateInstance(in Guid rclsid, nint pUnkOuter, uint dwClsContext, in Guid riid, out nint ppv); + + [LibraryImport("shell32.dll", StringMarshalling = StringMarshalling.Utf16)] + private static partial int SHCreateItemFromParsingName(string pszPath, nint pbc, in Guid riid, out nint ppv); + + // IModalWindow — base of IFileDialog. Only Show is called. + [GeneratedComInterface] + [Guid("b4db1657-70d7-485e-8e3e-6fcb5a5c1802")] + internal partial interface IModalWindow + { + [PreserveSig] + int Show(nint parent); + } + + // IFileDialog : IModalWindow. Declared in full vtable order; unused slots use blittable + // [PreserveSig] signatures so we never generate marshalling for methods we don't call. + [GeneratedComInterface] + [Guid("42f85136-db7e-439c-85f1-e4075d135fc8")] + internal partial interface IFileDialog : IModalWindow + { + void SetFileTypes(uint cFileTypes, nint rgFilterSpec); + + void SetFileTypeIndex(uint iFileType); + + [PreserveSig] + int GetFileTypeIndex(out uint piFileType); + + [PreserveSig] + int Advise(nint pfde, out uint pdwCookie); + + [PreserveSig] + int Unadvise(uint dwCookie); + + void SetOptions(uint fos); + + [PreserveSig] + int GetOptions(out uint pfos); + + [PreserveSig] + int SetDefaultFolder(nint psi); + + void SetFolder(IShellItem psi); + + [PreserveSig] + int GetFolder(out nint ppsi); + + [PreserveSig] + int GetCurrentSelection(out nint ppsi); + + void SetFileName([MarshalAs(UnmanagedType.LPWStr)] string pszName); + + [PreserveSig] + int GetFileName(out nint pszName); + + void SetTitle([MarshalAs(UnmanagedType.LPWStr)] string pszTitle); + + [PreserveSig] + int SetOkButtonLabel(nint pszText); + + [PreserveSig] + int SetFileNameLabel(nint pszLabel); + + void GetResult(out nint ppsi); + + [PreserveSig] + int AddPlace(nint psi, int fdap); + + void SetDefaultExtension([MarshalAs(UnmanagedType.LPWStr)] string pszDefaultExtension); + + [PreserveSig] + int Close(int hr); + + [PreserveSig] + int SetClientGuid(in Guid guid); + + [PreserveSig] + int ClearClientData(); + + [PreserveSig] + int SetFilter(nint pFilter); + } + + // IFileOpenDialog : IFileDialog. Adds GetResults / GetSelectedItems (unused; single-select + // uses IFileDialog.GetResult). Slots declared to keep the type usable if ever needed. + [GeneratedComInterface] + [Guid("d57c7288-d4ad-4768-be02-9d969532d960")] + internal partial interface IFileOpenDialog : IFileDialog + { + [PreserveSig] + int GetResults(out nint ppenum); + + [PreserveSig] + int GetSelectedItems(out nint ppsai); + } + + // IFileSaveDialog : IFileDialog. Extra slots unused (SetSaveAsItem/SetProperties/...). + [GeneratedComInterface] + [Guid("84bccd23-5fde-4cdb-aea4-af64b83d78ab")] + internal partial interface IFileSaveDialog : IFileDialog + { + [PreserveSig] + int SetSaveAsItem(nint psi); + + [PreserveSig] + int SetProperties(nint pStore); + + [PreserveSig] + int SetCollectedProperties(nint pList, int fAppendDefault); + + [PreserveSig] + int GetProperties(out nint ppStore); + + [PreserveSig] + int ApplyProperties(nint psi, nint pStore, nint hwnd, nint pSink); + } + + [GeneratedComInterface] + [Guid("43826d1e-e718-42ee-bc55-a1e261c37bfe")] + internal partial interface IShellItem + { + [PreserveSig] + int BindToHandler(nint pbc, in Guid bhid, in Guid riid, out nint ppv); + + [PreserveSig] + int GetParent(out nint ppsi); + + void GetDisplayName(uint sigdnName, out nint ppszName); + + [PreserveSig] + int GetAttributes(uint sfgaoMask, out uint psfgaoAttribs); + + [PreserveSig] + int Compare(nint psi, uint hint, out int piOrder); + } +} diff --git a/QuickShell/Interop/Win32Clipboard.cs b/QuickShell/Interop/Win32Clipboard.cs new file mode 100644 index 00000000..b86141bf --- /dev/null +++ b/QuickShell/Interop/Win32Clipboard.cs @@ -0,0 +1,160 @@ +using System.Runtime.InteropServices; + +namespace QuickShell.Interop; + +/// +/// Win32 clipboard text read/write via user32/kernel32 P/Invoke. Replaces +/// System.Windows.Forms.Clipboard so the app no longer forces UseWindowsForms. +/// Behaves like WinForms Clipboard.SetText/GetText — the OS owns the CF_UNICODETEXT +/// data after SetClipboardData, so it persists after the calling thread exits (no +/// WinRT-style Flush needed). Callers already marshal to an STA thread, which the +/// clipboard API requires. +/// +internal static partial class Win32Clipboard +{ + private const uint CF_UNICODETEXT = 13; + private const uint GMEM_MOVEABLE = 0x0002; + + public static string? GetText() + { + if (!IsClipboardFormatAvailable(CF_UNICODETEXT)) + { + return null; + } + + if (!OpenClipboard(0)) + { + return null; + } + + try + { + nint handle = GetClipboardData(CF_UNICODETEXT); + if (handle == 0) + { + return null; + } + + nint locked = GlobalLock(handle); + if (locked == 0) + { + return null; + } + + try + { + return Marshal.PtrToStringUni(locked); + } + finally + { + GlobalUnlock(handle); + } + } + finally + { + CloseClipboard(); + } + } + + public static bool SetText(string text) + { + ArgumentNullException.ThrowIfNull(text); + + if (!OpenClipboard(0)) + { + return false; + } + + try + { + if (!EmptyClipboard()) + { + return false; + } + + // Include the terminating null. GlobalAlloc GMEM_MOVEABLE is required for + // clipboard data; ownership transfers to the system on SetClipboardData. + int bytes = (text.Length + 1) * sizeof(char); + nint hGlobal = GlobalAlloc(GMEM_MOVEABLE, bytes); + if (hGlobal == 0) + { + return false; + } + + bool ownershipTransferred = false; + try + { + nint target = GlobalLock(hGlobal); + if (target == 0) + { + return false; + } + + try + { + Marshal.Copy(text.ToCharArray(), 0, target, text.Length); + Marshal.WriteInt16(target, text.Length * sizeof(char), 0); + } + finally + { + GlobalUnlock(hGlobal); + } + + if (SetClipboardData(CF_UNICODETEXT, hGlobal) == 0) + { + return false; + } + + ownershipTransferred = true; + return true; + } + finally + { + // Only free on failure; on success the system owns the memory. + if (!ownershipTransferred) + { + GlobalFree(hGlobal); + } + } + } + finally + { + CloseClipboard(); + } + } + + [LibraryImport("user32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + private static partial bool OpenClipboard(nint hWndNewOwner); + + [LibraryImport("user32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + private static partial bool CloseClipboard(); + + [LibraryImport("user32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + private static partial bool EmptyClipboard(); + + [LibraryImport("user32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + private static partial bool IsClipboardFormatAvailable(uint format); + + [LibraryImport("user32.dll", SetLastError = true)] + private static partial nint GetClipboardData(uint uFormat); + + [LibraryImport("user32.dll", SetLastError = true)] + private static partial nint SetClipboardData(uint uFormat, nint hMem); + + [LibraryImport("kernel32.dll", SetLastError = true)] + private static partial nint GlobalAlloc(uint uFlags, nint dwBytes); + + [LibraryImport("kernel32.dll", SetLastError = true)] + private static partial nint GlobalFree(nint hMem); + + [LibraryImport("kernel32.dll", SetLastError = true)] + private static partial nint GlobalLock(nint hMem); + + [LibraryImport("kernel32.dll", SetLastError = true)] + [return: MarshalAs(UnmanagedType.Bool)] + private static partial bool GlobalUnlock(nint hMem); +} diff --git a/QuickShell/QuickShell.csproj b/QuickShell/QuickShell.csproj index 3d555539..f2adba76 100644 --- a/QuickShell/QuickShell.csproj +++ b/QuickShell/QuickShell.csproj @@ -14,8 +14,8 @@ CN=31D3C278-3D55-4A42-BA6A-24E16D15B863 true enable - true - PerMonitorV2 + + true @@ -105,8 +105,12 @@ true 2 - - IL2081;$(WarningsNotAsErrors) + + IL2081;IL2104;$(WarningsNotAsErrors) true @@ -150,15 +154,19 @@ false - + - false - false - + true + true + en;fr;de;it;ru;zh-Hans;zh-Hant;es;ja;ko;pt-BR + + + + diff --git a/QuickShell/Services/FolderPickerService.cs b/QuickShell/Services/FolderPickerService.cs index 0218ae62..844027c9 100644 --- a/QuickShell/Services/FolderPickerService.cs +++ b/QuickShell/Services/FolderPickerService.cs @@ -1,5 +1,6 @@ using System.Runtime.InteropServices; using System.Threading; +using QuickShell.Interop; namespace QuickShell.Services; @@ -14,109 +15,46 @@ internal static class FolderPickerService if (Thread.CurrentThread.GetApartmentState() == ApartmentState.STA) { - return PickFolderOnStaThread(initialDirectory, ownerHandle, dialogHwnd: null); + return PickFolderOnStaThread(initialDirectory, ownerHandle); } string? selected = null; - var dialogHwnd = new DialogHwndBox(); - var thread = new Thread(() => selected = PickFolderOnStaThread(initialDirectory, ownerHandle, dialogHwnd)) + var thread = new Thread(() => selected = PickFolderOnStaThread(initialDirectory, ownerHandle)) { IsBackground = true, }; thread.SetApartmentState(ApartmentState.STA); thread.Start(); - // Happy path: PickFolderOnStaThread auto-closes at DialogTimeout and the STA thread - // exits soon after. JoinGracePeriod covers dismiss + ShowDialog unwind. If the thread - // is still alive, force another WM_CLOSE to the captured dialog hwnd, wait briefly, - // then cancel so the caller never hangs and we do not leave a live orphan dialog behind. + // Happy path: the modal IFileDialog blocks the STA thread until the user dismisses it, + // then the thread exits. DialogTimeout + JoinGracePeriod bounds a stuck dialog; on + // timeout, post WM_CLOSE to the foreground window (the modal dialog is topmost) so the + // STA thread unblocks and the caller never hangs on a live orphan dialog. if (thread.Join(DialogTimeout + JoinGracePeriod)) { return selected; } - TryCloseDialog(dialogHwnd.Value, ownerHandle); + TryCloseForegroundDialog(ownerHandle); return thread.Join(JoinGracePeriod) ? selected : null; } - private static string? PickFolderOnStaThread(string? initialDirectory, nint ownerHandle, DialogHwndBox? dialogHwnd) + private static string? PickFolderOnStaThread(string? initialDirectory, nint ownerHandle) { - using var dialog = new System.Windows.Forms.FolderBrowserDialog + var initial = initialDirectory; + if (!string.IsNullOrWhiteSpace(initialDirectory) + && WslPathResolver.TryParse(initialDirectory, out var wsl) + && !string.IsNullOrWhiteSpace(wsl.UncPath)) { - UseDescriptionForTitle = true, - Description = "Select a folder for this shortcut. Tab through the dialog; type a path in the address bar to jump to a folder.", - ShowNewFolderButton = true, - AutoUpgradeEnabled = true, - OkRequiresInteraction = false, - }; - - if (!string.IsNullOrWhiteSpace(initialDirectory)) - { - var initial = initialDirectory; - if (WslPathResolver.TryParse(initialDirectory, out var wsl) && !string.IsNullOrWhiteSpace(wsl.UncPath)) - { - initial = wsl.UncPath; - } - - if (Directory.Exists(initial)) - { - dialog.InitialDirectory = initial; - dialog.SelectedPath = initial; - } + initial = wsl.UncPath; } - // Capture the dialog hwnd once it becomes foreground (same STA message loop as - // ShowDialog) so auto-close and the caller-side safety net can PostMessage WM_CLOSE - // without EnumThreadWindows / GetCurrentThreadId. - using var captureHwnd = new System.Windows.Forms.Timer - { - Interval = 50, - }; - captureHwnd.Tick += (_, _) => - { - var hwnd = GetForegroundWindow(); - if (hwnd == 0 || hwnd == ownerHandle) - { - return; - } - - if (dialogHwnd is not null) - { - dialogHwnd.Value = hwnd; - } - - captureHwnd.Stop(); - }; - - // Auto-close the modal dialog if it is left open past the timeout. Without this the - // background STA thread keeps the native dialog alive after the caller gives up, and - // repeated calls stack orphaned dialogs. The Timer ticks on this thread's ShowDialog - // message loop; WM_CLOSE dismisses the dialog and lets the thread exit. - using var autoClose = new System.Windows.Forms.Timer - { - Interval = (int)DialogTimeout.TotalMilliseconds, - }; - autoClose.Tick += (_, _) => - { - autoClose.Stop(); - var captured = dialogHwnd?.Value ?? 0; - TryCloseDialog(captured != 0 ? captured : GetForegroundWindow(), ownerHandle); - }; - - captureHwnd.Start(); - autoClose.Start(); - - var owner = ownerHandle != 0 ? new NativeWindowWrapper(ownerHandle) : null; - var result = dialog.ShowDialog(owner); - captureHwnd.Stop(); - autoClose.Stop(); - return result == System.Windows.Forms.DialogResult.OK - ? dialog.SelectedPath - : null; + return ShellFileDialog.PickFolder(ownerHandle, initial); } - private static void TryCloseDialog(nint hwnd, nint ownerHandle) + private static void TryCloseForegroundDialog(nint ownerHandle) { + var hwnd = GetForegroundWindow(); if (hwnd == 0 || hwnd == ownerHandle) { return; @@ -133,20 +71,4 @@ private static void TryCloseDialog(nint hwnd, nint ownerHandle) [DllImport("user32.dll")] [return: MarshalAs(UnmanagedType.Bool)] private static extern bool PostMessage(nint hWnd, int msg, nint wParam, nint lParam); - - private sealed class DialogHwndBox - { - private nint _value; - - public nint Value - { - get => Volatile.Read(ref _value); - set => Volatile.Write(ref _value, value); - } - } - - private sealed class NativeWindowWrapper(nint handle) : System.Windows.Forms.IWin32Window - { - public nint Handle { get; } = handle; - } -} \ No newline at end of file +} diff --git a/QuickShell/Services/ShortcutFilePickerService.cs b/QuickShell/Services/ShortcutFilePickerService.cs index 56a19787..8a3a2db3 100644 --- a/QuickShell/Services/ShortcutFilePickerService.cs +++ b/QuickShell/Services/ShortcutFilePickerService.cs @@ -1,98 +1,68 @@ -using System.Runtime.InteropServices; using System.Threading; +using System.Runtime.InteropServices; +using QuickShell.Interop; namespace QuickShell.Services; internal static class ShortcutFilePickerService { - private const string JsonFilter = "JSON files (*.json)|*.json|All files (*.*)|*.*"; + private static readonly (string Name, string Spec)[] JsonFilters = + { + ("JSON files (*.json)", "*.json"), + ("All files (*.*)", "*.*"), + }; + + private static readonly (string Name, string Spec)[] ExecutableFilters = + { + ("Applications (*.exe;*.lnk;*.bat;*.cmd)", "*.exe;*.lnk;*.bat;*.cmd"), + ("All files (*.*)", "*.*"), + }; + private static readonly TimeSpan DialogTimeout = TimeSpan.FromMinutes(2); public static string? PickExportFile(IQuickShellServices services) { ArgumentNullException.ThrowIfNull(services); var defaultName = $"quickshell-workspaces-{DateTime.Now:yyyyMMdd-HHmmss}.json"; - var initialDirectory = services.Shortcuts.ConfigDirectory; - - return RunOnStaThread(() => - { - using var dialog = new System.Windows.Forms.SaveFileDialog - { - Title = $"Export {QuickShellBrand.DisplayName} workspaces", - Filter = JsonFilter, - DefaultExt = "json", - AddExtension = true, - FileName = defaultName, - OverwritePrompt = true, - }; - - if (Directory.Exists(initialDirectory)) - { - dialog.InitialDirectory = initialDirectory; - } - - return ShowDialog(dialog); - }); + var initialDirectory = DirectoryOrNull(services.Shortcuts.ConfigDirectory); + + return RunOnStaThread(() => ShellFileDialog.PickSaveFile( + GetForegroundWindow(), + $"Export {QuickShellBrand.DisplayName} workspaces", + JsonFilters, + defaultExt: "json", + defaultFileName: defaultName, + initialDirectory: initialDirectory)); } public static string? PickImportFile(IQuickShellServices services) { ArgumentNullException.ThrowIfNull(services); - var initialDirectory = services.Shortcuts.ConfigDirectory; - - return RunOnStaThread(() => - { - using var dialog = new System.Windows.Forms.OpenFileDialog - { - Title = $"Import {QuickShellBrand.DisplayName} workspaces", - Filter = JsonFilter, - DefaultExt = "json", - CheckFileExists = true, - Multiselect = false, - }; - - if (Directory.Exists(initialDirectory)) - { - dialog.InitialDirectory = initialDirectory; - } - - return ShowDialog(dialog); - }); + var initialDirectory = DirectoryOrNull(services.Shortcuts.ConfigDirectory); + + return RunOnStaThread(() => ShellFileDialog.PickOpenFile( + GetForegroundWindow(), + $"Import {QuickShellBrand.DisplayName} workspaces", + JsonFilters, + defaultExt: "json", + initialDirectory: initialDirectory)); } public static string? PickExecutableFile() { var programFiles = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles); - var initialDirectory = Directory.Exists(programFiles) ? programFiles : null; - - return RunOnStaThread(() => - { - using var dialog = new System.Windows.Forms.OpenFileDialog - { - Title = "Choose companion app", - Filter = "Applications (*.exe;*.lnk;*.bat;*.cmd)|*.exe;*.lnk;*.bat;*.cmd|All files (*.*)|*.*", - DefaultExt = "exe", - CheckFileExists = true, - Multiselect = false, - }; - - if (initialDirectory is not null) - { - dialog.InitialDirectory = initialDirectory; - } - - return ShowDialog(dialog); - }); + var initialDirectory = DirectoryOrNull(programFiles); + + return RunOnStaThread(() => ShellFileDialog.PickOpenFile( + GetForegroundWindow(), + "Choose companion app", + ExecutableFilters, + defaultExt: "exe", + initialDirectory: initialDirectory)); } - private static string? ShowDialog(System.Windows.Forms.FileDialog dialog) - { - var ownerHandle = GetForegroundWindow(); - var owner = ownerHandle != 0 ? new NativeWindowWrapper(ownerHandle) : null; - return dialog.ShowDialog(owner) == System.Windows.Forms.DialogResult.OK - ? dialog.FileName - : null; - } + private static string? DirectoryOrNull(string? path) => + !string.IsNullOrWhiteSpace(path) && Directory.Exists(path) ? path : null; private static string? RunOnStaThread(Func action) { @@ -114,9 +84,4 @@ internal static class ShortcutFilePickerService [DllImport("user32.dll")] private static extern nint GetForegroundWindow(); - - private sealed class NativeWindowWrapper(nint handle) : System.Windows.Forms.IWin32Window - { - public nint Handle { get; } = handle; - } } diff --git a/QuickShell/Services/StaClipboard.cs b/QuickShell/Services/StaClipboard.cs index 247c5a03..658f8bd5 100644 --- a/QuickShell/Services/StaClipboard.cs +++ b/QuickShell/Services/StaClipboard.cs @@ -1,5 +1,5 @@ using System.Threading; -using System.Windows.Forms; +using QuickShell.Interop; namespace QuickShell.Services; @@ -29,9 +29,13 @@ public static bool TrySetText(string text) private static bool SetTextOnStaThread(string text) { - try { Clipboard.SetText(text); return true; } + try { return Win32Clipboard.SetText(text); } catch { return false; } } - private static string? ReadTextOnStaThread() => Clipboard.ContainsText() ? Clipboard.GetText() : null; + private static string? ReadTextOnStaThread() + { + try { return Win32Clipboard.GetText(); } + catch { return null; } + } } From d330c690bf9ea2c7993cf4c4b19eccb68fc01d26 Mon Sep 17 00:00:00 2001 From: tonythethompson Date: Sat, 25 Jul 2026 05:22:45 -0700 Subject: [PATCH 2/8] Address CodeRabbit review: share interop and harden dialogs. Move ShellFileDialog/Win32Clipboard into Core, bound clipboard reads with GlobalSize, retry OpenClipboard, close timed-out pickers via the STA thread's dialog HWND, and capture file-picker owners before starting the STA thread. Co-authored-by: Cursor --- .../Interop/ShellFileDialog.cs | 0 QuickShell.Core/Interop/StaDialogCloser.cs | 96 +++++ .../Interop/Win32Clipboard.cs | 43 ++- QuickShell.Core/Models/FormChoiceJson.cs | 2 +- QuickShell.Core/Models/TaskTypeChoiceJson.cs | 2 +- QuickShell.Core/QuickShell.Core.csproj | 2 + QuickShell.Run/QuickShell.Run.csproj | 2 - .../Services/FolderPickerService.cs | 30 +- QuickShell/Interop/ShellFileDialog.cs | 360 ------------------ QuickShell/Interop/Win32Clipboard.cs | 160 -------- QuickShell/QuickShell.csproj | 2 - QuickShell/Services/FolderPickerService.cs | 29 +- .../Services/ShortcutFilePickerService.cs | 67 ++-- 13 files changed, 201 insertions(+), 594 deletions(-) rename {QuickShell.Run => QuickShell.Core}/Interop/ShellFileDialog.cs (100%) create mode 100644 QuickShell.Core/Interop/StaDialogCloser.cs rename {QuickShell.Run => QuickShell.Core}/Interop/Win32Clipboard.cs (77%) delete mode 100644 QuickShell/Interop/ShellFileDialog.cs delete mode 100644 QuickShell/Interop/Win32Clipboard.cs diff --git a/QuickShell.Run/Interop/ShellFileDialog.cs b/QuickShell.Core/Interop/ShellFileDialog.cs similarity index 100% rename from QuickShell.Run/Interop/ShellFileDialog.cs rename to QuickShell.Core/Interop/ShellFileDialog.cs diff --git a/QuickShell.Core/Interop/StaDialogCloser.cs b/QuickShell.Core/Interop/StaDialogCloser.cs new file mode 100644 index 00000000..e0227919 --- /dev/null +++ b/QuickShell.Core/Interop/StaDialogCloser.cs @@ -0,0 +1,96 @@ +using System.Runtime.InteropServices; + +namespace QuickShell.Interop; + +/// +/// Closes a modal common-dialog window owned by a known native STA thread. +/// Prefer the dialog class #32770; fall back to the first visible top-level +/// window on that thread that is not the owner. Avoids posting WM_CLOSE to an +/// arbitrary foreground window (which may belong to another app). +/// +internal static class StaDialogCloser +{ + private const int WM_CLOSE = 0x0010; + private const uint GA_ROOT = 2; + + public static void TryCloseThreadDialog(int nativeThreadId, nint ownerHandle) + { + if (nativeThreadId == 0) + { + return; + } + + nint dialog = 0; + nint fallback = 0; + EnumThreadWindows( + nativeThreadId, + (hwnd, _) => + { + if (hwnd == 0 || hwnd == ownerHandle || !IsWindowVisible(hwnd)) + { + return true; + } + + // Top-level only (skip DirectUI / child chrome inside the dialog). + if (GetAncestor(hwnd, GA_ROOT) != hwnd) + { + return true; + } + + if (IsDialogClass(hwnd)) + { + dialog = hwnd; + return false; + } + + if (fallback == 0) + { + fallback = hwnd; + } + + return true; + }, + 0); + + var target = dialog != 0 ? dialog : fallback; + if (target != 0) + { + PostMessage(target, WM_CLOSE, nint.Zero, nint.Zero); + } + } + + private static unsafe bool IsDialogClass(nint hwnd) + { + Span buffer = stackalloc char[64]; + int length; + fixed (char* p = buffer) + { + length = GetClassNameW(hwnd, p, buffer.Length); + } + + return length > 0 && buffer[..length].SequenceEqual("#32770"); + } + + private delegate bool EnumThreadWndProc(nint hWnd, nint lParam); + + [DllImport("user32.dll")] + [return: MarshalAs(UnmanagedType.Bool)] + private static extern bool EnumThreadWindows(int dwThreadId, EnumThreadWndProc lpfn, nint lParam); + + [DllImport("user32.dll")] + [return: MarshalAs(UnmanagedType.Bool)] + private static extern bool IsWindowVisible(nint hWnd); + + [DllImport("user32.dll")] + private static extern nint GetAncestor(nint hWnd, uint gaFlags); + + [DllImport("user32.dll", CharSet = CharSet.Unicode, ExactSpelling = true)] + private static extern unsafe int GetClassNameW(nint hWnd, char* lpClassName, int nMaxCount); + + [DllImport("user32.dll")] + [return: MarshalAs(UnmanagedType.Bool)] + private static extern bool PostMessage(nint hWnd, int msg, nint wParam, nint lParam); + + [DllImport("kernel32.dll")] + public static extern int GetCurrentThreadId(); +} diff --git a/QuickShell.Run/Interop/Win32Clipboard.cs b/QuickShell.Core/Interop/Win32Clipboard.cs similarity index 77% rename from QuickShell.Run/Interop/Win32Clipboard.cs rename to QuickShell.Core/Interop/Win32Clipboard.cs index b86141bf..06874fb0 100644 --- a/QuickShell.Run/Interop/Win32Clipboard.cs +++ b/QuickShell.Core/Interop/Win32Clipboard.cs @@ -1,4 +1,5 @@ using System.Runtime.InteropServices; +using System.Threading; namespace QuickShell.Interop; @@ -14,6 +15,8 @@ internal static partial class Win32Clipboard { private const uint CF_UNICODETEXT = 13; private const uint GMEM_MOVEABLE = 0x0002; + private const int OpenRetryCount = 10; + private const int OpenRetryDelayMs = 50; public static string? GetText() { @@ -22,7 +25,7 @@ internal static partial class Win32Clipboard return null; } - if (!OpenClipboard(0)) + if (!TryOpenClipboard()) { return null; } @@ -43,7 +46,16 @@ internal static partial class Win32Clipboard try { - return Marshal.PtrToStringUni(locked); + // Bound the read: clipboard memory is owned by another process and may omit + // a terminator. GlobalSize is the allocated byte length of the HGLOBAL. + nint byteLen = GlobalSize(handle); + if (byteLen <= 0) + { + return null; + } + + int charCount = (int)(byteLen / sizeof(char)); + return Marshal.PtrToStringUni(locked, charCount)?.TrimEnd('\0'); } finally { @@ -60,7 +72,7 @@ public static bool SetText(string text) { ArgumentNullException.ThrowIfNull(text); - if (!OpenClipboard(0)) + if (!TryOpenClipboard()) { return false; } @@ -123,6 +135,28 @@ public static bool SetText(string text) } } + /// + /// WinForms Clipboard retried OpenClipboard briefly when another app held it. + /// Match that so transient contention does not fail copy/paste. + /// + private static bool TryOpenClipboard() + { + for (var attempt = 0; attempt < OpenRetryCount; attempt++) + { + if (OpenClipboard(0)) + { + return true; + } + + if (attempt + 1 < OpenRetryCount) + { + Thread.Sleep(OpenRetryDelayMs); + } + } + + return false; + } + [LibraryImport("user32.dll", SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] private static partial bool OpenClipboard(nint hWndNewOwner); @@ -157,4 +191,7 @@ public static bool SetText(string text) [LibraryImport("kernel32.dll", SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] private static partial bool GlobalUnlock(nint hMem); + + [LibraryImport("kernel32.dll", SetLastError = true)] + private static partial nint GlobalSize(nint hMem); } diff --git a/QuickShell.Core/Models/FormChoiceJson.cs b/QuickShell.Core/Models/FormChoiceJson.cs index d11bb4b5..2e06a622 100644 --- a/QuickShell.Core/Models/FormChoiceJson.cs +++ b/QuickShell.Core/Models/FormChoiceJson.cs @@ -10,6 +10,6 @@ namespace QuickShell.Models; /// preserves the exact lowercase JSON keys the /// previous anonymous-type serialization produced (the CmdPal host expects this casing). /// -internal sealed record FormChoiceJson( +internal readonly record struct FormChoiceJson( [property: JsonPropertyName("title")] string Title, [property: JsonPropertyName("value")] string Value); diff --git a/QuickShell.Core/Models/TaskTypeChoiceJson.cs b/QuickShell.Core/Models/TaskTypeChoiceJson.cs index a3cf3f37..de7a74c3 100644 --- a/QuickShell.Core/Models/TaskTypeChoiceJson.cs +++ b/QuickShell.Core/Models/TaskTypeChoiceJson.cs @@ -10,7 +10,7 @@ namespace QuickShell.Models; /// preserves the exact lowercase JSON keys the /// previous anonymous-type serialization produced (the CmdPal host expects this casing). /// -internal sealed record TaskTypeChoiceJson( +internal readonly record struct TaskTypeChoiceJson( [property: JsonPropertyName("title")] string Title, [property: JsonPropertyName("value")] string Value, [property: JsonPropertyName("tooltip")] string Tooltip); diff --git a/QuickShell.Core/QuickShell.Core.csproj b/QuickShell.Core/QuickShell.Core.csproj index b5b6f576..687f6bb5 100644 --- a/QuickShell.Core/QuickShell.Core.csproj +++ b/QuickShell.Core/QuickShell.Core.csproj @@ -9,6 +9,8 @@ $(Platform) + + true diff --git a/QuickShell.Run/QuickShell.Run.csproj b/QuickShell.Run/QuickShell.Run.csproj index 9108ba03..b508cbc0 100644 --- a/QuickShell.Run/QuickShell.Run.csproj +++ b/QuickShell.Run/QuickShell.Run.csproj @@ -7,8 +7,6 @@ enable enable true - - true x64;ARM64 $(Platform) diff --git a/QuickShell.Run/Services/FolderPickerService.cs b/QuickShell.Run/Services/FolderPickerService.cs index 2e656a09..f6a47cf6 100644 --- a/QuickShell.Run/Services/FolderPickerService.cs +++ b/QuickShell.Run/Services/FolderPickerService.cs @@ -20,7 +20,12 @@ internal static class FolderPickerService } string? selected = null; - var thread = new Thread(() => selected = PickFolderOnStaThread(initialDirectory, ownerHandle)) + var nativeThreadId = 0; + var thread = new Thread(() => + { + nativeThreadId = StaDialogCloser.GetCurrentThreadId(); + selected = PickFolderOnStaThread(initialDirectory, ownerHandle); + }) { IsBackground = true, }; @@ -28,14 +33,14 @@ internal static class FolderPickerService thread.Start(); // The modal IFileDialog blocks the STA thread until dismissed. DialogTimeout + - // JoinGracePeriod bounds a stuck dialog; on timeout, post WM_CLOSE to the foreground - // window (the modal dialog is topmost) so the thread unblocks and the caller never hangs. + // JoinGracePeriod bounds a stuck dialog; on timeout, post WM_CLOSE only to a + // dialog owned by that STA thread (not an arbitrary foreground window). if (thread.Join(DialogTimeout + JoinGracePeriod)) { return selected; } - TryCloseForegroundDialog(ownerHandle); + StaDialogCloser.TryCloseThreadDialog(Volatile.Read(ref nativeThreadId), ownerHandle); return thread.Join(JoinGracePeriod) ? selected : null; } @@ -48,23 +53,6 @@ internal static class FolderPickerService return ShellFileDialog.PickFolder(ownerHandle, initial); } - private static void TryCloseForegroundDialog(nint ownerHandle) - { - var hwnd = GetForegroundWindow(); - if (hwnd == 0 || hwnd == ownerHandle) - { - return; - } - - PostMessage(hwnd, WM_CLOSE, nint.Zero, nint.Zero); - } - [DllImport("user32.dll")] private static extern nint GetForegroundWindow(); - - private const int WM_CLOSE = 0x0010; - - [DllImport("user32.dll")] - [return: MarshalAs(UnmanagedType.Bool)] - private static extern bool PostMessage(nint hWnd, int msg, nint wParam, nint lParam); } diff --git a/QuickShell/Interop/ShellFileDialog.cs b/QuickShell/Interop/ShellFileDialog.cs deleted file mode 100644 index b8dafed9..00000000 --- a/QuickShell/Interop/ShellFileDialog.cs +++ /dev/null @@ -1,360 +0,0 @@ -using System.IO; -using System.Runtime.InteropServices; -using System.Runtime.InteropServices.Marshalling; - -namespace QuickShell.Interop; - -/// -/// Minimal wrapper over the Win32 shell Common Item Dialog (IFileOpenDialog / -/// IFileSaveDialog). Replaces the WinForms FolderBrowserDialog / OpenFileDialog / -/// SaveFileDialog so the packaged app no longer forces UseWindowsForms (which blocks -/// trimming). Calls are synchronous and modal — IFileDialog.Show blocks the calling -/// thread exactly like the old ShowDialog, so the existing background-STA-thread -/// callers keep working unchanged. -/// -/// Uses source-generated COM () so the -/// interop stays AOT/trim-safe (the project is IsAotCompatible=true). The full -/// IFileDialog vtable is declared in order; slots we never call are declared with -/// blittable [PreserveSig] signatures purely to preserve vtable layout. -/// -internal static partial class ShellFileDialog -{ - // A single StrategyBasedComWrappers marshals the raw IUnknown pointers returned by - // CoCreateInstance / SHCreateItemFromParsingName into our generated interfaces. - private static readonly StrategyBasedComWrappers ComWrappers = new(); - - private static readonly Guid CLSID_FileOpenDialog = new("DC1C5A9C-E88A-4dde-A5A1-60F82A20AEF7"); - private static readonly Guid CLSID_FileSaveDialog = new("C0B4E2F3-BA21-4773-8DBA-335EC946EB8B"); - private static readonly Guid IID_IFileOpenDialog = new("d57c7288-d4ad-4768-be02-9d969532d960"); - private static readonly Guid IID_IFileSaveDialog = new("84bccd23-5fde-4cdb-aea4-af64b83d78ab"); - private static readonly Guid IID_IShellItem = new("43826d1e-e718-42ee-bc55-a1e261c37bfe"); - - private const uint FOS_OVERWRITEPROMPT = 0x00000002; - private const uint FOS_NOCHANGEDIR = 0x00000008; - private const uint FOS_PICKFOLDERS = 0x00000020; - private const uint FOS_FORCEFILESYSTEM = 0x00000040; - private const uint FOS_PATHMUSTEXIST = 0x00000800; - private const uint FOS_FILEMUSTEXIST = 0x00001000; - - private const uint SIGDN_FILESYSPATH = 0x80058000; - - private const uint CLSCTX_INPROC_SERVER = 0x1; - - // HRESULT_FROM_WIN32(ERROR_CANCELLED) — returned by Show when the user cancels. - private const int ERROR_CANCELLED_HRESULT = unchecked((int)0x800704C7); - - /// Modern folder picker (IFileOpenDialog with FOS_PICKFOLDERS). - public static string? PickFolder(nint owner, string? initialDirectory) - { - var dialog = (IFileOpenDialog)CreateInstance(CLSID_FileOpenDialog, IID_IFileOpenDialog); - try - { - dialog.SetOptions(FOS_PICKFOLDERS | FOS_FORCEFILESYSTEM | FOS_PATHMUSTEXIST | FOS_NOCHANGEDIR); - ApplyInitialDirectory(dialog, initialDirectory); - return ShowAndGetPath(dialog, owner); - } - finally - { - Release(dialog); - } - } - - /// Open-file picker. pairs display name to a - /// semicolon-delimited pattern (e.g. "JSON files (*.json)" -> "*.json"). - public static string? PickOpenFile(nint owner, string title, (string Name, string Spec)[] filters, string? defaultExt, string? initialDirectory) - { - var dialog = (IFileOpenDialog)CreateInstance(CLSID_FileOpenDialog, IID_IFileOpenDialog); - try - { - dialog.SetOptions(FOS_FORCEFILESYSTEM | FOS_FILEMUSTEXIST | FOS_PATHMUSTEXIST | FOS_NOCHANGEDIR); - dialog.SetTitle(title); - ApplyFilters(dialog, filters, defaultExt); - ApplyInitialDirectory(dialog, initialDirectory); - return ShowAndGetPath(dialog, owner); - } - finally - { - Release(dialog); - } - } - - /// Save-file picker with overwrite prompt and a suggested file name. - public static string? PickSaveFile(nint owner, string title, (string Name, string Spec)[] filters, string? defaultExt, string? defaultFileName, string? initialDirectory) - { - var dialog = (IFileSaveDialog)CreateInstance(CLSID_FileSaveDialog, IID_IFileSaveDialog); - try - { - dialog.SetOptions(FOS_FORCEFILESYSTEM | FOS_OVERWRITEPROMPT | FOS_PATHMUSTEXIST | FOS_NOCHANGEDIR); - dialog.SetTitle(title); - ApplyFilters(dialog, filters, defaultExt); - if (!string.IsNullOrEmpty(defaultFileName)) - { - dialog.SetFileName(defaultFileName); - } - - ApplyInitialDirectory(dialog, initialDirectory); - return ShowAndGetPath(dialog, owner); - } - finally - { - Release(dialog); - } - } - - private static string? ShowAndGetPath(IFileDialog dialog, nint owner) - { - int hr = dialog.Show(owner); - if (hr == ERROR_CANCELLED_HRESULT) - { - return null; - } - - Marshal.ThrowExceptionForHR(hr); - - dialog.GetResult(out nint itemPtr); - if (itemPtr == 0) - { - return null; - } - - var item = (IShellItem)ComWrappers.GetOrCreateObjectForComInstance(itemPtr, CreateObjectFlags.UniqueInstance); - Marshal.Release(itemPtr); - try - { - item.GetDisplayName(SIGDN_FILESYSPATH, out nint pathPtr); - if (pathPtr == 0) - { - return null; - } - - try - { - return Marshal.PtrToStringUni(pathPtr); - } - finally - { - Marshal.FreeCoTaskMem(pathPtr); - } - } - finally - { - Release(item); - } - } - - private static void ApplyInitialDirectory(IFileDialog dialog, string? initialDirectory) - { - if (string.IsNullOrWhiteSpace(initialDirectory) || !Directory.Exists(initialDirectory)) - { - return; - } - - int hr = SHCreateItemFromParsingName(initialDirectory!, 0, in IID_IShellItem, out nint itemPtr); - if (hr < 0 || itemPtr == 0) - { - return; // Non-fatal: fall back to the shell's default folder. - } - - var item = (IShellItem)ComWrappers.GetOrCreateObjectForComInstance(itemPtr, CreateObjectFlags.UniqueInstance); - Marshal.Release(itemPtr); - try - { - dialog.SetFolder(item); - } - finally - { - Release(item); - } - } - - private static void ApplyFilters(IFileDialog dialog, (string Name, string Spec)[] filters, string? defaultExt) - { - if (filters.Length == 0) - { - return; - } - - // COMDLG_FILTERSPEC is { LPWSTR pszName; LPWSTR pszSpec; }. Build the native array - // by hand so the interface method stays blittable (no struct marshalling in the vtable). - nint block = Marshal.AllocCoTaskMem(filters.Length * nint.Size * 2); - var strings = new List(filters.Length * 2); - try - { - for (int i = 0; i < filters.Length; i++) - { - nint namePtr = Marshal.StringToCoTaskMemUni(filters[i].Name); - nint specPtr = Marshal.StringToCoTaskMemUni(filters[i].Spec); - strings.Add(namePtr); - strings.Add(specPtr); - Marshal.WriteIntPtr(block, i * nint.Size * 2, namePtr); - Marshal.WriteIntPtr(block, (i * nint.Size * 2) + nint.Size, specPtr); - } - - dialog.SetFileTypes((uint)filters.Length, block); - dialog.SetFileTypeIndex(1); - } - finally - { - foreach (nint s in strings) - { - Marshal.FreeCoTaskMem(s); - } - - Marshal.FreeCoTaskMem(block); - } - - if (!string.IsNullOrEmpty(defaultExt)) - { - dialog.SetDefaultExtension(defaultExt); - } - } - - private static object CreateInstance(Guid clsid, Guid iid) - { - int hr = CoCreateInstance(in clsid, 0, CLSCTX_INPROC_SERVER, in iid, out nint ptr); - Marshal.ThrowExceptionForHR(hr); - object instance = ComWrappers.GetOrCreateObjectForComInstance(ptr, CreateObjectFlags.UniqueInstance); - Marshal.Release(ptr); - return instance; - } - - // Source-generated COM wrappers (UniqueInstance) release their underlying COM reference - // on Dispose — Marshal.FinalReleaseComObject does not work with them (SYSLIB1099). - private static void Release(object comObject) => (comObject as IDisposable)?.Dispose(); - - [LibraryImport("ole32.dll")] - private static partial int CoCreateInstance(in Guid rclsid, nint pUnkOuter, uint dwClsContext, in Guid riid, out nint ppv); - - [LibraryImport("shell32.dll", StringMarshalling = StringMarshalling.Utf16)] - private static partial int SHCreateItemFromParsingName(string pszPath, nint pbc, in Guid riid, out nint ppv); - - // IModalWindow — base of IFileDialog. Only Show is called. - [GeneratedComInterface] - [Guid("b4db1657-70d7-485e-8e3e-6fcb5a5c1802")] - internal partial interface IModalWindow - { - [PreserveSig] - int Show(nint parent); - } - - // IFileDialog : IModalWindow. Declared in full vtable order; unused slots use blittable - // [PreserveSig] signatures so we never generate marshalling for methods we don't call. - [GeneratedComInterface] - [Guid("42f85136-db7e-439c-85f1-e4075d135fc8")] - internal partial interface IFileDialog : IModalWindow - { - void SetFileTypes(uint cFileTypes, nint rgFilterSpec); - - void SetFileTypeIndex(uint iFileType); - - [PreserveSig] - int GetFileTypeIndex(out uint piFileType); - - [PreserveSig] - int Advise(nint pfde, out uint pdwCookie); - - [PreserveSig] - int Unadvise(uint dwCookie); - - void SetOptions(uint fos); - - [PreserveSig] - int GetOptions(out uint pfos); - - [PreserveSig] - int SetDefaultFolder(nint psi); - - void SetFolder(IShellItem psi); - - [PreserveSig] - int GetFolder(out nint ppsi); - - [PreserveSig] - int GetCurrentSelection(out nint ppsi); - - void SetFileName([MarshalAs(UnmanagedType.LPWStr)] string pszName); - - [PreserveSig] - int GetFileName(out nint pszName); - - void SetTitle([MarshalAs(UnmanagedType.LPWStr)] string pszTitle); - - [PreserveSig] - int SetOkButtonLabel(nint pszText); - - [PreserveSig] - int SetFileNameLabel(nint pszLabel); - - void GetResult(out nint ppsi); - - [PreserveSig] - int AddPlace(nint psi, int fdap); - - void SetDefaultExtension([MarshalAs(UnmanagedType.LPWStr)] string pszDefaultExtension); - - [PreserveSig] - int Close(int hr); - - [PreserveSig] - int SetClientGuid(in Guid guid); - - [PreserveSig] - int ClearClientData(); - - [PreserveSig] - int SetFilter(nint pFilter); - } - - // IFileOpenDialog : IFileDialog. Adds GetResults / GetSelectedItems (unused; single-select - // uses IFileDialog.GetResult). Slots declared to keep the type usable if ever needed. - [GeneratedComInterface] - [Guid("d57c7288-d4ad-4768-be02-9d969532d960")] - internal partial interface IFileOpenDialog : IFileDialog - { - [PreserveSig] - int GetResults(out nint ppenum); - - [PreserveSig] - int GetSelectedItems(out nint ppsai); - } - - // IFileSaveDialog : IFileDialog. Extra slots unused (SetSaveAsItem/SetProperties/...). - [GeneratedComInterface] - [Guid("84bccd23-5fde-4cdb-aea4-af64b83d78ab")] - internal partial interface IFileSaveDialog : IFileDialog - { - [PreserveSig] - int SetSaveAsItem(nint psi); - - [PreserveSig] - int SetProperties(nint pStore); - - [PreserveSig] - int SetCollectedProperties(nint pList, int fAppendDefault); - - [PreserveSig] - int GetProperties(out nint ppStore); - - [PreserveSig] - int ApplyProperties(nint psi, nint pStore, nint hwnd, nint pSink); - } - - [GeneratedComInterface] - [Guid("43826d1e-e718-42ee-bc55-a1e261c37bfe")] - internal partial interface IShellItem - { - [PreserveSig] - int BindToHandler(nint pbc, in Guid bhid, in Guid riid, out nint ppv); - - [PreserveSig] - int GetParent(out nint ppsi); - - void GetDisplayName(uint sigdnName, out nint ppszName); - - [PreserveSig] - int GetAttributes(uint sfgaoMask, out uint psfgaoAttribs); - - [PreserveSig] - int Compare(nint psi, uint hint, out int piOrder); - } -} diff --git a/QuickShell/Interop/Win32Clipboard.cs b/QuickShell/Interop/Win32Clipboard.cs deleted file mode 100644 index b86141bf..00000000 --- a/QuickShell/Interop/Win32Clipboard.cs +++ /dev/null @@ -1,160 +0,0 @@ -using System.Runtime.InteropServices; - -namespace QuickShell.Interop; - -/// -/// Win32 clipboard text read/write via user32/kernel32 P/Invoke. Replaces -/// System.Windows.Forms.Clipboard so the app no longer forces UseWindowsForms. -/// Behaves like WinForms Clipboard.SetText/GetText — the OS owns the CF_UNICODETEXT -/// data after SetClipboardData, so it persists after the calling thread exits (no -/// WinRT-style Flush needed). Callers already marshal to an STA thread, which the -/// clipboard API requires. -/// -internal static partial class Win32Clipboard -{ - private const uint CF_UNICODETEXT = 13; - private const uint GMEM_MOVEABLE = 0x0002; - - public static string? GetText() - { - if (!IsClipboardFormatAvailable(CF_UNICODETEXT)) - { - return null; - } - - if (!OpenClipboard(0)) - { - return null; - } - - try - { - nint handle = GetClipboardData(CF_UNICODETEXT); - if (handle == 0) - { - return null; - } - - nint locked = GlobalLock(handle); - if (locked == 0) - { - return null; - } - - try - { - return Marshal.PtrToStringUni(locked); - } - finally - { - GlobalUnlock(handle); - } - } - finally - { - CloseClipboard(); - } - } - - public static bool SetText(string text) - { - ArgumentNullException.ThrowIfNull(text); - - if (!OpenClipboard(0)) - { - return false; - } - - try - { - if (!EmptyClipboard()) - { - return false; - } - - // Include the terminating null. GlobalAlloc GMEM_MOVEABLE is required for - // clipboard data; ownership transfers to the system on SetClipboardData. - int bytes = (text.Length + 1) * sizeof(char); - nint hGlobal = GlobalAlloc(GMEM_MOVEABLE, bytes); - if (hGlobal == 0) - { - return false; - } - - bool ownershipTransferred = false; - try - { - nint target = GlobalLock(hGlobal); - if (target == 0) - { - return false; - } - - try - { - Marshal.Copy(text.ToCharArray(), 0, target, text.Length); - Marshal.WriteInt16(target, text.Length * sizeof(char), 0); - } - finally - { - GlobalUnlock(hGlobal); - } - - if (SetClipboardData(CF_UNICODETEXT, hGlobal) == 0) - { - return false; - } - - ownershipTransferred = true; - return true; - } - finally - { - // Only free on failure; on success the system owns the memory. - if (!ownershipTransferred) - { - GlobalFree(hGlobal); - } - } - } - finally - { - CloseClipboard(); - } - } - - [LibraryImport("user32.dll", SetLastError = true)] - [return: MarshalAs(UnmanagedType.Bool)] - private static partial bool OpenClipboard(nint hWndNewOwner); - - [LibraryImport("user32.dll", SetLastError = true)] - [return: MarshalAs(UnmanagedType.Bool)] - private static partial bool CloseClipboard(); - - [LibraryImport("user32.dll", SetLastError = true)] - [return: MarshalAs(UnmanagedType.Bool)] - private static partial bool EmptyClipboard(); - - [LibraryImport("user32.dll", SetLastError = true)] - [return: MarshalAs(UnmanagedType.Bool)] - private static partial bool IsClipboardFormatAvailable(uint format); - - [LibraryImport("user32.dll", SetLastError = true)] - private static partial nint GetClipboardData(uint uFormat); - - [LibraryImport("user32.dll", SetLastError = true)] - private static partial nint SetClipboardData(uint uFormat, nint hMem); - - [LibraryImport("kernel32.dll", SetLastError = true)] - private static partial nint GlobalAlloc(uint uFlags, nint dwBytes); - - [LibraryImport("kernel32.dll", SetLastError = true)] - private static partial nint GlobalFree(nint hMem); - - [LibraryImport("kernel32.dll", SetLastError = true)] - private static partial nint GlobalLock(nint hMem); - - [LibraryImport("kernel32.dll", SetLastError = true)] - [return: MarshalAs(UnmanagedType.Bool)] - private static partial bool GlobalUnlock(nint hMem); -} diff --git a/QuickShell/QuickShell.csproj b/QuickShell/QuickShell.csproj index f2adba76..2b4e57d5 100644 --- a/QuickShell/QuickShell.csproj +++ b/QuickShell/QuickShell.csproj @@ -14,8 +14,6 @@ CN=31D3C278-3D55-4A42-BA6A-24E16D15B863 true enable - - true diff --git a/QuickShell/Services/FolderPickerService.cs b/QuickShell/Services/FolderPickerService.cs index 844027c9..fe12f80d 100644 --- a/QuickShell/Services/FolderPickerService.cs +++ b/QuickShell/Services/FolderPickerService.cs @@ -19,7 +19,12 @@ internal static class FolderPickerService } string? selected = null; - var thread = new Thread(() => selected = PickFolderOnStaThread(initialDirectory, ownerHandle)) + var nativeThreadId = 0; + var thread = new Thread(() => + { + nativeThreadId = StaDialogCloser.GetCurrentThreadId(); + selected = PickFolderOnStaThread(initialDirectory, ownerHandle); + }) { IsBackground = true, }; @@ -28,14 +33,13 @@ internal static class FolderPickerService // Happy path: the modal IFileDialog blocks the STA thread until the user dismisses it, // then the thread exits. DialogTimeout + JoinGracePeriod bounds a stuck dialog; on - // timeout, post WM_CLOSE to the foreground window (the modal dialog is topmost) so the - // STA thread unblocks and the caller never hangs on a live orphan dialog. + // timeout, post WM_CLOSE only to a dialog owned by that STA thread. if (thread.Join(DialogTimeout + JoinGracePeriod)) { return selected; } - TryCloseForegroundDialog(ownerHandle); + StaDialogCloser.TryCloseThreadDialog(Volatile.Read(ref nativeThreadId), ownerHandle); return thread.Join(JoinGracePeriod) ? selected : null; } @@ -52,23 +56,6 @@ internal static class FolderPickerService return ShellFileDialog.PickFolder(ownerHandle, initial); } - private static void TryCloseForegroundDialog(nint ownerHandle) - { - var hwnd = GetForegroundWindow(); - if (hwnd == 0 || hwnd == ownerHandle) - { - return; - } - - PostMessage(hwnd, WM_CLOSE, nint.Zero, nint.Zero); - } - [DllImport("user32.dll")] private static extern nint GetForegroundWindow(); - - private const int WM_CLOSE = 0x0010; - - [DllImport("user32.dll")] - [return: MarshalAs(UnmanagedType.Bool)] - private static extern bool PostMessage(nint hWnd, int msg, nint wParam, nint lParam); } diff --git a/QuickShell/Services/ShortcutFilePickerService.cs b/QuickShell/Services/ShortcutFilePickerService.cs index 8a3a2db3..a0b06285 100644 --- a/QuickShell/Services/ShortcutFilePickerService.cs +++ b/QuickShell/Services/ShortcutFilePickerService.cs @@ -1,5 +1,5 @@ -using System.Threading; using System.Runtime.InteropServices; +using System.Threading; using QuickShell.Interop; namespace QuickShell.Services; @@ -19,52 +19,62 @@ private static readonly (string Name, string Spec)[] ExecutableFilters = }; private static readonly TimeSpan DialogTimeout = TimeSpan.FromMinutes(2); + private static readonly TimeSpan JoinGracePeriod = TimeSpan.FromSeconds(5); public static string? PickExportFile(IQuickShellServices services) { ArgumentNullException.ThrowIfNull(services); var defaultName = $"quickshell-workspaces-{DateTime.Now:yyyyMMdd-HHmmss}.json"; var initialDirectory = DirectoryOrNull(services.Shortcuts.ConfigDirectory); + var ownerHandle = GetForegroundWindow(); - return RunOnStaThread(() => ShellFileDialog.PickSaveFile( - GetForegroundWindow(), - $"Export {QuickShellBrand.DisplayName} workspaces", - JsonFilters, - defaultExt: "json", - defaultFileName: defaultName, - initialDirectory: initialDirectory)); + return RunOnStaThread( + ownerHandle, + () => ShellFileDialog.PickSaveFile( + ownerHandle, + $"Export {QuickShellBrand.DisplayName} workspaces", + JsonFilters, + defaultExt: "json", + defaultFileName: defaultName, + initialDirectory: initialDirectory)); } public static string? PickImportFile(IQuickShellServices services) { ArgumentNullException.ThrowIfNull(services); var initialDirectory = DirectoryOrNull(services.Shortcuts.ConfigDirectory); + var ownerHandle = GetForegroundWindow(); - return RunOnStaThread(() => ShellFileDialog.PickOpenFile( - GetForegroundWindow(), - $"Import {QuickShellBrand.DisplayName} workspaces", - JsonFilters, - defaultExt: "json", - initialDirectory: initialDirectory)); + return RunOnStaThread( + ownerHandle, + () => ShellFileDialog.PickOpenFile( + ownerHandle, + $"Import {QuickShellBrand.DisplayName} workspaces", + JsonFilters, + defaultExt: "json", + initialDirectory: initialDirectory)); } public static string? PickExecutableFile() { var programFiles = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles); var initialDirectory = DirectoryOrNull(programFiles); + var ownerHandle = GetForegroundWindow(); - return RunOnStaThread(() => ShellFileDialog.PickOpenFile( - GetForegroundWindow(), - "Choose companion app", - ExecutableFilters, - defaultExt: "exe", - initialDirectory: initialDirectory)); + return RunOnStaThread( + ownerHandle, + () => ShellFileDialog.PickOpenFile( + ownerHandle, + "Choose companion app", + ExecutableFilters, + defaultExt: "exe", + initialDirectory: initialDirectory)); } private static string? DirectoryOrNull(string? path) => !string.IsNullOrWhiteSpace(path) && Directory.Exists(path) ? path : null; - private static string? RunOnStaThread(Func action) + private static string? RunOnStaThread(nint ownerHandle, Func action) { if (Thread.CurrentThread.GetApartmentState() == ApartmentState.STA) { @@ -72,14 +82,25 @@ private static readonly (string Name, string Spec)[] ExecutableFilters = } string? result = null; - var thread = new Thread(() => result = action()) + var nativeThreadId = 0; + var thread = new Thread(() => + { + nativeThreadId = StaDialogCloser.GetCurrentThreadId(); + result = action(); + }) { IsBackground = true, }; thread.SetApartmentState(ApartmentState.STA); thread.Start(); - return thread.Join(DialogTimeout) ? result : null; + if (thread.Join(DialogTimeout + JoinGracePeriod)) + { + return result; + } + + StaDialogCloser.TryCloseThreadDialog(Volatile.Read(ref nativeThreadId), ownerHandle); + return thread.Join(JoinGracePeriod) ? result : null; } [DllImport("user32.dll")] From 30f17205a53793838dd01d14e4f87396beb71c41 Mon Sep 17 00:00:00 2001 From: tonythethompson Date: Sat, 25 Jul 2026 05:30:32 -0700 Subject: [PATCH 3/8] Address CodeQL notes for Win32 pickers and clipboard. Remove generic catch around Win32Clipboard, centralize GetForegroundWindow, document intentional unmanaged call sites, and exclude the quality-only cs/call-to-unmanaged-code query via CodeQL config. Co-authored-by: Cursor --- .github/codeql/codeql-config.yml | 12 +++++++++++ .github/workflows/codeql.yml | 5 ++++- .../Interop/NativeForegroundWindow.cs | 20 +++++++++++++++++++ QuickShell.Core/Interop/StaDialogCloser.cs | 13 +++++++++++- .../Services/FolderPickerService.cs | 8 ++------ QuickShell.Run/Services/StaClipboard.cs | 7 ++----- QuickShell/Services/FolderPickerService.cs | 8 ++------ .../Services/ShortcutFilePickerService.cs | 12 ++++------- QuickShell/Services/StaClipboard.cs | 14 ++++--------- 9 files changed, 62 insertions(+), 37 deletions(-) create mode 100644 .github/codeql/codeql-config.yml create mode 100644 QuickShell.Core/Interop/NativeForegroundWindow.cs diff --git a/.github/codeql/codeql-config.yml b/.github/codeql/codeql-config.yml new file mode 100644 index 00000000..fc8d0d38 --- /dev/null +++ b/.github/codeql/codeql-config.yml @@ -0,0 +1,12 @@ +name: QuickShell CodeQL configuration + +queries: + - uses: security-and-quality + +query-filters: + # Quality note (not a vuln). Quick Shell is a Windows shell host that must call + # Win32/COM for folder/file dialogs, clipboard, and dialog timeout recovery after + # dropping WinForms for MSIX trimming. Inline codeql[] suppressions also document + # intentional call sites under QuickShell.Core/Interop. + - exclude: + id: cs/call-to-unmanaged-code diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index b6450d58..4d260f40 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -11,6 +11,7 @@ on: - "**/*.sln" - "QuickShell.Raycast/**" - ".github/workflows/codeql.yml" + - ".github/codeql/**" pull_request: paths: - "**/*.cs" @@ -18,6 +19,7 @@ on: - "**/*.sln" - "QuickShell.Raycast/**" - ".github/workflows/codeql.yml" + - ".github/codeql/**" schedule: - cron: "17 3 * * 0" workflow_dispatch: @@ -51,6 +53,7 @@ jobs: - '**/*.csproj' - '**/*.sln' - '.github/workflows/codeql.yml' + - '.github/codeql/**' raycast: - 'QuickShell.Raycast/**' - '.github/workflows/codeql.yml' @@ -86,7 +89,7 @@ jobs: uses: github/codeql-action/init@v4 with: languages: csharp - queries: security-and-quality + config-file: ./.github/codeql/codeql-config.yml - name: Restore solution run: dotnet restore QuickShell.sln --verbosity minimal diff --git a/QuickShell.Core/Interop/NativeForegroundWindow.cs b/QuickShell.Core/Interop/NativeForegroundWindow.cs new file mode 100644 index 00000000..53c07cd9 --- /dev/null +++ b/QuickShell.Core/Interop/NativeForegroundWindow.cs @@ -0,0 +1,20 @@ +using System.Runtime.InteropServices; + +namespace QuickShell.Interop; + +/// +/// Owner HWND for modal shell dialogs. Win32 is intentional: after dropping WinForms +/// there is no BCL/WinRT equivalent that works for both the packaged CmdPal host and +/// the PowerToys Run plugin. +/// +internal static partial class NativeForegroundWindow +{ + public static nint Get() + { + // codeql[cs/call-to-unmanaged-code]: Required for IFileDialog owner hwnd; no managed API after WinForms removal. + return GetForegroundWindow(); + } + + [LibraryImport("user32.dll")] + private static partial nint GetForegroundWindow(); +} diff --git a/QuickShell.Core/Interop/StaDialogCloser.cs b/QuickShell.Core/Interop/StaDialogCloser.cs index e0227919..879a7f52 100644 --- a/QuickShell.Core/Interop/StaDialogCloser.cs +++ b/QuickShell.Core/Interop/StaDialogCloser.cs @@ -22,16 +22,19 @@ public static void TryCloseThreadDialog(int nativeThreadId, nint ownerHandle) nint dialog = 0; nint fallback = 0; + // codeql[cs/call-to-unmanaged-code]: Enumerate picker STA thread windows to find the modal dialog HWND. EnumThreadWindows( nativeThreadId, (hwnd, _) => { + // codeql[cs/call-to-unmanaged-code]: Skip invisible windows when locating the dialog. if (hwnd == 0 || hwnd == ownerHandle || !IsWindowVisible(hwnd)) { return true; } // Top-level only (skip DirectUI / child chrome inside the dialog). + // codeql[cs/call-to-unmanaged-code]: Restrict close target to top-level HWNDs on the STA thread. if (GetAncestor(hwnd, GA_ROOT) != hwnd) { return true; @@ -55,16 +58,24 @@ public static void TryCloseThreadDialog(int nativeThreadId, nint ownerHandle) var target = dialog != 0 ? dialog : fallback; if (target != 0) { + // codeql[cs/call-to-unmanaged-code]: Unblock a timed-out IFileDialog by closing its HWND. PostMessage(target, WM_CLOSE, nint.Zero, nint.Zero); } } + public static int CurrentNativeThreadId() + { + // codeql[cs/call-to-unmanaged-code]: Native thread id required for EnumThreadWindows on the picker STA thread. + return GetCurrentThreadId(); + } + private static unsafe bool IsDialogClass(nint hwnd) { Span buffer = stackalloc char[64]; int length; fixed (char* p = buffer) { + // codeql[cs/call-to-unmanaged-code]: Classify #32770 dialog HWNDs for timeout close. length = GetClassNameW(hwnd, p, buffer.Length); } @@ -92,5 +103,5 @@ private static unsafe bool IsDialogClass(nint hwnd) private static extern bool PostMessage(nint hWnd, int msg, nint wParam, nint lParam); [DllImport("kernel32.dll")] - public static extern int GetCurrentThreadId(); + private static extern int GetCurrentThreadId(); } diff --git a/QuickShell.Run/Services/FolderPickerService.cs b/QuickShell.Run/Services/FolderPickerService.cs index f6a47cf6..a4a308b8 100644 --- a/QuickShell.Run/Services/FolderPickerService.cs +++ b/QuickShell.Run/Services/FolderPickerService.cs @@ -1,5 +1,4 @@ using System.IO; -using System.Runtime.InteropServices; using System.Threading; using QuickShell.Interop; @@ -12,7 +11,7 @@ internal static class FolderPickerService public static string? PickFolder(string? initialDirectory = null) { - var ownerHandle = GetForegroundWindow(); + var ownerHandle = NativeForegroundWindow.Get(); if (Thread.CurrentThread.GetApartmentState() == ApartmentState.STA) { @@ -23,7 +22,7 @@ internal static class FolderPickerService var nativeThreadId = 0; var thread = new Thread(() => { - nativeThreadId = StaDialogCloser.GetCurrentThreadId(); + nativeThreadId = StaDialogCloser.CurrentNativeThreadId(); selected = PickFolderOnStaThread(initialDirectory, ownerHandle); }) { @@ -52,7 +51,4 @@ internal static class FolderPickerService return ShellFileDialog.PickFolder(ownerHandle, initial); } - - [DllImport("user32.dll")] - private static extern nint GetForegroundWindow(); } diff --git a/QuickShell.Run/Services/StaClipboard.cs b/QuickShell.Run/Services/StaClipboard.cs index b67ef696..e6228022 100644 --- a/QuickShell.Run/Services/StaClipboard.cs +++ b/QuickShell.Run/Services/StaClipboard.cs @@ -14,10 +14,7 @@ public static bool TrySetText(string text) thread.Start(); return thread.Join(TimeSpan.FromSeconds(5)) && success; - bool SetText() - { - try { return Win32Clipboard.SetText(text); } - catch { return false; } - } + // Win32Clipboard returns false on failure; no generic catch needed. + bool SetText() => Win32Clipboard.SetText(text); } } diff --git a/QuickShell/Services/FolderPickerService.cs b/QuickShell/Services/FolderPickerService.cs index fe12f80d..af334abe 100644 --- a/QuickShell/Services/FolderPickerService.cs +++ b/QuickShell/Services/FolderPickerService.cs @@ -1,4 +1,3 @@ -using System.Runtime.InteropServices; using System.Threading; using QuickShell.Interop; @@ -11,7 +10,7 @@ internal static class FolderPickerService public static string? PickFolder(string? initialDirectory = null) { - var ownerHandle = GetForegroundWindow(); + var ownerHandle = NativeForegroundWindow.Get(); if (Thread.CurrentThread.GetApartmentState() == ApartmentState.STA) { @@ -22,7 +21,7 @@ internal static class FolderPickerService var nativeThreadId = 0; var thread = new Thread(() => { - nativeThreadId = StaDialogCloser.GetCurrentThreadId(); + nativeThreadId = StaDialogCloser.CurrentNativeThreadId(); selected = PickFolderOnStaThread(initialDirectory, ownerHandle); }) { @@ -55,7 +54,4 @@ internal static class FolderPickerService return ShellFileDialog.PickFolder(ownerHandle, initial); } - - [DllImport("user32.dll")] - private static extern nint GetForegroundWindow(); } diff --git a/QuickShell/Services/ShortcutFilePickerService.cs b/QuickShell/Services/ShortcutFilePickerService.cs index a0b06285..ab7814e4 100644 --- a/QuickShell/Services/ShortcutFilePickerService.cs +++ b/QuickShell/Services/ShortcutFilePickerService.cs @@ -1,4 +1,3 @@ -using System.Runtime.InteropServices; using System.Threading; using QuickShell.Interop; @@ -26,7 +25,7 @@ private static readonly (string Name, string Spec)[] ExecutableFilters = ArgumentNullException.ThrowIfNull(services); var defaultName = $"quickshell-workspaces-{DateTime.Now:yyyyMMdd-HHmmss}.json"; var initialDirectory = DirectoryOrNull(services.Shortcuts.ConfigDirectory); - var ownerHandle = GetForegroundWindow(); + var ownerHandle = NativeForegroundWindow.Get(); return RunOnStaThread( ownerHandle, @@ -43,7 +42,7 @@ private static readonly (string Name, string Spec)[] ExecutableFilters = { ArgumentNullException.ThrowIfNull(services); var initialDirectory = DirectoryOrNull(services.Shortcuts.ConfigDirectory); - var ownerHandle = GetForegroundWindow(); + var ownerHandle = NativeForegroundWindow.Get(); return RunOnStaThread( ownerHandle, @@ -59,7 +58,7 @@ private static readonly (string Name, string Spec)[] ExecutableFilters = { var programFiles = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles); var initialDirectory = DirectoryOrNull(programFiles); - var ownerHandle = GetForegroundWindow(); + var ownerHandle = NativeForegroundWindow.Get(); return RunOnStaThread( ownerHandle, @@ -85,7 +84,7 @@ private static readonly (string Name, string Spec)[] ExecutableFilters = var nativeThreadId = 0; var thread = new Thread(() => { - nativeThreadId = StaDialogCloser.GetCurrentThreadId(); + nativeThreadId = StaDialogCloser.CurrentNativeThreadId(); result = action(); }) { @@ -102,7 +101,4 @@ private static readonly (string Name, string Spec)[] ExecutableFilters = StaDialogCloser.TryCloseThreadDialog(Volatile.Read(ref nativeThreadId), ownerHandle); return thread.Join(JoinGracePeriod) ? result : null; } - - [DllImport("user32.dll")] - private static extern nint GetForegroundWindow(); } diff --git a/QuickShell/Services/StaClipboard.cs b/QuickShell/Services/StaClipboard.cs index 658f8bd5..207f8abc 100644 --- a/QuickShell/Services/StaClipboard.cs +++ b/QuickShell/Services/StaClipboard.cs @@ -27,15 +27,9 @@ public static bool TrySetText(string text) return thread.Join(ReadTimeout) && success; } - private static bool SetTextOnStaThread(string text) - { - try { return Win32Clipboard.SetText(text); } - catch { return false; } - } + // Win32Clipboard returns false/null on failure and does not throw for the clipboard + // failure modes we care about (open contention, empty format, alloc failure). + private static bool SetTextOnStaThread(string text) => Win32Clipboard.SetText(text); - private static string? ReadTextOnStaThread() - { - try { return Win32Clipboard.GetText(); } - catch { return null; } - } + private static string? ReadTextOnStaThread() => Win32Clipboard.GetText(); } From fb99a07eccfdd851e7b5b5aa1a1841d74a76be7c Mon Sep 17 00:00:00 2001 From: tonythethompson Date: Sat, 25 Jul 2026 05:57:03 -0700 Subject: [PATCH 4/8] Harden shell dialogs after CodeRabbit follow-up. Keep COMDLG filter native memory alive until after Show, truncate clipboard text at the first null, and share STA timeout/close/exception handling via StaModalDialogRunner with Volatile thread-id publication. Co-authored-by: Cursor --- QuickShell.Core/Interop/ShellFileDialog.cs | 67 ++++++++++++++++--- QuickShell.Core/Interop/Win32Clipboard.cs | 11 ++- .../Services/StaModalDialogRunner.cs | 67 +++++++++++++++++++ .../Services/FolderPickerService.cs | 35 ++-------- QuickShell/Services/FolderPickerService.cs | 35 ++-------- .../Services/ShortcutFilePickerService.cs | 48 ++++--------- 6 files changed, 157 insertions(+), 106 deletions(-) create mode 100644 QuickShell.Core/Services/StaModalDialogRunner.cs diff --git a/QuickShell.Core/Interop/ShellFileDialog.cs b/QuickShell.Core/Interop/ShellFileDialog.cs index b8dafed9..a28e8707 100644 --- a/QuickShell.Core/Interop/ShellFileDialog.cs +++ b/QuickShell.Core/Interop/ShellFileDialog.cs @@ -64,16 +64,18 @@ internal static partial class ShellFileDialog public static string? PickOpenFile(nint owner, string title, (string Name, string Spec)[] filters, string? defaultExt, string? initialDirectory) { var dialog = (IFileOpenDialog)CreateInstance(CLSID_FileOpenDialog, IID_IFileOpenDialog); + FilterNativeMemory? filterMemory = null; try { dialog.SetOptions(FOS_FORCEFILESYSTEM | FOS_FILEMUSTEXIST | FOS_PATHMUSTEXIST | FOS_NOCHANGEDIR); dialog.SetTitle(title); - ApplyFilters(dialog, filters, defaultExt); + filterMemory = ApplyFilters(dialog, filters, defaultExt); ApplyInitialDirectory(dialog, initialDirectory); return ShowAndGetPath(dialog, owner); } finally { + filterMemory?.Dispose(); Release(dialog); } } @@ -82,11 +84,12 @@ internal static partial class ShellFileDialog public static string? PickSaveFile(nint owner, string title, (string Name, string Spec)[] filters, string? defaultExt, string? defaultFileName, string? initialDirectory) { var dialog = (IFileSaveDialog)CreateInstance(CLSID_FileSaveDialog, IID_IFileSaveDialog); + FilterNativeMemory? filterMemory = null; try { dialog.SetOptions(FOS_FORCEFILESYSTEM | FOS_OVERWRITEPROMPT | FOS_PATHMUSTEXIST | FOS_NOCHANGEDIR); dialog.SetTitle(title); - ApplyFilters(dialog, filters, defaultExt); + filterMemory = ApplyFilters(dialog, filters, defaultExt); if (!string.IsNullOrEmpty(defaultFileName)) { dialog.SetFileName(defaultFileName); @@ -97,6 +100,7 @@ internal static partial class ShellFileDialog } finally { + filterMemory?.Dispose(); Release(dialog); } } @@ -167,11 +171,21 @@ private static void ApplyInitialDirectory(IFileDialog dialog, string? initialDir } } - private static void ApplyFilters(IFileDialog dialog, (string Name, string Spec)[] filters, string? defaultExt) + /// + /// Builds COMDLG_FILTERSPEC native memory for . + /// The returned disposable must stay alive until after ; + /// the dialog may keep pointers into this buffer for the lifetime of the modal call. + /// + private static FilterNativeMemory? ApplyFilters(IFileDialog dialog, (string Name, string Spec)[] filters, string? defaultExt) { if (filters.Length == 0) { - return; + if (!string.IsNullOrEmpty(defaultExt)) + { + dialog.SetDefaultExtension(defaultExt); + } + + return null; } // COMDLG_FILTERSPEC is { LPWSTR pszName; LPWSTR pszSpec; }. Build the native array @@ -192,20 +206,55 @@ private static void ApplyFilters(IFileDialog dialog, (string Name, string Spec)[ dialog.SetFileTypes((uint)filters.Length, block); dialog.SetFileTypeIndex(1); + if (!string.IsNullOrEmpty(defaultExt)) + { + dialog.SetDefaultExtension(defaultExt); + } + + // Ownership transfers to the caller until Show completes. + var memory = new FilterNativeMemory(block, strings); + block = 0; + strings = null; + return memory; } finally { - foreach (nint s in strings) + if (strings is not null) { - Marshal.FreeCoTaskMem(s); + foreach (nint s in strings) + { + Marshal.FreeCoTaskMem(s); + } } - Marshal.FreeCoTaskMem(block); + if (block != 0) + { + Marshal.FreeCoTaskMem(block); + } } + } - if (!string.IsNullOrEmpty(defaultExt)) + private sealed class FilterNativeMemory(nint block, List strings) : IDisposable + { + private nint _block = block; + private List? _strings = strings; + + public void Dispose() { - dialog.SetDefaultExtension(defaultExt); + var ownedStrings = Interlocked.Exchange(ref _strings, null); + if (ownedStrings is not null) + { + foreach (nint s in ownedStrings) + { + Marshal.FreeCoTaskMem(s); + } + } + + var ownedBlock = Interlocked.Exchange(ref _block, 0); + if (ownedBlock != 0) + { + Marshal.FreeCoTaskMem(ownedBlock); + } } } diff --git a/QuickShell.Core/Interop/Win32Clipboard.cs b/QuickShell.Core/Interop/Win32Clipboard.cs index 06874fb0..3d34539f 100644 --- a/QuickShell.Core/Interop/Win32Clipboard.cs +++ b/QuickShell.Core/Interop/Win32Clipboard.cs @@ -55,7 +55,16 @@ internal static partial class Win32Clipboard } int charCount = (int)(byteLen / sizeof(char)); - return Marshal.PtrToStringUni(locked, charCount)?.TrimEnd('\0'); + var text = Marshal.PtrToStringUni(locked, charCount); + if (text is null) + { + return null; + } + + // CF_UNICODETEXT is a single null-terminated string; drop anything after the + // first embedded '\0' (padding in the HGLOBAL), not trailing-only with TrimEnd. + var terminator = text.IndexOf('\0'); + return terminator < 0 ? text : text[..terminator]; } finally { diff --git a/QuickShell.Core/Services/StaModalDialogRunner.cs b/QuickShell.Core/Services/StaModalDialogRunner.cs new file mode 100644 index 00000000..fed6bb2b --- /dev/null +++ b/QuickShell.Core/Services/StaModalDialogRunner.cs @@ -0,0 +1,67 @@ +using System.Threading; +using QuickShell.Interop; + +namespace QuickShell.Services; + +/// +/// Runs a modal shell dialog on an STA thread with timeout recovery. Publishes the +/// native thread id with so timeout close can find the dialog, +/// and swallows worker exceptions into a null result so they never tear down the process. +/// +internal static class StaModalDialogRunner +{ + public static string? Run( + nint ownerHandle, + Func action, + TimeSpan dialogTimeout, + TimeSpan joinGracePeriod) + { + ArgumentNullException.ThrowIfNull(action); + + if (Thread.CurrentThread.GetApartmentState() == ApartmentState.STA) + { + try + { + return action(); + } + catch (Exception) + { + return null; + } + } + + string? selected = null; + Exception? fault = null; + var nativeThreadId = 0; + var thread = new Thread(() => + { + Volatile.Write(ref nativeThreadId, StaDialogCloser.CurrentNativeThreadId()); + try + { + selected = action(); + } + catch (Exception ex) + { + fault = ex; + } + }) + { + IsBackground = true, + }; + thread.SetApartmentState(ApartmentState.STA); + thread.Start(); + + if (thread.Join(dialogTimeout + joinGracePeriod)) + { + return fault is null ? selected : null; + } + + StaDialogCloser.TryCloseThreadDialog(Volatile.Read(ref nativeThreadId), ownerHandle); + if (!thread.Join(joinGracePeriod)) + { + return null; + } + + return fault is null ? selected : null; + } +} diff --git a/QuickShell.Run/Services/FolderPickerService.cs b/QuickShell.Run/Services/FolderPickerService.cs index a4a308b8..4e4ac6bb 100644 --- a/QuickShell.Run/Services/FolderPickerService.cs +++ b/QuickShell.Run/Services/FolderPickerService.cs @@ -1,5 +1,4 @@ using System.IO; -using System.Threading; using QuickShell.Interop; namespace QuickShell.Services; @@ -12,35 +11,11 @@ internal static class FolderPickerService public static string? PickFolder(string? initialDirectory = null) { var ownerHandle = NativeForegroundWindow.Get(); - - if (Thread.CurrentThread.GetApartmentState() == ApartmentState.STA) - { - return PickFolderOnStaThread(initialDirectory, ownerHandle); - } - - string? selected = null; - var nativeThreadId = 0; - var thread = new Thread(() => - { - nativeThreadId = StaDialogCloser.CurrentNativeThreadId(); - selected = PickFolderOnStaThread(initialDirectory, ownerHandle); - }) - { - IsBackground = true, - }; - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - - // The modal IFileDialog blocks the STA thread until dismissed. DialogTimeout + - // JoinGracePeriod bounds a stuck dialog; on timeout, post WM_CLOSE only to a - // dialog owned by that STA thread (not an arbitrary foreground window). - if (thread.Join(DialogTimeout + JoinGracePeriod)) - { - return selected; - } - - StaDialogCloser.TryCloseThreadDialog(Volatile.Read(ref nativeThreadId), ownerHandle); - return thread.Join(JoinGracePeriod) ? selected : null; + return StaModalDialogRunner.Run( + ownerHandle, + () => PickFolderOnStaThread(initialDirectory, ownerHandle), + DialogTimeout, + JoinGracePeriod); } private static string? PickFolderOnStaThread(string? initialDirectory, nint ownerHandle) diff --git a/QuickShell/Services/FolderPickerService.cs b/QuickShell/Services/FolderPickerService.cs index af334abe..aa1bb19f 100644 --- a/QuickShell/Services/FolderPickerService.cs +++ b/QuickShell/Services/FolderPickerService.cs @@ -1,4 +1,3 @@ -using System.Threading; using QuickShell.Interop; namespace QuickShell.Services; @@ -11,35 +10,11 @@ internal static class FolderPickerService public static string? PickFolder(string? initialDirectory = null) { var ownerHandle = NativeForegroundWindow.Get(); - - if (Thread.CurrentThread.GetApartmentState() == ApartmentState.STA) - { - return PickFolderOnStaThread(initialDirectory, ownerHandle); - } - - string? selected = null; - var nativeThreadId = 0; - var thread = new Thread(() => - { - nativeThreadId = StaDialogCloser.CurrentNativeThreadId(); - selected = PickFolderOnStaThread(initialDirectory, ownerHandle); - }) - { - IsBackground = true, - }; - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - - // Happy path: the modal IFileDialog blocks the STA thread until the user dismisses it, - // then the thread exits. DialogTimeout + JoinGracePeriod bounds a stuck dialog; on - // timeout, post WM_CLOSE only to a dialog owned by that STA thread. - if (thread.Join(DialogTimeout + JoinGracePeriod)) - { - return selected; - } - - StaDialogCloser.TryCloseThreadDialog(Volatile.Read(ref nativeThreadId), ownerHandle); - return thread.Join(JoinGracePeriod) ? selected : null; + return StaModalDialogRunner.Run( + ownerHandle, + () => PickFolderOnStaThread(initialDirectory, ownerHandle), + DialogTimeout, + JoinGracePeriod); } private static string? PickFolderOnStaThread(string? initialDirectory, nint ownerHandle) diff --git a/QuickShell/Services/ShortcutFilePickerService.cs b/QuickShell/Services/ShortcutFilePickerService.cs index ab7814e4..a03271c1 100644 --- a/QuickShell/Services/ShortcutFilePickerService.cs +++ b/QuickShell/Services/ShortcutFilePickerService.cs @@ -1,4 +1,3 @@ -using System.Threading; using QuickShell.Interop; namespace QuickShell.Services; @@ -27,7 +26,7 @@ private static readonly (string Name, string Spec)[] ExecutableFilters = var initialDirectory = DirectoryOrNull(services.Shortcuts.ConfigDirectory); var ownerHandle = NativeForegroundWindow.Get(); - return RunOnStaThread( + return StaModalDialogRunner.Run( ownerHandle, () => ShellFileDialog.PickSaveFile( ownerHandle, @@ -35,7 +34,9 @@ private static readonly (string Name, string Spec)[] ExecutableFilters = JsonFilters, defaultExt: "json", defaultFileName: defaultName, - initialDirectory: initialDirectory)); + initialDirectory: initialDirectory), + DialogTimeout, + JoinGracePeriod); } public static string? PickImportFile(IQuickShellServices services) @@ -44,14 +45,16 @@ private static readonly (string Name, string Spec)[] ExecutableFilters = var initialDirectory = DirectoryOrNull(services.Shortcuts.ConfigDirectory); var ownerHandle = NativeForegroundWindow.Get(); - return RunOnStaThread( + return StaModalDialogRunner.Run( ownerHandle, () => ShellFileDialog.PickOpenFile( ownerHandle, $"Import {QuickShellBrand.DisplayName} workspaces", JsonFilters, defaultExt: "json", - initialDirectory: initialDirectory)); + initialDirectory: initialDirectory), + DialogTimeout, + JoinGracePeriod); } public static string? PickExecutableFile() @@ -60,45 +63,18 @@ private static readonly (string Name, string Spec)[] ExecutableFilters = var initialDirectory = DirectoryOrNull(programFiles); var ownerHandle = NativeForegroundWindow.Get(); - return RunOnStaThread( + return StaModalDialogRunner.Run( ownerHandle, () => ShellFileDialog.PickOpenFile( ownerHandle, "Choose companion app", ExecutableFilters, defaultExt: "exe", - initialDirectory: initialDirectory)); + initialDirectory: initialDirectory), + DialogTimeout, + JoinGracePeriod); } private static string? DirectoryOrNull(string? path) => !string.IsNullOrWhiteSpace(path) && Directory.Exists(path) ? path : null; - - private static string? RunOnStaThread(nint ownerHandle, Func action) - { - if (Thread.CurrentThread.GetApartmentState() == ApartmentState.STA) - { - return action(); - } - - string? result = null; - var nativeThreadId = 0; - var thread = new Thread(() => - { - nativeThreadId = StaDialogCloser.CurrentNativeThreadId(); - result = action(); - }) - { - IsBackground = true, - }; - thread.SetApartmentState(ApartmentState.STA); - thread.Start(); - - if (thread.Join(DialogTimeout + JoinGracePeriod)) - { - return result; - } - - StaDialogCloser.TryCloseThreadDialog(Volatile.Read(ref nativeThreadId), ownerHandle); - return thread.Join(JoinGracePeriod) ? result : null; - } } From 20673a040ae1b8fb564924e984c53ad539fba9c0 Mon Sep 17 00:00:00 2001 From: Anthony Thompson Date: Sat, 25 Jul 2026 10:42:19 -0700 Subject: [PATCH 5/8] Potential fix for pull request finding 'CodeQL / Unmanaged code' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Signed-off-by: Anthony Thompson --- QuickShell.Core/Interop/StaDialogCloser.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/QuickShell.Core/Interop/StaDialogCloser.cs b/QuickShell.Core/Interop/StaDialogCloser.cs index 879a7f52..6c508799 100644 --- a/QuickShell.Core/Interop/StaDialogCloser.cs +++ b/QuickShell.Core/Interop/StaDialogCloser.cs @@ -65,8 +65,7 @@ public static void TryCloseThreadDialog(int nativeThreadId, nint ownerHandle) public static int CurrentNativeThreadId() { - // codeql[cs/call-to-unmanaged-code]: Native thread id required for EnumThreadWindows on the picker STA thread. - return GetCurrentThreadId(); + return Environment.CurrentManagedThreadId; } private static unsafe bool IsDialogClass(nint hwnd) @@ -102,6 +101,5 @@ private static unsafe bool IsDialogClass(nint hwnd) [return: MarshalAs(UnmanagedType.Bool)] private static extern bool PostMessage(nint hWnd, int msg, nint wParam, nint lParam); - [DllImport("kernel32.dll")] - private static extern int GetCurrentThreadId(); + } From 69e54b199739dd45c7bdeec8b230178e486ca894 Mon Sep 17 00:00:00 2001 From: "coderabbitai[bot]" <136622811+coderabbitai[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 17:42:34 +0000 Subject: [PATCH 6/8] =?UTF-8?q?=F0=9F=93=9D=20Add=20docstrings=20to=20`ref?= =?UTF-8?q?actor/drop-winforms-shrink-msix`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docstrings generation was requested by @tonythethompson. The following files were modified: * `QuickShell.Core/Classification/ProjectAnalysisService.cs` * `QuickShell.Core/Interop/NativeForegroundWindow.cs` * `QuickShell.Core/Interop/ShellFileDialog.cs` * `QuickShell.Core/Interop/StaDialogCloser.cs` * `QuickShell.Core/Interop/Win32Clipboard.cs` * `QuickShell.Core/Services/CompanionAppCatalog.cs` * `QuickShell.Core/Services/StaModalDialogRunner.cs` * `QuickShell.Core/Services/WorkspaceSecurityPolicy.cs` * `QuickShell.Run/Services/FolderPickerService.cs` * `QuickShell.Run/Services/StaClipboard.cs` * `QuickShell/Services/FolderPickerService.cs` * `QuickShell/Services/ShortcutFilePickerService.cs` * `QuickShell/Services/StaClipboard.cs` These files were ignored: * `QuickShell.Core.Tests/WorkspaceSecurityPolicyTests.cs` These file types are not supported: * `.github/codeql/codeql-config.yml` * `.github/workflows/codeql.yml` * `.github/workflows/release-extension.yml` * `.github/workflows/release-run-plugin.yml` * `Directory.Packages.props` * `QuickShell.Core/QuickShell.Core.csproj` * `QuickShell.Run/QuickShell.Run.csproj` * `QuickShell/ILLink.Descriptors.xml` * `QuickShell/QuickShell.csproj` --- .../Classification/ProjectAnalysisService.cs | 14 +- .../Interop/NativeForegroundWindow.cs | 8 + QuickShell.Core/Interop/ShellFileDialog.cs | 257 ++++++++++++++++-- QuickShell.Core/Interop/StaDialogCloser.cs | 42 ++- QuickShell.Core/Interop/Win32Clipboard.cs | 48 ++++ .../Services/CompanionAppCatalog.cs | 11 +- .../Services/StaModalDialogRunner.cs | 9 + .../Services/WorkspaceSecurityPolicy.cs | 5 + .../Services/FolderPickerService.cs | 11 + QuickShell.Run/Services/StaClipboard.cs | 5 + QuickShell/Services/FolderPickerService.cs | 11 + .../Services/ShortcutFilePickerService.cs | 21 +- QuickShell/Services/StaClipboard.cs | 16 +- 13 files changed, 434 insertions(+), 24 deletions(-) diff --git a/QuickShell.Core/Classification/ProjectAnalysisService.cs b/QuickShell.Core/Classification/ProjectAnalysisService.cs index 244c08fb..5673e8a4 100644 --- a/QuickShell.Core/Classification/ProjectAnalysisService.cs +++ b/QuickShell.Core/Classification/ProjectAnalysisService.cs @@ -79,6 +79,13 @@ public string GetTaskTypeChoiceTooltip(string? directory, string? taskType, Task return $"Suggests: {first.Command} · also {alternates}"; } + /// + /// Builds a JSON payload containing the available task type choices. + /// + /// The project directory used to determine available choices. + /// Context used to select task type suggestions. + /// Whether to include the choice for adding a new command row. + /// A JSON representation of the task type choices. public string BuildTaskTypeChoicesJson( string? directory = null, TaskTypePickContext? pickContext = null, @@ -110,7 +117,12 @@ public string BuildTaskTypeChoicesJson( return System.Text.Json.JsonSerializer.Serialize(choices, QuickShellJsonContext.Default.ListTaskTypeChoiceJson); } - public CompanionAppSuggestion? TrySuggestCompanionApp(string directory) => + /// + /// Suggests a companion app for the specified project directory. + /// + /// The project directory to inspect. + /// A companion app suggestion, or null when no suggestion is available. + public CompanionAppSuggestion? TrySuggestCompanionApp(string directory) => _companionAppDetector.TrySuggest(directory); public string? TryDetectDevServerUrl(string directory) => diff --git a/QuickShell.Core/Interop/NativeForegroundWindow.cs b/QuickShell.Core/Interop/NativeForegroundWindow.cs index 53c07cd9..37b385fd 100644 --- a/QuickShell.Core/Interop/NativeForegroundWindow.cs +++ b/QuickShell.Core/Interop/NativeForegroundWindow.cs @@ -9,12 +9,20 @@ namespace QuickShell.Interop; /// internal static partial class NativeForegroundWindow { + /// + /// Gets the handle of the current foreground window. + /// + /// The handle of the current foreground window. public static nint Get() { // codeql[cs/call-to-unmanaged-code]: Required for IFileDialog owner hwnd; no managed API after WinForms removal. return GetForegroundWindow(); } + /// + /// Retrieves the handle of the current foreground window. + /// + /// The handle of the foreground window, or zero if no foreground window exists. [LibraryImport("user32.dll")] private static partial nint GetForegroundWindow(); } diff --git a/QuickShell.Core/Interop/ShellFileDialog.cs b/QuickShell.Core/Interop/ShellFileDialog.cs index a28e8707..14107d97 100644 --- a/QuickShell.Core/Interop/ShellFileDialog.cs +++ b/QuickShell.Core/Interop/ShellFileDialog.cs @@ -43,7 +43,12 @@ internal static partial class ShellFileDialog // HRESULT_FROM_WIN32(ERROR_CANCELLED) — returned by Show when the user cancels. private const int ERROR_CANCELLED_HRESULT = unchecked((int)0x800704C7); - /// Modern folder picker (IFileOpenDialog with FOS_PICKFOLDERS). + /// + /// Displays a folder picker and obtains the selected folder path. + /// + /// The handle of the parent window. + /// The directory initially displayed by the picker, when valid. + /// The selected folder path, or null if the picker is canceled. public static string? PickFolder(nint owner, string? initialDirectory) { var dialog = (IFileOpenDialog)CreateInstance(CLSID_FileOpenDialog, IID_IFileOpenDialog); @@ -60,7 +65,15 @@ internal static partial class ShellFileDialog } /// Open-file picker. pairs display name to a - /// semicolon-delimited pattern (e.g. "JSON files (*.json)" -> "*.json"). + /// + /// Displays a file selection dialog and obtains the selected file path. + /// + /// The handle of the dialog's owner window. + /// The dialog title. + /// The file type filters to display. + /// The default file extension. + /// The directory initially displayed by the dialog. + /// The selected file path, or null if the dialog is canceled. public static string? PickOpenFile(nint owner, string title, (string Name, string Spec)[] filters, string? defaultExt, string? initialDirectory) { var dialog = (IFileOpenDialog)CreateInstance(CLSID_FileOpenDialog, IID_IFileOpenDialog); @@ -80,7 +93,11 @@ internal static partial class ShellFileDialog } } - /// Save-file picker with overwrite prompt and a suggested file name. + /// + /// Opens a save-file dialog with overwrite confirmation and optional file name, extension, filter, and initial directory settings. + /// + /// The suggested file name. + /// The selected file's filesystem path, or null if the dialog is canceled. public static string? PickSaveFile(nint owner, string title, (string Name, string Spec)[] filters, string? defaultExt, string? defaultFileName, string? initialDirectory) { var dialog = (IFileSaveDialog)CreateInstance(CLSID_FileSaveDialog, IID_IFileSaveDialog); @@ -105,6 +122,12 @@ internal static partial class ShellFileDialog } } + /// + /// Displays the file dialog and retrieves the selected item's filesystem path. + /// + /// The file dialog to display. + /// The handle of the dialog's owner window. + /// The selected filesystem path, or null if the dialog is canceled or no path is available. private static string? ShowAndGetPath(IFileDialog dialog, nint owner) { int hr = dialog.Show(owner); @@ -146,6 +169,11 @@ internal static partial class ShellFileDialog } } + /// + /// Applies an existing directory as the dialog's initial folder. + /// + /// The file dialog to configure. + /// The directory to use initially, or to use the shell's default folder. private static void ApplyInitialDirectory(IFileDialog dialog, string? initialDirectory) { if (string.IsNullOrWhiteSpace(initialDirectory) || !Directory.Exists(initialDirectory)) @@ -175,7 +203,12 @@ private static void ApplyInitialDirectory(IFileDialog dialog, string? initialDir /// Builds COMDLG_FILTERSPEC native memory for . /// The returned disposable must stay alive until after ; /// the dialog may keep pointers into this buffer for the lifetime of the modal call. + /// + /// Applies file type filters and an optional default extension to a file dialog. /// + /// The display names and wildcard specifications for the file types. + /// The default file name extension. + /// Native memory that owns the applied filter specifications, or null when no filters are provided. private static FilterNativeMemory? ApplyFilters(IFileDialog dialog, (string Name, string Spec)[] filters, string? defaultExt) { if (filters.Length == 0) @@ -239,6 +272,9 @@ private sealed class FilterNativeMemory(nint block, List strings) : IDispo private nint _block = block; private List? _strings = strings; + /// + /// Releases the native memory allocated for the filter specifications. + /// public void Dispose() { var ownedStrings = Interlocked.Exchange(ref _strings, null); @@ -258,6 +294,12 @@ public void Dispose() } } + /// + /// Creates and wraps an instance of the specified COM class and interface. + /// + /// The class identifier of the COM class to create. + /// The interface identifier to expose. + /// The wrapped COM object. private static object CreateInstance(Guid clsid, Guid iid) { int hr = CoCreateInstance(in clsid, 0, CLSCTX_INPROC_SERVER, in iid, out nint ptr); @@ -271,9 +313,26 @@ private static object CreateInstance(Guid clsid, Guid iid) // on Dispose — Marshal.FinalReleaseComObject does not work with them (SYSLIB1099). private static void Release(object comObject) => (comObject as IDisposable)?.Dispose(); + /// + /// Creates a COM object for the specified class and interface. + /// + /// The class identifier of the COM object to create. + /// The controlling unknown for aggregation, or zero. + /// The execution context in which the COM object runs. + /// The interface identifier to retrieve. + /// Receives a pointer to the requested interface. + /// An HRESULT indicating whether the object was created successfully. [LibraryImport("ole32.dll")] private static partial int CoCreateInstance(in Guid rclsid, nint pUnkOuter, uint dwClsContext, in Guid riid, out nint ppv); + /// + /// Creates a Shell item from a filesystem path. + /// + /// The filesystem path used to create the Shell item. + /// The optional bind context, or zero. + /// The interface identifier requested for the Shell item. + /// Receives a pointer to the requested interface. + /// An HRESULT indicating whether the Shell item was created. [LibraryImport("shell32.dll", StringMarshalling = StringMarshalling.Utf16)] private static partial int SHCreateItemFromParsingName(string pszPath, nint pbc, in Guid riid, out nint ppv); @@ -282,6 +341,11 @@ private static object CreateInstance(Guid clsid, Guid iid) [Guid("b4db1657-70d7-485e-8e3e-6fcb5a5c1802")] internal partial interface IModalWindow { + /// + /// Displays the modal window. + /// + /// The handle of the window that owns the modal window. + /// An HRESULT indicating whether the window was displayed successfully. [PreserveSig] int Show(nint parent); } @@ -292,64 +356,158 @@ internal partial interface IModalWindow [Guid("42f85136-db7e-439c-85f1-e4075d135fc8")] internal partial interface IFileDialog : IModalWindow { - void SetFileTypes(uint cFileTypes, nint rgFilterSpec); - - void SetFileTypeIndex(uint iFileType); + /// +/// Configures the file types displayed by the dialog. +/// +/// The number of file type specifications. +/// A pointer to the array of file type specifications. +void SetFileTypes(uint cFileTypes, nint rgFilterSpec); + /// +/// Selects the default file type in the dialog's file type list. +/// +/// The one-based index of the file type to select. +void SetFileTypeIndex(uint iFileType); + + /// + /// Retrieves the index of the currently selected file type. + /// + /// Receives the one-based index of the selected file type. + /// An HRESULT indicating whether the operation succeeded. [PreserveSig] int GetFileTypeIndex(out uint piFileType); [PreserveSig] int Advise(nint pfde, out uint pdwCookie); + /// + /// Removes the registered file dialog event handler. + /// + /// The connection cookie returned when the event handler was registered. + /// An HRESULT indicating whether the event handler was removed successfully. [PreserveSig] int Unadvise(uint dwCookie); - void SetOptions(uint fos); - + /// +/// Configures the file dialog options. +/// +/// The bitwise combination of file dialog option flags. +void SetOptions(uint fos); + + /// + /// Retrieves the current options configured for the file dialog. + /// + /// Receives the configured file dialog option flags. + /// An HRESULT indicating whether the options were retrieved successfully. [PreserveSig] int GetOptions(out uint pfos); + /// + /// Sets the dialog's default folder. + /// + /// A pointer to the shell item representing the default folder. + /// An HRESULT indicating whether the operation succeeded. [PreserveSig] int SetDefaultFolder(nint psi); - void SetFolder(IShellItem psi); - + /// +/// Sets the folder displayed by the file dialog. +/// +/// The shell item representing the folder to display. +void SetFolder(IShellItem psi); + + /// + /// Retrieves the dialog's current folder. + /// + /// Receives a pointer to the folder's shell item. + /// An HRESULT indicating whether the operation succeeded. [PreserveSig] int GetFolder(out nint ppsi); + /// + /// Retrieves the shell item currently selected in the dialog. + /// + /// Receives a pointer to the selected shell item. + /// An HRESULT indicating whether the operation succeeded. [PreserveSig] int GetCurrentSelection(out nint ppsi); - void SetFileName([MarshalAs(UnmanagedType.LPWStr)] string pszName); - + /// +/// Sets the file name displayed in the dialog. +/// +/// The file name to display. +void SetFileName([MarshalAs(UnmanagedType.LPWStr)] string pszName); + + /// + /// Retrieves the current file name displayed by the dialog. + /// + /// Receives a pointer to the file name. + /// An HRESULT indicating whether the file name was retrieved successfully. [PreserveSig] int GetFileName(out nint pszName); - void SetTitle([MarshalAs(UnmanagedType.LPWStr)] string pszTitle); + /// +/// Sets the dialog title. +/// +void SetTitle([MarshalAs(UnmanagedType.LPWStr)] string pszTitle); + /// + /// Sets the label of the dialog's OK button. + /// + /// A pointer to a null-terminated string containing the button label. + /// An HRESULT indicating whether the label was set successfully. [PreserveSig] int SetOkButtonLabel(nint pszText); + /// + /// Sets the label for the file name input control. + /// + /// A pointer to the label string. + /// An HRESULT indicating whether the operation succeeded. [PreserveSig] int SetFileNameLabel(nint pszLabel); - void GetResult(out nint ppsi); - + /// +/// Retrieves the selected shell item. +/// +/// Receives a pointer to the selected shell item. +/// +void GetResult(out nint ppsi); + + /// + /// Adds a location to the dialog's navigation pane. + /// + /// A pointer to the shell item representing the location. + /// The placement of the location in the navigation pane. + /// An HRESULT indicating whether the location was added successfully. [PreserveSig] int AddPlace(nint psi, int fdap); void SetDefaultExtension([MarshalAs(UnmanagedType.LPWStr)] string pszDefaultExtension); + /// + /// Closes the dialog with the specified result code. + /// + /// The result code to associate with the dialog closure. + /// The HRESULT returned by the operation. [PreserveSig] int Close(int hr); [PreserveSig] int SetClientGuid(in Guid guid); + /// + /// Clears the dialog's client data. + /// + /// An HRESULT indicating whether the operation succeeded. [PreserveSig] int ClearClientData(); + /// + /// Sets the dialog's filter configuration. + /// + /// A pointer to the filter configuration. + /// An HRESULT indicating whether the operation succeeded. [PreserveSig] int SetFilter(nint pFilter); } @@ -360,9 +518,19 @@ internal partial interface IFileDialog : IModalWindow [Guid("d57c7288-d4ad-4768-be02-9d969532d960")] internal partial interface IFileOpenDialog : IFileDialog { + /// + /// Retrieves an enumerator for the selected items. + /// + /// Receives a pointer to the selected-items enumerator. + /// An HRESULT indicating whether the operation succeeded. [PreserveSig] int GetResults(out nint ppenum); + /// + /// Retrieves the items selected in the dialog. + /// + /// Receives a pointer to the selected items collection. + /// An HRESULT indicating whether the operation succeeded. [PreserveSig] int GetSelectedItems(out nint ppsai); } @@ -372,18 +540,42 @@ internal partial interface IFileOpenDialog : IFileDialog [Guid("84bccd23-5fde-4cdb-aea4-af64b83d78ab")] internal partial interface IFileSaveDialog : IFileDialog { + /// + /// Sets the item to use as the save destination. + /// + /// A pointer to the shell item. + /// An HRESULT indicating whether the operation succeeded. [PreserveSig] int SetSaveAsItem(nint psi); [PreserveSig] int SetProperties(nint pStore); + /// + /// Configures the properties collected by the dialog. + /// + /// A pointer to the property description list. + /// A value indicating whether to append the default properties. + /// An HRESULT indicating the operation result. [PreserveSig] int SetCollectedProperties(nint pList, int fAppendDefault); + /// + /// Retrieves the property store for the shell item. + /// + /// Receives a pointer to the property store. + /// An HRESULT indicating whether the operation succeeded. [PreserveSig] int GetProperties(out nint ppStore); + /// + /// Applies the specified property store to a shell item. + /// + /// A pointer to the shell item. + /// A pointer to the property store containing the properties to apply. + /// The handle of the parent window. + /// A pointer to the progress notification sink. + /// An HRESULT indicating whether the properties were applied successfully. [PreserveSig] int ApplyProperties(nint psi, nint pStore, nint hwnd, nint pSink); } @@ -392,17 +584,48 @@ internal partial interface IFileSaveDialog : IFileDialog [Guid("43826d1e-e718-42ee-bc55-a1e261c37bfe")] internal partial interface IShellItem { + /// + /// Binds the shell item to the specified handler and interface. + /// + /// The optional bind context. + /// The identifier of the handler to use. + /// The identifier of the requested interface. + /// Receives a pointer to the requested interface. + /// An HRESULT indicating whether the binding succeeded. [PreserveSig] int BindToHandler(nint pbc, in Guid bhid, in Guid riid, out nint ppv); + /// + /// Retrieves the parent shell item. + /// + /// Receives a pointer to the parent shell item. + /// An HRESULT indicating whether the operation succeeded. [PreserveSig] int GetParent(out nint ppsi); - void GetDisplayName(uint sigdnName, out nint ppszName); - + /// +/// Retrieves the item's display name in the specified format. +/// +/// The display name format to retrieve. +/// Receives a pointer to the allocated display-name string. +void GetDisplayName(uint sigdnName, out nint ppszName); + + /// + /// Retrieves the specified attributes of the shell item. + /// + /// The attributes to retrieve. + /// Receives the retrieved attributes. + /// An HRESULT indicating whether the operation succeeded. [PreserveSig] int GetAttributes(uint sfgaoMask, out uint psfgaoAttribs); + /// + /// Compares this shell item with another shell item. + /// + /// The shell item to compare with this item. + /// The comparison criteria. + /// Receives a value indicating the relative ordering of the shell items. + /// An HRESULT indicating whether the comparison succeeded. [PreserveSig] int Compare(nint psi, uint hint, out int piOrder); } diff --git a/QuickShell.Core/Interop/StaDialogCloser.cs b/QuickShell.Core/Interop/StaDialogCloser.cs index 6c508799..a7bd651e 100644 --- a/QuickShell.Core/Interop/StaDialogCloser.cs +++ b/QuickShell.Core/Interop/StaDialogCloser.cs @@ -13,6 +13,11 @@ internal static class StaDialogCloser private const int WM_CLOSE = 0x0010; private const uint GA_ROOT = 2; + /// + /// Requests closure of a visible top-level dialog owned by the specified native thread. + /// + /// The native thread identifier whose dialog windows are searched. + /// The window handle to exclude from consideration. public static void TryCloseThreadDialog(int nativeThreadId, nint ownerHandle) { if (nativeThreadId == 0) @@ -63,11 +68,21 @@ public static void TryCloseThreadDialog(int nativeThreadId, nint ownerHandle) } } + /// + /// Gets the native identifier of the current thread. + /// + /// The native thread identifier. public static int CurrentNativeThreadId() { - return Environment.CurrentManagedThreadId; + // codeql[cs/call-to-unmanaged-code]: Native thread id required for EnumThreadWindows on the picker STA thread. + return GetCurrentThreadId(); } + /// + /// Determines whether a window uses the standard dialog window class. + /// + /// The window handle to inspect. + /// true if the window class is #32770; false otherwise. private static unsafe bool IsDialogClass(nint hwnd) { Span buffer = stackalloc char[64]; @@ -83,6 +98,13 @@ private static unsafe bool IsDialogClass(nint hwnd) private delegate bool EnumThreadWndProc(nint hWnd, nint lParam); + /// + /// Enumerates the top-level windows associated with a thread. + /// + /// The identifier of the thread whose windows are enumerated. + /// The callback invoked for each enumerated window. + /// Application-defined data passed to the callback. + /// true if the enumeration was initiated successfully; otherwise, false. [DllImport("user32.dll")] [return: MarshalAs(UnmanagedType.Bool)] private static extern bool EnumThreadWindows(int dwThreadId, EnumThreadWndProc lpfn, nint lParam); @@ -94,12 +116,28 @@ private static unsafe bool IsDialogClass(nint hwnd) [DllImport("user32.dll")] private static extern nint GetAncestor(nint hWnd, uint gaFlags); + /// + /// Retrieves the class name of the specified window. + /// + /// The handle of the window. + /// The buffer that receives the window class name. + /// The maximum number of characters to copy to the buffer. + /// The number of characters copied to the buffer, excluding the terminating null character. [DllImport("user32.dll", CharSet = CharSet.Unicode, ExactSpelling = true)] private static extern unsafe int GetClassNameW(nint hWnd, char* lpClassName, int nMaxCount); + /// + /// Posts a message to the specified window. + /// + /// The handle of the window that receives the message. + /// The message to post. + /// Additional message information. + /// Additional message information. + /// true if the message was posted successfully; otherwise, false. [DllImport("user32.dll")] [return: MarshalAs(UnmanagedType.Bool)] private static extern bool PostMessage(nint hWnd, int msg, nint wParam, nint lParam); - + [DllImport("kernel32.dll")] + private static extern int GetCurrentThreadId(); } diff --git a/QuickShell.Core/Interop/Win32Clipboard.cs b/QuickShell.Core/Interop/Win32Clipboard.cs index 3d34539f..a5f08c13 100644 --- a/QuickShell.Core/Interop/Win32Clipboard.cs +++ b/QuickShell.Core/Interop/Win32Clipboard.cs @@ -18,6 +18,10 @@ internal static partial class Win32Clipboard private const int OpenRetryCount = 10; private const int OpenRetryDelayMs = 50; + /// + /// Reads Unicode text from the Windows clipboard. + /// + /// The clipboard text, or null if Unicode text is unavailable or cannot be read. public static string? GetText() { if (!IsClipboardFormatAvailable(CF_UNICODETEXT)) @@ -77,6 +81,12 @@ internal static partial class Win32Clipboard } } + /// + /// Places Unicode text on the system clipboard. + /// + /// The text to place on the clipboard. + /// true if the text is set successfully; false if the clipboard cannot be updated. + /// Thrown when is null. public static bool SetText(string text) { ArgumentNullException.ThrowIfNull(text); @@ -147,7 +157,10 @@ public static bool SetText(string text) /// /// WinForms Clipboard retried OpenClipboard briefly when another app held it. /// Match that so transient contention does not fail copy/paste. + /// + /// Attempts to open the clipboard, retrying after transient failures. /// + /// true if the clipboard is opened successfully; false if all attempts fail. private static bool TryOpenClipboard() { for (var attempt = 0; attempt < OpenRetryCount; attempt++) @@ -166,6 +179,11 @@ private static bool TryOpenClipboard() return false; } + /// + /// Opens the clipboard for access by the specified window. + /// + /// The handle of the window that owns the open clipboard, or zero for no owner. + /// true if the clipboard is opened successfully; otherwise, false. [LibraryImport("user32.dll", SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] private static partial bool OpenClipboard(nint hWndNewOwner); @@ -178,10 +196,20 @@ private static bool TryOpenClipboard() [return: MarshalAs(UnmanagedType.Bool)] private static partial bool EmptyClipboard(); + /// + /// Determines whether the specified clipboard format is available. + /// + /// The clipboard format to check. + /// true if the format is available; otherwise, false. [LibraryImport("user32.dll", SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] private static partial bool IsClipboardFormatAvailable(uint format); + /// + /// Retrieves clipboard data in the specified format. + /// + /// The clipboard format to retrieve. + /// A handle to the clipboard data, or zero if the data is unavailable. [LibraryImport("user32.dll", SetLastError = true)] private static partial nint GetClipboardData(uint uFormat); @@ -191,16 +219,36 @@ private static bool TryOpenClipboard() [LibraryImport("kernel32.dll", SetLastError = true)] private static partial nint GlobalAlloc(uint uFlags, nint dwBytes); + /// + /// Releases globally allocated memory. + /// + /// A handle to the memory block to release. + /// A null handle if the memory is released successfully; otherwise, the handle remains valid. [LibraryImport("kernel32.dll", SetLastError = true)] private static partial nint GlobalFree(nint hMem); + /// + /// Locks a global memory object and returns a pointer to its memory. + /// + /// The handle of the global memory object to lock. + /// A pointer to the locked memory, or zero if the memory could not be locked. [LibraryImport("kernel32.dll", SetLastError = true)] private static partial nint GlobalLock(nint hMem); + /// + /// Releases the lock on a global memory object. + /// + /// A handle to the global memory object. + /// true if the memory object was unlocked; otherwise, false. [LibraryImport("kernel32.dll", SetLastError = true)] [return: MarshalAs(UnmanagedType.Bool)] private static partial bool GlobalUnlock(nint hMem); + /// + /// Retrieves the size of a global memory block in bytes. + /// + /// A handle to the global memory block. + /// The size of the memory block in bytes. [LibraryImport("kernel32.dll", SetLastError = true)] private static partial nint GlobalSize(nint hMem); } diff --git a/QuickShell.Core/Services/CompanionAppCatalog.cs b/QuickShell.Core/Services/CompanionAppCatalog.cs index 69d3fb75..1339f138 100644 --- a/QuickShell.Core/Services/CompanionAppCatalog.cs +++ b/QuickShell.Core/Services/CompanionAppCatalog.cs @@ -165,11 +165,20 @@ private static void EnsureFormChoicesCached() } } - private static string SerializeFormChoices(IReadOnlyList<(string Id, string Title)> choices) => + /// + /// Serializes form choices into the JSON representation used by the companion app selector. + /// + /// The form choice identifiers and display titles. + /// The serialized form choices. + private static string SerializeFormChoices(IReadOnlyList<(string Id, string Title)> choices) => JsonSerializer.Serialize( choices.Select(choice => new FormChoiceJson(choice.Title, choice.Id)).ToList(), QuickShellJsonContext.Default.ListFormChoiceJson); + /// + /// Builds the form choices for installed companion apps, including the options for no companion app and a custom app. + /// + /// The available companion-app choices. private static List<(string Id, string Title)> BuildInstalledFormChoicesUncached() { var choices = new List<(string Id, string Title)> diff --git a/QuickShell.Core/Services/StaModalDialogRunner.cs b/QuickShell.Core/Services/StaModalDialogRunner.cs index fed6bb2b..09032c05 100644 --- a/QuickShell.Core/Services/StaModalDialogRunner.cs +++ b/QuickShell.Core/Services/StaModalDialogRunner.cs @@ -10,6 +10,15 @@ namespace QuickShell.Services; /// internal static class StaModalDialogRunner { + /// + /// Executes an action in an STA context with timeout-based dialog recovery. + /// + /// The native handle of the dialog owner. + /// The action to execute. + /// The initial time allowed for the action to complete. + /// The additional time allowed for completion and recovery. + /// The action's selected value, or if the action fails or does not complete within the recovery period. + /// is . public static string? Run( nint ownerHandle, Func action, diff --git a/QuickShell.Core/Services/WorkspaceSecurityPolicy.cs b/QuickShell.Core/Services/WorkspaceSecurityPolicy.cs index 42dfeaf7..d29ccf40 100644 --- a/QuickShell.Core/Services/WorkspaceSecurityPolicy.cs +++ b/QuickShell.Core/Services/WorkspaceSecurityPolicy.cs @@ -407,6 +407,11 @@ public static bool MatchesReviewToken(StoredWorkspace workspace, WorkspaceReview && workspace.Revision == token.Revision && string.Equals(ComputeDigest(workspace.Content), token.Digest, StringComparison.Ordinal); + /// + /// Computes a hexadecimal SHA-256 digest for the workspace content. + /// + /// The workspace content to digest. + /// The hexadecimal SHA-256 digest of the workspace content. public static string ComputeDigest(TerminalShortcut workspace) { // Named DTO + QuickShellDigestJsonContext (not QuickShellJsonContext): the digest diff --git a/QuickShell.Run/Services/FolderPickerService.cs b/QuickShell.Run/Services/FolderPickerService.cs index 4e4ac6bb..27528555 100644 --- a/QuickShell.Run/Services/FolderPickerService.cs +++ b/QuickShell.Run/Services/FolderPickerService.cs @@ -8,6 +8,11 @@ internal static class FolderPickerService private static readonly TimeSpan DialogTimeout = TimeSpan.FromMinutes(2); private static readonly TimeSpan JoinGracePeriod = TimeSpan.FromSeconds(5); + /// + /// Displays a folder-picker dialog and obtains the selected folder path. + /// + /// The directory to display initially, if it exists. + /// The selected folder path, or null if no folder is selected. public static string? PickFolder(string? initialDirectory = null) { var ownerHandle = NativeForegroundWindow.Get(); @@ -18,6 +23,12 @@ internal static class FolderPickerService JoinGracePeriod); } + /// + /// Displays a folder picker using the specified owner window and initial directory. + /// + /// The directory to show initially when it exists; otherwise, no initial directory is used. + /// The native handle of the window that owns the folder picker. + /// The selected folder path, or null if no folder is selected. private static string? PickFolderOnStaThread(string? initialDirectory, nint ownerHandle) { var initial = !string.IsNullOrWhiteSpace(initialDirectory) && Directory.Exists(initialDirectory) diff --git a/QuickShell.Run/Services/StaClipboard.cs b/QuickShell.Run/Services/StaClipboard.cs index e6228022..f76d6606 100644 --- a/QuickShell.Run/Services/StaClipboard.cs +++ b/QuickShell.Run/Services/StaClipboard.cs @@ -5,6 +5,11 @@ namespace QuickShell.Services; internal static class StaClipboard { + /// + /// Attempts to set the clipboard text, using an STA thread when required. + /// + /// The text to place on the clipboard. + /// true if the clipboard text is set successfully within five seconds; false otherwise. public static bool TrySetText(string text) { if (Thread.CurrentThread.GetApartmentState() == ApartmentState.STA) return SetText(); diff --git a/QuickShell/Services/FolderPickerService.cs b/QuickShell/Services/FolderPickerService.cs index aa1bb19f..84996c4e 100644 --- a/QuickShell/Services/FolderPickerService.cs +++ b/QuickShell/Services/FolderPickerService.cs @@ -7,6 +7,11 @@ internal static class FolderPickerService private static readonly TimeSpan DialogTimeout = TimeSpan.FromMinutes(2); private static readonly TimeSpan JoinGracePeriod = TimeSpan.FromSeconds(5); + /// + /// Prompts the user to select a folder. + /// + /// The directory to display initially, if specified. + /// The selected folder path, or null if no folder is selected. public static string? PickFolder(string? initialDirectory = null) { var ownerHandle = NativeForegroundWindow.Get(); @@ -17,6 +22,12 @@ internal static class FolderPickerService JoinGracePeriod); } + /// + /// Selects a folder using the specified owner window and initial directory. + /// + /// The directory to display initially, optionally specified as a WSL path. + /// The native handle of the dialog's owner window. + /// The selected folder path, or null if no folder is selected. private static string? PickFolderOnStaThread(string? initialDirectory, nint ownerHandle) { var initial = initialDirectory; diff --git a/QuickShell/Services/ShortcutFilePickerService.cs b/QuickShell/Services/ShortcutFilePickerService.cs index a03271c1..82572bbb 100644 --- a/QuickShell/Services/ShortcutFilePickerService.cs +++ b/QuickShell/Services/ShortcutFilePickerService.cs @@ -19,6 +19,11 @@ private static readonly (string Name, string Spec)[] ExecutableFilters = private static readonly TimeSpan DialogTimeout = TimeSpan.FromMinutes(2); private static readonly TimeSpan JoinGracePeriod = TimeSpan.FromSeconds(5); + /// + /// Prompts the user to choose a destination for exporting workspace data. + /// + /// The shell services used to determine the workspace configuration directory. + /// The selected export file path, or null if no file is selected. public static string? PickExportFile(IQuickShellServices services) { ArgumentNullException.ThrowIfNull(services); @@ -39,6 +44,11 @@ private static readonly (string Name, string Spec)[] ExecutableFilters = JoinGracePeriod); } + /// + /// Prompts the user to select a workspace import file. + /// + /// The shell services used to locate the shortcuts configuration directory. + /// The selected JSON file path, or null if no file is selected. public static string? PickImportFile(IQuickShellServices services) { ArgumentNullException.ThrowIfNull(services); @@ -57,6 +67,10 @@ private static readonly (string Name, string Spec)[] ExecutableFilters = JoinGracePeriod); } + /// + /// Prompts the user to select a companion executable file. + /// + /// The selected executable file path, or null if no file is selected. public static string? PickExecutableFile() { var programFiles = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles); @@ -75,6 +89,11 @@ private static readonly (string Name, string Spec)[] ExecutableFilters = JoinGracePeriod); } - private static string? DirectoryOrNull(string? path) => + /// + /// Gets the path when it identifies an existing directory. + /// + /// The directory path to check. + /// The existing directory path, or null when the path is blank or does not exist. + private static string? DirectoryOrNull(string? path) => !string.IsNullOrWhiteSpace(path) && Directory.Exists(path) ? path : null; } diff --git a/QuickShell/Services/StaClipboard.cs b/QuickShell/Services/StaClipboard.cs index 207f8abc..70423395 100644 --- a/QuickShell/Services/StaClipboard.cs +++ b/QuickShell/Services/StaClipboard.cs @@ -17,6 +17,11 @@ internal static class StaClipboard return thread.Join(ReadTimeout) ? text : null; } + /// + /// Attempts to place text on the Windows clipboard. + /// + /// The text to place on the clipboard. + /// true if the text was placed successfully within the timeout; false otherwise. public static bool TrySetText(string text) { if (Thread.CurrentThread.GetApartmentState() == ApartmentState.STA) return SetTextOnStaThread(text); @@ -28,8 +33,15 @@ public static bool TrySetText(string text) } // Win32Clipboard returns false/null on failure and does not throw for the clipboard - // failure modes we care about (open contention, empty format, alloc failure). + /// +/// Attempts to place text on the Windows clipboard. +/// +/// true if the text was set successfully; false otherwise. private static bool SetTextOnStaThread(string text) => Win32Clipboard.SetText(text); - private static string? ReadTextOnStaThread() => Win32Clipboard.GetText(); + /// +/// Reads text from the Windows clipboard on an STA thread. +/// +/// The clipboard text, or if no text is available. +private static string? ReadTextOnStaThread() => Win32Clipboard.GetText(); } From 78a96e50063155f52bab5fb38cef83a3dd3ae668 Mon Sep 17 00:00:00 2001 From: tonythethompson Date: Sat, 25 Jul 2026 11:05:18 -0700 Subject: [PATCH 7/8] Use using for dialog filter memory; document STA catch intent. Address leftover GHAS notes from the StaModalDialogRunner follow-up without reopening already-fixed CodeRabbit findings on older commits. Co-authored-by: Cursor --- QuickShell.Core/Interop/ShellFileDialog.cs | 53 +++++++++++-------- .../Services/StaModalDialogRunner.cs | 25 +++++---- 2 files changed, 46 insertions(+), 32 deletions(-) diff --git a/QuickShell.Core/Interop/ShellFileDialog.cs b/QuickShell.Core/Interop/ShellFileDialog.cs index 14107d97..82124719 100644 --- a/QuickShell.Core/Interop/ShellFileDialog.cs +++ b/QuickShell.Core/Interop/ShellFileDialog.cs @@ -64,31 +64,30 @@ internal static partial class ShellFileDialog } } - /// Open-file picker. pairs display name to a /// /// Displays a file selection dialog and obtains the selected file path. /// /// The handle of the dialog's owner window. /// The dialog title. - /// The file type filters to display. + /// The file type filters to display (display name + wildcard spec pairs). /// The default file extension. /// The directory initially displayed by the dialog. /// The selected file path, or null if the dialog is canceled. public static string? PickOpenFile(nint owner, string title, (string Name, string Spec)[] filters, string? defaultExt, string? initialDirectory) { var dialog = (IFileOpenDialog)CreateInstance(CLSID_FileOpenDialog, IID_IFileOpenDialog); - FilterNativeMemory? filterMemory = null; try { dialog.SetOptions(FOS_FORCEFILESYSTEM | FOS_FILEMUSTEXIST | FOS_PATHMUSTEXIST | FOS_NOCHANGEDIR); dialog.SetTitle(title); - filterMemory = ApplyFilters(dialog, filters, defaultExt); - ApplyInitialDirectory(dialog, initialDirectory); - return ShowAndGetPath(dialog, owner); + using (ApplyFilters(dialog, filters, defaultExt)) + { + ApplyInitialDirectory(dialog, initialDirectory); + return ShowAndGetPath(dialog, owner); + } } finally { - filterMemory?.Dispose(); Release(dialog); } } @@ -101,23 +100,23 @@ internal static partial class ShellFileDialog public static string? PickSaveFile(nint owner, string title, (string Name, string Spec)[] filters, string? defaultExt, string? defaultFileName, string? initialDirectory) { var dialog = (IFileSaveDialog)CreateInstance(CLSID_FileSaveDialog, IID_IFileSaveDialog); - FilterNativeMemory? filterMemory = null; try { dialog.SetOptions(FOS_FORCEFILESYSTEM | FOS_OVERWRITEPROMPT | FOS_PATHMUSTEXIST | FOS_NOCHANGEDIR); dialog.SetTitle(title); - filterMemory = ApplyFilters(dialog, filters, defaultExt); - if (!string.IsNullOrEmpty(defaultFileName)) + using (ApplyFilters(dialog, filters, defaultExt)) { - dialog.SetFileName(defaultFileName); - } + if (!string.IsNullOrEmpty(defaultFileName)) + { + dialog.SetFileName(defaultFileName); + } - ApplyInitialDirectory(dialog, initialDirectory); - return ShowAndGetPath(dialog, owner); + ApplyInitialDirectory(dialog, initialDirectory); + return ShowAndGetPath(dialog, owner); + } } finally { - filterMemory?.Dispose(); Release(dialog); } } @@ -203,13 +202,13 @@ private static void ApplyInitialDirectory(IFileDialog dialog, string? initialDir /// Builds COMDLG_FILTERSPEC native memory for . /// The returned disposable must stay alive until after ; /// the dialog may keep pointers into this buffer for the lifetime of the modal call. - /// - /// Applies file type filters and an optional default extension to a file dialog. + /// Always returns a disposable (no-op when there are no filters). /// + /// The file dialog to configure. /// The display names and wildcard specifications for the file types. /// The default file name extension. - /// Native memory that owns the applied filter specifications, or null when no filters are provided. - private static FilterNativeMemory? ApplyFilters(IFileDialog dialog, (string Name, string Spec)[] filters, string? defaultExt) + /// Native memory that owns the applied filter specifications. + private static FilterNativeMemory ApplyFilters(IFileDialog dialog, (string Name, string Spec)[] filters, string? defaultExt) { if (filters.Length == 0) { @@ -218,7 +217,7 @@ private static void ApplyInitialDirectory(IFileDialog dialog, string? initialDir dialog.SetDefaultExtension(defaultExt); } - return null; + return FilterNativeMemory.Empty; } // COMDLG_FILTERSPEC is { LPWSTR pszName; LPWSTR pszSpec; }. Build the native array @@ -267,10 +266,18 @@ private static void ApplyInitialDirectory(IFileDialog dialog, string? initialDir } } - private sealed class FilterNativeMemory(nint block, List strings) : IDisposable + private sealed class FilterNativeMemory : IDisposable { - private nint _block = block; - private List? _strings = strings; + public static FilterNativeMemory Empty { get; } = new(0, null); + + private nint _block; + private List? _strings; + + public FilterNativeMemory(nint block, List? strings) + { + _block = block; + _strings = strings; + } /// /// Releases the native memory allocated for the filter specifications. diff --git a/QuickShell.Core/Services/StaModalDialogRunner.cs b/QuickShell.Core/Services/StaModalDialogRunner.cs index 09032c05..f5b94be0 100644 --- a/QuickShell.Core/Services/StaModalDialogRunner.cs +++ b/QuickShell.Core/Services/StaModalDialogRunner.cs @@ -6,7 +6,7 @@ namespace QuickShell.Services; /// /// Runs a modal shell dialog on an STA thread with timeout recovery. Publishes the /// native thread id with so timeout close can find the dialog, -/// and swallows worker exceptions into a null result so they never tear down the process. +/// and maps worker failures to a null result so they never tear down the process. /// internal static class StaModalDialogRunner { @@ -29,14 +29,7 @@ internal static class StaModalDialogRunner if (Thread.CurrentThread.GetApartmentState() == ApartmentState.STA) { - try - { - return action(); - } - catch (Exception) - { - return null; - } + return TryInvoke(action); } string? selected = null; @@ -49,6 +42,7 @@ internal static class StaModalDialogRunner { selected = action(); } + // codeql[cs/catch-of-all-exceptions]: Background STA must not crash the host; picker failure is null. catch (Exception ex) { fault = ex; @@ -73,4 +67,17 @@ internal static class StaModalDialogRunner return fault is null ? selected : null; } + + private static string? TryInvoke(Func action) + { + try + { + return action(); + } + // codeql[cs/catch-of-all-exceptions]: Same host-stability rule on the calling STA thread. + catch (Exception) + { + return null; + } + } } From e331b7a1bf06785371c0b51b54e9b1a223be9dbd Mon Sep 17 00:00:00 2001 From: tonythethompson Date: Sat, 25 Jul 2026 11:08:51 -0700 Subject: [PATCH 8/8] Quiet GHAS quality notes for intentional Win32 interop. Exclude cs/unmanaged-code alongside call-site alerts, catch known picker failures specifically, keep a documented background STA catch-all, and document that EnumThreadWindows needs the native thread id. Co-authored-by: Cursor --- .github/codeql/codeql-config.yml | 10 ++++--- .../Interop/NativeForegroundWindow.cs | 2 +- QuickShell.Core/Interop/StaDialogCloser.cs | 16 ++++++----- .../Services/StaModalDialogRunner.cs | 27 ++++++++++++++++--- 4 files changed, 40 insertions(+), 15 deletions(-) diff --git a/.github/codeql/codeql-config.yml b/.github/codeql/codeql-config.yml index fc8d0d38..d6eb3cfe 100644 --- a/.github/codeql/codeql-config.yml +++ b/.github/codeql/codeql-config.yml @@ -4,9 +4,13 @@ queries: - uses: security-and-quality query-filters: - # Quality note (not a vuln). Quick Shell is a Windows shell host that must call + # Quality notes (not vulns). Quick Shell is a Windows shell host that must call # Win32/COM for folder/file dialogs, clipboard, and dialog timeout recovery after - # dropping WinForms for MSIX trimming. Inline codeql[] suppressions also document - # intentional call sites under QuickShell.Core/Interop. + # dropping WinForms for MSIX trimming. + # + # cs/call-to-unmanaged-code: call sites of extern methods + # cs/unmanaged-code: DllImport/LibraryImport declarations themselves - exclude: id: cs/call-to-unmanaged-code + - exclude: + id: cs/unmanaged-code diff --git a/QuickShell.Core/Interop/NativeForegroundWindow.cs b/QuickShell.Core/Interop/NativeForegroundWindow.cs index 37b385fd..ccf33ce3 100644 --- a/QuickShell.Core/Interop/NativeForegroundWindow.cs +++ b/QuickShell.Core/Interop/NativeForegroundWindow.cs @@ -15,7 +15,7 @@ internal static partial class NativeForegroundWindow /// The handle of the current foreground window. public static nint Get() { - // codeql[cs/call-to-unmanaged-code]: Required for IFileDialog owner hwnd; no managed API after WinForms removal. + // codeql[cs/call-to-unmanaged-code] Required for IFileDialog owner hwnd; no managed API after WinForms removal. return GetForegroundWindow(); } diff --git a/QuickShell.Core/Interop/StaDialogCloser.cs b/QuickShell.Core/Interop/StaDialogCloser.cs index a7bd651e..f33c2bdd 100644 --- a/QuickShell.Core/Interop/StaDialogCloser.cs +++ b/QuickShell.Core/Interop/StaDialogCloser.cs @@ -27,19 +27,19 @@ public static void TryCloseThreadDialog(int nativeThreadId, nint ownerHandle) nint dialog = 0; nint fallback = 0; - // codeql[cs/call-to-unmanaged-code]: Enumerate picker STA thread windows to find the modal dialog HWND. + // codeql[cs/call-to-unmanaged-code] Enumerate picker STA thread windows to find the modal dialog HWND. EnumThreadWindows( nativeThreadId, (hwnd, _) => { - // codeql[cs/call-to-unmanaged-code]: Skip invisible windows when locating the dialog. + // codeql[cs/call-to-unmanaged-code] Skip invisible windows when locating the dialog. if (hwnd == 0 || hwnd == ownerHandle || !IsWindowVisible(hwnd)) { return true; } // Top-level only (skip DirectUI / child chrome inside the dialog). - // codeql[cs/call-to-unmanaged-code]: Restrict close target to top-level HWNDs on the STA thread. + // codeql[cs/call-to-unmanaged-code] Restrict close target to top-level HWNDs on the STA thread. if (GetAncestor(hwnd, GA_ROOT) != hwnd) { return true; @@ -63,18 +63,20 @@ public static void TryCloseThreadDialog(int nativeThreadId, nint ownerHandle) var target = dialog != 0 ? dialog : fallback; if (target != 0) { - // codeql[cs/call-to-unmanaged-code]: Unblock a timed-out IFileDialog by closing its HWND. + // codeql[cs/call-to-unmanaged-code] Unblock a timed-out IFileDialog by closing its HWND. PostMessage(target, WM_CLOSE, nint.Zero, nint.Zero); } } /// - /// Gets the native identifier of the current thread. + /// Gets the native Win32 identifier of the current thread. + /// Do not substitute — + /// EnumThreadWindows requires the OS thread id. /// /// The native thread identifier. public static int CurrentNativeThreadId() { - // codeql[cs/call-to-unmanaged-code]: Native thread id required for EnumThreadWindows on the picker STA thread. + // codeql[cs/call-to-unmanaged-code] Native thread id required for EnumThreadWindows on the picker STA thread. return GetCurrentThreadId(); } @@ -89,7 +91,7 @@ private static unsafe bool IsDialogClass(nint hwnd) int length; fixed (char* p = buffer) { - // codeql[cs/call-to-unmanaged-code]: Classify #32770 dialog HWNDs for timeout close. + // codeql[cs/call-to-unmanaged-code] Classify #32770 dialog HWNDs for timeout close. length = GetClassNameW(hwnd, p, buffer.Length); } diff --git a/QuickShell.Core/Services/StaModalDialogRunner.cs b/QuickShell.Core/Services/StaModalDialogRunner.cs index f5b94be0..4cec6c00 100644 --- a/QuickShell.Core/Services/StaModalDialogRunner.cs +++ b/QuickShell.Core/Services/StaModalDialogRunner.cs @@ -1,3 +1,4 @@ +using System.Runtime.InteropServices; using System.Threading; using QuickShell.Interop; @@ -6,7 +7,7 @@ namespace QuickShell.Services; /// /// Runs a modal shell dialog on an STA thread with timeout recovery. Publishes the /// native thread id with so timeout close can find the dialog, -/// and maps worker failures to a null result so they never tear down the process. +/// and maps known picker failures to a null result so they never tear down the process. /// internal static class StaModalDialogRunner { @@ -37,12 +38,17 @@ internal static class StaModalDialogRunner var nativeThreadId = 0; var thread = new Thread(() => { + // Must be the Win32 thread id for EnumThreadWindows — not Environment.CurrentManagedThreadId. Volatile.Write(ref nativeThreadId, StaDialogCloser.CurrentNativeThreadId()); try { selected = action(); } - // codeql[cs/catch-of-all-exceptions]: Background STA must not crash the host; picker failure is null. + catch (Exception ex) when (IsPickerFailure(ex)) + { + fault = ex; + } + // codeql[cs/catch-of-all-exceptions] Background STA must not crash the host; any other failure is a canceled pick. catch (Exception ex) { fault = ex; @@ -74,10 +80,23 @@ internal static class StaModalDialogRunner { return action(); } - // codeql[cs/catch-of-all-exceptions]: Same host-stability rule on the calling STA thread. - catch (Exception) + catch (Exception ex) when (IsPickerFailure(ex)) { return null; } } + + /// + /// COM/interop and allocation failures from IFileDialog / clipboard paths. + /// Unexpected exceptions still propagate so real bugs are not swallowed. + /// + private static bool IsPickerFailure(Exception ex) => + ex is COMException + or ExternalException + or SEHException + or InvalidOperationException + or ArgumentException + or OutOfMemoryException + or NotSupportedException + or UnauthorizedAccessException; }