From 3acaf4193a32c3fe2f4f67f9e525b94973b84797 Mon Sep 17 00:00:00 2001 From: Tom Doherty Date: Wed, 11 Dec 2024 11:01:28 +0000 Subject: [PATCH] throw a TypeError when trying to merge a list with a mapping when `merge_lists=True` --- changelog/67092.fixed.md | 1 + salt/utils/dictupdate.py | 17 ++++++++++++++++- tests/unit/utils/test_dictupdate.py | 12 ++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 changelog/67092.fixed.md diff --git a/changelog/67092.fixed.md b/changelog/67092.fixed.md new file mode 100644 index 000000000000..3fb853724b63 --- /dev/null +++ b/changelog/67092.fixed.md @@ -0,0 +1 @@ +dictupdate.update: throw a TypeError when trying to merge a list with a mapping when ``merge_lists=True``. diff --git a/salt/utils/dictupdate.py b/salt/utils/dictupdate.py index 49c0436e0930..81e45152f6f7 100644 --- a/salt/utils/dictupdate.py +++ b/salt/utils/dictupdate.py @@ -16,7 +16,7 @@ log = logging.getLogger(__name__) -def update(dest, upd, recursive_update=True, merge_lists=False): +def update(dest, upd, recursive_update=True, merge_lists=False, strict=False): """ Recursive version of the default dict.update @@ -30,9 +30,15 @@ def update(dest, upd, recursive_update=True, merge_lists=False): is ``dest[key] + upd[key]``. This behavior is only activated when recursive_update=True. By default merge_lists=False. + if strict=True, then we will raise a TypeError when trying to merge a list + with a mapping when ``merge_lists=True``. + .. versionchanged:: 2016.11.6 When merging lists, duplicate values are removed. Values already present in the ``dest`` list are not added from the ``upd`` list. + .. versionchanged:: 3008.0 + Throw a TypeError when trying to merge a list with a mapping when + ``merge_lists=True``. """ if (not isinstance(dest, Mapping)) or (not isinstance(upd, Mapping)): raise TypeError("Cannot update using non-dict types in dictupdate.update()") @@ -56,6 +62,15 @@ def update(dest, upd, recursive_update=True, merge_lists=False): dest[key] = merged else: dest[key] = upd[key] + elif ( + strict + and merge_lists + and isinstance(dest_subkey, list) + and isinstance(val, Mapping) + ): + raise TypeError( + f"With {merge_lists=} we cannot update list under {key=} with mapping containing keys {val.keys()=} in dictupdate.update()" + ) else: dest[key] = upd[key] return dest diff --git a/tests/unit/utils/test_dictupdate.py b/tests/unit/utils/test_dictupdate.py index b68f7ea79052..fbce39dbe83f 100644 --- a/tests/unit/utils/test_dictupdate.py +++ b/tests/unit/utils/test_dictupdate.py @@ -343,3 +343,15 @@ def test_deep_update_illegal_update(self): dictupdate.update_dict_key_value( {}, "foo", update_with, ordered_dict=True ) + + def test_update_with_merge_lists_and_mapping(self): + """ + Test that updating a list with a mapping raises a TypeError + when merge_lists=True. + """ + mdict = copy.deepcopy(self.dict1) + mdict["A"] = [1, 2] + with self.assertRaises(TypeError): + dictupdate.update( + copy.deepcopy(mdict), {"A": {"key": "value"}}, merge_lists=True, strict=True + )