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 <alcooper@chromium.org>
Reviewed-by: Mark Foltz <mfoltz@chromium.org>
Commit-Queue: Mark Foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#37872}
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 c76fa73..02d9d2e 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 <string>
@@ -67,18 +68,26 @@
GDBusConnection* connection,
std::string& session_handle,
guint& session_closed_signal_id) {
- uint32_t portal_response;
+ uint32_t portal_response = 2;
Scoped<GVariant> 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<GVariant> 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 46668d8..8e45af7 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::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 @@
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 @@
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 4a74b9f..75dbf2b 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 @@
}
}
+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<char> 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 edd0466..f6ac92b 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 @@
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,
