From 1f84fa590ece5bb25ad1d8ce9de0c441d3071e94 Mon Sep 17 00:00:00 2001 From: Zhongjie Li Date: Sun, 25 Oct 2026 15:17:02 +0800 Subject: [PATCH 1/2] fix(windows): stop trusting recycled HWNDs in the capsule hit-test subclass `install_capsule_hit_test_proc` keys its bookkeeping on the numeric HWND, but Windows recycles HWND values. Once a capsule/card window is destroyed and its handle value is handed to a new window, a lookup by that key still finds the dead window's entry, and three things go wrong: - the fast path reuses the stale entry and returns early, so the *new* window never gets the subclass procedure installed -- hit-testing silently stops working for it; - `WM_NCHITTEST` hit-tests the live window against the dead window's geometry; - message forwarding can call a `previous_proc` that belonged to another window. The only reliable identity check is whether *our own* procedure is still installed on the handle: a recycled HWND carries whatever procedure its new owner installed. Every read of the map now proves that first, and an entry that fails the check is dropped and re-installed instead of trusted. Two supporting fixes while in here: - `install_capsule_hit_test_proc` now refuses to record itself as the predecessor. When the map has been wiped but the procedure is already on the window, `SetWindowLongPtrW` returns our own address; storing it would make every forwarded message recurse into this same procedure. - `WM_NCDESTROY` only retires the entry this window owns. Adds `reinstall_keeps_the_real_predecessor_and_never_self_wraps`, which uses a real `HWND_MESSAGE` window because the identity check reads the live `GWLP_WNDPROC` and cannot be mocked. Note on running tests: `cargo test --lib` exits with `0xC0000139` (STATUS_ENTRYPOINT_NOT_FOUND) on Windows before any test runs. The crate imports `comctl32!TaskDialogIndirect`, which lives in Common-Controls v6, and v6 is bound only through the `build.rs` `/manifestdependency` link arg -- the cargo test harness links separately and does not get that manifest. Embedding a v6 manifest in the test binary makes it run: mt.exe -nologo -manifest cc6.manifest "-outputresource:;#1" With that, the new test passes (1 passed; 0 failed), as does `cargo check` (0 errors) and `cargo build --release`. --- .../src-tauri/src/tauri_coordinator_host.rs | 211 +++++++++++++++--- 1 file changed, 186 insertions(+), 25 deletions(-) diff --git a/openless-all/app/src-tauri/src/tauri_coordinator_host.rs b/openless-all/app/src-tauri/src/tauri_coordinator_host.rs index 86df885be..58ddf7816 100644 --- a/openless-all/app/src-tauri/src/tauri_coordinator_host.rs +++ b/openless-all/app/src-tauri/src/tauri_coordinator_host.rs @@ -148,6 +148,22 @@ struct CapsuleHitTestEntry { control_bottom: i32, } +/// Whether `entry` still describes the live window behind `hwnd`. +/// +/// The map is keyed by the numeric HWND, and Windows recycles HWND values: a +/// destroyed capsule/card can hand its handle to an unrelated new window. The one +/// fact that distinguishes the two is whether *our* subclass procedure is still +/// installed on the handle. A recycled HWND carries whatever procedure its new +/// owner installed, so a mismatch means the entry is stale and must be dropped and +/// re-installed instead of trusted. +#[cfg(target_os = "windows")] +unsafe fn capsule_hit_test_entry_matches(hwnd: windows::Win32::Foundation::HWND) -> bool { + use windows::Win32::UI::WindowsAndMessaging::{GetWindowLongPtrW, GWLP_WNDPROC}; + + let installed = unsafe { GetWindowLongPtrW(hwnd, GWLP_WNDPROC) }; + installed == capsule_hit_test_window_proc as *const () as usize as isize +} + #[cfg(target_os = "windows")] static CAPSULE_HIT_TEST_ENTRIES: OnceLock>> = OnceLock::new(); @@ -244,11 +260,20 @@ unsafe extern "system" fn capsule_hit_test_window_proc( }; if msg == WM_NCDESTROY { - let previous_proc = capsule_hit_test_entries() - .lock() - .remove(&(hwnd.0 as isize)) - .map(|entry| entry.previous_proc) - .unwrap_or_default(); + let key = hwnd.0 as isize; + // Only retire the entry this window owns. A recycled HWND can reach + // WM_NCDESTROY for a *different* window than the one in the map, and + // removing that survivor's entry would strand it with our procedure + // installed but no recorded `previous_proc` to forward to. + let previous_proc = if unsafe { capsule_hit_test_entry_matches(hwnd) } { + capsule_hit_test_entries() + .lock() + .remove(&key) + .map(|entry| entry.previous_proc) + .unwrap_or_default() + } else { + 0 + }; if previous_proc != 0 { let previous: windows::Win32::UI::WindowsAndMessaging::WNDPROC = std::mem::transmute(previous_proc); @@ -258,9 +283,18 @@ unsafe extern "system" fn capsule_hit_test_window_proc( return DefWindowProcW(hwnd, msg, wparam, lparam); } + // Every read of the map must prove the entry still belongs to this window + // instance. A recycled HWND would otherwise be hit-tested against another + // window's geometry and forwarded to another window's `previous_proc`. + let live = unsafe { capsule_hit_test_entry_matches(hwnd) }; + let entry = if live { + let entries = capsule_hit_test_entries().lock(); + entries.get(&(hwnd.0 as isize)).copied() + } else { + None + }; + if msg == WM_NCHITTEST { - let key = hwnd.0 as isize; - let entry = capsule_hit_test_entries().lock().get(&key).copied(); if let Some(entry) = entry { let screen_x = (lparam.0 as i32 & 0xffff) as u16 as i16 as i32; let screen_y = ((lparam.0 >> 16) & 0xffff) as u16 as i16 as i32; @@ -279,11 +313,7 @@ unsafe extern "system" fn capsule_hit_test_window_proc( } } - let previous_proc = capsule_hit_test_entries() - .lock() - .get(&(hwnd.0 as isize)) - .map(|entry| entry.previous_proc) - .unwrap_or_default(); + let previous_proc = entry.map(|entry| entry.previous_proc).unwrap_or_default(); if previous_proc != 0 { let previous: windows::Win32::UI::WindowsAndMessaging::WNDPROC = std::mem::transmute(previous_proc); @@ -309,26 +339,39 @@ fn install_capsule_hit_test_proc( let key = hwnd.0 as isize; let mut entries = capsule_hit_test_entries().lock(); - if let Some(entry) = entries.get_mut(&key) { - entry.control_left = rect.left; - entry.control_right = rect.right; - entry.control_top = rect.top; - entry.control_bottom = rect.bottom; - return Ok(()); + // The handle alone is not identity: Windows recycles HWND values, so a matching + // key can belong to a window that was destroyed and replaced. Re-install for this + // instance instead of writing geometry into a dead window's entry — that path + // both lost the subclass and left the map describing the wrong window, which is + // how a live window ends up dispatched to a stale `previous_proc`. + if unsafe { capsule_hit_test_entry_matches(hwnd) } { + if let Some(entry) = entries.get_mut(&key) { + entry.control_left = rect.left; + entry.control_right = rect.right; + entry.control_top = rect.top; + entry.control_bottom = rect.bottom; + return Ok(()); + } + // Our procedure is installed but the map has no entry: the window is ours + // yet nothing can forward for it. Fall through and re-install so the entry + // is rebuilt; the self-install guard below keeps that from recursing. } + entries.remove(&key); unsafe { SetLastError(ERROR_SUCCESS) }; - let previous = unsafe { - SetWindowLongPtrW( - hwnd, - GWLP_WNDPROC, - capsule_hit_test_window_proc as usize as isize, - ) - }; + let installed = capsule_hit_test_window_proc as *const () as usize as isize; + let previous = unsafe { SetWindowLongPtrW(hwnd, GWLP_WNDPROC, installed) }; if previous == 0 && unsafe { GetLastError() } != ERROR_SUCCESS { return Err(tauri::Error::Anyhow(anyhow::anyhow!( "SetWindowLongPtrW(GWLP_WNDPROC) failed" ))); } + if previous == installed { + // Already wrapped: recording ourselves as `previous_proc` would make every + // forwarded message recurse into this same procedure. There is no real + // predecessor to call, so leave the map empty and let `DefWindowProcW` take + // the traffic instead of overflowing the stack. + return Ok(()); + } entries.insert( key, CapsuleHitTestEntry { @@ -2466,6 +2509,124 @@ impl TauriCoordinatorHost { mod tests { use super::*; + /// Close the gap that made the recycled-HWND bug possible: re-installing the + /// hit-test procedure on a window that already carries it must not overwrite + /// the recorded predecessor with ourselves, and must not lose the entry. + /// + /// Uses a real `HWND_MESSAGE` window because the identity check reads the live + /// `GWLP_WNDPROC`, which no mock can stand in for. + #[cfg(target_os = "windows")] + #[test] + fn reinstall_keeps_the_real_predecessor_and_never_self_wraps() { + use windows::Win32::Foundation::{HINSTANCE, HWND, LPARAM, WPARAM}; + use windows::Win32::UI::WindowsAndMessaging::{ + CreateWindowExW, DefWindowProcW, DestroyWindow, GetWindowLongPtrW, RegisterClassW, + UnregisterClassW, GWLP_WNDPROC, HWND_MESSAGE, WINDOW_EX_STYLE, WINDOW_STYLE, + WNDCLASSW, + }; + + static CLASS_NAME: [u16; 20] = [ + 'O' as u16, 'L' as u16, 'H' as u16, 'i' as u16, 't' as u16, 'T' as u16, 'e' as u16, + 's' as u16, 't' as u16, 'C' as u16, 'l' as u16, 's' as u16, 0, 0, 0, 0, 0, 0, 0, 0, + ]; + + unsafe extern "system" fn plain_proc( + hwnd: HWND, + msg: u32, + wparam: WPARAM, + lparam: LPARAM, + ) -> windows::Win32::Foundation::LRESULT { + unsafe { DefWindowProcW(hwnd, msg, wparam, lparam) } + } + + // `HINSTANCE::default()` is NULL, which is what a class registered by the + // process itself wants; the `Win32_System_LibraryLoader` feature that + // `GetModuleHandleW` needs is not enabled in this crate. + let hinstance = HINSTANCE::default(); + let class = WNDCLASSW { + lpfnWndProc: Some(plain_proc), + hInstance: hinstance, + lpszClassName: windows::core::PCWSTR(CLASS_NAME.as_ptr()), + ..Default::default() + }; + assert_ne!(unsafe { RegisterClassW(&class) }, 0, "RegisterClassW"); + + let hwnd = unsafe { + CreateWindowExW( + WINDOW_EX_STYLE(0), + windows::core::PCWSTR(CLASS_NAME.as_ptr()), + windows::core::PCWSTR(std::ptr::null()), + WINDOW_STYLE(0), + 0, + 0, + 0, + 0, + HWND_MESSAGE, + None, + hinstance, + None, + ) + } + .expect("CreateWindowExW"); + + let rect = CapsuleHitTestRect { + left: 10, + right: 20, + top: 30, + bottom: 40, + }; + install_capsule_hit_test_proc(hwnd, rect).expect("first install"); + let after_first = unsafe { GetWindowLongPtrW(hwnd, GWLP_WNDPROC) }; + assert_eq!( + after_first, + capsule_hit_test_window_proc as *const () as usize as isize, + "our procedure must be installed" + ); + + // Second install takes the fast path and only refreshes geometry. + install_capsule_hit_test_proc( + hwnd, + CapsuleHitTestRect { + left: 1, + right: 2, + top: 3, + bottom: 4, + }, + ) + .expect("second install"); + { + let entries = capsule_hit_test_entries().lock(); + let entry = entries.get(&(hwnd.0 as isize)).expect("entry survives"); + assert_eq!( + entry.previous_proc, + plain_proc as *const () as usize as isize, + "the recorded predecessor must stay the original window procedure" + ); + assert_eq!( + (entry.control_left, entry.control_right), + (1, 2), + "geometry is refreshed in place" + ); + } + + // Third install with the map wiped exercises the defensive branch: our + // procedure is on the window but nothing records it, so `SetWindowLongPtrW` + // returns our own address. Recording that as `previous_proc` would send + // every message straight back into this same procedure. + capsule_hit_test_entries().lock().remove(&(hwnd.0 as isize)); + install_capsule_hit_test_proc(hwnd, rect).expect("third install"); + assert!( + capsule_hit_test_entries() + .lock() + .get(&(hwnd.0 as isize)) + .is_none(), + "a self-install must not be recorded, or forwarding recurses forever" + ); + + unsafe { DestroyWindow(hwnd) }.expect("DestroyWindow"); + let _ = unsafe { UnregisterClassW(windows::core::PCWSTR(CLASS_NAME.as_ptr()), hinstance) }; + } + #[test] fn display_mode_changes_invalidate_mirrors_without_changing_style() { let state = CapsuleWindowState::default(); From b24e17e00f8dd5c813620959cdbc9e0e2ac00ef5 Mon Sep 17 00:00:00 2001 From: Chris233 Date: Thu, 29 Oct 2026 11:15:22 +0800 Subject: [PATCH 2/2] fix(windows): preserve capsule subclass chains Track hit-test state with the window's system subclass, not the current outer WndProc. Preserve message forwarding and cleanup when another callback wraps the capsule, and keep recycled HWNDs state-free. Run real-window regressions in the standalone backend tests with a Common Controls v6 manifest. Refs #1267 --- .../app/scripts/windows-ui-config.test.mjs | 26 +- .../app/src-tauri/backend-tests/Cargo.toml | 2 +- .../app/src-tauri/backend-tests/build.rs | 11 + .../backend-tests/tests/capsule_hit_test.rs | 4 + .../app/src-tauri/src/capsule_hit_test.rs | 365 ++++++++++++++++++ .../src-tauri/src/tauri_coordinator_host.rs | 312 +-------------- 6 files changed, 404 insertions(+), 316 deletions(-) create mode 100644 openless-all/app/src-tauri/backend-tests/build.rs create mode 100644 openless-all/app/src-tauri/backend-tests/tests/capsule_hit_test.rs create mode 100644 openless-all/app/src-tauri/src/capsule_hit_test.rs diff --git a/openless-all/app/scripts/windows-ui-config.test.mjs b/openless-all/app/scripts/windows-ui-config.test.mjs index cc7b263bb..37d72f26e 100644 --- a/openless-all/app/scripts/windows-ui-config.test.mjs +++ b/openless-all/app/scripts/windows-ui-config.test.mjs @@ -33,6 +33,10 @@ const sharedTypesRs = await readFile( ); // Contract check over the union of the capsule subsystem compiled into the binary; native // window manipulation belongs to the explicit Tauri Host. +const capsuleHitTestRs = await readFile( + new URL('../src-tauri/src/capsule_hit_test.rs', import.meta.url), + 'utf-8', +); const coordinatorRs = (await readFile(new URL('../src-tauri/src/coordinator.rs', import.meta.url), 'utf-8')) + '\n' + @@ -43,7 +47,9 @@ const coordinatorRs = '\n' + (await readFile(new URL('../src-tauri/src/capsule_snapshot.rs', import.meta.url), 'utf-8')) + '\n' + - (await readFile(new URL('../src-tauri/src/tauri_coordinator_host.rs', import.meta.url), 'utf-8')); + (await readFile(new URL('../src-tauri/src/tauri_coordinator_host.rs', import.meta.url), 'utf-8')) + + '\n' + + capsuleHitTestRs; const tauriEventsRs = await readFile( new URL('../src-tauri/src/tauri_events.rs', import.meta.url), 'utf-8', @@ -404,9 +410,9 @@ assertMatch( 'capsule hit testing should cover X/Y and use a real native input region', ); assertMatch( - coordinatorRs, - /control_left:[\s\S]*?control_right:[\s\S]*?control_top:[\s\S]*?control_bottom:/, - 'capsule hit-test state should store both X and Y bounds', + capsuleHitTestRs, + /struct CapsuleHitTestRect \{[\s\S]*?left: i32,[\s\S]*?right: i32,[\s\S]*?top: i32,[\s\S]*?bottom: i32,/, + 'window-owned capsule hit-test state should store both X and Y bounds', ); assertMatch( coordinatorRs, @@ -414,14 +420,14 @@ assertMatch( 'capsule input region should be native', ); assertMatch( - coordinatorRs, - /WM_NCDESTROY[\s\S]*?remove\([\s\S]*?SetWindowLongPtrW/, - 'subclass teardown should restore the old WndProc and remove its table entry', + capsuleHitTestRs, + /WM_NCDESTROY[\s\S]*?RemoveWindowSubclass[\s\S]*?drop\(Box::from_raw\([\s\S]*?DefSubclassProc/, + 'subclass teardown should release window-owned state and preserve message forwarding', ); assertMatch( - coordinatorRs, - /SetLastError[\s\S]*?GetLastError/, - 'WndProc installation should detect SetWindowLongPtrW failure', + capsuleHitTestRs, + /if !SetWindowSubclass\([\s\S]*?\.as_bool\(\)[\s\S]*?drop\(Box::from_raw\(data\)\);[\s\S]*?return Err/, + 'failed subclass installation should release its allocation and return an error', ); assertMatch( coordinatorRs, diff --git a/openless-all/app/src-tauri/backend-tests/Cargo.toml b/openless-all/app/src-tauri/backend-tests/Cargo.toml index 1b3248046..8fd711041 100644 --- a/openless-all/app/src-tauri/backend-tests/Cargo.toml +++ b/openless-all/app/src-tauri/backend-tests/Cargo.toml @@ -35,7 +35,7 @@ parking_lot = "0.12" futures-util = "0.3" [target.'cfg(windows)'.dev-dependencies] -windows = { version = "0.58", features = ["Win32_Foundation", "Win32_System_Threading", "Win32_UI_WindowsAndMessaging", "Win32_UI_Input_KeyboardAndMouse"] } +windows = { version = "0.58", features = ["Win32_Foundation", "Win32_Graphics_Gdi", "Win32_System_Threading", "Win32_UI_Shell", "Win32_UI_WindowsAndMessaging", "Win32_UI_Input_KeyboardAndMouse"] } [target.'cfg(target_os = "linux")'.dev-dependencies] # Exercise the production recorder worker without the retired Linux Tauri host. diff --git a/openless-all/app/src-tauri/backend-tests/build.rs b/openless-all/app/src-tauri/backend-tests/build.rs new file mode 100644 index 000000000..4c9cabc97 --- /dev/null +++ b/openless-all/app/src-tauri/backend-tests/build.rs @@ -0,0 +1,11 @@ +fn main() { + if std::env::var("CARGO_CFG_TARGET_OS").as_deref() == Ok("windows") + && std::env::var("CARGO_CFG_TARGET_ENV").as_deref() == Ok("msvc") + { + // Subclass helper exports require Common Controls v6 in the test executables. + println!("cargo:rustc-link-arg-tests=/MANIFEST:EMBED"); + println!( + r#"cargo:rustc-link-arg-tests=/MANIFESTDEPENDENCY:type='win32' name='Microsoft.Windows.Common-Controls' version='6.0.0.0' processorArchitecture='*' publicKeyToken='6595b64144ccf1df' language='*'"# + ); + } +} diff --git a/openless-all/app/src-tauri/backend-tests/tests/capsule_hit_test.rs b/openless-all/app/src-tauri/backend-tests/tests/capsule_hit_test.rs new file mode 100644 index 000000000..a6bf8eb04 --- /dev/null +++ b/openless-all/app/src-tauri/backend-tests/tests/capsule_hit_test.rs @@ -0,0 +1,4 @@ +#![cfg(target_os = "windows")] + +#[path = "../../src/capsule_hit_test.rs"] +mod capsule_hit_test; diff --git a/openless-all/app/src-tauri/src/capsule_hit_test.rs b/openless-all/app/src-tauri/src/capsule_hit_test.rs new file mode 100644 index 000000000..c5927882f --- /dev/null +++ b/openless-all/app/src-tauri/src/capsule_hit_test.rs @@ -0,0 +1,365 @@ +//! Window-owned hit-test state; repeated installs must preserve other subclasses. + +use windows::Win32::Foundation::{HWND, LPARAM, LRESULT, RECT, WPARAM}; +use windows::Win32::UI::Shell::{ + DefSubclassProc, GetWindowSubclass, RemoveWindowSubclass, SetWindowSubclass, +}; +use windows::Win32::UI::WindowsAndMessaging::{ + GetWindowRect, HTTRANSPARENT, WM_NCDESTROY, WM_NCHITTEST, +}; + +const SUBCLASS_ID: usize = 1; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct CapsuleHitTestRect { + pub(crate) left: i32, + pub(crate) right: i32, + pub(crate) top: i32, + pub(crate) bottom: i32, +} + +unsafe extern "system" fn capsule_hit_test_subclass_proc( + hwnd: HWND, + msg: u32, + wparam: WPARAM, + lparam: LPARAM, + subclass_id: usize, + data: usize, +) -> LRESULT { + if msg == WM_NCDESTROY { + let _ = RemoveWindowSubclass(hwnd, Some(capsule_hit_test_subclass_proc), subclass_id); + drop(Box::from_raw(data as *mut CapsuleHitTestRect)); + return DefSubclassProc(hwnd, msg, wparam, lparam); + } + + if msg == WM_NCHITTEST { + let control = *(data as *const CapsuleHitTestRect); + let screen_x = (lparam.0 as i32 & 0xffff) as u16 as i16 as i32; + let screen_y = ((lparam.0 >> 16) & 0xffff) as u16 as i16 as i32; + let mut rect = RECT::default(); + if GetWindowRect(hwnd, &mut rect).is_ok() { + let client_x = screen_x - rect.left; + let client_y = screen_y - rect.top; + if client_x < control.left + || client_x >= control.right + || client_y < control.top + || client_y >= control.bottom + { + return LRESULT(HTTRANSPARENT as isize); + } + } + } + DefSubclassProc(hwnd, msg, wparam, lparam) +} + +/// Called on the window's owning thread by the capsule/card layout paths. +pub(crate) fn install_capsule_hit_test_proc( + hwnd: HWND, + rect: CapsuleHitTestRect, +) -> windows::core::Result<()> { + if hwnd.0.is_null() { + return Err(windows::core::Error::from_hresult( + windows::Win32::Foundation::E_INVALIDARG, + )); + } + + let mut data = 0; + unsafe { + // A subclass can remain in the chain even when another callback is on top. + if GetWindowSubclass( + hwnd, + Some(capsule_hit_test_subclass_proc), + SUBCLASS_ID, + Some(&mut data), + ) + .as_bool() + { + *(data as *mut CapsuleHitTestRect) = rect; + return Ok(()); + } + let data = Box::into_raw(Box::new(rect)); + if !SetWindowSubclass( + hwnd, + Some(capsule_hit_test_subclass_proc), + SUBCLASS_ID, + data as usize, + ) + .as_bool() + { + drop(Box::from_raw(data)); + return Err(windows::core::Error::from_hresult( + windows::Win32::Foundation::E_FAIL, + )); + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::cell::Cell; + use std::sync::atomic::{AtomicUsize, Ordering}; + use windows::core::PCWSTR; + use windows::Win32::Foundation::HINSTANCE; + use windows::Win32::UI::WindowsAndMessaging::{ + CallWindowProcW, CreateWindowExW, DefWindowProcW, DestroyWindow, GetWindowLongPtrW, + RegisterClassW, SendMessageW, SetWindowLongPtrW, UnregisterClassW, GWLP_USERDATA, + GWLP_WNDPROC, HTCLIENT, HWND_MESSAGE, WINDOW_EX_STYLE, WINDOW_STYLE, WM_APP, WNDCLASSW, + }; + + const PROBE_MESSAGE: u32 = WM_APP + 42; + const PROBE_RESULT: isize = 12345; + static NEXT_CLASS: AtomicUsize = AtomicUsize::new(0); + + #[derive(Default)] + struct Probe { + messages: Cell, + destroyed: Cell, + upper_calls: Cell, + upper_previous: Cell, + } + + unsafe extern "system" fn original_proc( + hwnd: HWND, + msg: u32, + wparam: WPARAM, + lparam: LPARAM, + ) -> LRESULT { + let data = GetWindowLongPtrW(hwnd, GWLP_USERDATA) as *const Probe; + if !data.is_null() { + let probe = &*data; + match msg { + PROBE_MESSAGE => { + probe.messages.set(probe.messages.get() + 1); + return LRESULT(PROBE_RESULT); + } + WM_NCDESTROY => probe.destroyed.set(true), + WM_NCHITTEST => return LRESULT(HTCLIENT as isize), + _ => {} + } + } + DefWindowProcW(hwnd, msg, wparam, lparam) + } + + unsafe extern "system" fn upper_proc( + hwnd: HWND, + msg: u32, + wparam: WPARAM, + lparam: LPARAM, + ) -> LRESULT { + let probe = &*(GetWindowLongPtrW(hwnd, GWLP_USERDATA) as *const Probe); + if msg == PROBE_MESSAGE { + probe.upper_calls.set(probe.upper_calls.get() + 1); + // Bound the failing case instead of overflowing the test process stack. + if probe.upper_calls.get() > 4 { + return LRESULT(-1); + } + } + CallWindowProcW( + std::mem::transmute::( + probe.upper_previous.get(), + ), + hwnd, + msg, + wparam, + lparam, + ) + } + + struct TestWindow { + hwnd: HWND, + class: Vec, + probe: Box, + } + + impl TestWindow { + fn new() -> Self { + let class: Vec = format!( + "OpenLessHitTest{}", + NEXT_CLASS.fetch_add(1, Ordering::Relaxed) + ) + .encode_utf16() + .chain(Some(0)) + .collect(); + let wc = WNDCLASSW { + lpfnWndProc: Some(original_proc), + lpszClassName: PCWSTR(class.as_ptr()), + ..Default::default() + }; + assert_ne!(unsafe { RegisterClassW(&wc) }, 0); + let hwnd = unsafe { + CreateWindowExW( + WINDOW_EX_STYLE(0), + PCWSTR(class.as_ptr()), + PCWSTR::null(), + WINDOW_STYLE(0), + 0, + 0, + 100, + 100, + HWND_MESSAGE, + None, + HINSTANCE::default(), + None, + ) + } + .expect("CreateWindowExW"); + let probe = Box::::default(); + unsafe { + SetWindowLongPtrW(hwnd, GWLP_USERDATA, &*probe as *const Probe as isize); + } + Self { hwnd, class, probe } + } + + fn add_upper_subclass(&self) { + let previous = unsafe { + SetWindowLongPtrW(self.hwnd, GWLP_WNDPROC, upper_proc as *const () as isize) + }; + assert_ne!(previous, 0); + self.probe.upper_previous.set(previous); + } + + fn send_probe(&self) { + assert_eq!( + unsafe { SendMessageW(self.hwnd, PROBE_MESSAGE, WPARAM(0), LPARAM(0)) }.0, + PROBE_RESULT, + ); + } + + fn hit_test(&self, x: i32, y: i32) -> isize { + let mut bounds = RECT::default(); + unsafe { GetWindowRect(self.hwnd, &mut bounds) }.unwrap(); + let x = (bounds.left + x) as u16 as u32; + let y = (bounds.top + y) as u16 as u32; + unsafe { + SendMessageW( + self.hwnd, + WM_NCHITTEST, + WPARAM(0), + LPARAM((x | (y << 16)) as isize), + ) + } + .0 + } + + fn destroy(&mut self) { + unsafe { DestroyWindow(self.hwnd) }.expect("DestroyWindow"); + self.hwnd = HWND::default(); + } + } + + impl Drop for TestWindow { + fn drop(&mut self) { + if !self.hwnd.0.is_null() { + self.destroy(); + } + unsafe { UnregisterClassW(PCWSTR(self.class.as_ptr()), HINSTANCE::default()) } + .expect("UnregisterClassW"); + } + } + + fn rect() -> CapsuleHitTestRect { + CapsuleHitTestRect { + left: 10, + right: 20, + top: 30, + bottom: 40, + } + } + + #[test] + fn reinstall_refreshes_geometry_and_keeps_message_forwarding() { + let window = TestWindow::new(); + install_capsule_hit_test_proc(window.hwnd, rect()).unwrap(); + assert_eq!(window.hit_test(15, 35), HTCLIENT as isize); + assert_eq!(window.hit_test(0, 0), HTTRANSPARENT as isize); + install_capsule_hit_test_proc( + window.hwnd, + CapsuleHitTestRect { + left: 0, + right: 5, + top: 0, + bottom: 5, + }, + ) + .unwrap(); + assert_eq!(window.hit_test(15, 35), HTTRANSPARENT as isize); + assert_eq!(window.hit_test(1, 1), HTCLIENT as isize); + window.send_probe(); + assert_eq!(window.probe.messages.get(), 1); + } + + #[test] + fn upper_subclass_survives_forwarding_and_reinstall() { + let window = TestWindow::new(); + install_capsule_hit_test_proc(window.hwnd, rect()).unwrap(); + window.add_upper_subclass(); + window.send_probe(); + install_capsule_hit_test_proc(window.hwnd, rect()).unwrap(); + assert_eq!( + unsafe { GetWindowLongPtrW(window.hwnd, GWLP_WNDPROC) }, + upper_proc as *const () as isize + ); + window.send_probe(); + assert_eq!(window.probe.messages.get(), 2); + assert_eq!(window.probe.upper_calls.get(), 2); + assert_eq!(window.hit_test(0, 0), HTTRANSPARENT as isize); + } + + #[test] + fn destroy_through_upper_subclass_reaches_original_cleanup() { + let mut window = TestWindow::new(); + install_capsule_hit_test_proc(window.hwnd, rect()).unwrap(); + window.add_upper_subclass(); + window.destroy(); + assert!(window.probe.destroyed.get()); + } + + #[test] + fn rectangles_belong_to_their_window_and_do_not_survive_recreation() { + let mut first = TestWindow::new(); + let second = TestWindow::new(); + install_capsule_hit_test_proc(first.hwnd, rect()).unwrap(); + install_capsule_hit_test_proc( + second.hwnd, + CapsuleHitTestRect { + left: 0, + right: 5, + top: 0, + bottom: 5, + }, + ) + .unwrap(); + assert_eq!(first.hit_test(15, 35), HTCLIENT as isize); + assert_eq!(second.hit_test(15, 35), HTTRANSPARENT as isize); + first.destroy(); + assert!(first.probe.destroyed.get()); + let recreated = TestWindow::new(); + assert!(!unsafe { + GetWindowSubclass( + recreated.hwnd, + Some(capsule_hit_test_subclass_proc), + SUBCLASS_ID, + None, + ) + } + .as_bool()); + install_capsule_hit_test_proc(recreated.hwnd, rect()).unwrap(); + assert_eq!(recreated.hit_test(15, 35), HTCLIENT as isize); + second.send_probe(); + } + + #[test] + fn destroyed_window_rejects_installation() { + let mut window = TestWindow::new(); + let hwnd = window.hwnd; + window.destroy(); + assert!(install_capsule_hit_test_proc(hwnd, rect()).is_err()); + } + + #[test] + fn null_window_is_rejected() { + assert!(install_capsule_hit_test_proc(HWND::default(), rect()).is_err()); + } +} diff --git a/openless-all/app/src-tauri/src/tauri_coordinator_host.rs b/openless-all/app/src-tauri/src/tauri_coordinator_host.rs index 58ddf7816..950f52e95 100644 --- a/openless-all/app/src-tauri/src/tauri_coordinator_host.rs +++ b/openless-all/app/src-tauri/src/tauri_coordinator_host.rs @@ -4,13 +4,15 @@ //! [`tauri::AppHandle`] and keeps window, main-thread and managed-state access //! out of the coordinator's business paths. -#[cfg(target_os = "windows")] -use std::collections::HashMap; use std::future::Future; use std::sync::atomic::{AtomicBool, AtomicU64, AtomicU8, Ordering}; use std::sync::Arc; + +#[cfg(target_os = "windows")] +#[path = "capsule_hit_test.rs"] +mod capsule_hit_test; #[cfg(target_os = "windows")] -use std::sync::OnceLock; +use capsule_hit_test::{install_capsule_hit_test_proc, CapsuleHitTestRect}; #[path = "capsule_snapshot.rs"] mod capsule_snapshot; @@ -138,50 +140,6 @@ fn position_rail_for_capsule( Ok(()) } -#[cfg(target_os = "windows")] -#[derive(Clone, Copy)] -struct CapsuleHitTestEntry { - previous_proc: isize, - control_left: i32, - control_right: i32, - control_top: i32, - control_bottom: i32, -} - -/// Whether `entry` still describes the live window behind `hwnd`. -/// -/// The map is keyed by the numeric HWND, and Windows recycles HWND values: a -/// destroyed capsule/card can hand its handle to an unrelated new window. The one -/// fact that distinguishes the two is whether *our* subclass procedure is still -/// installed on the handle. A recycled HWND carries whatever procedure its new -/// owner installed, so a mismatch means the entry is stale and must be dropped and -/// re-installed instead of trusted. -#[cfg(target_os = "windows")] -unsafe fn capsule_hit_test_entry_matches(hwnd: windows::Win32::Foundation::HWND) -> bool { - use windows::Win32::UI::WindowsAndMessaging::{GetWindowLongPtrW, GWLP_WNDPROC}; - - let installed = unsafe { GetWindowLongPtrW(hwnd, GWLP_WNDPROC) }; - installed == capsule_hit_test_window_proc as *const () as usize as isize -} - -#[cfg(target_os = "windows")] -static CAPSULE_HIT_TEST_ENTRIES: OnceLock>> = - OnceLock::new(); - -#[cfg(target_os = "windows")] -fn capsule_hit_test_entries() -> &'static Mutex> { - CAPSULE_HIT_TEST_ENTRIES.get_or_init(|| Mutex::new(HashMap::new())) -} - -#[cfg(target_os = "windows")] -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -struct CapsuleHitTestRect { - left: i32, - right: i32, - top: i32, - bottom: i32, -} - #[cfg(target_os = "windows")] fn capsule_control_hit_rect( style: CapsuleStyle, @@ -246,145 +204,6 @@ fn capsule_card_hit_rect(window_width: i32, window_height: i32) -> CapsuleHitTes } } -#[cfg(target_os = "windows")] -unsafe extern "system" fn capsule_hit_test_window_proc( - hwnd: windows::Win32::Foundation::HWND, - msg: u32, - wparam: windows::Win32::Foundation::WPARAM, - lparam: windows::Win32::Foundation::LPARAM, -) -> windows::Win32::Foundation::LRESULT { - use windows::Win32::Foundation::RECT; - use windows::Win32::UI::WindowsAndMessaging::{ - CallWindowProcW, DefWindowProcW, GetWindowRect, SetWindowLongPtrW, GWLP_WNDPROC, - HTTRANSPARENT, WM_NCDESTROY, WM_NCHITTEST, - }; - - if msg == WM_NCDESTROY { - let key = hwnd.0 as isize; - // Only retire the entry this window owns. A recycled HWND can reach - // WM_NCDESTROY for a *different* window than the one in the map, and - // removing that survivor's entry would strand it with our procedure - // installed but no recorded `previous_proc` to forward to. - let previous_proc = if unsafe { capsule_hit_test_entry_matches(hwnd) } { - capsule_hit_test_entries() - .lock() - .remove(&key) - .map(|entry| entry.previous_proc) - .unwrap_or_default() - } else { - 0 - }; - if previous_proc != 0 { - let previous: windows::Win32::UI::WindowsAndMessaging::WNDPROC = - std::mem::transmute(previous_proc); - let _ = SetWindowLongPtrW(hwnd, GWLP_WNDPROC, previous_proc); - return CallWindowProcW(previous, hwnd, msg, wparam, lparam); - } - return DefWindowProcW(hwnd, msg, wparam, lparam); - } - - // Every read of the map must prove the entry still belongs to this window - // instance. A recycled HWND would otherwise be hit-tested against another - // window's geometry and forwarded to another window's `previous_proc`. - let live = unsafe { capsule_hit_test_entry_matches(hwnd) }; - let entry = if live { - let entries = capsule_hit_test_entries().lock(); - entries.get(&(hwnd.0 as isize)).copied() - } else { - None - }; - - if msg == WM_NCHITTEST { - if let Some(entry) = entry { - let screen_x = (lparam.0 as i32 & 0xffff) as u16 as i16 as i32; - let screen_y = ((lparam.0 >> 16) & 0xffff) as u16 as i16 as i32; - let mut rect = RECT::default(); - if GetWindowRect(hwnd, &mut rect).is_ok() { - let client_x = screen_x - rect.left; - let client_y = screen_y - rect.top; - if client_x < entry.control_left - || client_x >= entry.control_right - || client_y < entry.control_top - || client_y >= entry.control_bottom - { - return windows::Win32::Foundation::LRESULT(HTTRANSPARENT as isize); - } - } - } - } - - let previous_proc = entry.map(|entry| entry.previous_proc).unwrap_or_default(); - if previous_proc != 0 { - let previous: windows::Win32::UI::WindowsAndMessaging::WNDPROC = - std::mem::transmute(previous_proc); - CallWindowProcW(previous, hwnd, msg, wparam, lparam) - } else { - DefWindowProcW(hwnd, msg, wparam, lparam) - } -} - -#[cfg(target_os = "windows")] -fn install_capsule_hit_test_proc( - hwnd: windows::Win32::Foundation::HWND, - rect: CapsuleHitTestRect, -) -> tauri::Result<()> { - use windows::Win32::Foundation::{GetLastError, SetLastError, ERROR_SUCCESS}; - use windows::Win32::UI::WindowsAndMessaging::{SetWindowLongPtrW, GWLP_WNDPROC}; - - if hwnd.0.is_null() { - return Err(tauri::Error::Anyhow(anyhow::anyhow!( - "cannot install hit-test proc for a null HWND" - ))); - } - - let key = hwnd.0 as isize; - let mut entries = capsule_hit_test_entries().lock(); - // The handle alone is not identity: Windows recycles HWND values, so a matching - // key can belong to a window that was destroyed and replaced. Re-install for this - // instance instead of writing geometry into a dead window's entry — that path - // both lost the subclass and left the map describing the wrong window, which is - // how a live window ends up dispatched to a stale `previous_proc`. - if unsafe { capsule_hit_test_entry_matches(hwnd) } { - if let Some(entry) = entries.get_mut(&key) { - entry.control_left = rect.left; - entry.control_right = rect.right; - entry.control_top = rect.top; - entry.control_bottom = rect.bottom; - return Ok(()); - } - // Our procedure is installed but the map has no entry: the window is ours - // yet nothing can forward for it. Fall through and re-install so the entry - // is rebuilt; the self-install guard below keeps that from recursing. - } - entries.remove(&key); - unsafe { SetLastError(ERROR_SUCCESS) }; - let installed = capsule_hit_test_window_proc as *const () as usize as isize; - let previous = unsafe { SetWindowLongPtrW(hwnd, GWLP_WNDPROC, installed) }; - if previous == 0 && unsafe { GetLastError() } != ERROR_SUCCESS { - return Err(tauri::Error::Anyhow(anyhow::anyhow!( - "SetWindowLongPtrW(GWLP_WNDPROC) failed" - ))); - } - if previous == installed { - // Already wrapped: recording ourselves as `previous_proc` would make every - // forwarded message recurse into this same procedure. There is no real - // predecessor to call, so leave the map empty and let `DefWindowProcW` take - // the traffic instead of overflowing the stack. - return Ok(()); - } - entries.insert( - key, - CapsuleHitTestEntry { - previous_proc: previous, - control_left: rect.left, - control_right: rect.right, - control_top: rect.top, - control_bottom: rect.bottom, - }, - ); - Ok(()) -} - #[cfg(target_os = "windows")] fn set_capsule_input_region( hwnd: windows::Win32::Foundation::HWND, @@ -469,7 +288,7 @@ fn configure_capsule_hit_test( set_capsule_input_region(hwnd, rect)?; if let Err(error) = install_capsule_hit_test_proc(hwnd, rect) { clear_capsule_input_region(hwnd); - return Err(error); + return Err(tauri::Error::Anyhow(error.into())); } Ok(()) } @@ -501,6 +320,7 @@ fn configure_card_hit_test( let rect = capsule_card_hit_rect(client.right - client.left, client.bottom - client.top); clear_capsule_input_region(hwnd); install_capsule_hit_test_proc(hwnd, rect) + .map_err(|error| tauri::Error::Anyhow(error.into())) } #[cfg(target_os = "windows")] @@ -2509,124 +2329,6 @@ impl TauriCoordinatorHost { mod tests { use super::*; - /// Close the gap that made the recycled-HWND bug possible: re-installing the - /// hit-test procedure on a window that already carries it must not overwrite - /// the recorded predecessor with ourselves, and must not lose the entry. - /// - /// Uses a real `HWND_MESSAGE` window because the identity check reads the live - /// `GWLP_WNDPROC`, which no mock can stand in for. - #[cfg(target_os = "windows")] - #[test] - fn reinstall_keeps_the_real_predecessor_and_never_self_wraps() { - use windows::Win32::Foundation::{HINSTANCE, HWND, LPARAM, WPARAM}; - use windows::Win32::UI::WindowsAndMessaging::{ - CreateWindowExW, DefWindowProcW, DestroyWindow, GetWindowLongPtrW, RegisterClassW, - UnregisterClassW, GWLP_WNDPROC, HWND_MESSAGE, WINDOW_EX_STYLE, WINDOW_STYLE, - WNDCLASSW, - }; - - static CLASS_NAME: [u16; 20] = [ - 'O' as u16, 'L' as u16, 'H' as u16, 'i' as u16, 't' as u16, 'T' as u16, 'e' as u16, - 's' as u16, 't' as u16, 'C' as u16, 'l' as u16, 's' as u16, 0, 0, 0, 0, 0, 0, 0, 0, - ]; - - unsafe extern "system" fn plain_proc( - hwnd: HWND, - msg: u32, - wparam: WPARAM, - lparam: LPARAM, - ) -> windows::Win32::Foundation::LRESULT { - unsafe { DefWindowProcW(hwnd, msg, wparam, lparam) } - } - - // `HINSTANCE::default()` is NULL, which is what a class registered by the - // process itself wants; the `Win32_System_LibraryLoader` feature that - // `GetModuleHandleW` needs is not enabled in this crate. - let hinstance = HINSTANCE::default(); - let class = WNDCLASSW { - lpfnWndProc: Some(plain_proc), - hInstance: hinstance, - lpszClassName: windows::core::PCWSTR(CLASS_NAME.as_ptr()), - ..Default::default() - }; - assert_ne!(unsafe { RegisterClassW(&class) }, 0, "RegisterClassW"); - - let hwnd = unsafe { - CreateWindowExW( - WINDOW_EX_STYLE(0), - windows::core::PCWSTR(CLASS_NAME.as_ptr()), - windows::core::PCWSTR(std::ptr::null()), - WINDOW_STYLE(0), - 0, - 0, - 0, - 0, - HWND_MESSAGE, - None, - hinstance, - None, - ) - } - .expect("CreateWindowExW"); - - let rect = CapsuleHitTestRect { - left: 10, - right: 20, - top: 30, - bottom: 40, - }; - install_capsule_hit_test_proc(hwnd, rect).expect("first install"); - let after_first = unsafe { GetWindowLongPtrW(hwnd, GWLP_WNDPROC) }; - assert_eq!( - after_first, - capsule_hit_test_window_proc as *const () as usize as isize, - "our procedure must be installed" - ); - - // Second install takes the fast path and only refreshes geometry. - install_capsule_hit_test_proc( - hwnd, - CapsuleHitTestRect { - left: 1, - right: 2, - top: 3, - bottom: 4, - }, - ) - .expect("second install"); - { - let entries = capsule_hit_test_entries().lock(); - let entry = entries.get(&(hwnd.0 as isize)).expect("entry survives"); - assert_eq!( - entry.previous_proc, - plain_proc as *const () as usize as isize, - "the recorded predecessor must stay the original window procedure" - ); - assert_eq!( - (entry.control_left, entry.control_right), - (1, 2), - "geometry is refreshed in place" - ); - } - - // Third install with the map wiped exercises the defensive branch: our - // procedure is on the window but nothing records it, so `SetWindowLongPtrW` - // returns our own address. Recording that as `previous_proc` would send - // every message straight back into this same procedure. - capsule_hit_test_entries().lock().remove(&(hwnd.0 as isize)); - install_capsule_hit_test_proc(hwnd, rect).expect("third install"); - assert!( - capsule_hit_test_entries() - .lock() - .get(&(hwnd.0 as isize)) - .is_none(), - "a self-install must not be recorded, or forwarding recurses forever" - ); - - unsafe { DestroyWindow(hwnd) }.expect("DestroyWindow"); - let _ = unsafe { UnregisterClassW(windows::core::PCWSTR(CLASS_NAME.as_ptr()), hinstance) }; - } - #[test] fn display_mode_changes_invalidate_mirrors_without_changing_style() { let state = CapsuleWindowState::default();