Merge pull request #10499 from Icinga/remove-override-frozen

No longer allow overriding the frozen attribute of containers
This commit is contained in:
Yonas Habteab 2025-07-08 17:22:58 +02:00 committed by GitHub
commit 241cf08263
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 55 additions and 86 deletions

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);

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)

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. */

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

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;

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);
}

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;

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();

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);

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

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());

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;

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:

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);

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) {