Merge pull request #9537 from Icinga/replace-some-raw-pointer-with-intrusive-ptr

FilterUtility: Replace some nested raw pointers by `unique_ptr<>*`
This commit is contained in:
Alexander Aleksandrovič Klimov 2022-12-06 13:07:24 +01:00 committed by GitHub
commit ca328627cd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 21 additions and 22 deletions

View File

@ -10,6 +10,7 @@
#include "base/logger.hpp" #include "base/logger.hpp"
#include "base/utility.hpp" #include "base/utility.hpp"
#include <boost/algorithm/string/case_conv.hpp> #include <boost/algorithm/string/case_conv.hpp>
#include <memory>
using namespace icinga; using namespace icinga;
@ -138,7 +139,7 @@ static void FilteredAddTarget(ScriptFrame& permissionFrame, Expression *permissi
* *
* @return bool * @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<Expression>* permissionFilter)
{ {
if (permissionFilter) if (permissionFilter)
*permissionFilter = nullptr; *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)); FunctionCallExpression *fexpr = new FunctionCallExpression(std::move(indexer), std::move(args));
if (!*permissionFilter) if (!*permissionFilter)
*permissionFilter = fexpr; permissionFilter->reset(fexpr);
else else
*permissionFilter = new LogicalOrExpression(std::unique_ptr<Expression>(*permissionFilter), std::unique_ptr<Expression>(fexpr)); *permissionFilter = std::make_unique<LogicalOrExpression>(std::move(*permissionFilter), std::unique_ptr<Expression>(fexpr));
} }
} }
} }
@ -191,7 +192,7 @@ bool FilterUtility::HasPermission(const ApiUser::Ptr& user, const String& permis
return foundPermission; 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<Expression>* permissionFilter)
{ {
if (!HasPermission(user, permission, permissionFilter)) { if (!HasPermission(user, permission, permissionFilter)) {
BOOST_THROW_EXCEPTION(ScriptError("Missing permission: " + permission.ToLower())); BOOST_THROW_EXCEPTION(ScriptError("Missing permission: " + permission.ToLower()));
@ -209,7 +210,7 @@ std::vector<Value> FilterUtility::GetFilterTargets(const QueryDescription& qd, c
else else
provider = new ConfigObjectTargetProvider(); provider = new ConfigObjectTargetProvider();
Expression *permissionFilter; std::unique_ptr<Expression> permissionFilter;
CheckPermission(user, qd.Permission, &permissionFilter); CheckPermission(user, qd.Permission, &permissionFilter);
Namespace::Ptr permissionFrameNS = new Namespace(); Namespace::Ptr permissionFrameNS = new Namespace();
@ -226,7 +227,7 @@ std::vector<Value> FilterUtility::GetFilterTargets(const QueryDescription& qd, c
String name = HttpUtility::GetLastParameter(query, attr); String name = HttpUtility::GetLastParameter(query, attr);
Object::Ptr target = provider->GetTargetByName(type, name); 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 + "'")); BOOST_THROW_EXCEPTION(ScriptError("Access denied to object '" + name + "' of type '" + type + "'"));
result.emplace_back(std::move(target)); result.emplace_back(std::move(target));
@ -242,7 +243,7 @@ std::vector<Value> FilterUtility::GetFilterTargets(const QueryDescription& qd, c
for (const String& name : names) { for (const String& name : names) {
Object::Ptr target = provider->GetTargetByName(type, name); 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 + "'")); BOOST_THROW_EXCEPTION(ScriptError("Access denied to object '" + name + "' of type '" + type + "'"));
result.emplace_back(std::move(target)); result.emplace_back(std::move(target));
@ -279,15 +280,15 @@ std::vector<Value> FilterUtility::GetFilterTargets(const QueryDescription& qd, c
} }
} }
provider->FindTargets(type, [&permissionFrame, permissionFilter, &frame, &ufilter, &result, variableName](const Object::Ptr& target) { provider->FindTargets(type, [&permissionFrame, &permissionFilter, &frame, &ufilter, &result, variableName](const Object::Ptr& target) {
FilteredAddTarget(permissionFrame, permissionFilter, frame, &*ufilter, result, variableName, target); FilteredAddTarget(permissionFrame, permissionFilter.get(), frame, &*ufilter, result, variableName, target);
}); });
} else { } else {
/* Ensure to pass a nullptr as filter expression. /* Ensure to pass a nullptr as filter expression.
* GCC 8.1.1 on F28 causes problems, see GH #6533. * GCC 8.1.1 on F28 causes problems, see GH #6533.
*/ */
provider->FindTargets(type, [&permissionFrame, permissionFilter, &frame, &result, variableName](const Object::Ptr& target) { provider->FindTargets(type, [&permissionFrame, &permissionFilter, &frame, &result, variableName](const Object::Ptr& target) {
FilteredAddTarget(permissionFrame, permissionFilter, frame, nullptr, result, variableName, target); FilteredAddTarget(permissionFrame, permissionFilter.get(), frame, nullptr, result, variableName, target);
}); });
} }
} }

