From c1f73fbc1d75166426ccc6616a232416317a27e6 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Wed, 12 Oct 2022 16:08:52 +0200 Subject: [PATCH] FilterUtility: Replace some nested raw pointers by our `unique_ptr*` --- lib/remote/filterutility.cpp | 23 ++++++++++++----------- lib/remote/filterutility.hpp | 4 ++-- lib/remote/objectqueryhandler.cpp | 16 +++++++--------- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/lib/remote/filterutility.cpp b/lib/remote/filterutility.cpp index 9bcc60f71..57564e954 100644 --- a/lib/remote/filterutility.cpp +++ b/lib/remote/filterutility.cpp @@ -10,6 +10,7 @@ #include "base/logger.hpp" #include "base/utility.hpp" #include +#include using namespace icinga; @@ -138,7 +139,7 @@ static void FilteredAddTarget(ScriptFrame& permissionFrame, Expression *permissi * * @return bool */ -bool FilterUtility::HasPermission(const ApiUser::Ptr& user, const String& permission, Expression **permissionFilter) +bool FilterUtility::HasPermission(const ApiUser::Ptr& user, const String& permission, std::unique_ptr* permissionFilter) { if (permissionFilter) *permissionFilter = nullptr; @@ -176,9 +177,9 @@ bool FilterUtility::HasPermission(const ApiUser::Ptr& user, const String& permis FunctionCallExpression *fexpr = new FunctionCallExpression(std::move(indexer), std::move(args)); if (!*permissionFilter) - *permissionFilter = fexpr; + permissionFilter->reset(fexpr); else - *permissionFilter = new LogicalOrExpression(std::unique_ptr(*permissionFilter), std::unique_ptr(fexpr)); + *permissionFilter = std::make_unique(std::move(*permissionFilter), std::unique_ptr(fexpr)); } } } @@ -191,7 +192,7 @@ bool FilterUtility::HasPermission(const ApiUser::Ptr& user, const String& permis return foundPermission; } -void FilterUtility::CheckPermission(const ApiUser::Ptr& user, const String& permission, Expression **permissionFilter) +void FilterUtility::CheckPermission(const ApiUser::Ptr& user, const String& permission, std::unique_ptr* permissionFilter) { if (!HasPermission(user, permission, permissionFilter)) { BOOST_THROW_EXCEPTION(ScriptError("Missing permission: " + permission.ToLower())); @@ -209,7 +210,7 @@ std::vector FilterUtility::GetFilterTargets(const QueryDescription& qd, c else provider = new ConfigObjectTargetProvider(); - Expression *permissionFilter; + std::unique_ptr permissionFilter; CheckPermission(user, qd.Permission, &permissionFilter); Namespace::Ptr permissionFrameNS = new Namespace(); @@ -226,7 +227,7 @@ std::vector FilterUtility::GetFilterTargets(const QueryDescription& qd, c String name = HttpUtility::GetLastParameter(query, attr); Object::Ptr target = provider->GetTargetByName(type, name); - if (!FilterUtility::EvaluateFilter(permissionFrame, permissionFilter, target, variableName)) + if (!FilterUtility::EvaluateFilter(permissionFrame, permissionFilter.get(), target, variableName)) BOOST_THROW_EXCEPTION(ScriptError("Access denied to object '" + name + "' of type '" + type + "'")); result.emplace_back(std::move(target)); @@ -242,7 +243,7 @@ std::vector FilterUtility::GetFilterTargets(const QueryDescription& qd, c for (const String& name : names) { Object::Ptr target = provider->GetTargetByName(type, name); - if (!FilterUtility::EvaluateFilter(permissionFrame, permissionFilter, target, variableName)) + if (!FilterUtility::EvaluateFilter(permissionFrame, permissionFilter.get(), target, variableName)) BOOST_THROW_EXCEPTION(ScriptError("Access denied to object '" + name + "' of type '" + type + "'")); result.emplace_back(std::move(target)); @@ -279,15 +280,15 @@ std::vector FilterUtility::GetFilterTargets(const QueryDescription& qd, c } } - provider->FindTargets(type, [&permissionFrame, permissionFilter, &frame, &ufilter, &result, variableName](const Object::Ptr& target) { - FilteredAddTarget(permissionFrame, permissionFilter, frame, &*ufilter, result, variableName, target); + provider->FindTargets(type, [&permissionFrame, &permissionFilter, &frame, &ufilter, &result, variableName](const Object::Ptr& target) { + FilteredAddTarget(permissionFrame, permissionFilter.get(), frame, &*ufilter, result, variableName, target); }); } else { /* Ensure to pass a nullptr as filter expression. * GCC 8.1.1 on F28 causes problems, see GH #6533. */ - provider->FindTargets(type, [&permissionFrame, permissionFilter, &frame, &result, variableName](const Object::Ptr& target) { - FilteredAddTarget(permissionFrame, permissionFilter, frame, nullptr, result, variableName, target); + provider->FindTargets(type, [&permissionFrame, &permissionFilter, &frame, &result, variableName](const Object::Ptr& target) { + FilteredAddTarget(permissionFrame, permissionFilter.get(), frame, nullptr, result, variableName, target); }); } } diff --git a/lib/remote/filterutility.hpp b/lib/remote/filterutility.hpp index 1cebffc0b..7271367b4 100644 --- a/lib/remote/filterutility.hpp +++ b/lib/remote/filterutility.hpp @@ -51,8 +51,8 @@ class FilterUtility { public: static Type::Ptr TypeFromPluralName(const String& pluralName); - static void CheckPermission(const ApiUser::Ptr& user, const String& permission, Expression **filter = nullptr); - static bool HasPermission(const ApiUser::Ptr& user, const String& permission, Expression **permissionFilter = nullptr); + static void CheckPermission(const ApiUser::Ptr& user, const String& permission, std::unique_ptr* filter = nullptr); + static bool HasPermission(const ApiUser::Ptr& user, const String& permission, std::unique_ptr* permissionFilter = nullptr); static std::vector GetFilterTargets(const QueryDescription& qd, const Dictionary::Ptr& query, const ApiUser::Ptr& user, const String& variableName = String()); static bool EvaluateFilter(ScriptFrame& frame, Expression *filter, diff --git a/lib/remote/objectqueryhandler.cpp b/lib/remote/objectqueryhandler.cpp index f059c0328..ad73030da 100644 --- a/lib/remote/objectqueryhandler.cpp +++ b/lib/remote/objectqueryhandler.cpp @@ -190,7 +190,7 @@ bool ObjectQueryHandler::HandleRequest( joinAttrs.insert(field.Name); } - std::unordered_map> typePermissions; + std::unordered_map>> typePermissions; std::unordered_map objectAccessAllowed; for (const ConfigObject::Ptr& obj : objs) { @@ -262,23 +262,21 @@ bool ObjectQueryHandler::HandleRequest( continue; Type::Ptr reflectionType = joinedObj->GetReflectionType(); - Expression::Ptr permissionFilter; - auto it = typePermissions.find(reflectionType.get()); bool granted; if (it == typePermissions.end()) { String permission = "objects/query/" + reflectionType->GetName(); - Expression *filter = nullptr; - granted = FilterUtility::HasPermission(user, permission, &filter); - permissionFilter = filter; + std::unique_ptr permissionFilter; + granted = FilterUtility::HasPermission(user, permission, &permissionFilter); - typePermissions.insert({reflectionType.get(), std::make_pair(granted, permissionFilter)}); - } else { - std::tie(granted, permissionFilter) = it->second; + it = typePermissions.insert({reflectionType.get(), std::make_pair(granted, std::move(permissionFilter))}).first; } + granted = it->second.first; + const std::unique_ptr& permissionFilter = it->second.second; + if (!granted) { // Not authorized continue;