config: Discard prior mutables before applying

If we only clear existing mutables *after* applying, we get into an
inconsistent state if there was an error in one of the config values:

The improper value lingers around in self._mutables, and then gets
returned when get_mutable_obj() (or update_mutables()) gets called the
next time.

Reproducer:

    qutebrowser --debug --temp-basedir \
        ':config-dict-add content.javascript.log_message.levels example.org bla' \
        ':later 1000 config-dict-add content.javascript.log_message.levels example.org bla'

Results in:

    ERROR: Invalid value 'bla' - expected a value of type list but got str.
    ERROR: example.org already exists in content.javascript.log_message - use --replace to overwrite!

Fixes the second part of #7343.

nb: As before, the mutable updating actually gets interrupted by a
failing update, instead of it e.g. collecting all errors but carrying
on. With this change, the remaining updates will thus also be discarded,
but that does not seem to be a problem with how mutables are currently
used. Ideally, we should get rid of the mutable handling entirely
anyways, at least for qutebrowser internal code - see #4344.

2 files changed, 16 insertions, 2 deletions

diff --git a/qutebrowser/config/config.py b/qutebrowser/config/config.py
index 918c9b894..b2736158f 100644
--- a/qutebrowser/config/config.py
+++ b/qutebrowser/config/config.py
@@ -553,11 +553,12 @@ class Config(QObject):
         Here, we check all those saved copies for mutations, and if something
         mutated, we call set_obj again so we save the new value.
         """
-        for name, (old_value, new_value) in self._mutables.items():
+        mutables = self._mutables.items()
+        self._mutables = {}
+        for name, (old_value, new_value) in mutables:
             if old_value != new_value:
                 log.config.debug("{} was mutated, updating".format(name))
                 self.set_obj(name, new_value, save_yaml=save_yaml)
-        self._mutables = {}
 
     def dump_userconfig(self) -> str:
         """Get the part of the config which was changed by the user.
diff --git a/tests/unit/config/test_config.py b/tests/unit/config/test_config.py
index ab1fba789..fc6c6fc0c 100644
--- a/tests/unit/config/test_config.py
+++ b/tests/unit/config/test_config.py
@@ -613,6 +613,19 @@ class TestConfig:
         expected = {'X-Foo': 'fooval', 'X-Bar': 'barval'}
         assert conf.get_obj(option) == expected
 
+    def test_get_mutable_invalid_value(self, conf):
+        """Make sure invalid values aren't stored in mutables."""
+        option = 'keyhint.blacklist'
+        obj = conf.get_mutable_obj(option)
+        assert obj == []
+        obj.append(42)
+
+        with pytest.raises(configexc.ValidationError):
+            conf.update_mutables()
+
+        obj = conf.get_mutable_obj(option)
+        assert obj == []
+
     def test_get_obj_unknown_mutable(self, conf):
         """Make sure we don't have unknown mutable types."""
         with pytest.raises(AssertionError):