View File

@ -51,8 +51,8 @@ class FilterUtility
{ {
public: public:
static Type::Ptr TypeFromPluralName(const String& pluralName); static Type::Ptr TypeFromPluralName(const String& pluralName);
static void CheckPermission(const ApiUser::Ptr& user, const String& permission, Expression **filter = nullptr); static void CheckPermission(const ApiUser::Ptr& user, const String& permission, std::unique_ptr<Expression>* filter = nullptr);
static bool HasPermission(const ApiUser::Ptr& user, const String& permission, Expression **permissionFilter = nullptr); static bool HasPermission(const ApiUser::Ptr& user, const String& permission, std::unique_ptr<Expression>* permissionFilter = nullptr);
static std::vector<Value> GetFilterTargets(const QueryDescription& qd, const Dictionary::Ptr& query, static std::vector<Value> GetFilterTargets(const QueryDescription& qd, const Dictionary::Ptr& query,
const ApiUser::Ptr& user, const String& variableName = String()); const ApiUser::Ptr& user, const String& variableName = String());
static bool EvaluateFilter(ScriptFrame& frame, Expression *filter, static bool EvaluateFilter(ScriptFrame& frame, Expression *filter,

View File

@ -190,7 +190,7 @@ bool ObjectQueryHandler::HandleRequest(
joinAttrs.insert(field.Name); joinAttrs.insert(field.Name);
} }
std::unordered_map<Type*, std::pair<bool, Expression::Ptr>> typePermissions; std::unordered_map<Type*, std::pair<bool, std::unique_ptr<Expression>>> typePermissions;
std::unordered_map<Object*, bool> objectAccessAllowed; std::unordered_map<Object*, bool> objectAccessAllowed;
for (const ConfigObject::Ptr& obj : objs) { for (const ConfigObject::Ptr& obj : objs) {
@ -262,23 +262,21 @@ bool ObjectQueryHandler::HandleRequest(
continue; continue;
Type::Ptr reflectionType = joinedObj->GetReflectionType(); Type::Ptr reflectionType = joinedObj->GetReflectionType();
Expression::Ptr permissionFilter;
auto it = typePermissions.find(reflectionType.get()); auto it = typePermissions.find(reflectionType.get());
bool granted; bool granted;
if (it == typePermissions.end()) { if (it == typePermissions.end()) {
String permission = "objects/query/" + reflectionType->GetName(); String permission = "objects/query/" + reflectionType->GetName();
Expression *filter = nullptr; std::unique_ptr<Expression> permissionFilter;
granted = FilterUtility::HasPermission(user, permission, &filter); granted = FilterUtility::HasPermission(user, permission, &permissionFilter);
permissionFilter = filter;
typePermissions.insert({reflectionType.get(), std::make_pair(granted, permissionFilter)}); it = typePermissions.insert({reflectionType.get(), std::make_pair(granted, std::move(permissionFilter))}).first;
} else {
std::tie(granted, permissionFilter) = it->second;
} }
granted = it->second.first;
const std::unique_ptr<Expression>& permissionFilter = it->second.second;
if (!granted) { if (!granted) {
// Not authorized // Not authorized
continue; continue;