From b5aa8f65b168c668e41ce61c966dae8198e233ed Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Fri, 27 Sep 2024 16:53:29 -0400 Subject: [PATCH] WebDriver: Ensure that the session's current window handle is valid After closing a window, it is the client's job to switch to another window before executing any other command. Currently, we will crash if that did not happen when we try to send an IPC to a window handle that we no longer hold. This patch makes us return a "no such window" error instead. The exceptions to this new check are the "Switch to Window" and "Get Window Handles" commands. --- Userland/Services/WebDriver/Client.cpp | 12 ++++++++---- Userland/Services/WebDriver/Client.h | 6 +++++- Userland/Services/WebDriver/Session.cpp | 7 +++++++ Userland/Services/WebDriver/Session.h | 1 + 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/Userland/Services/WebDriver/Client.cpp b/Userland/Services/WebDriver/Client.cpp index 4d698e3bcaa..9613432387d 100644 --- a/Userland/Services/WebDriver/Client.cpp +++ b/Userland/Services/WebDriver/Client.cpp @@ -37,14 +37,18 @@ Client::Client(NonnullOwnPtr socket, LaunchBrowserCallb Client::~Client() = default; -ErrorOr, Web::WebDriver::Error> Client::find_session_with_id(StringView session_id) +ErrorOr, Web::WebDriver::Error> Client::find_session_with_id(StringView session_id, AllowInvalidWindowHandle allow_invalid_window_handle) { auto session_id_or_error = session_id.to_number(); if (!session_id_or_error.has_value()) return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidSessionId, "Invalid session id"); - if (auto session = s_sessions.get(*session_id_or_error); session.has_value()) + if (auto session = s_sessions.get(*session_id_or_error); session.has_value()) { + if (allow_invalid_window_handle == AllowInvalidWindowHandle::No) + TRY(session.value()->ensure_current_window_handle_is_valid()); + return *session.release_value(); + } return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidSessionId, "Invalid session id"); } @@ -311,7 +315,7 @@ Web::WebDriver::Response Client::close_window(Web::WebDriver::Parameters paramet Web::WebDriver::Response Client::switch_to_window(Web::WebDriver::Parameters parameters, AK::JsonValue payload) { dbgln_if(WEBDRIVER_DEBUG, "Handling POST /session//window"); - auto session = TRY(find_session_with_id(parameters[0])); + auto session = TRY(find_session_with_id(parameters[0], AllowInvalidWindowHandle::Yes)); if (!payload.is_object()) return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidArgument, "Payload is not a JSON object"); @@ -331,7 +335,7 @@ Web::WebDriver::Response Client::switch_to_window(Web::WebDriver::Parameters par Web::WebDriver::Response Client::get_window_handles(Web::WebDriver::Parameters parameters, JsonValue) { dbgln_if(WEBDRIVER_DEBUG, "Handling GET /session//window/handles"); - auto session = TRY(find_session_with_id(parameters[0])); + auto session = TRY(find_session_with_id(parameters[0], AllowInvalidWindowHandle::Yes)); return session->get_window_handles(); } diff --git a/Userland/Services/WebDriver/Client.h b/Userland/Services/WebDriver/Client.h index 156b4c97f06..573806abb5e 100644 --- a/Userland/Services/WebDriver/Client.h +++ b/Userland/Services/WebDriver/Client.h @@ -35,7 +35,11 @@ public: private: Client(NonnullOwnPtr, LaunchBrowserCallbacks, Core::EventReceiver* parent); - ErrorOr, Web::WebDriver::Error> find_session_with_id(StringView session_id); + enum class AllowInvalidWindowHandle { + No, + Yes, + }; + ErrorOr, Web::WebDriver::Error> find_session_with_id(StringView session_id, AllowInvalidWindowHandle = AllowInvalidWindowHandle::No); virtual Web::WebDriver::Response new_session(Web::WebDriver::Parameters parameters, JsonValue payload) override; virtual Web::WebDriver::Response delete_session(Web::WebDriver::Parameters parameters, JsonValue payload) override; diff --git a/Userland/Services/WebDriver/Session.cpp b/Userland/Services/WebDriver/Session.cpp index 68c6635c65b..3612d097d67 100644 --- a/Userland/Services/WebDriver/Session.cpp +++ b/Userland/Services/WebDriver/Session.cpp @@ -169,6 +169,13 @@ Web::WebDriver::Response Session::get_window_handles() const return JsonValue { move(handles) }; } +ErrorOr Session::ensure_current_window_handle_is_valid() const +{ + if (auto current_window = m_windows.get(m_current_window_handle); current_window.has_value()) + return {}; + return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::NoSuchWindow, "Window not found"sv); +} + Web::WebDriver::Response Session::execute_script(JsonValue payload, ScriptMode mode) const { ScopeGuard guard { [&]() { web_content_connection().on_script_executed = nullptr; } }; diff --git a/Userland/Services/WebDriver/Session.h b/Userland/Services/WebDriver/Session.h index fa0988f9647..173dca2e6fe 100644 --- a/Userland/Services/WebDriver/Session.h +++ b/Userland/Services/WebDriver/Session.h @@ -53,6 +53,7 @@ public: Web::WebDriver::Response close_window(); Web::WebDriver::Response switch_to_window(StringView); Web::WebDriver::Response get_window_handles() const; + ErrorOr ensure_current_window_handle_is_valid() const; enum class ScriptMode { Sync,