From 46e5311d5a384907817ca1b60281d04d1f0bd5e1 Mon Sep 17 00:00:00 2001 From: Mario Liebisch Date: Fri, 10 Mar 2023 17:26:34 +0100 Subject: [PATCH 1/3] Fixed read-only dictionaries adding missing keys When querying a non-existing key on a read-only dictionary, the key was still added (albeit never set). This fixes #74726. --- core/variant/dictionary.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/variant/dictionary.cpp b/core/variant/dictionary.cpp index 0429508cc54..7ab150a4f84 100644 --- a/core/variant/dictionary.cpp +++ b/core/variant/dictionary.cpp @@ -83,7 +83,12 @@ Variant &Dictionary::operator[](const Variant &p_key) { if (unlikely(_p->read_only)) { if (p_key.get_type() == Variant::STRING_NAME) { const StringName *sn = VariantInternal::get_string_name(&p_key); - *_p->read_only = _p->variant_map[sn->operator String()]; + const String &key = sn->operator String(); + if (_p->variant_map.has(key)) { + *_p->read_only = _p->variant_map[key]; + } else { + *_p->read_only = Variant(); + } } else { *_p->read_only = _p->variant_map[p_key]; } From 4e72e093436b9f187ee7b015be1655ef9c16f607 Mon Sep 17 00:00:00 2001 From: Mario Liebisch Date: Sat, 11 Mar 2023 01:08:23 +0100 Subject: [PATCH 2/3] Added the missing second case for string keys --- core/variant/dictionary.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/variant/dictionary.cpp b/core/variant/dictionary.cpp index 7ab150a4f84..f019273735f 100644 --- a/core/variant/dictionary.cpp +++ b/core/variant/dictionary.cpp @@ -84,13 +84,15 @@ Variant &Dictionary::operator[](const Variant &p_key) { if (p_key.get_type() == Variant::STRING_NAME) { const StringName *sn = VariantInternal::get_string_name(&p_key); const String &key = sn->operator String(); - if (_p->variant_map.has(key)) { + if (likely(_p->variant_map.has(key))) { *_p->read_only = _p->variant_map[key]; } else { *_p->read_only = Variant(); } - } else { + } else if (likely(_p->variant_map.has(p_key))) { *_p->read_only = _p->variant_map[p_key]; + } else { + *_p->read_only = Variant(); } return *_p->read_only; From 62e1a5e2354a75973135b56b90f0e8237f6ad756 Mon Sep 17 00:00:00 2001 From: Mario Liebisch Date: Sat, 11 Mar 2023 18:53:17 +0100 Subject: [PATCH 3/3] Added two test cases --- tests/core/variant/test_dictionary.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/core/variant/test_dictionary.h b/tests/core/variant/test_dictionary.h index 2d73910d3db..4571de64871 100644 --- a/tests/core/variant/test_dictionary.h +++ b/tests/core/variant/test_dictionary.h @@ -88,6 +88,12 @@ TEST_CASE("[Dictionary] Assignment using bracket notation ([])") { CHECK(int(map[0]) == 400); // Check that assigning 0 doesn't overwrite the value for `false`. CHECK(int(map[false]) == 128); + + // Ensure read-only maps aren't modified by non-existing keys. + const auto length = map.size(); + map.make_read_only(); + CHECK(int(map["This key does not exist"].get_type()) == Variant::NIL); + CHECK(map.size() == length); } TEST_CASE("[Dictionary] get_key_lists()") {