gitpython-developers
diff --git a/‎git/config.py‎
Lines changed: 22 additions & 5 deletions b/‎git/config.py‎
Lines changed: 22 additions & 5 deletions
diff --git a/‎test/test_config.py‎
Lines changed: 54 additions & 7 deletions b/‎test/test_config.py‎
Lines changed: 54 additions & 7 deletions
@@ -462,7 +462,8 @@ def string_decode(v: str) -> str:
                 v = v[:-1]
             # END cut trailing escapes to prevent decode error
 
-            return v.encode(defenc).decode("unicode_escape")
+            escapes = {"b": "\b", "n": "\n", "r": "\r", "t": "\t", '"': '"', "\\": "\\"}
+            return re.sub(r"\\(.)", lambda match: escapes.get(match.group(1), match.group(0)), v)
 
         # END string_decode
 
@@ -517,10 +518,12 @@ def string_decode(v: str) -> str:
                         # Opens quoting and does not close: appears to start multi-line quoting.
                         is_multi_line = True
                         optval = string_decode(optval[1:])
-                    elif optval.find("\\", 1, -1) == -1 and optval.find('"', 1, -1) == -1:
-                        # Opens and closes quoting. Single line, and all we need is quote removal.
-                        optval = optval[1:-1]
-                    # TODO: Handle other quoted content, especially well-formed backslash escapes.
+                    elif re.search(r'(?:^|[^\\])(?:\\\\)*"', optval[1:-1]):
+                        # Preserve malformed values containing unescaped quotes.
+                        pass
+                    else:
+                        # Opens and closes quoting.
+                        optval = string_decode(optval[1:-1])
 
                     # Preserves multiple values for duplicate optnames.
                     cursect.add(optname, optval)
@@ -768,6 +771,20 @@ def write(self) -> None:
             return
         # END stop if we have include files
 
+        sections: List[_OMD] = [self._defaults]
+        section: _OMD
+        stored_section: _OMD
+        values: List[Any]
+        raw_value: Any
+        for _, stored_section in self._sections.items():
+            sections.append(stored_section)
+        for section in sections:
+            for key, values in section.items_all():
+                if key != "__name__":
+                    for raw_value in values:
+                        if "\r" in self._value_to_string(raw_value) or "\x00" in self._value_to_string(raw_value):
+                            raise ValueError("Git config values must not contain CR or NUL")
+
         fp = self._file_or_files
 
         # We have a physical file on disk, so get a lock.
 
@@ -190,6 +190,54 @@ def test_writer_escapes_special_characters_without_newline(self, rw_dir):
         with open(config_path, "rb") as config_file:
             self.assertNotIn(b"\x08", config_file.read())
 
+    @with_rw_directory
+    def test_writer_preserves_escaped_and_non_ascii_values_safely(self, rw_dir):
+        config_path = osp.join(rw_dir, "config")
+        with open(config_path, "wb") as config_file:
+            config_file.write(
+                (
+                    '[section]\nnewline = "first\\nsecond"\nquote = "a\\"b"\nbackslash = "a\\\\b"\n'
+                    'unicode = "café\\\\path"\n'
+                ).encode()
+            )
+
+        with GitConfigParser(config_path, read_only=False) as config:
+            config.set_value("unrelated", "key", "value")
+
+        expected = {
+            "newline": "first\nsecond",
+            "quote": 'a"b',
+            "backslash": "a\\b",
+            "unicode": "café\\path",
+        }
+        with GitConfigParser(config_path, read_only=True) as config:
+            for key, value in expected.items():
+                self.assertEqual(config.get_value("section", key), value)
+                self.assertEqual(
+                    subprocess.run(
+                        ["git", "config", "--file", config_path, "--get", "section.%s" % key],
+                        stdout=subprocess.PIPE,
+                        check=True,
+                    ).stdout,
+                    value.encode() + b"\n",
+                )
+
+        with open(config_path, "rb") as config_file:
+            contents = config_file.read()
+        self.assertNotIn(b"\r", contents)
+        self.assertNotIn(b"\x00", contents)
+
+        for name, value in (("return", b"first\\rsecond"), ("nul", b"first\x00second")):
+            unsafe_path = osp.join(rw_dir, "%s-config" % name)
+            unsafe_contents = b'[section]\nvalue = "' + value + b'"\n'
+            with open(unsafe_path, "wb") as config_file:
+                config_file.write(unsafe_contents)
+            with self.assertRaisesRegex(ValueError, "CR or NUL"):
+                with GitConfigParser(unsafe_path, read_only=False) as config:
+                    config.set_value("unrelated", "key", "value")
+            with open(unsafe_path, "rb") as config_file:
+                self.assertEqual(config_file.read(), unsafe_contents)
+
     @with_rw_directory
     def test_set_value_rejects_config_injection(self, rw_dir):
         config_path = osp.join(rw_dir, "config")
@@ -745,15 +793,14 @@ def test_config_with_quotes_with_whitespace_outside_value(self):
         self.assertEqual(cr.get("init", "defaultBranch"), "trunk")
 
     def test_config_with_quotes_containing_escapes(self):
-        """For now just suppress quote removal. But it would be good to interpret most of these."""
+        """Interpret Git's quoted escapes without changing malformed values."""
         cr = GitConfigParser(fixture_path("git_config_with_quotes_escapes"), read_only=True)
 
-        # These can eventually be supported by substituting the represented character.
-        self.assertEqual(cr.get("custom", "hasnewline"), R'"first\nsecond"')
-        self.assertEqual(cr.get("custom", "hasbackslash"), R'"foo\\bar"')
-        self.assertEqual(cr.get("custom", "hasquote"), R'"ab\"cd"')
-        self.assertEqual(cr.get("custom", "hastrailingbackslash"), R'"word\\"')
-        self.assertEqual(cr.get("custom", "hasunrecognized"), R'"p\qrs"')
+        self.assertEqual(cr.get("custom", "hasnewline"), "first\nsecond")
+        self.assertEqual(cr.get("custom", "hasbackslash"), R"foo\bar")
+        self.assertEqual(cr.get("custom", "hasquote"), 'ab"cd')
+        self.assertEqual(cr.get("custom", "hastrailingbackslash"), "word\\")
+        self.assertEqual(cr.get("custom", "hasunrecognized"), R"p\qrs")
 
         # It is less obvious whether and what to eventually do with this.
         self.assertEqual(cr.get("custom", "hasunescapedquotes"), '"ab"cd"e"')