Skip to content

Commit

Permalink
FilterUtility: Replace some nested raw pointers by our ::Ptr*
Browse files Browse the repository at this point in the history
  • Loading branch information
yhabteab committed Oct 13, 2022
1 parent 91cbb85 commit 3434f48
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 21 deletions.
36 changes: 20 additions & 16 deletions lib/remote/filterutility.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "base/logger.hpp"
#include "base/utility.hpp"
#include <boost/algorithm/string/case_conv.hpp>
#include <memory>

using namespace icinga;

Expand Down Expand Up @@ -114,11 +115,11 @@ bool FilterUtility::EvaluateFilter(ScriptFrame& frame, Expression *filter,
return Convert::ToBool(filter->Evaluate(frame));
}

static void FilteredAddTarget(ScriptFrame& permissionFrame, Expression *permissionFilter,
ScriptFrame& frame, Expression *ufilter, std::vector<Value>& result, const String& variableName, const Object::Ptr& target)
static void FilteredAddTarget(ScriptFrame& permissionFrame, const Expression::Ptr& permissionFilter,
ScriptFrame& frame, const Expression::Ptr& ufilter, std::vector<Value>& result, const String& variableName, const Object::Ptr& target)
{
if (FilterUtility::EvaluateFilter(permissionFrame, permissionFilter, target, variableName)) {
if (FilterUtility::EvaluateFilter(frame, ufilter, target, variableName)) {
if (FilterUtility::EvaluateFilter(permissionFrame, permissionFilter.get(), target, variableName)) {
if (FilterUtility::EvaluateFilter(frame, ufilter.get(), target, variableName)) {
result.emplace_back(std::move(target));
}
}
Expand All @@ -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, Expression::Ptr* permissionFilter)
{
if (permissionFilter)
*permissionFilter = nullptr;
Expand All @@ -149,6 +150,7 @@ bool FilterUtility::HasPermission(const ApiUser::Ptr& user, const String& permis
bool foundPermission = false;
String requiredPermission = permission.ToLower();

std::unique_ptr<Expression> filtersBuffer;
Array::Ptr permissions = user->GetPermissions();
if (permissions) {
ObjectLock olock(permissions);
Expand All @@ -175,14 +177,16 @@ bool FilterUtility::HasPermission(const ApiUser::Ptr& user, const String& permis
std::unique_ptr<Expression> indexer{new IndexerExpression(std::unique_ptr<Expression>(MakeLiteral(filter)), std::unique_ptr<Expression>(MakeLiteral("call")))};
FunctionCallExpression *fexpr = new FunctionCallExpression(std::move(indexer), std::move(args));

if (!*permissionFilter)
*permissionFilter = fexpr;
if (!filtersBuffer)
filtersBuffer.reset(fexpr);
else
*permissionFilter = new LogicalOrExpression(std::unique_ptr<Expression>(*permissionFilter), std::unique_ptr<Expression>(fexpr));
filtersBuffer = std::make_unique<LogicalOrExpression>(std::move(filtersBuffer), std::unique_ptr<Expression>(fexpr));
}
}
}

*permissionFilter = filtersBuffer.release();

if (!foundPermission) {
Log(LogWarning, "FilterUtility")
<< "Missing permission: " << requiredPermission;
Expand All @@ -191,7 +195,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, Expression::Ptr* permissionFilter)
{
if (!HasPermission(user, permission, permissionFilter)) {
BOOST_THROW_EXCEPTION(ScriptError("Missing permission: " + permission.ToLower()));
Expand All @@ -209,7 +213,7 @@ std::vector<Value> FilterUtility::GetFilterTargets(const QueryDescription& qd, c
else
provider = new ConfigObjectTargetProvider();

Expression *permissionFilter;
Expression::Ptr permissionFilter;
CheckPermission(user, qd.Permission, &permissionFilter);

Namespace::Ptr permissionFrameNS = new Namespace();
Expand All @@ -226,7 +230,7 @@ std::vector<Value> 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));
Expand All @@ -242,7 +246,7 @@ std::vector<Value> 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));
Expand All @@ -269,7 +273,7 @@ std::vector<Value> FilterUtility::GetFilterTargets(const QueryDescription& qd, c

if (query->Contains("filter")) {
String filter = HttpUtility::GetLastParameter(query, "filter");
std::unique_ptr<Expression> ufilter = ConfigCompiler::CompileText("<API query>", filter);
Expression::Ptr ufilter = ConfigCompiler::CompileText("<API query>", filter).release();

Dictionary::Ptr filter_vars = query->Get("filter_vars");
if (filter_vars) {
Expand All @@ -279,14 +283,14 @@ std::vector<Value> 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, 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) {
provider->FindTargets(type, [&permissionFrame, &permissionFilter, &frame, &result, variableName](const Object::Ptr& target) {
FilteredAddTarget(permissionFrame, permissionFilter, frame, nullptr, result, variableName, target);
});
}
Expand Down
4 changes: 2 additions & 2 deletions lib/remote/filterutility.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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, Expression::Ptr* filter = nullptr);
static bool HasPermission(const ApiUser::Ptr& user, const String& permission, Expression::Ptr* permissionFilter = nullptr);
static std::vector<Value> GetFilterTargets(const QueryDescription& qd, const Dictionary::Ptr& query,
const ApiUser::Ptr& user, const String& variableName = String());
static bool EvaluateFilter(ScriptFrame& frame, Expression *filter,
Expand Down
4 changes: 1 addition & 3 deletions lib/remote/objectqueryhandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,7 @@ bool ObjectQueryHandler::HandleRequest(
if (it == typePermissions.end()) {
String permission = "objects/query/" + reflectionType->GetName();

Expression *filter = nullptr;
granted = FilterUtility::HasPermission(user, permission, &filter);
permissionFilter = filter;
granted = FilterUtility::HasPermission(user, permission, &permissionFilter);

typePermissions.insert({reflectionType.get(), std::make_pair(granted, permissionFilter)});
} else {
Expand Down

0 comments on commit 3434f48

Please sign in to comment.