From cc04ab920214437a81d53ac93e82d7ec1eeb482a Mon Sep 17 00:00:00 2001 From: stktyagi Date: Tue, 10 Jun 2025 19:24:54 +0530 Subject: [PATCH 1/6] [Fix] Merging with wrong format now raises Validation error #351 An attempt to merge config with wrong format will now raise a validation error instead of failing. Fixes #351 --- netjsonconfig/utils.py | 28 ++++++++++++++++++++++++---- tests/openwrt/test_default.py | 29 ++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/netjsonconfig/utils.py b/netjsonconfig/utils.py index de9a08bf9..65a042cfd 100644 --- a/netjsonconfig/utils.py +++ b/netjsonconfig/utils.py @@ -3,6 +3,12 @@ from copy import deepcopy +class ValidationError(Exception): + """exception for validation errors during config merge""" + + pass + + def merge_config(template, config, list_identifiers=None): """ Merges ``config`` on top of ``template``. @@ -23,10 +29,24 @@ def merge_config(template, config, list_identifiers=None): result = deepcopy(template) for key, value in config.items(): if isinstance(value, dict): - node = result.get(key, OrderedDict()) - result[key] = merge_config(node, value) - elif isinstance(value, list) and isinstance(result.get(key), list): - result[key] = merge_list(result[key], value, list_identifiers) + if isinstance(result.get(key), dict) or key not in result: + node = result.get(key, OrderedDict()) + result[key] = merge_config(node, value) + else: + raise ValidationError( + f"Cannot merge dict into non-dict for key '{key}': " + f"{type(result.get(key)).__name__} vs dict" + ) + elif isinstance(value, list): + if isinstance(result.get(key), list): + result[key] = merge_list(result[key], value, list_identifiers) + elif key in result: + raise ValidationError( + f"Cannot merge list into non-list for key '{key}': " + f"{type(result.get(key)).__name__} vs list" + ) + else: + result[key] = value else: result[key] = value return result diff --git a/tests/openwrt/test_default.py b/tests/openwrt/test_default.py index 9a7eb3eaa..5d65d93a6 100644 --- a/tests/openwrt/test_default.py +++ b/tests/openwrt/test_default.py @@ -3,7 +3,7 @@ from openwisp_utils.tests import capture_stdout from netjsonconfig import OpenWrt -from netjsonconfig.utils import _TabsMixin +from netjsonconfig.utils import ValidationError, _TabsMixin class TestDefault(unittest.TestCase, _TabsMixin): @@ -251,3 +251,30 @@ def test_render_invalid_uci_name(self): """ ) self.assertEqual(o.render(), expected) + + def test_merge_invalid_format(self): + invalid = { + "dhcp": { + "lan": { + "interface": "lan", + "start": 100, + "limit": 150, + "leasetime": "12h", + } + } + } + valid = { + "dhcp": [ + { + "dhcpv6": "disabled", + "ignore": True, + "ra": "disabled", + "config_value": "lan", + "config_name": "dhcp", + } + ] + } + with self.assertRaises(ValidationError) as context: + OpenWrt({}, templates=[valid, invalid]).validate() + + self.assertIn("Cannot merge dict into non-dict", str(context.exception)) From a9969bfdc4edd386715023db2fa7d726b829263b Mon Sep 17 00:00:00 2001 From: stktyagi Date: Sun, 26 Oct 2025 16:37:32 +0530 Subject: [PATCH 2/6] [Refactor] Change aggressive validation in merge_config #351 It only prevents the original AttributeError crash and correctly lets the .validate() method handle all validation of the final merged configuration. Fixes #351 --- netjsonconfig/utils.py | 29 ++++------------------------- tests/openwrt/test_default.py | 8 +++----- 2 files changed, 7 insertions(+), 30 deletions(-) diff --git a/netjsonconfig/utils.py b/netjsonconfig/utils.py index 65a042cfd..c91feeab8 100644 --- a/netjsonconfig/utils.py +++ b/netjsonconfig/utils.py @@ -3,12 +3,6 @@ from copy import deepcopy -class ValidationError(Exception): - """exception for validation errors during config merge""" - - pass - - def merge_config(template, config, list_identifiers=None): """ Merges ``config`` on top of ``template``. @@ -28,25 +22,10 @@ def merge_config(template, config, list_identifiers=None): """ result = deepcopy(template) for key, value in config.items(): - if isinstance(value, dict): - if isinstance(result.get(key), dict) or key not in result: - node = result.get(key, OrderedDict()) - result[key] = merge_config(node, value) - else: - raise ValidationError( - f"Cannot merge dict into non-dict for key '{key}': " - f"{type(result.get(key)).__name__} vs dict" - ) - elif isinstance(value, list): - if isinstance(result.get(key), list): - result[key] = merge_list(result[key], value, list_identifiers) - elif key in result: - raise ValidationError( - f"Cannot merge list into non-list for key '{key}': " - f"{type(result.get(key)).__name__} vs list" - ) - else: - result[key] = value + if isinstance(value, dict) and isinstance(result.get(key), dict): + result[key] = merge_config(result.get(key), value, list_identifiers) + elif isinstance(value, list) and isinstance(result.get(key), list): + result[key] = merge_list(result[key], value, list_identifiers) else: result[key] = value return result diff --git a/tests/openwrt/test_default.py b/tests/openwrt/test_default.py index 5d65d93a6..b5e487b20 100644 --- a/tests/openwrt/test_default.py +++ b/tests/openwrt/test_default.py @@ -3,7 +3,7 @@ from openwisp_utils.tests import capture_stdout from netjsonconfig import OpenWrt -from netjsonconfig.utils import ValidationError, _TabsMixin +from netjsonconfig.utils import _TabsMixin class TestDefault(unittest.TestCase, _TabsMixin): @@ -274,7 +274,5 @@ def test_merge_invalid_format(self): } ] } - with self.assertRaises(ValidationError) as context: - OpenWrt({}, templates=[valid, invalid]).validate() - - self.assertIn("Cannot merge dict into non-dict", str(context.exception)) + o = OpenWrt({}, templates=[valid, invalid]) + o.validate() From 9d0c8b132f20048913b10e946a8822a60266e7f3 Mon Sep 17 00:00:00 2001 From: stktyagi Date: Fri, 13 Mar 2026 11:14:07 +0530 Subject: [PATCH 3/6] [fix] Riase ValidationError on incompatible types #351 Merge_config now raises ValidationError when templates have incompatible types for the same key (e.g. list vs dict), instead of silently overwriting. Test updated to assert ValidationError. Fixes #351 --- netjsonconfig/utils.py | 12 ++++++++++++ tests/openwrt/test_default.py | 5 +++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/netjsonconfig/utils.py b/netjsonconfig/utils.py index c91feeab8..771c85bde 100644 --- a/netjsonconfig/utils.py +++ b/netjsonconfig/utils.py @@ -2,6 +2,10 @@ from collections import OrderedDict from copy import deepcopy +from jsonschema import ValidationError as JsonSchemaError + +from .exceptions import ValidationError + def merge_config(template, config, list_identifiers=None): """ @@ -19,6 +23,7 @@ def merge_config(template, config, list_identifiers=None): :param config: config ``dict`` :param list_identifiers: ``list`` or ``None`` :returns: merged ``dict`` + :raises ValidationError: if incompatible types are found """ result = deepcopy(template) for key, value in config.items(): @@ -26,6 +31,13 @@ def merge_config(template, config, list_identifiers=None): result[key] = merge_config(result.get(key), value, list_identifiers) elif isinstance(value, list) and isinstance(result.get(key), list): result[key] = merge_list(result[key], value, list_identifiers) + elif result.get(key) is not None and type(value) is not type(result.get(key)): + raise ValidationError( + JsonSchemaError( + f"incompatible types for '{key}': expected {type(result.get(key)).__name__}, " + f"got {type(value).__name__}" + ) + ) else: result[key] = value return result diff --git a/tests/openwrt/test_default.py b/tests/openwrt/test_default.py index 1bc2b7ea6..a22802b5b 100644 --- a/tests/openwrt/test_default.py +++ b/tests/openwrt/test_default.py @@ -3,6 +3,7 @@ from openwisp_utils.tests import capture_stdout from netjsonconfig import OpenWrt +from netjsonconfig.exceptions import ValidationError from netjsonconfig.utils import _TabsMixin @@ -268,5 +269,5 @@ def test_merge_invalid_format(self): } ] } - o = OpenWrt({}, templates=[valid, invalid]) - o.validate() + with self.assertRaises(ValidationError): + OpenWrt({}, templates=[valid, invalid]) From aae764512bbd62d2f9ea1c4233edf919224c2ecf Mon Sep 17 00:00:00 2001 From: stktyagi Date: Fri, 13 Mar 2026 11:25:22 +0530 Subject: [PATCH 4/6] [fix] Scope type check to container-type conflicts onl #351 Narrowed the incompatible type check in merge_config to only raise ValidationError when a dict or list conflicts with a different type. Scalar overrides (e.g. str to int) are still allowed as before. Fixes #351 --- netjsonconfig/utils.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/netjsonconfig/utils.py b/netjsonconfig/utils.py index 771c85bde..8088ca51c 100644 --- a/netjsonconfig/utils.py +++ b/netjsonconfig/utils.py @@ -27,14 +27,19 @@ def merge_config(template, config, list_identifiers=None): """ result = deepcopy(template) for key, value in config.items(): - if isinstance(value, dict) and isinstance(result.get(key), dict): - result[key] = merge_config(result.get(key), value, list_identifiers) - elif isinstance(value, list) and isinstance(result.get(key), list): - result[key] = merge_list(result[key], value, list_identifiers) - elif result.get(key) is not None and type(value) is not type(result.get(key)): + existing = result.get(key) + if isinstance(value, dict) and isinstance(existing, dict): + result[key] = merge_config(existing, value, list_identifiers) + elif isinstance(value, list) and isinstance(existing, list): + result[key] = merge_list(existing, value, list_identifiers) + elif ( + existing is not None + and (isinstance(value, (dict, list)) or isinstance(existing, (dict, list))) + and type(value) is not type(existing) + ): raise ValidationError( JsonSchemaError( - f"incompatible types for '{key}': expected {type(result.get(key)).__name__}, " + f"incompatible types for '{key}': expected {type(existing).__name__}, " f"got {type(value).__name__}" ) ) From b87a1ea305be66fece8dc18fcfa6f52d30f053cc Mon Sep 17 00:00:00 2001 From: stktyagi Date: Sat, 16 May 2026 23:04:58 +0530 Subject: [PATCH 5/6] [fix] Fix uncaught exception on incompatible merge types Make NetJsonConfigException tolerant of message-only jsonschema errors to prevent TypeError during stringification. Scope type mismatch validation in merge_config strictly to complex shape conflicts (dict vs list). --- netjsonconfig/exceptions.py | 6 ++++++ netjsonconfig/utils.py | 3 ++- tests/test_utils.py | 15 +++++++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/netjsonconfig/exceptions.py b/netjsonconfig/exceptions.py index 8a38a3439..b4393af00 100644 --- a/netjsonconfig/exceptions.py +++ b/netjsonconfig/exceptions.py @@ -7,6 +7,10 @@ def _list_errors(e): :param e: ``jsonschema.exceptions.ValidationError`` instance """ error_list = [] + if not getattr(e, "context", None): + return error_list + if not getattr(e, "validator_value", None): + return error_list for value, error in zip(e.validator_value, e.context): error_list.append((value, error.message)) if error.context: @@ -25,6 +29,8 @@ def __str__(self): self.details, ) errors = _list_errors(self.details) + if not errors: + return message separator = "\nAgainst schema %s\n%s\n" details = reduce(lambda x, y: x + separator % y, errors, "") return message + details diff --git a/netjsonconfig/utils.py b/netjsonconfig/utils.py index 8088ca51c..0d3132357 100644 --- a/netjsonconfig/utils.py +++ b/netjsonconfig/utils.py @@ -34,7 +34,8 @@ def merge_config(template, config, list_identifiers=None): result[key] = merge_list(existing, value, list_identifiers) elif ( existing is not None - and (isinstance(value, (dict, list)) or isinstance(existing, (dict, list))) + and isinstance(existing, (dict, list)) + and isinstance(value, (dict, list)) and type(value) is not type(existing) ): raise ValidationError( diff --git a/tests/test_utils.py b/tests/test_utils.py index 0d83b5d13..9ffcb1cb2 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,5 +1,6 @@ import unittest +from netjsonconfig.exceptions import ValidationError from netjsonconfig.utils import evaluate_vars, get_copy, merge_config, merge_list @@ -55,6 +56,20 @@ def test_merge_list_of_dicts_unchanged(self): result, {"list": [{"a": "original"}, {"b": "changed"}, {"c": "original"}]} ) + def test_merge_config_list_scalar_overrides(self): + template = {"server": ["1.1.1.1"]} + config = {"server": "8.8.8.8"} + result = merge_config(template, config) + self.assertEqual(result, {"server": "8.8.8.8"}) + + def test_merge_config_incompatible_dict_list(self): + template = {"a": {"b": "c"}} + config = {"a": ["x"]} + with self.assertRaises(ValidationError) as context: + merge_config(template, config) + self.assertIn("incompatible types", str(context.exception)) + self.assertIn("ValidationError", str(context.exception)) + def test_evaluate_vars(self): self.assertEqual(evaluate_vars("{{ tz }}", {"tz": "UTC"}), "UTC") self.assertEqual(evaluate_vars("tz: {{ tz }}", {"tz": "UTC"}), "tz: UTC") From 68343b89642acf9eeff3ba1b8068fb79fa658270 Mon Sep 17 00:00:00 2001 From: Federico Capoano Date: Mon, 18 May 2026 14:00:48 -0300 Subject: [PATCH 6/6] [chores] Minor improvements --- netjsonconfig/utils.py | 4 ++-- tests/test_utils.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/netjsonconfig/utils.py b/netjsonconfig/utils.py index 0d3132357..617e2d681 100644 --- a/netjsonconfig/utils.py +++ b/netjsonconfig/utils.py @@ -40,8 +40,8 @@ def merge_config(template, config, list_identifiers=None): ): raise ValidationError( JsonSchemaError( - f"incompatible types for '{key}': expected {type(existing).__name__}, " - f"got {type(value).__name__}" + f"Incompatible type for '{key}': expected {type(existing).__name__}, " + f"got {type(value).__name__}." ) ) else: diff --git a/tests/test_utils.py b/tests/test_utils.py index 9ffcb1cb2..052734b7a 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -67,7 +67,7 @@ def test_merge_config_incompatible_dict_list(self): config = {"a": ["x"]} with self.assertRaises(ValidationError) as context: merge_config(template, config) - self.assertIn("incompatible types", str(context.exception)) + self.assertIn("Incompatible type", str(context.exception)) self.assertIn("ValidationError", str(context.exception)) def test_evaluate_vars(self):