tyler.durden/icinga2
1
0
Fork 0
mirror of https://github.com/Icinga/icinga2.git synced 2025-07-20 12:14:44 +02:00
Code Issues Packages Projects Releases Wiki Activity

No longer allow overriding the frozen attribute of containers

Browse Source 
The Array, Dictionary, and Namespace types provide a Freeze() method that makes
them read-only. So far, there was the possibility to call some methods with
`overrideFrozen=true` which would then bypass the corresponding check and allow
modification of the data structures nonetheless.

With 24b57f0d3a222835178e88489eabd595755ed883, this possibility was already
removed from the Namespace type. However, for interface compatibility, it kept
the parameter and just ignores it, throwing an exception on any modification on
a frozen instance.

The only place using `overrideFrozen` was processing of the `-D`/`--define`
command line flag that allows setting additional variables in the DSL. At the
time it is evaluated, there are no user-created data structures yet that could
be frozen, so the only frozen objects that could be encountered are Namespaces
(Icinga doesn't freeze other types by itself) and for these, `overrideFrozen`
already has no effect.

Hence, there is no harm in removing `overrideFrozen` altogether. This
simplifies the code and also means that frozen objects are now indeed read-only
without exceptions, allowing further optimizations regarding locking in the
future.
This commit is contained in:
Julian Brost 2025-07-08 13:52:40 +02:00
parent a13751d972
commit 0ebcd2662d
15 changed files with 55 additions and 86 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
icinga-app/icinga.cpp
View File

@ -420,12 +420,10 @@ static int Main()
for (size_t i = 1; i < keyTokens.size(); i++) {
std::unique_ptr<IndexerExpression> indexerExpr{new IndexerExpression(std::move(expr), MakeLiteral(keyTokens[i]))};
indexerExpr->SetOverrideFrozen();
expr = std::move(indexerExpr);
}
std::unique_ptr<SetExpression> setExpr{new SetExpression(std::move(expr), OpSetLiteral, MakeLiteral(value))};
setExpr->SetOverrideFrozen();
try {
ScriptFrame frame(true);

52
lib/base/array.cpp
View File

@ -45,13 +45,12 @@ Value Array::Get(SizeType index) const
*
* @param index The index.
* @param value The value.
* @param overrideFrozen Whether to allow modifying frozen arrays.
*/
void Array::Set(SizeType index, const Value& value, bool overrideFrozen)
void Array::Set(SizeType index, const Value& value)
{
ObjectLock olock(this);
if (m_Frozen && !overrideFrozen)
if (m_Frozen)
BOOST_THROW_EXCEPTION(std::invalid_argument("Value in array must not be modified."));
m_Data.at(index) = value;
@ -62,13 +61,12 @@ void Array::Set(SizeType index, const Value& value, bool overrideFrozen)
*
* @param index The index.
* @param value The value.
* @param overrideFrozen Whether to allow modifying frozen arrays.
*/
void Array::Set(SizeType index, Value&& value, bool overrideFrozen)
void Array::Set(SizeType index, Value&& value)
{
ObjectLock olock(this);
if (m_Frozen && !overrideFrozen)
if (m_Frozen)
BOOST_THROW_EXCEPTION(std::invalid_argument("Array must not be modified."));
m_Data.at(index).Swap(value);
@ -78,13 +76,12 @@ void Array::Set(SizeType index, Value&& value, bool overrideFrozen)
* Adds a value to the array.
*
* @param value The value.
* @param overrideFrozen Whether to allow modifying frozen arrays.
*/
void Array::Add(Value value, bool overrideFrozen)
void Array::Add(Value value)
{
ObjectLock olock(this);
if (m_Frozen && !overrideFrozen)
if (m_Frozen)
BOOST_THROW_EXCEPTION(std::invalid_argument("Array must not be modified."));
m_Data.push_back(std::move(value));
@ -148,15 +145,14 @@ bool Array::Contains(const Value& value) const
*
* @param index The index
* @param value The value to add
* @param overrideFrozen Whether to allow modifying frozen arrays.
*/
void Array::Insert(SizeType index, Value value, bool overrideFrozen)
void Array::Insert(SizeType index, Value value)
{
ObjectLock olock(this);
ASSERT(index <= m_Data.size());
if (m_Frozen && !overrideFrozen)
if (m_Frozen)
BOOST_THROW_EXCEPTION(std::invalid_argument("Array must not be modified."));
m_Data.insert(m_Data.begin() + index, std::move(value));
@ -166,13 +162,12 @@ void Array::Insert(SizeType index, Value value, bool overrideFrozen)
* Removes the specified index from the array.
*
* @param index The index.
* @param overrideFrozen Whether to allow modifying frozen arrays.
*/
void Array::Remove(SizeType index, bool overrideFrozen)
void Array::Remove(SizeType index)
{
ObjectLock olock(this);
if (m_Frozen && !overrideFrozen)
if (m_Frozen)
BOOST_THROW_EXCEPTION(std::invalid_argument("Array must not be modified."));
if (index >= m_Data.size())
@ -185,43 +180,42 @@ void Array::Remove(SizeType index, bool overrideFrozen)
* Removes the item specified by the iterator from the array.
*
* @param it The iterator.
* @param overrideFrozen Whether to allow modifying frozen arrays.
*/
void Array::Remove(Array::Iterator it, bool overrideFrozen)
void Array::Remove(Array::Iterator it)
{
ASSERT(OwnsLock());
if (m_Frozen && !overrideFrozen)
if (m_Frozen)
BOOST_THROW_EXCEPTION(std::invalid_argument("Array must not be modified."));
m_Data.erase(it);
}
void Array::Resize(SizeType newSize, bool overrideFrozen)
void Array::Resize(SizeType newSize)
{
ObjectLock olock(this);
if (m_Frozen && !overrideFrozen)
if (m_Frozen)
BOOST_THROW_EXCEPTION(std::invalid_argument("Array must not be modified."));
m_Data.resize(newSize);
}
void Array::Clear(bool overrideFrozen)
void Array::Clear()
{
ObjectLock olock(this);
if (m_Frozen && !overrideFrozen)
if (m_Frozen)
BOOST_THROW_EXCEPTION(std::invalid_argument("Array must not be modified."));
m_Data.clear();
}
void Array::Reserve(SizeType newSize, bool overrideFrozen)
void Array::Reserve(SizeType newSize)
{
ObjectLock olock(this);
if (m_Frozen && !overrideFrozen)
if (m_Frozen)
BOOST_THROW_EXCEPTION(std::invalid_argument("Array must not be modified."));
m_Data.reserve(newSize);
@ -280,11 +274,11 @@ Array::Ptr Array::Reverse() const
return result;
}
void Array::Sort(bool overrideFrozen)
void Array::Sort()
{
ObjectLock olock(this);
if (m_Frozen && !overrideFrozen)
if (m_Frozen)
BOOST_THROW_EXCEPTION(std::invalid_argument("Array must not be modified."));
std::sort(m_Data.begin(), m_Data.end());
@ -354,7 +348,7 @@ Value Array::GetFieldByName(const String& field, bool sandboxed, const DebugInfo
return Get(index);
}
void Array::SetFieldByName(const String& field, const Value& value, bool overrideFrozen, const DebugInfo& debugInfo)
void Array::SetFieldByName(const String& field, const Value& value, const DebugInfo& debugInfo)
{
ObjectLock olock(this);
@ -364,9 +358,9 @@ void Array::SetFieldByName(const String& field, const Value& value, bool overrid
BOOST_THROW_EXCEPTION(ScriptError("Array index '" + Convert::ToString(index) + "' is out of bounds.", debugInfo));
if (static_cast<size_t>(index) >= GetLength())
Resize(index + 1, overrideFrozen);
Resize(index + 1);
Set(index, value, overrideFrozen);
Set(index, value);
}
Array::Iterator icinga::begin(const Array::Ptr& x)

22
lib/base/array.hpp
View File

@ -38,9 +38,9 @@ public:
Array(std::initializer_list<Value> init);
Value Get(SizeType index) const;
void Set(SizeType index, const Value& value, bool overrideFrozen = false);
void Set(SizeType index, Value&& value, bool overrideFrozen = false);
void Add(Value value, bool overrideFrozen = false);
void Set(SizeType index, const Value& value);
void Set(SizeType index, Value&& value);
void Add(Value value);
Iterator Begin();
Iterator End();
@ -48,14 +48,14 @@ public:
size_t GetLength() const;
bool Contains(const Value& value) const;
void Insert(SizeType index, Value value, bool overrideFrozen = false);
void Remove(SizeType index, bool overrideFrozen = false);
void Remove(Iterator it, bool overrideFrozen = false);
void Insert(SizeType index, Value value);
void Remove(SizeType index);
void Remove(Iterator it);
void Resize(SizeType newSize, bool overrideFrozen = false);
void Clear(bool overrideFrozen = false);
void Resize(SizeType newSize);
void Clear();
void Reserve(SizeType newSize, bool overrideFrozen = false);
void Reserve(SizeType newSize);
void CopyTo(const Array::Ptr& dest) const;
Array::Ptr ShallowClone() const;
@ -91,7 +91,7 @@ public:
Array::Ptr Reverse() const;
void Sort(bool overrideFrozen = false);
void Sort();
String ToString() const override;
Value Join(const Value& separator) const;
@ -100,7 +100,7 @@ public:
void Freeze();
Value GetFieldByName(const String& field, bool sandboxed, const DebugInfo& debugInfo) const override;
void SetFieldByName(const String& field, const Value& value, bool overrideFrozen, const DebugInfo& debugInfo) override;
void SetFieldByName(const String& field, const Value& value, const DebugInfo& debugInfo) override;
private:
std::vector<Value> m_Data; /**< The data for the array. */

9
lib/base/dictionary.cpp
View File

@ -86,14 +86,13 @@ const Value * Dictionary::GetRef(const String& key) const
*
* @param key The key.
* @param value The value.
* @param overrideFrozen Whether to allow modifying frozen dictionaries.
*/
void Dictionary::Set(const String& key, Value value, bool overrideFrozen)
void Dictionary::Set(const String& key, Value value)
{
ObjectLock olock(this);
std::unique_lock<std::shared_timed_mutex> lock (m_DataMutex);
if (m_Frozen && !overrideFrozen)
if (m_Frozen)
BOOST_THROW_EXCEPTION(std::invalid_argument("Value in dictionary must not be modified."));
m_Data[key] = std::move(value);
@ -290,9 +289,9 @@ Value Dictionary::GetFieldByName(const String& field, bool, const DebugInfo& deb
return GetPrototypeField(const_cast<Dictionary *>(this), field, false, debugInfo);
}
void Dictionary::SetFieldByName(const String& field, const Value& value, bool overrideFrozen, const DebugInfo&)
void Dictionary::SetFieldByName(const String& field, const Value& value, const DebugInfo&)
{
Set(field, value, overrideFrozen);
Set(field, value);
}
bool Dictionary::HasOwnField(const String& field) const

4
lib/base/dictionary.hpp
View File

@ -43,7 +43,7 @@ public:
Value Get(const String& key) const;
bool Get(const String& key, Value *result) const;
const Value * GetRef(const String& key) const;
void Set(const String& key, Value value, bool overrideFrozen = false);
void Set(const String& key, Value value);
bool Contains(const String& key) const;
Iterator Begin();
@ -71,7 +71,7 @@ public:
void Freeze();
Value GetFieldByName(const String& field, bool sandboxed, const DebugInfo& debugInfo) const override;
void SetFieldByName(const String& field, const Value& value, bool overrideFrozen, const DebugInfo& debugInfo) override;
void SetFieldByName(const String& field, const Value& value, const DebugInfo& debugInfo) override;
bool HasOwnField(const String& field) const override;
bool GetOwnField(const String& field, Value *result) const override;

7
lib/base/namespace.cpp
View File

@ -143,13 +143,8 @@ Value Namespace::GetFieldByName(const String& field, bool, const DebugInfo& debu
return GetPrototypeField(const_cast<Namespace *>(this), field, false, debugInfo); /* Ignore indexer not found errors similar to the Dictionary class. */
}
void Namespace::SetFieldByName(const String& field, const Value& value, bool overrideFrozen, const DebugInfo& debugInfo)
void Namespace::SetFieldByName(const String& field, const Value& value, const DebugInfo& debugInfo)
{
// The override frozen parameter is mandated by the interface but ignored here. If the namespace is frozen, this
// disables locking for read operations, so it must not be modified again to ensure the consistency of the internal
// data structures.
(void) overrideFrozen;
Set(field, value, false, debugInfo);
}

2
lib/base/namespace.hpp
View File

@ -80,7 +80,7 @@ public:
size_t GetLength() const;
Value GetFieldByName(const String& field, bool sandboxed, const DebugInfo& debugInfo) const override;
void SetFieldByName(const String& field, const Value& value, bool overrideFrozen, const DebugInfo& debugInfo) override;
void SetFieldByName(const String& field, const Value& value, const DebugInfo& debugInfo) override;
bool HasOwnField(const String& field) const override;
bool GetOwnField(const String& field, Value *result) const override;

2
lib/base/object.cpp
View File

@ -125,7 +125,7 @@ Value Object::GetFieldByName(const String& field, bool sandboxed, const DebugInf
return GetField(fid);
}
void Object::SetFieldByName(const String& field, const Value& value, bool overrideFrozen, const DebugInfo& debugInfo)
void Object::SetFieldByName(const String& field, const Value& value, const DebugInfo& debugInfo)
{
Type::Ptr type = GetReflectionType();

2
lib/base/object.hpp
View File

@ -171,7 +171,7 @@ public:
virtual void SetField(int id, const Value& value, bool suppress_events = false, const Value& cookie = Empty);
virtual Value GetField(int id) const;
virtual Value GetFieldByName(const String& field, bool sandboxed, const DebugInfo& debugInfo) const;
virtual void SetFieldByName(const String& field, const Value& value, bool overrideFrozen, const DebugInfo& debugInfo);
virtual void SetFieldByName(const String& field, const Value& value, const DebugInfo& debugInfo);
virtual bool HasOwnField(const String& field) const;
virtual bool GetOwnField(const String& field, Value *result) const;
virtual void ValidateField(int id, const Lazy<Value>& lvalue, const ValidationUtils& utils);

2
lib/base/reference.cpp
View File

@ -24,7 +24,7 @@ Value Reference::Get() const
void Reference::Set(const Value& value)
{
m_Parent->SetFieldByName(m_Index, value, false, DebugInfo());
m_Parent->SetFieldByName(m_Index, value, DebugInfo());
}
Object::Ptr Reference::GetParent() const

14
lib/config/expression.cpp
View File

@ -652,7 +652,7 @@ ExpressionResult SetExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dhint)
}
}
VMOps::SetField(parent, index, operand2.GetValue(), m_OverrideFrozen, m_DebugInfo);
VMOps::SetField(parent, index, operand2.GetValue(), m_DebugInfo);
if (psdhint) {
psdhint->AddMessage("=", m_DebugInfo);
@ -666,11 +666,6 @@ ExpressionResult SetExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dhint)
return Empty;
}
void SetExpression::SetOverrideFrozen()
{
m_OverrideFrozen = true;
}
ExpressionResult SetConstExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const
{
auto globals = ScriptGlobal::GetGlobals();
@ -772,7 +767,7 @@ bool IndexerExpression::GetReference(ScriptFrame& frame, bool init_dict, Value *
old_value = VMOps::GetField(vparent, vindex, frame.Sandboxed, m_Operand1->GetDebugInfo());
if (old_value.IsEmpty() && !old_value.IsString())
VMOps::SetField(vparent, vindex, new Dictionary(), m_OverrideFrozen, m_Operand1->GetDebugInfo());
VMOps::SetField(vparent, vindex, new Dictionary(), m_Operand1->GetDebugInfo());
}
*parent = VMOps::GetField(vparent, vindex, frame.Sandboxed, m_DebugInfo);
@ -798,11 +793,6 @@ bool IndexerExpression::GetReference(ScriptFrame& frame, bool init_dict, Value *
return true;
}
void IndexerExpression::SetOverrideFrozen()
{
m_OverrideFrozen = true;
}
void icinga::BindToScope(std::unique_ptr<Expression>& expr, ScopeSpecifier scopeSpec)
{
auto *dexpr = dynamic_cast<DictExpression *>(expr.get());

7
lib/config/expression.hpp
View File

@ -657,14 +657,11 @@ public:
: BinaryExpression(std::move(operand1), std::move(operand2), debugInfo), m_Op(op)
{ }
void SetOverrideFrozen();
protected:
ExpressionResult DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const override;
private:
CombinedSetOp m_Op;
bool m_OverrideFrozen{false};
friend void BindToScope(std::unique_ptr<Expression>& expr, ScopeSpecifier scopeSpec);
};
@ -755,11 +752,7 @@ public:
: BinaryExpression(std::move(operand1), std::move(operand2), debugInfo)
{ }
void SetOverrideFrozen();
protected:
bool m_OverrideFrozen{false};
ExpressionResult DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const override;
bool GetReference(ScriptFrame& frame, bool init_dict, Value *parent, String *index, DebugHint **dhint) const override;

4
lib/config/vmops.hpp
View File

@ -246,12 +246,12 @@ public:
return object->GetFieldByName(field, sandboxed, debugInfo);
}
static inline void SetField(const Object::Ptr& context, const String& field, const Value& value, bool overrideFrozen, const DebugInfo& debugInfo = DebugInfo())
static inline void SetField(const Object::Ptr& context, const String& field, const Value& value, const DebugInfo& debugInfo = DebugInfo())
{
if (!context)
BOOST_THROW_EXCEPTION(ScriptError("Cannot set field '" + field + "' on a value that is not an object.", debugInfo));
return context->SetFieldByName(field, value, overrideFrozen, debugInfo);
return context->SetFieldByName(field, value, debugInfo);
}
private:

6
test/icinga-notification.cpp
View File

@ -36,9 +36,9 @@ struct DuplicateDueToFilterHelper
nc->SetExecute(new Function("", [this]() { ++called; }), true);
nc->Register();
n->SetFieldByName("host_name", "example.com", false, DebugInfo());
n->SetFieldByName("service_name", "disk", false, DebugInfo());
n->SetFieldByName("command", "mail", false, DebugInfo());
n->SetFieldByName("host_name", "example.com", DebugInfo());
n->SetFieldByName("service_name", "disk", DebugInfo());
n->SetFieldByName("command", "mail", DebugInfo());
n->SetUsersRaw(new Array({"jdoe"}), true);
n->SetTypeFilter(typeFilter);
n->SetStateFilter(stateFilter);

6
test/methods-pluginnotificationtask.cpp
View File

@ -56,9 +56,9 @@ BOOST_AUTO_TEST_CASE(truncate_long_output)
nc->SetName("mail", true);
nc->Register();
n->SetFieldByName("host_name", "example.com", false, DebugInfo());
n->SetFieldByName("service_name", "disk", false, DebugInfo());
n->SetFieldByName("command", "mail", false, DebugInfo());
n->SetFieldByName("host_name", "example.com", DebugInfo());
n->SetFieldByName("service_name", "disk", DebugInfo());
n->SetFieldByName("command", "mail", DebugInfo());
n->OnAllConfigLoaded(); // link Service
Checkable::ExecuteCommandProcessFinishedHandler = [&promise](const Value&, const ProcessResult& pr) {