From d3a548687230ba39f3970abd07f83bc81445fd65 Mon Sep 17 00:00:00 2001 From: Chris233 Date: Sun, 23 Aug 2026 23:14:10 +0800 Subject: [PATCH 1/3] fix(windows-ime): make TSF shutdown fully cancelable Remove the uncancelable named-pipe flush, post owned submit requests to the TSF owner thread, cancel pending edit sessions during shutdown, and keep the DLL loaded while asynchronous edit sessions remain alive.\n\nRelated to #954. --- .../windows-ime-lifecycle-contract.test.mjs | 52 +++++ .../app/src-tauri/src/windows_ime_ipc.rs | 2 +- .../app/windows-ime/OpenLessIme.vcxproj | 5 + openless-all/app/windows-ime/src/dllmain.cpp | 8 +- .../app/windows-ime/src/edit_session.cpp | 21 +- .../app/windows-ime/src/edit_session.h | 5 +- .../app/windows-ime/src/ipc_client.cpp | 94 ++++++--- openless-all/app/windows-ime/src/ipc_client.h | 7 +- .../app/windows-ime/src/text_service.cpp | 197 ++++++++++++++---- .../app/windows-ime/src/text_service.h | 7 +- 10 files changed, 315 insertions(+), 83 deletions(-) create mode 100644 openless-all/app/scripts/windows-ime-lifecycle-contract.test.mjs diff --git a/openless-all/app/scripts/windows-ime-lifecycle-contract.test.mjs b/openless-all/app/scripts/windows-ime-lifecycle-contract.test.mjs new file mode 100644 index 000000000..45c28f6db --- /dev/null +++ b/openless-all/app/scripts/windows-ime-lifecycle-contract.test.mjs @@ -0,0 +1,52 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const appRoot = join(dirname(fileURLToPath(import.meta.url)), ".."); +const imeRoot = join(appRoot, "windows-ime", "src"); +const ipcClient = readFileSync(join(imeRoot, "ipc_client.cpp"), "utf8"); +const ipcHeader = readFileSync(join(imeRoot, "ipc_client.h"), "utf8"); +const textService = readFileSync(join(imeRoot, "text_service.cpp"), "utf8"); +const editSession = readFileSync(join(imeRoot, "edit_session.cpp"), "utf8"); + +assert.doesNotMatch( + ipcClient, + /FlushFileBuffers\(/, + "IME shutdown must not block the host UI thread waiting for the pipe client", +); +assert.match( + ipcClient, + /WaitForClientDisconnect/, + "IME replies should wait for client disconnect through cancelable overlapped I/O", +); +assert.match(ipcHeader, /HRESULT Start\(/, "IME activation should report pipe server startup failures"); +assert.match(ipcHeader, /void Run\(\) noexcept/, "IME worker exceptions must not terminate the host process"); + +assert.doesNotMatch( + textService, + /SendMessageTimeoutW\(/, + "IME worker must not synchronously send a stack request to the owner thread", +); +assert.match(textService, /PostMessageW\(/, "IME worker should post an owned request to the owner thread"); +assert.match( + textService, + /WaitForMultipleObjects\(/, + "IME owner-thread and async edit waits should be cancelable during shutdown", +); +assert.match( + textService, + /PeekMessageW[\s\S]*PM_REMOVE/, + "IME shutdown should release queued submit requests before destroying the message window", +); + +assert.match( + editSession, + /InterlockedIncrement\(&g_object_count\)/, + "IME edit sessions should keep the COM DLL loaded while TSF holds them", +); +assert.match( + editSession, + /InterlockedDecrement\(&g_object_count\)/, + "IME edit sessions should release the COM DLL lifetime count when destroyed", +); diff --git a/openless-all/app/src-tauri/src/windows_ime_ipc.rs b/openless-all/app/src-tauri/src/windows_ime_ipc.rs index 921b15609..9a3b330ca 100644 --- a/openless-all/app/src-tauri/src/windows_ime_ipc.rs +++ b/openless-all/app/src-tauri/src/windows_ime_ipc.rs @@ -10,7 +10,7 @@ const IME_SUBMIT_TIMEOUT_MARGIN_MS: u64 = 1000; const IME_NATIVE_ASYNC_COMMIT_TIMEOUT_MS: u64 = IME_OWNER_THREAD_MESSAGE_TIMEOUT_MS + IME_ASYNC_EDIT_SESSION_TIMEOUT_MS; -// Must exceed the IME DLL owner-thread SendMessageTimeoutW wait plus the +// Must exceed the IME DLL owner-thread posted-request wait plus the // async edit session wait, otherwise Rust can fall back while the DLL later // commits and duplicates insertion. pub const IME_SUBMIT_TIMEOUT: Duration = diff --git a/openless-all/app/windows-ime/OpenLessIme.vcxproj b/openless-all/app/windows-ime/OpenLessIme.vcxproj index 956cd1fae..01542deac 100644 --- a/openless-all/app/windows-ime/OpenLessIme.vcxproj +++ b/openless-all/app/windows-ime/OpenLessIme.vcxproj @@ -68,6 +68,11 @@ + + + /utf-8 %(AdditionalOptions) + + Level4 diff --git a/openless-all/app/windows-ime/src/dllmain.cpp b/openless-all/app/windows-ime/src/dllmain.cpp index 624c79f75..38c2f509f 100644 --- a/openless-all/app/windows-ime/src/dllmain.cpp +++ b/openless-all/app/windows-ime/src/dllmain.cpp @@ -25,11 +25,15 @@ BOOL APIENTRY DllMain(HINSTANCE instance, DWORD reason, LPVOID reserved) { return TRUE; } -STDAPI DllCanUnloadNow() { +__control_entrypoint(DllExport) +STDAPI DllCanUnloadNow(void) { return (g_lock_count == 0 && g_object_count == 0) ? S_OK : S_FALSE; } -STDAPI DllGetClassObject(REFCLSID clsid, REFIID iid, void** object) { +_Check_return_ +STDAPI DllGetClassObject(_In_ REFCLSID clsid, + _In_ REFIID iid, + _Outptr_ LPVOID FAR* object) { if (object == nullptr) { return E_POINTER; } diff --git a/openless-all/app/windows-ime/src/edit_session.cpp b/openless-all/app/windows-ime/src/edit_session.cpp index 6e1a32f15..1960dc25a 100644 --- a/openless-all/app/windows-ime/src/edit_session.cpp +++ b/openless-all/app/windows-ime/src/edit_session.cpp @@ -2,6 +2,8 @@ #include +extern LONG g_object_count; + OpenLessAsyncEditState::OpenLessAsyncEditState() : event(CreateEventW(nullptr, TRUE, FALSE, nullptr)) { if (event == nullptr) { @@ -23,10 +25,13 @@ bool OpenLessAsyncEditState::IsValid() const { OpenLessEditSession::OpenLessEditSession( ITfContext* context, std::wstring text, - std::shared_ptr async_state) + std::shared_ptr async_state, + std::shared_ptr> cancellation) : context_(context), text_(std::move(text)), - async_state_(std::move(async_state)) { + async_state_(std::move(async_state)), + cancellation_(std::move(cancellation)) { + InterlockedIncrement(&g_object_count); if (context_ != nullptr) { context_->AddRef(); } @@ -37,6 +42,7 @@ OpenLessEditSession::~OpenLessEditSession() { context_->Release(); context_ = nullptr; } + InterlockedDecrement(&g_object_count); } STDMETHODIMP OpenLessEditSession::QueryInterface(REFIID iid, void** object) { @@ -67,7 +73,9 @@ STDMETHODIMP_(ULONG) OpenLessEditSession::Release() { } STDMETHODIMP OpenLessEditSession::DoEditSession(TfEditCookie edit_cookie) { - const HRESULT hr = InsertText(edit_cookie); + const HRESULT hr = cancellation_ && cancellation_->load() + ? HRESULT_FROM_WIN32(ERROR_CANCELLED) + : InsertText(edit_cookie); if (async_state_) { async_state_->result = hr; if (async_state_->event != nullptr) { @@ -81,6 +89,9 @@ HRESULT OpenLessEditSession::InsertText(TfEditCookie edit_cookie) { if (context_ == nullptr) { return E_UNEXPECTED; } + if (cancellation_ && cancellation_->load()) { + return HRESULT_FROM_WIN32(ERROR_CANCELLED); + } ITfInsertAtSelection* insert_at_selection = nullptr; HRESULT hr = context_->QueryInterface(IID_ITfInsertAtSelection, @@ -99,6 +110,10 @@ HRESULT OpenLessEditSession::InsertText(TfEditCookie edit_cookie) { query_range = nullptr; } + if (SUCCEEDED(hr) && cancellation_ && cancellation_->load()) { + hr = HRESULT_FROM_WIN32(ERROR_CANCELLED); + } + if (SUCCEEDED(hr)) { ITfRange* committed_range = nullptr; hr = insert_at_selection->InsertTextAtSelection( diff --git a/openless-all/app/windows-ime/src/edit_session.h b/openless-all/app/windows-ime/src/edit_session.h index 0185b6d95..35b505576 100644 --- a/openless-all/app/windows-ime/src/edit_session.h +++ b/openless-all/app/windows-ime/src/edit_session.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include #include @@ -23,7 +24,8 @@ class OpenLessEditSession final : public ITfEditSession { OpenLessEditSession( ITfContext* context, std::wstring text, - std::shared_ptr async_state = nullptr); + std::shared_ptr async_state = nullptr, + std::shared_ptr> cancellation = nullptr); OpenLessEditSession(const OpenLessEditSession&) = delete; OpenLessEditSession& operator=(const OpenLessEditSession&) = delete; ~OpenLessEditSession(); @@ -40,4 +42,5 @@ class OpenLessEditSession final : public ITfEditSession { ITfContext* context_ = nullptr; std::wstring text_; std::shared_ptr async_state_; + std::shared_ptr> cancellation_; }; diff --git a/openless-all/app/windows-ime/src/ipc_client.cpp b/openless-all/app/windows-ime/src/ipc_client.cpp index 4b32ee10e..e4f5df787 100644 --- a/openless-all/app/windows-ime/src/ipc_client.cpp +++ b/openless-all/app/windows-ime/src/ipc_client.cpp @@ -1,7 +1,10 @@ #include "ipc_client.h" #include +#include +#include #include +#include #include "text_service.h" @@ -204,7 +207,11 @@ bool ParseJsonInteger(const std::string& json, size_t* pos, int* value) { int parsed = 0; while (*pos < json.size() && json[*pos] >= '0' && json[*pos] <= '9') { - parsed = parsed * 10 + (json[*pos] - '0'); + const int digit = json[*pos] - '0'; + if (parsed > ((std::numeric_limits::max)() - digit) / 10) { + return false; + } + parsed = parsed * 10 + digit; ++(*pos); } @@ -342,9 +349,12 @@ OpenLessPipeServer::~OpenLessPipeServer() { Stop(); } -void OpenLessPipeServer::Start(OpenLessTextService* service) { - if (service == nullptr || thread_.joinable()) { - return; +HRESULT OpenLessPipeServer::Start(OpenLessTextService* service) { + if (service == nullptr) { + return E_INVALIDARG; + } + if (thread_.joinable()) { + return HRESULT_FROM_WIN32(ERROR_ALREADY_INITIALIZED); } stop_requested_.store(false); @@ -355,21 +365,29 @@ void OpenLessPipeServer::Start(OpenLessTextService* service) { stop_event_ = CreateEventW(nullptr, TRUE, FALSE, nullptr); io_event_ = CreateEventW(nullptr, TRUE, FALSE, nullptr); if (stop_event_ == nullptr || io_event_ == nullptr) { - if (stop_event_ != nullptr) { - CloseHandle(stop_event_); - stop_event_ = nullptr; - } - if (io_event_ != nullptr) { - CloseHandle(io_event_); - io_event_ = nullptr; - } - return; - } - - pipe_name_ = PipeNameForCurrentThread(); - service_ = service; - service_->AddRef(); - thread_ = std::thread(&OpenLessPipeServer::Run, this); + const DWORD error = GetLastError(); + ResetServerState(); + return HRESULT_FROM_WIN32(error != ERROR_SUCCESS ? error + : ERROR_NOT_ENOUGH_MEMORY); + } + + try { + pipe_name_ = PipeNameForCurrentThread(); + service_ = service; + service_->AddRef(); + thread_ = std::thread(&OpenLessPipeServer::Run, this); + } catch (const std::bad_alloc&) { + ResetServerState(); + return E_OUTOFMEMORY; + } catch (const std::system_error&) { + ResetServerState(); + return E_FAIL; + } catch (...) { + ResetServerState(); + return E_UNEXPECTED; + } + + return S_OK; } void OpenLessPipeServer::Stop() { @@ -378,14 +396,16 @@ void OpenLessPipeServer::Stop() { SetEvent(stop_event_); } - // The worker parks in WaitForMultipleObjects on {io_event_, stop_event_}, so - // signaling stop_event_ wakes it deterministically. join() then returns at - // once instead of blocking the host UI thread on an uncancelable - // ConnectNamedPipe wait — the root cause of the explorer.exe AppHang. + // Every worker wait includes stop_event_, so joining cannot leave the host UI + // thread blocked on a pipe client or a synchronous pipe operation. if (thread_.joinable()) { thread_.join(); } + ResetServerState(); +} + +void OpenLessPipeServer::ResetServerState() { if (io_event_ != nullptr) { CloseHandle(io_event_); io_event_ = nullptr; @@ -399,9 +419,18 @@ void OpenLessPipeServer::Stop() { service_->Release(); service_ = nullptr; } + pipe_name_.clear(); +} + +void OpenLessPipeServer::Run() noexcept { + try { + RunLoop(); + } catch (...) { + // Never let an allocation or STL exception terminate the host process. + } } -void OpenLessPipeServer::Run() { +void OpenLessPipeServer::RunLoop() { const std::wstring pipe_name = pipe_name_; while (!stop_requested_.load()) { HANDLE pipe = CreateNamedPipeW( @@ -417,9 +446,9 @@ void OpenLessPipeServer::Run() { if (ReadJsonLine(pipe, &line)) { HandleSubmitLine(pipe, line); } + WaitForClientDisconnect(pipe); } - FlushFileBuffers(pipe); DisconnectNamedPipe(pipe); CloseHandle(pipe); } @@ -459,6 +488,18 @@ bool OpenLessPipeServer::WaitForClient(HANDLE pipe) { return false; } +void OpenLessPipeServer::WaitForClientDisconnect(HANDLE pipe) { + char buffer[256] = {}; + while (!stop_requested_.load()) { + DWORD bytes_read = 0; + if (!RunOverlapped(pipe, /*is_write=*/false, buffer, sizeof(buffer), + &bytes_read) || + bytes_read == 0) { + return; + } + } +} + bool OpenLessPipeServer::RunOverlapped(HANDLE pipe, bool is_write, void* buffer, @@ -539,7 +580,8 @@ void OpenLessPipeServer::HandleSubmitLine(HANDLE pipe, const std::string& line) } const HRESULT hr = - service_->SubmitTextFromPipe(message.session_id, message.text); + service_->SubmitTextFromPipe(message.session_id, message.text, + stop_event_); if (SUCCEEDED(hr)) { WriteResult(pipe, message.session_id, L"committed", nullptr); } else { diff --git a/openless-all/app/windows-ime/src/ipc_client.h b/openless-all/app/windows-ime/src/ipc_client.h index 8ca4c25f6..628bed2bd 100644 --- a/openless-all/app/windows-ime/src/ipc_client.h +++ b/openless-all/app/windows-ime/src/ipc_client.h @@ -14,12 +14,14 @@ class OpenLessPipeServer { OpenLessPipeServer& operator=(const OpenLessPipeServer&) = delete; ~OpenLessPipeServer(); - void Start(OpenLessTextService* service); + HRESULT Start(OpenLessTextService* service); void Stop(); private: - void Run(); + void Run() noexcept; + void RunLoop(); bool WaitForClient(HANDLE pipe); + void WaitForClientDisconnect(HANDLE pipe); bool ReadJsonLine(HANDLE pipe, std::string* line); void HandleSubmitLine(HANDLE pipe, const std::string& line); bool WriteResult(HANDLE pipe, @@ -35,6 +37,7 @@ class OpenLessPipeServer { void* buffer, DWORD length, DWORD* bytes); + void ResetServerState(); std::atomic stop_requested_{false}; std::thread thread_; diff --git a/openless-all/app/windows-ime/src/text_service.cpp b/openless-all/app/windows-ime/src/text_service.cpp index f303eac10..24876d93e 100644 --- a/openless-all/app/windows-ime/src/text_service.cpp +++ b/openless-all/app/windows-ime/src/text_service.cpp @@ -15,30 +15,81 @@ constexpr UINT kSubmitTextMessage = WM_APP + 1; constexpr UINT kSubmitTextTimeoutMs = 2000; struct SubmitTextRequest { - const std::wstring* session_id = nullptr; - const std::wstring* text = nullptr; + SubmitTextRequest() + : cancellation(std::make_shared>(false)), + completion_event(CreateEventW(nullptr, TRUE, FALSE, nullptr)) { + if (completion_event == nullptr) { + create_error = GetLastError(); + } + } + + ~SubmitTextRequest() { + if (completion_event != nullptr) { + CloseHandle(completion_event); + completion_event = nullptr; + } + } + + bool IsValid() const { + return completion_event != nullptr; + } + + std::wstring session_id; + std::wstring text; std::shared_ptr async_completion; bool wait_for_async_completion = false; HRESULT result = E_UNEXPECTED; + std::shared_ptr> cancellation; + HANDLE completion_event = nullptr; + DWORD create_error = ERROR_SUCCESS; }; -HRESULT WaitForAsyncEditCompletion( - const std::shared_ptr& completion) { - if (!completion || !completion->IsValid()) { - return HRESULT_FROM_WIN32(completion && completion->create_error != ERROR_SUCCESS - ? completion->create_error - : ERROR_INVALID_HANDLE); +using PostedSubmitRequest = std::shared_ptr; + +HRESULT WaitForCompletionOrCancellation( + HANDLE completion_event, + HANDLE cancellation_event, + const std::shared_ptr>& cancellation) { + if (completion_event == nullptr) { + return HRESULT_FROM_WIN32(ERROR_INVALID_HANDLE); + } + if (cancellation_event != nullptr && + WaitForSingleObject(cancellation_event, 0) == WAIT_OBJECT_0) { + cancellation->store(true); + return HRESULT_FROM_WIN32(ERROR_CANCELLED); } - const DWORD wait_result = - WaitForSingleObject(completion->event, kSubmitTextTimeoutMs); + const HANDLE wait_handles[2] = {completion_event, cancellation_event}; + const DWORD wait_count = cancellation_event != nullptr ? 2 : 1; + const DWORD wait_result = WaitForMultipleObjects( + wait_count, wait_handles, FALSE, kSubmitTextTimeoutMs); if (wait_result == WAIT_OBJECT_0) { - return completion->result; + return S_OK; + } + + cancellation->store(true); + if (wait_count == 2 && wait_result == WAIT_OBJECT_0 + 1) { + return HRESULT_FROM_WIN32(ERROR_CANCELLED); } if (wait_result == WAIT_TIMEOUT) { return HRESULT_FROM_WIN32(ERROR_TIMEOUT); } - return HRESULT_FROM_WIN32(GetLastError()); + const DWORD error = GetLastError(); + return HRESULT_FROM_WIN32(error != ERROR_SUCCESS ? error : ERROR_GEN_FAILURE); +} + +HRESULT WaitForAsyncEditCompletion( + const std::shared_ptr& completion, + HANDLE cancellation_event, + const std::shared_ptr>& cancellation) { + if (!completion || !completion->IsValid()) { + return HRESULT_FROM_WIN32(completion && completion->create_error != ERROR_SUCCESS + ? completion->create_error + : ERROR_INVALID_HANDLE); + } + const HRESULT wait_result = WaitForCompletionOrCancellation( + completion->event, cancellation_event, cancellation); + return FAILED(wait_result) ? wait_result : completion->result; } } // namespace @@ -133,38 +184,63 @@ STDMETHODIMP OpenLessTextService::Deactivate() { HRESULT OpenLessTextService::SubmitTextFromPipe( const std::wstring& session_id, - const std::wstring& text) { - if (GetCurrentThreadId() == owner_thread_id_) { - return CommitTextOnOwnerThread(session_id, text, nullptr, nullptr); - } + const std::wstring& text, + HANDLE cancellation_event) { + try { + if (GetCurrentThreadId() == owner_thread_id_) { + auto cancellation = std::make_shared>(false); + return CommitTextOnOwnerThread(session_id, text, nullptr, nullptr, + cancellation); + } - if (message_window_ == nullptr) { - return E_UNEXPECTED; - } + if (message_window_ == nullptr) { + return E_UNEXPECTED; + } - SubmitTextRequest request; - request.session_id = &session_id; - request.text = &text; - DWORD_PTR message_result = 0; - const LRESULT sent = SendMessageTimeoutW( - message_window_, kSubmitTextMessage, 0, - reinterpret_cast(&request), SMTO_ABORTIFHUNG, - kSubmitTextTimeoutMs, &message_result); - if (sent == 0) { - const DWORD error = GetLastError(); - return HRESULT_FROM_WIN32(error != ERROR_SUCCESS ? error : ERROR_TIMEOUT); - } + auto request = std::make_shared(); + if (!request->IsValid()) { + return HRESULT_FROM_WIN32(request->create_error != ERROR_SUCCESS + ? request->create_error + : ERROR_INVALID_HANDLE); + } + request->session_id = session_id; + request->text = text; - if (request.wait_for_async_completion) { - return WaitForAsyncEditCompletion(request.async_completion); - } + auto* posted_request = new (std::nothrow) PostedSubmitRequest(request); + if (posted_request == nullptr) { + return E_OUTOFMEMORY; + } + + if (!PostMessageW(message_window_, kSubmitTextMessage, 0, + reinterpret_cast(posted_request))) { + const DWORD error = GetLastError(); + delete posted_request; + return HRESULT_FROM_WIN32(error != ERROR_SUCCESS ? error + : ERROR_GEN_FAILURE); + } - return request.result; + const HRESULT wait_result = WaitForCompletionOrCancellation( + request->completion_event, cancellation_event, + request->cancellation); + if (FAILED(wait_result)) { + return wait_result; + } + + if (request->wait_for_async_completion) { + return WaitForAsyncEditCompletion(request->async_completion, + cancellation_event, + request->cancellation); + } + return request->result; + } catch (const std::bad_alloc&) { + return E_OUTOFMEMORY; + } catch (...) { + return E_UNEXPECTED; + } } HRESULT OpenLessTextService::StartIpcServer() { - pipe_server_.Start(this); - return S_OK; + return pipe_server_.Start(this); } void OpenLessTextService::StopIpcServer() { @@ -200,6 +276,11 @@ HRESULT OpenLessTextService::EnsureMessageWindow() { void OpenLessTextService::DestroyMessageWindow() { if (message_window_ != nullptr) { + MSG message = {}; + while (PeekMessageW(&message, message_window_, kSubmitTextMessage, + kSubmitTextMessage, PM_REMOVE)) { + delete reinterpret_cast(message.lParam); + } DestroyWindow(message_window_); message_window_ = nullptr; } @@ -209,12 +290,16 @@ HRESULT OpenLessTextService::CommitTextOnOwnerThread( const std::wstring& session_id, const std::wstring& text, std::shared_ptr* async_completion, - bool* wait_for_async_completion) { + bool* wait_for_async_completion, + const std::shared_ptr>& cancellation) { UNREFERENCED_PARAMETER(session_id); if (thread_mgr_ == nullptr || client_id_ == TF_CLIENTID_NULL) { return E_UNEXPECTED; } + if (cancellation && cancellation->load()) { + return HRESULT_FROM_WIN32(ERROR_CANCELLED); + } ITfDocumentMgr* document_mgr = nullptr; HRESULT hr = thread_mgr_->GetFocus(&document_mgr); @@ -236,7 +321,9 @@ HRESULT OpenLessTextService::CommitTextOnOwnerThread( return E_FAIL; } - auto* session = new (std::nothrow) OpenLessEditSession(context, text); + auto* session = + new (std::nothrow) OpenLessEditSession(context, text, nullptr, + cancellation); if (session == nullptr) { context->Release(); return E_OUTOFMEMORY; @@ -266,6 +353,11 @@ HRESULT OpenLessTextService::CommitTextOnOwnerThread( return edit_result; } + if (cancellation && cancellation->load()) { + context->Release(); + return HRESULT_FROM_WIN32(ERROR_CANCELLED); + } + auto completion = std::make_shared(); if (!completion->IsValid()) { context->Release(); @@ -275,7 +367,8 @@ HRESULT OpenLessTextService::CommitTextOnOwnerThread( } auto* async_session = - new (std::nothrow) OpenLessEditSession(context, text, completion); + new (std::nothrow) OpenLessEditSession(context, text, completion, + cancellation); if (async_session == nullptr) { context->Release(); return E_OUTOFMEMORY; @@ -316,15 +409,27 @@ LRESULT CALLBACK OpenLessTextService::MessageWindowProc(HWND window, auto* service = reinterpret_cast( GetWindowLongPtrW(window, GWLP_USERDATA)); if (message == kSubmitTextMessage && service != nullptr) { - auto* request = reinterpret_cast(lparam); - if (request == nullptr || request->session_id == nullptr || - request->text == nullptr) { + std::unique_ptr posted_request( + reinterpret_cast(lparam)); + if (!posted_request || !*posted_request) { return 0; } - request->result = service->CommitTextOnOwnerThread( - *request->session_id, *request->text, &request->async_completion, - &request->wait_for_async_completion); + const auto request = *posted_request; + if (request->cancellation->load()) { + request->result = HRESULT_FROM_WIN32(ERROR_CANCELLED); + } else { + try { + request->result = service->CommitTextOnOwnerThread( + request->session_id, request->text, &request->async_completion, + &request->wait_for_async_completion, request->cancellation); + } catch (const std::bad_alloc&) { + request->result = E_OUTOFMEMORY; + } catch (...) { + request->result = E_UNEXPECTED; + } + } + SetEvent(request->completion_event); return 1; } diff --git a/openless-all/app/windows-ime/src/text_service.h b/openless-all/app/windows-ime/src/text_service.h index c69190d05..1ad3ed618 100644 --- a/openless-all/app/windows-ime/src/text_service.h +++ b/openless-all/app/windows-ime/src/text_service.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include #include @@ -27,7 +28,8 @@ class OpenLessTextService final : public ITfTextInputProcessorEx { DWORD flags) override; HRESULT SubmitTextFromPipe(const std::wstring& session_id, - const std::wstring& text); + const std::wstring& text, + HANDLE cancellation_event = nullptr); private: HRESULT StartIpcServer(); @@ -38,7 +40,8 @@ class OpenLessTextService final : public ITfTextInputProcessorEx { const std::wstring& session_id, const std::wstring& text, std::shared_ptr* async_completion, - bool* wait_for_async_completion); + bool* wait_for_async_completion, + const std::shared_ptr>& cancellation); static LRESULT CALLBACK MessageWindowProc(HWND window, UINT message, From cd018a26c2a3db624a579f6cd2ee43a811ee2e04 Mon Sep 17 00:00:00 2001 From: Chris233 Date: Mon, 24 Aug 2026 01:14:43 +0800 Subject: [PATCH 2/3] fix(windows-ime): handle uncertain submits and startup failures --- .../windows-ime-lifecycle-contract.test.mjs | 10 +++ openless-all/app/src-tauri/src/coordinator.rs | 54 +++++++++--- .../app/src-tauri/src/windows_ime_ipc.rs | 85 +++++++++++++++---- .../app/src-tauri/src/windows_ime_session.rs | 33 +++++-- .../app/windows-ime/src/ipc_client.cpp | 72 ++++++++++++++-- openless-all/app/windows-ime/src/ipc_client.h | 5 +- 6 files changed, 217 insertions(+), 42 deletions(-) diff --git a/openless-all/app/scripts/windows-ime-lifecycle-contract.test.mjs b/openless-all/app/scripts/windows-ime-lifecycle-contract.test.mjs index 45c28f6db..a9293e4cc 100644 --- a/openless-all/app/scripts/windows-ime-lifecycle-contract.test.mjs +++ b/openless-all/app/scripts/windows-ime-lifecycle-contract.test.mjs @@ -21,6 +21,16 @@ assert.match( "IME replies should wait for client disconnect through cancelable overlapped I/O", ); assert.match(ipcHeader, /HRESULT Start\(/, "IME activation should report pipe server startup failures"); +assert.match( + ipcClient, + /WaitForSingleObject\(startup_event_/, + "IME activation must wait for the worker's first named-pipe creation result", +); +assert.match( + ipcClient, + /CreateNamedPipeW[\s\S]*ReportStartupResult/, + "IME worker must report the first CreateNamedPipeW result to activation", +); assert.match(ipcHeader, /void Run\(\) noexcept/, "IME worker exceptions must not terminate the host process"); assert.doesNotMatch( diff --git a/openless-all/app/src-tauri/src/coordinator.rs b/openless-all/app/src-tauri/src/coordinator.rs index 852d64d4f..ef1f375f4 100644 --- a/openless-all/app/src-tauri/src/coordinator.rs +++ b/openless-all/app/src-tauri/src/coordinator.rs @@ -62,7 +62,9 @@ use crate::types::{ #[cfg(target_os = "windows")] use crate::windows_ime_ipc::ImeSubmitTarget; #[cfg(target_os = "windows")] -use crate::windows_ime_session::{PreparedWindowsImeSession, WindowsImeSessionController}; +use crate::windows_ime_session::{ + PreparedWindowsImeSession, WindowsImeSessionController, WindowsImeSessionError, +}; mod asr_wiring; mod capsule_focus; @@ -3235,6 +3237,7 @@ async fn insert_with_windows_ime_first( if should_try_non_tsf_insertion_fallback( allow_non_tsf_insertion_fallback, InsertStatus::Failed, + true, ) { return insert_via_non_tsf_fallback(inner, polished, restore_clipboard, paste_shortcut); } @@ -3249,21 +3252,37 @@ async fn insert_with_windows_ime_first( target: ime_target, }; - let ime_status = match inner.windows_ime.submit_prepared(&prepared, request).await { - Ok(status) => status, + let (ime_status, outcome_known) = match inner + .windows_ime + .submit_prepared(&prepared, request) + .await + { + Ok(status) => (status, true), + Err(WindowsImeSessionError::OutcomeUnknown(error)) => { + log::warn!( + "[windows-ime] TSF submit outcome is unknown; suppressing automatic fallback: {error}" + ); + (InsertStatus::Failed, false) + } Err(error) => { log::warn!("[windows-ime] TSF submit failed: {error}"); - InsertStatus::Failed + (InsertStatus::Failed, true) } }; inner.windows_ime.restore_session(prepared); if ime_status == InsertStatus::Inserted { ime_status - } else if should_try_non_tsf_insertion_fallback(allow_non_tsf_insertion_fallback, ime_status) { + } else if should_try_non_tsf_insertion_fallback( + allow_non_tsf_insertion_fallback, + ime_status, + outcome_known, + ) { insert_via_non_tsf_fallback(inner, polished, restore_clipboard, paste_shortcut) } else { - log::warn!("[windows-ime] TSF did not insert; non-TSF insertion fallback is disabled"); + if outcome_known { + log::warn!("[windows-ime] TSF did not insert; non-TSF insertion fallback is disabled"); + } InsertStatus::Failed } } @@ -3272,8 +3291,9 @@ async fn insert_with_windows_ime_first( fn should_try_non_tsf_insertion_fallback( allow_non_tsf_insertion_fallback: bool, ime_status: InsertStatus, + outcome_known: bool, ) -> bool { - allow_non_tsf_insertion_fallback && ime_status != InsertStatus::Inserted + allow_non_tsf_insertion_fallback && outcome_known && ime_status != InsertStatus::Inserted } #[cfg(target_os = "windows")] @@ -5613,23 +5633,33 @@ mod tests { fn non_tsf_insertion_fallback_gate_blocks_only_when_disabled() { assert!(should_try_non_tsf_insertion_fallback( true, - InsertStatus::CopiedFallback + InsertStatus::CopiedFallback, + true )); assert!(should_try_non_tsf_insertion_fallback( true, - InsertStatus::Failed + InsertStatus::Failed, + true )); assert!(!should_try_non_tsf_insertion_fallback( true, - InsertStatus::Inserted + InsertStatus::Inserted, + true )); assert!(!should_try_non_tsf_insertion_fallback( false, - InsertStatus::CopiedFallback + InsertStatus::CopiedFallback, + true )); assert!(!should_try_non_tsf_insertion_fallback( false, - InsertStatus::Failed + InsertStatus::Failed, + true + )); + assert!(!should_try_non_tsf_insertion_fallback( + true, + InsertStatus::Failed, + false )); } diff --git a/openless-all/app/src-tauri/src/windows_ime_ipc.rs b/openless-all/app/src-tauri/src/windows_ime_ipc.rs index 9a3b330ca..6c357142f 100644 --- a/openless-all/app/src-tauri/src/windows_ime_ipc.rs +++ b/openless-all/app/src-tauri/src/windows_ime_ipc.rs @@ -22,12 +22,15 @@ const ERROR_PATH_NOT_FOUND: u32 = 3; const ERROR_SEM_TIMEOUT: u32 = 121; const ERROR_PIPE_BUSY: u32 = 231; const NMPWAIT_NOWAIT: u32 = 0x00000001; +const NATIVE_TIMEOUT_HRESULT: &str = "hresult:0x800705B4"; +const NATIVE_CANCELLED_HRESULT: &str = "hresult:0x800704C7"; #[derive(Debug, Clone, PartialEq, Eq)] pub enum WindowsImeIpcError { Unavailable(String), NoReadyClient, Timeout, + OutcomeUnknown(String), Protocol(String), Io(String), } @@ -35,7 +38,10 @@ pub enum WindowsImeIpcError { impl std::fmt::Display for WindowsImeIpcError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::Unavailable(message) | Self::Protocol(message) | Self::Io(message) => { + Self::Unavailable(message) + | Self::OutcomeUnknown(message) + | Self::Protocol(message) + | Self::Io(message) => { write!(f, "{message}") } Self::NoReadyClient => write!(f, "no OpenLess IME client is ready"), @@ -44,6 +50,24 @@ impl std::fmt::Display for WindowsImeIpcError { } } +fn classify_native_submit_result( + status: ImeSubmitStatus, + error_code: Option<&str>, +) -> WindowsImeIpcResult { + if status != ImeSubmitStatus::Committed + && matches!( + error_code, + Some(NATIVE_TIMEOUT_HRESULT | NATIVE_CANCELLED_HRESULT) + ) + { + return Err(WindowsImeIpcError::OutcomeUnknown(format!( + "OpenLess IME submit outcome is unknown after {}", + error_code.unwrap_or("native cancellation") + ))); + } + Ok(status) +} + impl std::error::Error for WindowsImeIpcError {} pub type WindowsImeIpcResult = Result; @@ -183,8 +207,8 @@ mod windows_pipe { use tokio::net::windows::named_pipe::{ClientOptions, NamedPipeClient}; use super::{ - ImeSubmitRequest, PendingImeSubmit, WindowsImeIpcError, WindowsImeIpcResult, - IME_CLIENT_WAIT_TIMEOUT, IME_PIPE_RETRY_INTERVAL, IME_SUBMIT_TIMEOUT, + classify_native_submit_result, ImeSubmitRequest, PendingImeSubmit, WindowsImeIpcError, + WindowsImeIpcResult, IME_CLIENT_WAIT_TIMEOUT, IME_PIPE_RETRY_INTERVAL, IME_SUBMIT_TIMEOUT, }; use crate::windows_ime_protocol::{ decode_message, encode_message, ime_pipe_candidate_names_for_target, @@ -218,28 +242,38 @@ mod windows_pipe { write_half .write_all(line.as_bytes()) .await - .map_err(|error| WindowsImeIpcError::Io(error.to_string()))?; - write_half - .flush() - .await - .map_err(|error| WindowsImeIpcError::Io(error.to_string()))?; + .map_err(|error| { + WindowsImeIpcError::OutcomeUnknown(format!( + "IME pipe write failed after submit dispatch began: {error}" + )) + })?; + write_half.flush().await.map_err(|error| { + WindowsImeIpcError::OutcomeUnknown(format!( + "IME pipe flush failed after submit dispatch: {error}" + )) + })?; let mut response = String::new(); - let bytes_read = reader - .read_line(&mut response) - .await - .map_err(|error| WindowsImeIpcError::Io(error.to_string()))?; + let bytes_read = reader.read_line(&mut response).await.map_err(|error| { + WindowsImeIpcError::OutcomeUnknown(format!( + "IME submit result read failed after dispatch: {error}" + )) + })?; if bytes_read == 0 { - return Err(WindowsImeIpcError::Io( - "IME pipe closed before submit result".to_string(), + return Err(WindowsImeIpcError::OutcomeUnknown( + "IME pipe closed before reporting the dispatched submit result".to_string(), )); } Ok(response) }) .await - .map_err(|_| WindowsImeIpcError::Timeout)??; + .map_err(|_| { + WindowsImeIpcError::OutcomeUnknown( + "IME submit timed out after request dispatch began".to_string(), + ) + })??; match decode_message(response.trim_end()) .map_err(|error| WindowsImeIpcError::Protocol(error.to_string()))? @@ -255,7 +289,8 @@ mod windows_pipe { "[windows-ime] submit result status={status:?} error_code={error_code:?}" ); } - pending.accept_result(&session_id, status) + let status = pending.accept_result(&session_id, status)?; + classify_native_submit_result(status, error_code.as_deref()) } ImePipeMessage::SubmitResult { protocol_version, .. @@ -391,6 +426,24 @@ mod tests { .is_err()); } + #[test] + fn native_timeout_or_cancellation_keeps_submit_outcome_unknown() { + for error_code in ["hresult:0x800705B4", "hresult:0x800704C7"] { + assert!(matches!( + classify_native_submit_result(ImeSubmitStatus::Rejected, Some(error_code)), + Err(WindowsImeIpcError::OutcomeUnknown(_)) + )); + } + } + + #[test] + fn definitive_native_rejection_remains_safe_to_fallback() { + assert_eq!( + classify_native_submit_result(ImeSubmitStatus::Rejected, Some("hresult:0x80004005")), + Ok(ImeSubmitStatus::Rejected) + ); + } + #[test] fn submit_timeout_covers_native_async_commit_path() { assert!(IME_SUBMIT_TIMEOUT > Duration::from_millis(IME_NATIVE_ASYNC_COMMIT_TIMEOUT_MS)); diff --git a/openless-all/app/src-tauri/src/windows_ime_session.rs b/openless-all/app/src-tauri/src/windows_ime_session.rs index a0104ec7e..5e6a812bb 100644 --- a/openless-all/app/src-tauri/src/windows_ime_session.rs +++ b/openless-all/app/src-tauri/src/windows_ime_session.rs @@ -1,6 +1,6 @@ #![allow(dead_code, unused_imports, unused_variables)] use crate::types::InsertStatus; -use crate::windows_ime_ipc::{ImeSubmitRequest, WindowsImeIpcServer}; +use crate::windows_ime_ipc::{ImeSubmitRequest, WindowsImeIpcError, WindowsImeIpcServer}; use crate::windows_ime_profile::{ is_openless_profile_snapshot, restore_decision, ImeProfileSnapshot, ProfileRestoreDecision, WindowsImeProfileManager, @@ -12,12 +12,15 @@ use crate::windows_ime_restore::{run_restore_flow, RESTORE_RETRY_DELAY_MS}; pub enum WindowsImeSessionError { Profile(String), Ipc(String), + OutcomeUnknown(String), } impl std::fmt::Display for WindowsImeSessionError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::Profile(message) | Self::Ipc(message) => write!(f, "{message}"), + Self::Profile(message) | Self::Ipc(message) | Self::OutcomeUnknown(message) => { + write!(f, "{message}") + } } } } @@ -35,6 +38,15 @@ pub fn should_fallback_after_ime_result(status: ImeSubmitStatus) -> bool { !matches!(status, ImeSubmitStatus::Committed) } +fn map_ipc_error(error: WindowsImeIpcError) -> WindowsImeSessionError { + match error { + WindowsImeIpcError::OutcomeUnknown(message) => { + WindowsImeSessionError::OutcomeUnknown(message) + } + error => WindowsImeSessionError::Ipc(error.to_string()), + } +} + fn describe_snapshot(snapshot: &ImeProfileSnapshot) -> String { format!( "kind={:?} lang=0x{:04X} clsid={} profile={}", @@ -147,11 +159,7 @@ impl WindowsImeSessionController { )); } - let status = self - .ipc - .submit_text(request) - .await - .map_err(|error| WindowsImeSessionError::Ipc(error.to_string()))?; + let status = self.ipc.submit_text(request).await.map_err(map_ipc_error)?; if should_fallback_after_ime_result(status) { log::warn!( "[windows-ime] TSF submit returned {status:?}; falling back to non-TSF insertion" @@ -232,6 +240,17 @@ mod tests { )); } + #[test] + fn unknown_ipc_outcome_stays_distinct_from_definitive_failure() { + assert!(matches!( + map_ipc_error(WindowsImeIpcError::OutcomeUnknown( + "native commit timed out".to_string() + )), + WindowsImeSessionError::OutcomeUnknown(message) + if message == "native commit timed out" + )); + } + #[tokio::test] async fn submit_prepared_reports_unavailable_session() { let controller = WindowsImeSessionController::new(); diff --git a/openless-all/app/windows-ime/src/ipc_client.cpp b/openless-all/app/windows-ime/src/ipc_client.cpp index e4f5df787..3fe4fbfc3 100644 --- a/openless-all/app/windows-ime/src/ipc_client.cpp +++ b/openless-all/app/windows-ime/src/ipc_client.cpp @@ -13,6 +13,7 @@ namespace { constexpr wchar_t kPipeNamePrefix[] = L"\\\\.\\pipe\\OpenLessImeSubmit"; constexpr DWORD kPipeBufferSize = 4096; constexpr size_t kMaxJsonLineBytes = 64 * 1024; +constexpr DWORD kPipeStartupTimeoutMs = 2000; struct SubmitMessage { std::wstring type; @@ -359,12 +360,15 @@ HRESULT OpenLessPipeServer::Start(OpenLessTextService* service) { stop_requested_.store(false); - // Manual-reset events: stop_event_ wakes every overlapped wait the moment - // Stop() runs; io_event_ is the completion event reused by each overlapped - // operation on the worker thread. + // Manual-reset events: startup_event_ returns the first CreateNamedPipeW + // result to Activate; stop_event_ wakes every overlapped wait the moment + // Stop() runs; io_event_ is reused by worker I/O. + startup_result_ = E_PENDING; + startup_event_ = CreateEventW(nullptr, TRUE, FALSE, nullptr); stop_event_ = CreateEventW(nullptr, TRUE, FALSE, nullptr); io_event_ = CreateEventW(nullptr, TRUE, FALSE, nullptr); - if (stop_event_ == nullptr || io_event_ == nullptr) { + if (startup_event_ == nullptr || stop_event_ == nullptr || + io_event_ == nullptr) { const DWORD error = GetLastError(); ResetServerState(); return HRESULT_FROM_WIN32(error != ERROR_SUCCESS ? error @@ -387,6 +391,23 @@ HRESULT OpenLessPipeServer::Start(OpenLessTextService* service) { return E_UNEXPECTED; } + const DWORD startup_wait = + WaitForSingleObject(startup_event_, kPipeStartupTimeoutMs); + if (startup_wait != WAIT_OBJECT_0) { + const DWORD error = startup_wait == WAIT_TIMEOUT ? ERROR_TIMEOUT + : GetLastError(); + Stop(); + return HRESULT_FROM_WIN32(error != ERROR_SUCCESS ? error + : ERROR_GEN_FAILURE); + } + + const HRESULT startup_result = startup_result_; + CloseHandle(startup_event_); + startup_event_ = nullptr; + if (FAILED(startup_result)) { + Stop(); + return startup_result; + } return S_OK; } @@ -406,6 +427,10 @@ void OpenLessPipeServer::Stop() { } void OpenLessPipeServer::ResetServerState() { + if (startup_event_ != nullptr) { + CloseHandle(startup_event_); + startup_event_ = nullptr; + } if (io_event_ != nullptr) { CloseHandle(io_event_); io_event_ = nullptr; @@ -423,14 +448,33 @@ void OpenLessPipeServer::ResetServerState() { } void OpenLessPipeServer::Run() noexcept { + bool startup_reported = false; try { - RunLoop(); + RunLoop(&startup_reported); + } catch (const std::bad_alloc&) { + if (!startup_reported) { + ReportStartupResult(E_OUTOFMEMORY); + } + } catch (const std::system_error&) { + if (!startup_reported) { + ReportStartupResult(E_FAIL); + } } catch (...) { // Never let an allocation or STL exception terminate the host process. + if (!startup_reported) { + ReportStartupResult(E_UNEXPECTED); + } } } -void OpenLessPipeServer::RunLoop() { +void OpenLessPipeServer::ReportStartupResult(HRESULT result) noexcept { + startup_result_ = result; + if (startup_event_ != nullptr) { + SetEvent(startup_event_); + } +} + +void OpenLessPipeServer::RunLoop(bool* startup_reported) { const std::wstring pipe_name = pipe_name_; while (!stop_requested_.load()) { HANDLE pipe = CreateNamedPipeW( @@ -438,9 +482,20 @@ void OpenLessPipeServer::RunLoop() { PIPE_TYPE_MESSAGE | PIPE_READMODE_BYTE | PIPE_WAIT, 1, kPipeBufferSize, kPipeBufferSize, 0, nullptr); if (pipe == INVALID_HANDLE_VALUE) { + if (!*startup_reported) { + const DWORD error = GetLastError(); + *startup_reported = true; + ReportStartupResult(HRESULT_FROM_WIN32( + error != ERROR_SUCCESS ? error : ERROR_GEN_FAILURE)); + } return; } + if (!*startup_reported) { + *startup_reported = true; + ReportStartupResult(S_OK); + } + if (WaitForClient(pipe) && !stop_requested_.load()) { std::string line; if (ReadJsonLine(pipe, &line)) { @@ -452,6 +507,11 @@ void OpenLessPipeServer::RunLoop() { DisconnectNamedPipe(pipe); CloseHandle(pipe); } + + if (!*startup_reported) { + *startup_reported = true; + ReportStartupResult(HRESULT_FROM_WIN32(ERROR_CANCELLED)); + } } bool OpenLessPipeServer::WaitForClient(HANDLE pipe) { diff --git a/openless-all/app/windows-ime/src/ipc_client.h b/openless-all/app/windows-ime/src/ipc_client.h index 628bed2bd..77ab899fc 100644 --- a/openless-all/app/windows-ime/src/ipc_client.h +++ b/openless-all/app/windows-ime/src/ipc_client.h @@ -19,7 +19,8 @@ class OpenLessPipeServer { private: void Run() noexcept; - void RunLoop(); + void RunLoop(bool* startup_reported); + void ReportStartupResult(HRESULT result) noexcept; bool WaitForClient(HANDLE pipe); void WaitForClientDisconnect(HANDLE pipe); bool ReadJsonLine(HANDLE pipe, std::string* line); @@ -41,8 +42,10 @@ class OpenLessPipeServer { std::atomic stop_requested_{false}; std::thread thread_; + HANDLE startup_event_ = nullptr; HANDLE stop_event_ = nullptr; HANDLE io_event_ = nullptr; + HRESULT startup_result_ = E_PENDING; std::wstring pipe_name_; OpenLessTextService* service_ = nullptr; }; From c3ed07c46cc813238670c7f8d680c7359d96aaf9 Mon Sep 17 00:00:00 2001 From: Chris233 Date: Tue, 25 Aug 2026 20:18:19 +0800 Subject: [PATCH 3/3] fix(windows-ime): keep untrusted responses outcome-unknown --- .../app/src-tauri/src/windows_ime_ipc.rs | 111 +++++++++++++----- 1 file changed, 80 insertions(+), 31 deletions(-) diff --git a/openless-all/app/src-tauri/src/windows_ime_ipc.rs b/openless-all/app/src-tauri/src/windows_ime_ipc.rs index 6c357142f..2334c3bd6 100644 --- a/openless-all/app/src-tauri/src/windows_ime_ipc.rs +++ b/openless-all/app/src-tauri/src/windows_ime_ipc.rs @@ -1,7 +1,9 @@ #![allow(dead_code, unused_imports, unused_variables)] use std::time::Duration; -use crate::windows_ime_protocol::ImeSubmitStatus; +use crate::windows_ime_protocol::{ + decode_message, ImePipeMessage, ImeSubmitStatus, OPENLESS_IME_PROTOCOL_VERSION, +}; pub const IME_CLIENT_WAIT_TIMEOUT: Duration = Duration::from_millis(700); const IME_OWNER_THREAD_MESSAGE_TIMEOUT_MS: u64 = 2000; @@ -124,6 +126,46 @@ impl PendingImeSubmit { } } +fn classify_dispatched_submit_response( + response: &str, + pending: &mut PendingImeSubmit, +) -> WindowsImeIpcResult { + let message = decode_message(response.trim_end()).map_err(|error| { + WindowsImeIpcError::OutcomeUnknown(format!( + "IME submit response could not be decoded after dispatch: {error}" + )) + })?; + + match message { + ImePipeMessage::SubmitResult { + protocol_version, + session_id, + status, + error_code, + } if protocol_version == OPENLESS_IME_PROTOCOL_VERSION => { + if status != ImeSubmitStatus::Committed { + log::warn!( + "[windows-ime] submit result status={status:?} error_code={error_code:?}" + ); + } + let status = pending.accept_result(&session_id, status).map_err(|error| { + WindowsImeIpcError::OutcomeUnknown(format!( + "IME submit response could not be trusted after dispatch: {error}" + )) + })?; + classify_native_submit_result(status, error_code.as_deref()) + } + ImePipeMessage::SubmitResult { + protocol_version, .. + } => Err(WindowsImeIpcError::OutcomeUnknown(format!( + "IME submit response used unsupported protocol version {protocol_version} after dispatch" + ))), + _ => Err(WindowsImeIpcError::OutcomeUnknown( + "IME response was not a submit result after dispatch".to_string(), + )), + } +} + #[derive(Debug, Clone)] pub struct ImeSubmitRequest { pub session_id: String, @@ -207,12 +249,13 @@ mod windows_pipe { use tokio::net::windows::named_pipe::{ClientOptions, NamedPipeClient}; use super::{ - classify_native_submit_result, ImeSubmitRequest, PendingImeSubmit, WindowsImeIpcError, - WindowsImeIpcResult, IME_CLIENT_WAIT_TIMEOUT, IME_PIPE_RETRY_INTERVAL, IME_SUBMIT_TIMEOUT, + classify_dispatched_submit_response, ImeSubmitRequest, PendingImeSubmit, + WindowsImeIpcError, WindowsImeIpcResult, IME_CLIENT_WAIT_TIMEOUT, IME_PIPE_RETRY_INTERVAL, + IME_SUBMIT_TIMEOUT, }; use crate::windows_ime_protocol::{ - decode_message, encode_message, ime_pipe_candidate_names_for_target, - ime_pipe_name_for_target, ImePipeMessage, OPENLESS_IME_PROTOCOL_VERSION, + encode_message, ime_pipe_candidate_names_for_target, ime_pipe_name_for_target, + ImePipeMessage, OPENLESS_IME_PROTOCOL_VERSION, }; extern "system" { @@ -275,32 +318,7 @@ mod windows_pipe { ) })??; - match decode_message(response.trim_end()) - .map_err(|error| WindowsImeIpcError::Protocol(error.to_string()))? - { - ImePipeMessage::SubmitResult { - protocol_version, - session_id, - status, - error_code, - } if protocol_version == OPENLESS_IME_PROTOCOL_VERSION => { - if status != crate::windows_ime_protocol::ImeSubmitStatus::Committed { - log::warn!( - "[windows-ime] submit result status={status:?} error_code={error_code:?}" - ); - } - let status = pending.accept_result(&session_id, status)?; - classify_native_submit_result(status, error_code.as_deref()) - } - ImePipeMessage::SubmitResult { - protocol_version, .. - } => Err(WindowsImeIpcError::Protocol(format!( - "unsupported IME protocol version {protocol_version}" - ))), - _ => Err(WindowsImeIpcError::Protocol( - "message is not a submit result".to_string(), - )), - } + classify_dispatched_submit_response(&response, &mut pending) } async fn open_pipe_with_retry( @@ -444,6 +462,37 @@ mod tests { ); } + #[test] + fn untrusted_dispatched_submit_responses_keep_outcome_unknown() { + for response in [ + "{", + r#"{"type":"submitResult","protocolVersion":2,"sessionId":"session-1","status":"committed","errorCode":null}"#, + r#"{"type":"submitResult","protocolVersion":1,"sessionId":"session-2","status":"committed","errorCode":null}"#, + r#"{"type":"ping","protocolVersion":1}"#, + ] { + let mut pending = PendingImeSubmit::new("session-1".to_string()); + assert!( + matches!( + classify_dispatched_submit_response(response, &mut pending), + Err(WindowsImeIpcError::OutcomeUnknown(_)) + ), + "response should keep the submit outcome unknown: {response}" + ); + } + } + + #[test] + fn validated_dispatched_rejection_remains_safe_to_fallback() { + let mut pending = PendingImeSubmit::new("session-1".to_string()); + assert_eq!( + classify_dispatched_submit_response( + r#"{"type":"submitResult","protocolVersion":1,"sessionId":"session-1","status":"rejected","errorCode":"hresult:0x80004005"}"#, + &mut pending, + ), + Ok(ImeSubmitStatus::Rejected) + ); + } + #[test] fn submit_timeout_covers_native_async_commit_path() { assert!(IME_SUBMIT_TIMEOUT > Duration::from_millis(IME_NATIVE_ASYNC_COMMIT_TIMEOUT_MS));