From: Adam Michalski Date: Thu, 19 May 2022 15:28:25 +0000 (+0200) Subject: Rework DBus exception handling in sessiond and libsessiond X-Git-Tag: submit/tizen/20220603.143851~1^2~7 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a2cee4d0889b237e8a04d938c4efadfeafcded2f;p=platform%2Fcore%2Fsystem%2Fsessiond.git Rework DBus exception handling in sessiond and libsessiond Change-Id: I591af8d9d1dd927bfffddbc45a48540488893699 --- diff --git a/libsessiond/include/sessiond-internal.h b/libsessiond/include/sessiond-internal.h new file mode 100644 index 0000000..832f425 --- /dev/null +++ b/libsessiond/include/sessiond-internal.h @@ -0,0 +1,42 @@ +/* MIT License + * + * Copyright (c) 2022 Samsung Electronics Co., Ltd. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is furnished + * to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. */ + +#pragma once +#ifdef __cplusplus +extern "C" { +#endif + +typedef struct { + const char *dbus_error_msg; + int code; +} error_mapping_t; + +static error_mapping_t error_mappings[] = { + { "org.tizen.sessiond.Error.InvalidParameter", SUBSESSION_ERROR_INVALID_PARAMETER }, + { "org.tizen.sessiond.Error.IOError", SUBSESSION_ERROR_IO_ERROR }, + { "org.tizen.sessiond.Error.SubsessionAlreadyExists", SUBSESSION_ERROR_ALREADY_EXISTS }, + { "org.tizen.sessiond.Error.SubsessionDoesNotExist", SUBSESSION_ERROR_NOT_AVAILABLE }, +}; + +#ifdef __cplusplus +} +#endif diff --git a/libsessiond/include/sessiond.h b/libsessiond/include/sessiond.h index 4c6e6be..4d7f702 100644 --- a/libsessiond/include/sessiond.h +++ b/libsessiond/include/sessiond.h @@ -37,7 +37,7 @@ typedef enum { SUBSESSION_ERROR_INVALID_PARAMETER = TIZEN_ERROR_INVALID_PARAMETER, /**< Invalid parameter */ SUBSESSION_ERROR_IO_ERROR = TIZEN_ERROR_IO_ERROR, /**< I/O error */ SUBSESSION_ERROR_OUT_OF_MEMORY = TIZEN_ERROR_OUT_OF_MEMORY, /**< Out of memory */ - SUBSESSION_ERROR_ALREADY_EXITS = TIZEN_ERROR_FILE_EXISTS, /**< Resource already registered */ + SUBSESSION_ERROR_ALREADY_EXISTS = TIZEN_ERROR_FILE_EXISTS, /**< Resource already registered */ SUBSESSION_ERROR_NOT_AVAILABLE = TIZEN_ERROR_NO_SUCH_DEVICE, /**< Resource unavailable */ SUBSESSION_ERROR_RESOURCE_BUSY = TIZEN_ERROR_RESOURCE_BUSY, /**< Resource busy */ SUBSESSION_ERROR_PERMISSION_DENIED = TIZEN_ERROR_PERMISSION_DENIED, /**< Permission denied */ diff --git a/libsessiond/src/lib.c b/libsessiond/src/lib.c index 919beb9..78251f5 100644 --- a/libsessiond/src/lib.c +++ b/libsessiond/src/lib.c @@ -1,4 +1,3 @@ - /* MIT License * * Copyright (c) 2022 Samsung Electronics Co., Ltd. @@ -30,6 +29,7 @@ #include #include "lib.h" #include "sessiond.h" +#include "sessiond-internal.h" const int libsessiond_default_timeout = 20000; @@ -150,6 +150,18 @@ typedef struct { int signal_subscribed; } signal_client_data_t; +int get_dbus_error_mapping_to_subsession_error(const char *dbus_error) +{ + size_t arr_size = sizeof(error_mappings) / sizeof(error_mappings[0]); + + for (size_t i = 0; i < arr_size; ++i) { + if (strstr(dbus_error, error_mappings[i].dbus_error_msg)) + return error_mappings[i].code; + } + + return SUBSESSION_ERROR_IO_ERROR; +} + int map_dbus_call_error_to_return_value(const GError *error) { g_assert(error); @@ -158,6 +170,9 @@ int map_dbus_call_error_to_return_value(const GError *error) { if (error->code == G_DBUS_ERROR_NAME_HAS_NO_OWNER || error->code == G_DBUS_ERROR_SERVICE_UNKNOWN) return SUBSESSION_ERROR_NOT_SUPPORTED; + if (error->domain == G_IO_ERROR && error->code == G_IO_ERROR_DBUS_ERROR) + return get_dbus_error_mapping_to_subsession_error(error->message); + return SUBSESSION_ERROR_IO_ERROR; } diff --git a/sessiond/src/fs_helpers.cpp b/sessiond/src/fs_helpers.cpp index 966f724..9722335 100644 --- a/sessiond/src/fs_helpers.cpp +++ b/sessiond/src/fs_helpers.cpp @@ -32,6 +32,7 @@ #include #include +#include "globals.hpp" #include "fs_helpers.h" namespace fs = std::filesystem; @@ -190,7 +191,8 @@ void fs_helpers::add_user_subsession(const int session_uid, const int subsession fs::path subsession_path { subsession_dir }; if (fs::exists(subsession_path)) - throw std::runtime_error("Subsession directory already exists"); + throw std::system_error(EEXIST, std::generic_category(), + "Subsession directory already exists"); fs::create_directory(subsession_dir); @@ -227,6 +229,12 @@ void fs_helpers::add_user_subsession(const int session_uid, const int subsession fs::rename(apps_rw_tmp_path, apps_rw_path); } + catch (std::system_error const &ex) { + std::cerr << "Logic exception " << ex.what() << std::endl + << "while copying user subsession data [session_uid=" << session_uid + << " subsession_id=" << subsession_id << "]" << std::endl; + throw; + } catch (std::exception const &ex) { std::cerr << "Exception " << ex.what() << std::endl << "while copying user subsession data [session_uid=" << session_uid @@ -244,10 +252,17 @@ void fs_helpers::remove_user_subsession(const int session_uid, const int subsess }; if (!fs::exists(subsession_path)) - throw std::runtime_error("Subsession directory does not exist"); + throw std::system_error(ENOENT, std::generic_category(), + "Subsession directory does not exist"); fs::remove_all(subsession_path); } + catch (std::system_error const &ex) { + std::cerr << "Logic exception " << ex.what() << std::endl + << "while removing user subsession data [session_uid=" << session_uid + << " subsession_id=" << subsession_id << "]" << std::endl; + throw; + } catch (std::exception const &ex) { std::cerr << "Exception " << ex.what() << std::endl << "while removing user subsession data [session_uid=" << session_uid diff --git a/sessiond/src/globals.hpp b/sessiond/src/globals.hpp index 6f72d07..4ab3e61 100644 --- a/sessiond/src/globals.hpp +++ b/sessiond/src/globals.hpp @@ -35,7 +35,7 @@ constexpr inline std::string_view bus_name = "org.tizen.sessiond"; constexpr inline std::string_view bus_object = "/org/tizen/sessiond"; constexpr inline std::string_view bus_iface = "org.tizen.sessiond.subsession.Manager"; -std::chrono::seconds timeout = 10s; +constexpr std::chrono::seconds timeout = 10s; [[noreturn]] inline void g_error_throw(GError *err, std::string message) { message += err->message; diff --git a/sessiond/src/main.cpp b/sessiond/src/main.cpp index 0c0d76c..07a76ea 100644 --- a/sessiond/src/main.cpp +++ b/sessiond/src/main.cpp @@ -122,14 +122,28 @@ struct sessiond_context { throw std::runtime_error("Bus name lost"); } + bool check_parameters_invalid(GDBusMethodInvocation *invocation, const int session_uid, const int subsession_id) + { + if (session_uid <= 0) { + g_dbus_method_invocation_return_dbus_error(invocation, + sessiond_errors[SUBSESSION_ERROR_INVALID_PARAMETER].second.data(), "Negative UID passed"); + return true; + } + if (subsession_id <= 0) { + g_dbus_method_invocation_return_dbus_error(invocation, + sessiond_errors[SUBSESSION_ERROR_INVALID_PARAMETER].second.data(), "Negative subsession_id passed"); + return true; + } + + return false; + } + void on_add_user(GDBusMethodInvocation *invocation, std::string_view sender, GVariant *parameters) { auto [ session_uid, subsession_id ] = tuple_from_g_variant(parameters); - if (session_uid <= 0) { - g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.InvalidArgs", "Negative UID passed"); + if (check_parameters_invalid(invocation, session_uid, subsession_id)) return; - } GError *err = nullptr; if (!g_dbus_connection_emit_signal(connection, nullptr, bus_object.data(), bus_iface.data(), "AddUserStarted", @@ -148,10 +162,8 @@ struct sessiond_context { { auto [ session_uid, subsession_id ] = tuple_from_g_variant(parameters); - if (session_uid <= 0) { - g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.InvalidArgs", "Negative UID passed"); + if (check_parameters_invalid(invocation, session_uid, subsession_id)) return; - } GError *err = nullptr; if (!g_dbus_connection_emit_signal(connection, nullptr, bus_object.data(), bus_iface.data(), "RemoveUserStarted", @@ -171,12 +183,20 @@ struct sessiond_context { auto [ session_uid, next_subsession_id ] = tuple_from_g_variant(parameters); if (session_uid <= 0) { - g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.InvalidArgs", "Negative UID passed"); + g_dbus_method_invocation_return_dbus_error(invocation, + sessiond_errors[SUBSESSION_ERROR_INVALID_PARAMETER].second.data(), "Negative UID passed"); + return; + } + // N.B. Switch to user '0' is possible and it means no subsession is currently active + if (next_subsession_id < 0) { + g_dbus_method_invocation_return_dbus_error(invocation, + sessiond_errors[SUBSESSION_ERROR_INVALID_PARAMETER].second.data(), "Negative subsession_id passed"); return; } if (!fs_helpers::subsession_exists(session_uid, next_subsession_id)) { - g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.FileNotFound", "Invalid UID or subsession passed"); + g_dbus_method_invocation_return_dbus_error(invocation, + sessiond_errors[SUBSESSION_ERROR_DOES_NOT_EXIST].second.data(), "Subsession does not exist"); return; } @@ -204,7 +224,8 @@ struct sessiond_context { auto [ session_uid ] = tuple_from_g_variant(parameters); if (session_uid <= 0) { - g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.InvalidArgs", "Negative UID passed"); + g_dbus_method_invocation_return_dbus_error(invocation, + sessiond_errors[SUBSESSION_ERROR_INVALID_PARAMETER].second.data(), "Negative UID passed"); return; } @@ -219,7 +240,8 @@ struct sessiond_context { auto [ session_uid ] = tuple_from_g_variant(parameters); if (session_uid <= 0) { - g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.InvalidArgs", "Negative UID passed"); + g_dbus_method_invocation_return_dbus_error(invocation, + sessiond_errors[SUBSESSION_ERROR_INVALID_PARAMETER].second.data(), "Negative UID passed"); return; } @@ -234,7 +256,8 @@ struct sessiond_context { auto [ session_uid ] = tuple_from_g_variant(parameters); if (session_uid <= 0) { - g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.InvalidArgs", "Negative UID passed"); + g_dbus_method_invocation_return_dbus_error(invocation, + sessiond_errors[SUBSESSION_ERROR_INVALID_PARAMETER].second.data(), "Negative UID passed"); return; } @@ -248,10 +271,8 @@ struct sessiond_context { { auto [ session_uid, subsession_id ] = tuple_from_g_variant(parameters); - if (session_uid <= 0) { - g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.InvalidArgs", "Negative UID passed"); + if (check_parameters_invalid(invocation, session_uid, subsession_id)) return; - } wait_add.try_emplace(session_uid, session_uid, connection, "AddUserCompleted"); wait_add.at(session_uid).on_client_done(subsession_id, std::string(sender)); @@ -263,10 +284,8 @@ struct sessiond_context { { auto [ session_uid, subsession_id ] = tuple_from_g_variant(parameters); - if (session_uid <= 0) { - g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.InvalidArgs", "Negative UID passed"); + if (check_parameters_invalid(invocation, session_uid, subsession_id)) return; - } wait_remove.try_emplace(session_uid, session_uid, connection, "RemoveUserCompleted"); wait_remove.at(session_uid).on_client_done(subsession_id, std::string(sender)); @@ -279,7 +298,8 @@ struct sessiond_context { auto [ session_uid, switch_id ] = tuple_from_g_variant(parameters); if (session_uid <= 0) { - g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.InvalidArgs", "Negative UID passed"); + g_dbus_method_invocation_return_dbus_error(invocation, + sessiond_errors[SUBSESSION_ERROR_INVALID_PARAMETER].second.data(), "Negative UID passed"); return; } @@ -294,7 +314,8 @@ struct sessiond_context { auto [ session_uid ] = tuple_from_g_variant(parameters); if (session_uid <= 0) { - g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.InvalidArgs", "Negative UID passed"); + g_dbus_method_invocation_return_dbus_error(invocation, + sessiond_errors[SUBSESSION_ERROR_INVALID_PARAMETER].second.data(), "Negative UID passed"); return; } @@ -314,7 +335,8 @@ struct sessiond_context { auto [ session_uid ] = tuple_from_g_variant(parameters); if (session_uid <= 0) { - g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.InvalidArgs", "Negative UID passed"); + g_dbus_method_invocation_return_dbus_error(invocation, + sessiond_errors[SUBSESSION_ERROR_INVALID_PARAMETER].second.data(), "Negative UID passed"); return; } @@ -349,11 +371,30 @@ struct sessiond_context { std::cout << "Handling " << method_name << " call from " << sender << std::endl; (self->*(to_call->second))(invocation, std::string_view(sender), parameters); } catch (const std::invalid_argument &ex) { - g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.InvalidArgs", ex.what()); + g_dbus_method_invocation_return_dbus_error(invocation, + sessiond_errors[SUBSESSION_ERROR_INVALID_PARAMETER].second.data(), ex.what()); + log_exception(ex, sender, method_name); + } catch (const std::system_error &ex) { + switch (ex.code().value()) { + case EEXIST: + g_dbus_method_invocation_return_dbus_error(invocation, + sessiond_errors[SUBSESSION_ERROR_ALREADY_EXISTS].second.data(), ex.what()); + break; + case ENOENT: + g_dbus_method_invocation_return_dbus_error(invocation, + sessiond_errors[SUBSESSION_ERROR_DOES_NOT_EXIST].second.data(), ex.what()); + break; + default: + g_dbus_method_invocation_return_dbus_error(invocation, + sessiond_errors[SUBSESSION_ERROR_IO_ERROR].second.data(), + (std::string("Unable to complete requested operation: ") + ex.what()).c_str()); + break; + } log_exception(ex, sender, method_name); } catch (const std::runtime_error &ex) { - g_dbus_method_invocation_return_dbus_error(invocation, "org.freedesktop.DBus.Error.Failed", - (std::string("Unable to complete requested operation: ") + std::string(ex.what())).c_str()); + g_dbus_method_invocation_return_dbus_error(invocation, + sessiond_errors[SUBSESSION_ERROR_IO_ERROR].second.data(), + (std::string("Unable to complete requested operation: ") + ex.what()).c_str()); log_exception(ex, sender, method_name); // Swallow the exception; the show must go on } @@ -460,6 +501,20 @@ struct sessiond_context { std::make_pair("GetCurrentUser"sv, &sessiond_context::on_get_current_user), }; + enum { + SUBSESSION_ERROR_INVALID_PARAMETER, + SUBSESSION_ERROR_IO_ERROR, + SUBSESSION_ERROR_ALREADY_EXISTS, + SUBSESSION_ERROR_DOES_NOT_EXIST, + }; + + constexpr static std::array sessiond_errors = { + std::make_pair(SUBSESSION_ERROR_INVALID_PARAMETER, "org.tizen.sessiond.Error.InvalidParameter"sv), + std::make_pair(SUBSESSION_ERROR_IO_ERROR, "org.tizen.sessiond.Error.IOError"sv), + std::make_pair(SUBSESSION_ERROR_ALREADY_EXISTS, "org.tizen.sessiond.Error.SubsessionAlreadyExists"sv), + std::make_pair(SUBSESSION_ERROR_DOES_NOT_EXIST, "org.tizen.sessiond.Error.SubsessionDoesNotExist"sv), + }; + // TODO: Currently, the first parameter is always a single-element tuple. // Consider simplifying wait_manager. // N.B. Although GLib is multi-threaded, the following data structures do not need