jsonutil: fix handling of None in json patches

A null key in a JSON patch means "delete the existing key at this
location", but the operation should be idempotent, and certainly if the
key is missing, we shouldn't have a crash.

Unfortunately, as a long-standing bug, we've been using .pop() without a
default value for this, which will indeed crash if the original value is
missing.  It seems that nobody has ever attempted that, and we don't
test for it either.

Let's use the non-failing form of .pop() with a default value and add a
test to make sure this is working: once for removing an existing
attribute, and once for removing a non-existing attribute.

src/cockpit/jsonutil.py

@@ -152,7 +152,7 @@ def json_merge_patch(current: JsonObject, patch: JsonObject) -> JsonObject:
         elif patch_value is not None:
             result[key] = patch_value
         else:
-            result.pop(key)
+            result.pop(key, None)
 
     return result
 

test/pytest/test_packages.py

@@ -80,20 +80,19 @@ def test_basic(pkgdir):
 
 
 def test_override_etc(pkgdir, confdir):
-    (confdir / 'basic.override.json').write_text('{"description": "overridden package", "priority": 5}')
+    (confdir / 'basic.override.json').write_text('{"description": null, "priority": 5, "does-not-exist": null}')
 
     packages = Packages()
     assert len(packages.packages) == 1
     # original attributes
     assert packages.packages['basic'].name == 'basic'
     assert packages.packages['basic'].manifest['requires'] == {'cockpit': '42'}
     # overridden attributes
-    assert packages.packages['basic'].manifest['description'] == 'overridden package'
+    assert 'description' not in packages.packages['basic'].manifest
     assert packages.packages['basic'].priority == 5
 
     assert json.loads(packages.manifests) == {
         'basic': {
-            'description': 'overridden package',
             'requires': {'cockpit': '42'},
             'priority': 5,
         }