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.
This commit is contained in:
Timothy Flynn 2024-09-27 16:53:29 -04:00 committed by Andreas Kling
parent 4ccc52e921
commit b5aa8f65b1
Notes: github-actions[bot] 2024-09-29 09:49:47 +00:00
4 changed files with 21 additions and 5 deletions

View file

@ -37,14 +37,18 @@ Client::Client(NonnullOwnPtr<Core::BufferedTCPSocket> socket, LaunchBrowserCallb
Client::~Client() = default; Client::~Client() = default;
ErrorOr<NonnullRefPtr<Session>, Web::WebDriver::Error> Client::find_session_with_id(StringView session_id) ErrorOr<NonnullRefPtr<Session>, 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<unsigned>(); auto session_id_or_error = session_id.to_number<unsigned>();
if (!session_id_or_error.has_value()) if (!session_id_or_error.has_value())
return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidSessionId, "Invalid session id"); 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 *session.release_value();
}
return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidSessionId, "Invalid session id"); 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) Web::WebDriver::Response Client::switch_to_window(Web::WebDriver::Parameters parameters, AK::JsonValue payload)
{ {
dbgln_if(WEBDRIVER_DEBUG, "Handling POST /session/<session_id>/window"); dbgln_if(WEBDRIVER_DEBUG, "Handling POST /session/<session_id>/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()) if (!payload.is_object())
return Web::WebDriver::Error::from_code(Web::WebDriver::ErrorCode::InvalidArgument, "Payload is not a JSON 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) Web::WebDriver::Response Client::get_window_handles(Web::WebDriver::Parameters parameters, JsonValue)
{ {
dbgln_if(WEBDRIVER_DEBUG, "Handling GET /session/<session_id>/window/handles"); dbgln_if(WEBDRIVER_DEBUG, "Handling GET /session/<session_id>/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(); return session->get_window_handles();
} }

View file

@ -35,7 +35,11 @@ public:
private: private:
Client(NonnullOwnPtr<Core::BufferedTCPSocket>, LaunchBrowserCallbacks, Core::EventReceiver* parent); Client(NonnullOwnPtr<Core::BufferedTCPSocket>, LaunchBrowserCallbacks, Core::EventReceiver* parent);
ErrorOr<NonnullRefPtr<Session>, Web::WebDriver::Error> find_session_with_id(StringView session_id); enum class AllowInvalidWindowHandle {
No,
Yes,
};
ErrorOr<NonnullRefPtr<Session>, 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 new_session(Web::WebDriver::Parameters parameters, JsonValue payload) override;
virtual Web::WebDriver::Response delete_session(Web::WebDriver::Parameters parameters, JsonValue payload) override; virtual Web::WebDriver::Response delete_session(Web::WebDriver::Parameters parameters, JsonValue payload) override;

View file

@ -169,6 +169,13 @@ Web::WebDriver::Response Session::get_window_handles() const
return JsonValue { move(handles) }; return JsonValue { move(handles) };
} }
ErrorOr<void, Web::WebDriver::Error> 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 Web::WebDriver::Response Session::execute_script(JsonValue payload, ScriptMode mode) const
{ {
ScopeGuard guard { [&]() { web_content_connection().on_script_executed = nullptr; } }; ScopeGuard guard { [&]() { web_content_connection().on_script_executed = nullptr; } };

View file

@ -53,6 +53,7 @@ public:
Web::WebDriver::Response close_window(); Web::WebDriver::Response close_window();
Web::WebDriver::Response switch_to_window(StringView); Web::WebDriver::Response switch_to_window(StringView);
Web::WebDriver::Response get_window_handles() const; Web::WebDriver::Response get_window_handles() const;
ErrorOr<void, Web::WebDriver::Error> ensure_current_window_handle_is_valid() const;
enum class ScriptMode { enum class ScriptMode {
Sync, Sync,