From f059e6b3c8ea36bf2e3b5a1365c881f9246ca2cd Mon Sep 17 00:00:00 2001 From: WolverinDEV Date: Wed, 26 May 2021 11:49:07 +0200 Subject: [PATCH] Fixed a query rename bug --- server/src/client/SpeakingClient.cpp | 67 -------------------- server/src/client/SpeakingClient.h | 1 - server/src/client/command_handler/groups.cpp | 45 +++++++++---- server/src/groups/GroupManager.cpp | 40 ++++++++---- shared | 2 +- 5 files changed, 61 insertions(+), 94 deletions(-) diff --git a/server/src/client/SpeakingClient.cpp b/server/src/client/SpeakingClient.cpp index bde353b..f0daa99 100644 --- a/server/src/client/SpeakingClient.cpp +++ b/server/src/client/SpeakingClient.cpp @@ -786,9 +786,6 @@ command_result SpeakingClient::handleCommand(Command &command) { return this->handleCommandRtcSessionDescribe(command); } else if(command.command() == "rtcicecandidate") { return this->handleCommandRtcIceCandidate(command); - } else if(command.command() == "rtcbroadcast") { - /* TODO: Remove this command once the first 1.5.0 stable is out */ - return this->handleCommandRtcBroadcast(command); } else if(command.command() == "rtcsessionreset") { return this->handleCommandRtcSessionReset(command); } else if(command.command() == "broadcastaudio") { @@ -994,61 +991,6 @@ command_result SpeakingClient::handleCommandBroadcastVideo(Command &command) { return broadcast_start_result_to_command_result(broadcast_result); } -command_result SpeakingClient::handleCommandRtcBroadcast(Command &command) { - CMD_REQ_SERVER; - CMD_CHK_AND_INC_FLOOD_POINTS(15); - - std::vector> broadcasts{}; - broadcasts.reserve(command.bulkCount()); - - ts::command_result_bulk result{}; - for(size_t index{0}; index < command.bulkCount(); index++) { - auto& bulk = command[index]; - - auto ssrc = bulk.has("ssrc") ? bulk["ssrc"].as() : (uint32_t) 0; - auto type = bulk["type"].as(); - - for(const auto& entry : broadcasts) { - if(std::get<0>(entry) == type) { - return ts::command_result{error::parameter_constraint_violation}; - } - } - - broadcasts.push_back(std::make_tuple(type, ssrc)); - switch(type) { - case 1: { - ts::Command cmd{""}; - cmd["ssrc"] = ssrc; - result.insert_result(this->handleCommandBroadcastAudio(cmd)); - break; - } - case 2: { - ts::Command cmd{""}; - cmd["broadcast_bitrate_max"] = 1500000; - cmd["broadcast_keyframe_interval"] = 0; - cmd["ssrc"] = ssrc; - cmd["type"] = (uint8_t) rtc::VideoBroadcastType::Camera; - result.insert_result(this->handleCommandBroadcastVideo(cmd)); - break; - } - case 3: { - ts::Command cmd{""}; - cmd["broadcast_bitrate_max"] = 1500000; - cmd["broadcast_keyframe_interval"] = 0; - cmd["ssrc"] = ssrc; - cmd["type"] = (uint8_t) rtc::VideoBroadcastType::Screen; - result.insert_result(this->handleCommandBroadcastVideo(cmd)); - break; - } - default: - result.emplace_result(error::parameter_invalid, "type"); - break; - } - } - - return ts::command_result{std::move(result)}; -} - command_result SpeakingClient::handleCommandBroadcastVideoJoin(Command &cmd) { CMD_REQ_SERVER; @@ -1214,15 +1156,6 @@ command_result SpeakingClient::handleCommandBroadcastVideoConfigure(Command &cmd result.release_data(); } - { - auto result = test_broadcast_options(*this, this->getChannelId(), options); - if(result.has_error()) { - return result; - } - - result.release_data(); - } - auto result = this->server->rtc_server().client_broadcast_video_configure(this->rtc_client_id, broadcast_type, &options); return broadcast_config_result_to_command_result(result); } \ No newline at end of file diff --git a/server/src/client/SpeakingClient.h b/server/src/client/SpeakingClient.h index 08cf77c..446bb87 100644 --- a/server/src/client/SpeakingClient.h +++ b/server/src/client/SpeakingClient.h @@ -70,7 +70,6 @@ namespace ts::server { virtual command_result handleCommandRtcSessionDescribe(Command &command); virtual command_result handleCommandRtcSessionReset(Command &command); virtual command_result handleCommandRtcIceCandidate(Command &); - virtual command_result handleCommandRtcBroadcast(Command &); /* Old method used 'till 1.5.0b4 */ virtual command_result handleCommandBroadcastAudio(Command &); virtual command_result handleCommandBroadcastVideo(Command &); virtual command_result handleCommandBroadcastVideoJoin(Command &); diff --git a/server/src/client/command_handler/groups.cpp b/server/src/client/command_handler/groups.cpp index d1e6031..4e72126 100644 --- a/server/src/client/command_handler/groups.cpp +++ b/server/src/client/command_handler/groups.cpp @@ -70,8 +70,9 @@ command_result ConnectedClient::handleCommandGroupAdd(Command &cmd, GroupTarget case groups::GroupType::GROUP_TYPE_NORMAL: if (!this->server) { - return command_result{error::parameter_invalid, "you cant create normal groups on the template server!"}; + return ts::command_result{error::parameter_invalid, "you cant create normal groups on the template server!"}; } + log_group_type = log::GroupType::NORMAL; group_manager = this->server->group_manager(); break; @@ -260,6 +261,7 @@ command_result ConnectedClient::handleCommandGroupCopy(Command &cmd, GroupTarget auto global_update = false; if (target_group) { + /* Copy permissions into an already existing group */ if(!target_group->permission_granted(permission::i_channel_group_needed_modify_power, this->calculate_permission(permission::i_channel_group_modify_power, 0), true)) { return ts::command_result{permission::i_channel_group_needed_modify_power}; } @@ -328,7 +330,7 @@ command_result ConnectedClient::handleCommandGroupCopy(Command &cmd, GroupTarget global_update = !this->server || target_group_global; } else { - //Copy to a new group + /* Copy permissions into a new group */ auto target_type = cmd["type"].as(); log::GroupType log_group_type; @@ -495,12 +497,12 @@ command_result ConnectedClient::handleCommandGroupRename(Command &cmd, GroupTarg } assert(group); - auto type = group->group_type(); + auto group_type = group->group_type(); log::GroupType log_group_type; - if (type == groups::GroupType::GROUP_TYPE_QUERY) { + if (group_type == groups::GroupType::GROUP_TYPE_QUERY) { ACTION_REQUIRES_GLOBAL_PERMISSION(permission::b_serverinstance_modify_querygroup, 1); log_group_type = log::GroupType::QUERY; - } else if (type == groups::GroupType::GROUP_TYPE_TEMPLATE) { + } else if (group_type == groups::GroupType::GROUP_TYPE_TEMPLATE) { ACTION_REQUIRES_GLOBAL_PERMISSION(permission::b_serverinstance_modify_templates, 1); log_group_type = log::GroupType::TEMPLATE; } else { @@ -542,14 +544,31 @@ command_result ConnectedClient::handleCommandGroupRename(Command &cmd, GroupTarg return ts::command_result{error::vs_critical}; } - serverInstance->action_logger()->group_logger.log_group_rename(this->getServerId(), this->ref(), log_group_target, log_group_type, group->group_id(), group->display_name(), old_name); + ServerId log_server_id; std::deque> server_updates{}; - if(!this->server) { - server_updates = serverInstance->getVoiceServerManager()->serverInstances(); - } else { - server_updates.push_back(this->server); + switch(group_type) { + case groups::GroupType::GROUP_TYPE_QUERY: + case groups::GroupType::GROUP_TYPE_TEMPLATE: + log_server_id = 0; + server_updates = serverInstance->getVoiceServerManager()->serverInstances(); + break; + + case groups::GroupType::GROUP_TYPE_NORMAL: + /* Normal groups can only exist on regular servers */ + assert(this->server); + server_updates.push_back(this->server); + log_server_id = this->getServerId(); + break; + + case groups::GroupType::GROUP_TYPE_UNKNOWN: + default: + log_server_id = 0; + assert(false); + break; } + serverInstance->action_logger()->group_logger.log_group_rename(log_server_id, this->ref(), log_group_target, log_group_type, group->group_id(), group->display_name(), old_name); + for(const auto& server : server_updates) { if(!server) { continue; @@ -658,15 +677,18 @@ command_result ConnectedClient::handleCommandGroupDel(Command &cmd, GroupTarget bool global_update; log::GroupType log_group_type; + ServerId log_server_id; switch (group->group_type()) { case groups::GroupType::GROUP_TYPE_QUERY: ACTION_REQUIRES_GLOBAL_PERMISSION(permission::b_serverinstance_modify_querygroup, 1); + log_server_id = 0; log_group_type = log::GroupType::QUERY; global_update = true; break; case groups::GroupType::GROUP_TYPE_TEMPLATE: ACTION_REQUIRES_GLOBAL_PERMISSION(permission::b_serverinstance_modify_templates, 1); + log_server_id = 0; log_group_type = log::GroupType::TEMPLATE; global_update = true; break; @@ -682,6 +704,7 @@ command_result ConnectedClient::handleCommandGroupDel(Command &cmd, GroupTarget break; } + log_server_id = this->getServerId(); log_group_type = log::GroupType::NORMAL; global_update = false; break; @@ -731,7 +754,7 @@ command_result ConnectedClient::handleCommandGroupDel(Command &cmd, GroupTarget return ts::command_result{error::vs_critical}; } - serverInstance->action_logger()->group_logger.log_group_delete(this->getServerId(), this->ref(), log_group_target, log_group_type, group->group_id(), group->display_name()); + serverInstance->action_logger()->group_logger.log_group_delete(log_server_id, this->ref(), log_group_target, log_group_type, group->group_id(), group->display_name()); std::deque> server_updates{}; if(global_update) { diff --git a/server/src/groups/GroupManager.cpp b/server/src/groups/GroupManager.cpp index 0266524..9dd9942 100644 --- a/server/src/groups/GroupManager.cpp +++ b/server/src/groups/GroupManager.cpp @@ -83,6 +83,8 @@ bool AbstractGroupManager::initialize(std::string &) { } GroupLoadResult AbstractGroupManager::load_data(bool initialize) { + (void) initialize; + std::lock_guard manage_lock{this->group_manage_mutex_}; { std::lock_guard list_lock{this->group_mutex_}; @@ -330,27 +332,29 @@ GroupCopyResult AbstractGroupManager::copy_group_(GroupId source, GroupType targ return this->copy_group_permissions_(source, result->group_id()); } +/* FIXME: This implementation is flawed since it may operates on parent groups without locking them properly. */ GroupCopyResult AbstractGroupManager::copy_group_permissions_(GroupId source, GroupId target) { std::lock_guard manage_lock{this->group_manage_mutex_}; - std::shared_ptr owning_manager{}; - auto source_group = this->find_group_(owning_manager, groups::GroupCalculateMode::GLOBAL, source); + std::shared_ptr source_owning_manager{}; + auto source_group = this->find_group_(source_owning_manager, groups::GroupCalculateMode::GLOBAL, source); if(!source_group) { return GroupCopyResult::UNKNOWN_SOURCE_GROUP; } - auto target_group = this->find_group_(owning_manager, groups::GroupCalculateMode::GLOBAL, target); + std::shared_ptr target_owning_manager{}; + auto target_group = this->find_group_(target_owning_manager, groups::GroupCalculateMode::GLOBAL, target); if(!target_group) { return GroupCopyResult::UNKNOWN_TARGET_GROUP; } - if(target_group->permissions()->require_db_updates()) { + if(source_group->permissions()->require_db_updates()) { /* TODO: Somehow flush all pending changes */ } { auto res = sql::command(this->sql_manager(), "DELETE FROM `permissions` WHERE `serverId` = :sid AND `type` = :type AND `id` = :id", - variable{":sid", this->server_id()}, + variable{":sid", source_owning_manager->server_id()}, variable{":type", ts::permission::SQL_PERM_GROUP}, variable{":id", target}).execute(); if(!res) { @@ -363,8 +367,8 @@ GroupCopyResult AbstractGroupManager::copy_group_permissions_(GroupId source, Gr constexpr static auto kSqlCommand = "INSERT INTO `permissions` (`serverId`, `type`, `id`, `channelId`, `permId`, `value`, `grant`, `flag_skip`, `flag_negate`) " "SELECT :tsid AS `serverId`, `type`, :target AS `id`, 0 AS `channelId`, `permId`, `value`,`grant`,`flag_skip`, `flag_negate` FROM `permissions` WHERE `serverId` = :ssid AND `type` = :type AND `id` = :source"; auto res = sql::command(this->sql_manager(), kSqlCommand, - variable{":ssid", source_group->virtual_server_id()}, - variable{":tsid", target_group->virtual_server_id()}, + variable{":ssid", source_owning_manager->server_id()}, + variable{":tsid", target_owning_manager->server_id()}, variable{":type", ts::permission::SQL_PERM_GROUP}, variable{":source", source}, variable{":target", target}).execute(); @@ -375,23 +379,30 @@ GroupCopyResult AbstractGroupManager::copy_group_permissions_(GroupId source, Gr } } - target_group->set_permissions(serverInstance->databaseHelper()->loadGroupPermissions(this->server_id(), target, (uint8_t) this->database_target_)); + target_group->set_permissions(serverInstance->databaseHelper()->loadGroupPermissions(target_owning_manager->server_id(), target, (uint8_t) this->database_target_)); return GroupCopyResult::SUCCESS; } GroupRenameResult AbstractGroupManager::rename_group_(GroupId group_id, const std::string &name) { std::lock_guard manage_lock{this->group_manage_mutex_}; + + std::shared_ptr owning_manager{}; + auto group = this->find_group_(owning_manager, groups::GroupCalculateMode::LOCAL, group_id); + if(!group) { + /* Group hasn't been found locally. Trying our parent manager. */ + if(this->parent_manager_) { + return this->parent_manager_->rename_group_(group_id, name); + } + + return GroupRenameResult::INVALID_GROUP_ID; + } + + assert(&*owning_manager == this); auto name_length = utf8::count_characters(name); if(name_length <= 0 || name_length > kGroupMaxNameLength) { return GroupRenameResult::NAME_INVALID; } - std::shared_ptr owning_manager{}; - auto group = this->find_group_(owning_manager, groups::GroupCalculateMode::GLOBAL, group_id); - if(!group) { - return GroupRenameResult::INVALID_GROUP_ID; - } - if(this->find_group_by_name_(GroupCalculateMode::LOCAL, name)) { return GroupRenameResult::NAME_ALREADY_USED; } @@ -399,6 +410,7 @@ GroupRenameResult AbstractGroupManager::rename_group_(GroupId group_id, const st sql::command(this->sql_manager(), "UPDATE `groups` SET `displayName` = :name WHERE `serverId` = :server AND `groupId` = :group_id AND `target` = :target", variable{":server", this->server_id()}, variable{":target", (uint8_t) this->database_target_}, + variable{":name", name}, variable{":group_id", group_id}).executeLater().waitAndGetLater(LOG_SQL_CMD, {-1, "future failed"}); group->name_ = name; return GroupRenameResult::SUCCESS; diff --git a/shared b/shared index b155bf8..9f03f33 160000 --- a/shared +++ b/shared @@ -1 +1 @@ -Subproject commit b155bf8d5f587b3b8970ef8ff81e0deb3b5bdf10 +Subproject commit 9f03f33a23de2a69b2f464311838f5174f5d78ac