From 65a9a515b51be1050bbb6fcb1edfbdff706434bf Mon Sep 17 00:00:00 2001 From: Jan Grulich Date: Mon, 22 Aug 2022 14:06:08 +0200 Subject: [PATCH] Add additional checks in case of early portal error In case ScreenCast portal fails right at the beginning, we need to check the response before trying to get session handle to avoid accessing non-existing portal data. Also on early failure do not continue making source request if we failed before and don't have session handle. Bug: webrtc:13429 Change-Id: I2bfbd2c6e96e3cda1e62aa9dc07f66d4c7496b53 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272400 Reviewed-by: Alexander Cooper Reviewed-by: Mark Foltz Commit-Queue: Mark Foltz Cr-Commit-Position: refs/heads/main@{#37872} --- .../screen_capture_portal_interface.cc | 17 +++++++++--- .../linux/wayland/screencast_portal.cc | 26 +++++++------------ .../linux/wayland/xdg_desktop_portal_utils.cc | 15 +++++++++++ .../linux/wayland/xdg_desktop_portal_utils.h | 2 ++ 4 files changed, 39 insertions(+), 21 deletions(-) diff --git a/modules/desktop_capture/linux/wayland/screen_capture_portal_interface.cc b/modules/desktop_capture/linux/wayland/screen_capture_portal_interface.cc index c76fa73dad..02d9d2e806 100644 --- a/modules/desktop_capture/linux/wayland/screen_capture_portal_interface.cc +++ b/modules/desktop_capture/linux/wayland/screen_capture_portal_interface.cc @@ -8,6 +8,7 @@ * be found in the AUTHORS file in the root of the source tree. */ #include "modules/desktop_capture/linux/wayland/screen_capture_portal_interface.h" +#include "modules/desktop_capture/linux/wayland/xdg_desktop_portal_utils.h" #include @@ -67,18 +68,26 @@ void ScreenCapturePortalInterface::RegisterSessionClosedSignalHandler( GDBusConnection* connection, std::string& session_handle, guint& session_closed_signal_id) { - uint32_t portal_response; + uint32_t portal_response = 2; Scoped response_data; g_variant_get(parameters, /*format_string=*/"(u@a{sv})", &portal_response, response_data.receive()); + + if (RequestResponseFromPortalResponse(portal_response) != + RequestResponse::kSuccess) { + RTC_LOG(LS_ERROR) << "Failed to request the session subscription."; + OnPortalDone(RequestResponse::kError); + return; + } + Scoped g_session_handle( g_variant_lookup_value(response_data.get(), /*key=*/"session_handle", /*expected_type=*/nullptr)); - session_handle = g_variant_dup_string( + session_handle = g_variant_get_string( /*value=*/g_session_handle.get(), /*length=*/nullptr); - if (session_handle.empty() || portal_response) { - RTC_LOG(LS_ERROR) << "Failed to request the session subscription."; + if (session_handle.empty()) { + RTC_LOG(LS_ERROR) << "Could not get session handle despite valid response"; OnPortalDone(RequestResponse::kError); return; } diff --git a/modules/desktop_capture/linux/wayland/screencast_portal.cc b/modules/desktop_capture/linux/wayland/screencast_portal.cc index 46668d80d1..8e45af7e24 100644 --- a/modules/desktop_capture/linux/wayland/screencast_portal.cc +++ b/modules/desktop_capture/linux/wayland/screencast_portal.cc @@ -29,21 +29,7 @@ using xdg_portal::SetupRequestResponseSignal; using xdg_portal::SetupSessionRequestHandlers; using xdg_portal::StartSessionRequest; using xdg_portal::TearDownSession; - -RequestResponse ToRequestResponseFromSignal(uint32_t signal_response) { - // See: - // https://docs.flatpak.org/en/latest/portal-api-reference.html#gdbus-signal-org-freedesktop-portal-Request.Response - switch (signal_response) { - case 0: - return RequestResponse::kSuccess; - case 1: - return RequestResponse::kUserCancelled; - case 2: - return RequestResponse::kError; - default: - return RequestResponse::kUnknown; - } -} +using xdg_portal::RequestResponseFromPortalResponse; } // namespace @@ -176,7 +162,13 @@ void ScreenCastPortal::OnSessionRequestResponseSignal( that->RegisterSessionClosedSignalHandler( OnSessionClosedSignal, parameters, that->connection_, that->session_handle_, that->session_closed_signal_id_); - that->SourcesRequest(); + + // Do not continue if we don't get session_handle back. The call above will + // already notify the capturer there is a failure, but we would still continue + // to make following request and crash on that. + if (!that->session_handle_.empty()) { + that->SourcesRequest(); + } } // static @@ -357,7 +349,7 @@ void ScreenCastPortal::OnStartRequestResponseSignal(GDBusConnection* connection, response_data.receive()); if (portal_response || !response_data) { RTC_LOG(LS_ERROR) << "Failed to start the screen cast session."; - that->OnPortalDone(ToRequestResponseFromSignal(portal_response)); + that->OnPortalDone(RequestResponseFromPortalResponse(portal_response)); return; } diff --git a/modules/desktop_capture/linux/wayland/xdg_desktop_portal_utils.cc b/modules/desktop_capture/linux/wayland/xdg_desktop_portal_utils.cc index 4a74b9f188..75dbf2bdf3 100644 --- a/modules/desktop_capture/linux/wayland/xdg_desktop_portal_utils.cc +++ b/modules/desktop_capture/linux/wayland/xdg_desktop_portal_utils.cc @@ -33,6 +33,21 @@ std::string RequestResponseToString(RequestResponse request) { } } +RequestResponse RequestResponseFromPortalResponse(uint32_t portal_response) { + // See: + // https://docs.flatpak.org/en/latest/portal-api-reference.html#gdbus-signal-org-freedesktop-portal-Request.Response + switch (portal_response) { + case 0: + return RequestResponse::kSuccess; + case 1: + return RequestResponse::kUserCancelled; + case 2: + return RequestResponse::kError; + default: + return RequestResponse::kUnknown; + } +} + std::string PrepareSignalHandle(absl::string_view token, GDBusConnection* connection) { Scoped sender( diff --git a/modules/desktop_capture/linux/wayland/xdg_desktop_portal_utils.h b/modules/desktop_capture/linux/wayland/xdg_desktop_portal_utils.h index edd046639b..f6ac92b5d1 100644 --- a/modules/desktop_capture/linux/wayland/xdg_desktop_portal_utils.h +++ b/modules/desktop_capture/linux/wayland/xdg_desktop_portal_utils.h @@ -57,6 +57,8 @@ using SessionStartRequestedHandler = void (*)(GDBusProxy*, std::string RequestResponseToString(RequestResponse request); +RequestResponse RequestResponseFromPortalResponse(uint32_t portal_response); + // Returns a string path for signal handle based on the provided connection and // token. std::string PrepareSignalHandle(absl::string_view token,