Preserve Git config value semantics

<!-- agent -->
Decode Git-supported quoted value escapes directly instead of routing UTF-8 text
through Python unicode_escape. This preserves newlines, quotes, backslashes, and
non-ASCII text when an unrelated config update rewrites existing values.
Quote values containing Git comment delimiters (# and ;) or leading/trailing
whitespace so Git does not truncate or trim their data. Escape LF, tab,
backspace, quote, and backslash, while rejecting carriage returns and NULs
before opening the destination.
Regression coverage round-trips these values through both GitPython and git
config and verifies unsafe control characters cannot alter the original file.
Behavior follows Git config.c parse_value() and write_pair().
Assisted-by: GPT 5.6
Co-authored-by: GPT 5.6 <codex@openai.com>

Author: Byron

git/config.py

@@ -462,7 +462,8 @@ def string_decode(v: str) -> str:
 v=v[:-1]
 # END cut trailing escapes to prevent decode error
 
-returnv.encode(defenc).decode("unicode_escape")
+escapes= {"b": "\b", "n": "\n", "r": "\r", "t": "\t", '"': '"', "\\": "\\"}
+returnre.sub(r"\\(.)", lambdamatch: 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:])
-elifoptval.find("\\", 1, -1) ==-1andoptval.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.
+elifre.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)
@@ -706,7 +709,7 @@ def write_section(name: str, section_dict: _OMD) -> None:
 
 forvinvalues:
 value=self._value_to_string(v)
-ifany(charinvalueforcharin'\n\t\b\\"'):
+ifany(charinvalueforcharin'\n\t\b\\"#;') orvalue[:1].isspace() orvalue[-1:].isspace():
 value=value.replace("\\", "\\\\").replace('"', '\\"')
 value='"%s\\\n"'%value.replace("\n", "\\n").replace("\t", "\\t").replace("\b", "\\b")
 fp.write(("\t%s = %s\n"% (key, value)).encode(defenc))
@@ -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_sectioninself._sections.items():
+sections.append(stored_section)
+forsectioninsections:
+forkey, valuesinsection.items_all():
+ifkey!="__name__":
+forraw_valueinvalues:
+if"\r"inself._value_to_string(raw_value) or"\x00"inself._value_to_string(raw_value):
+raiseValueError("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.

test/test_config.py

@@ -14,6 +14,7 @@
 importpytest
 
 fromgitimportGitConfigParser
+fromgit.compatimportdefenc
 fromgit.configimport_OMD, cp
 fromgit.utilimportcwd, rmfile
 fromtest.libimportSkipTest, TestCase, fixture_path, with_rw_directory
@@ -170,7 +171,16 @@ def test_rewriting_multiline_value_does_not_create_option(self, rw_dir):
 @with_rw_directory
 deftest_writer_escapes_special_characters_without_newline(self, rw_dir):
 config_path=osp.join(rw_dir, "config")
-values= {"tab": "\tvalue\t", "backspace": "a\bb", "quote": 'a"b', "backslash": "a\\qb"}
+values= {
+"tab": "\tvalue\t",
+"backspace": "a\bb",
+"quote": 'a"b',
+"backslash": "a\\qb",
+"hash": "value#fragment",
+"semicolon": "value;fragment",
+"leading": " value",
+"trailing": "value ",
+ }
 
 withGitConfigParser(config_path, read_only=False) asgit_config:
 forkey, valueinvalues.items():
@@ -185,11 +195,72 @@ def test_writer_escapes_special_characters_without_newline(self, rw_dir):
 stdout=subprocess.PIPE,
 check=True,
  ).stdout,
-value.encode() +b"\n",
+value.encode(defenc) +b"\n",
  )
 withopen(config_path, "rb") asconfig_file:
 self.assertNotIn(b"\x08", config_file.read())
 
+@with_rw_directory
+deftest_writer_preserves_escaped_and_non_ascii_values_safely(self, rw_dir):
+config_path=osp.join(rw_dir, "config")
+withopen(config_path, "wb") asconfig_file:
+config_file.write(
+ (
+'[section]\nnewline = "first\\nsecond"\nquote = "a\\"b"\nbackslash = "a\\\\b"\n'
+'unicode = "café\\\\path"\n'
+ ).encode(defenc)
+ )
+
+withGitConfigParser(config_path, read_only=False) asconfig:
+config.set_value("unrelated", "key", "value")
+
+expected= {
+"newline": "first\nsecond",
+"quote": 'a"b',
+"backslash": "a\\b",
+"unicode": "café\\path",
+ }
+withGitConfigParser(config_path, read_only=True) asconfig:
+forkey, valueinexpected.items():
+self.assertEqual(
+config.get_value("section", key),
+value,
+"GitPython should preserve values when rewriting unrelated entries",
+ )
+self.assertEqual(
+subprocess.run(
+ ["git", "config", "--file", config_path, "--get", "section.%s"%key],
+stdout=subprocess.PIPE,
+check=True,
+ ).stdout,
+value.encode(defenc) +b"\n",
+"git should read rewritten values with the same semantics",
+ )
+
+withopen(config_path, "rb") asconfig_file:
+contents=config_file.read()
+self.assertNotIn(b"\r", contents, "the writer should never emit carriage returns")
+self.assertNotIn(b"\x00", contents, "the writer should never emit NUL bytes")
+
+forname, valuein (("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'
+withopen(unsafe_path, "wb") asconfig_file:
+config_file.write(unsafe_contents)
+withself.assertRaisesRegex(
+ValueError,
+"CR or NUL",
+msg="unsafe existing values should abort rewrites",
+ ):
+withGitConfigParser(unsafe_path, read_only=False) asconfig:
+config.set_value("unrelated", "key", "value")
+withopen(unsafe_path, "rb") asconfig_file:
+self.assertEqual(
+config_file.read(),
+unsafe_contents,
+"rejected rewrites should leave the original file unchanged",
+ )
+
 @with_rw_directory
 deftest_set_value_rejects_config_injection(self, rw_dir):
 config_path=osp.join(rw_dir, "config")
@@ -745,15 +816,14 @@ def test_config_with_quotes_with_whitespace_outside_value(self):
 self.assertEqual(cr.get("init", "defaultBranch"), "trunk")
 
 deftest_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"')

git/config.py

@@ -462,7 +462,8 @@ def string_decode(v: str) -> str:
 v=v[:-1]
 # END cut trailing escapes to prevent decode error
 
-returnv.encode(defenc).decode("unicode_escape")
+escapes= {"b": "\b", "n": "\n", "r": "\r", "t": "\t", '"': '"', "\\": "\\"}
+returnre.sub(r"\\(.)", lambdamatch: 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:])
-elifoptval.find("\\", 1, -1) ==-1andoptval.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.
+elifre.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)
@@ -706,7 +709,7 @@ def write_section(name: str, section_dict: _OMD) -> None:
 
 forvinvalues:
 value=self._value_to_string(v)
-ifany(charinvalueforcharin'\n\t\b\\"'):
+ifany(charinvalueforcharin'\n\t\b\\"#;') orvalue[:1].isspace() orvalue[-1:].isspace():
 value=value.replace("\\", "\\\\").replace('"', '\\"')
 value='"%s\\\n"'%value.replace("\n", "\\n").replace("\t", "\\t").replace("\b", "\\b")
 fp.write(("\t%s = %s\n"% (key, value)).encode(defenc))
@@ -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_sectioninself._sections.items():
+sections.append(stored_section)
+forsectioninsections:
+forkey, valuesinsection.items_all():
+ifkey!="__name__":
+forraw_valueinvalues:
+if"\r"inself._value_to_string(raw_value) or"\x00"inself._value_to_string(raw_value):
+raiseValueError("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.

test/test_config.py

@@ -14,6 +14,7 @@
 importpytest
 
 fromgitimportGitConfigParser
+fromgit.compatimportdefenc
 fromgit.configimport_OMD, cp
 fromgit.utilimportcwd, rmfile
 fromtest.libimportSkipTest, TestCase, fixture_path, with_rw_directory
@@ -170,7 +171,16 @@ def test_rewriting_multiline_value_does_not_create_option(self, rw_dir):
 @with_rw_directory
 deftest_writer_escapes_special_characters_without_newline(self, rw_dir):
 config_path=osp.join(rw_dir, "config")
-values= {"tab": "\tvalue\t", "backspace": "a\bb", "quote": 'a"b', "backslash": "a\\qb"}
+values= {
+"tab": "\tvalue\t",
+"backspace": "a\bb",
+"quote": 'a"b',
+"backslash": "a\\qb",
+"hash": "value#fragment",
+"semicolon": "value;fragment",
+"leading": " value",
+"trailing": "value ",
+ }
 
 withGitConfigParser(config_path, read_only=False) asgit_config:
 forkey, valueinvalues.items():
@@ -185,11 +195,72 @@ def test_writer_escapes_special_characters_without_newline(self, rw_dir):
 stdout=subprocess.PIPE,
 check=True,
  ).stdout,
-value.encode() +b"\n",
+value.encode(defenc) +b"\n",
  )
 withopen(config_path, "rb") asconfig_file:
 self.assertNotIn(b"\x08", config_file.read())
 
+@with_rw_directory
+deftest_writer_preserves_escaped_and_non_ascii_values_safely(self, rw_dir):
+config_path=osp.join(rw_dir, "config")
+withopen(config_path, "wb") asconfig_file:
+config_file.write(
+ (
+'[section]\nnewline = "first\\nsecond"\nquote = "a\\"b"\nbackslash = "a\\\\b"\n'
+'unicode = "café\\\\path"\n'
+ ).encode(defenc)
+ )
+
+withGitConfigParser(config_path, read_only=False) asconfig:
+config.set_value("unrelated", "key", "value")
+
+expected= {
+"newline": "first\nsecond",
+"quote": 'a"b',
+"backslash": "a\\b",
+"unicode": "café\\path",
+ }
+withGitConfigParser(config_path, read_only=True) asconfig:
+forkey, valueinexpected.items():
+self.assertEqual(
+config.get_value("section", key),
+value,
+"GitPython should preserve values when rewriting unrelated entries",
+ )
+self.assertEqual(
+subprocess.run(
+ ["git", "config", "--file", config_path, "--get", "section.%s"%key],
+stdout=subprocess.PIPE,
+check=True,
+ ).stdout,
+value.encode(defenc) +b"\n",
+"git should read rewritten values with the same semantics",
+ )
+
+withopen(config_path, "rb") asconfig_file:
+contents=config_file.read()
+self.assertNotIn(b"\r", contents, "the writer should never emit carriage returns")
+self.assertNotIn(b"\x00", contents, "the writer should never emit NUL bytes")
+
+forname, valuein (("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'
+withopen(unsafe_path, "wb") asconfig_file:
+config_file.write(unsafe_contents)
+withself.assertRaisesRegex(
+ValueError,
+"CR or NUL",
+msg="unsafe existing values should abort rewrites",
+ ):
+withGitConfigParser(unsafe_path, read_only=False) asconfig:
+config.set_value("unrelated", "key", "value")
+withopen(unsafe_path, "rb") asconfig_file:
+self.assertEqual(
+config_file.read(),
+unsafe_contents,
+"rejected rewrites should leave the original file unchanged",
+ )
+
 @with_rw_directory
 deftest_set_value_rejects_config_injection(self, rw_dir):
 config_path=osp.join(rw_dir, "config")
@@ -745,15 +816,14 @@ def test_config_with_quotes_with_whitespace_outside_value(self):
 self.assertEqual(cr.get("init", "defaultBranch"), "trunk")
 
 deftest_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"')

git/config.py

@@ -462,7 +462,8 @@ def string_decode(v: str) -> str:
 v=v[:-1]
 # END cut trailing escapes to prevent decode error
 
-returnv.encode(defenc).decode("unicode_escape")
+escapes= {"b": "\b", "n": "\n", "r": "\r", "t": "\t", '"': '"', "\\": "\\"}
+returnre.sub(r"\\(.)", lambdamatch: 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:])
-elifoptval.find("\\", 1, -1) ==-1andoptval.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.
+elifre.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)
@@ -706,7 +709,7 @@ def write_section(name: str, section_dict: _OMD) -> None:
 
 forvinvalues:
 value=self._value_to_string(v)
-ifany(charinvalueforcharin'\n\t\b\\"'):
+ifany(charinvalueforcharin'\n\t\b\\"#;') orvalue[:1].isspace() orvalue[-1:].isspace():
 value=value.replace("\\", "\\\\").replace('"', '\\"')
 value='"%s\\\n"'%value.replace("\n", "\\n").replace("\t", "\\t").replace("\b", "\\b")
 fp.write(("\t%s = %s\n"% (key, value)).encode(defenc))
@@ -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_sectioninself._sections.items():
+sections.append(stored_section)
+forsectioninsections:
+forkey, valuesinsection.items_all():
+ifkey!="__name__":
+forraw_valueinvalues:
+if"\r"inself._value_to_string(raw_value) or"\x00"inself._value_to_string(raw_value):
+raiseValueError("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.

test/test_config.py

@@ -14,6 +14,7 @@
 importpytest
 
 fromgitimportGitConfigParser
+fromgit.compatimportdefenc
 fromgit.configimport_OMD, cp
 fromgit.utilimportcwd, rmfile
 fromtest.libimportSkipTest, TestCase, fixture_path, with_rw_directory
@@ -170,7 +171,16 @@ def test_rewriting_multiline_value_does_not_create_option(self, rw_dir):
 @with_rw_directory
 deftest_writer_escapes_special_characters_without_newline(self, rw_dir):
 config_path=osp.join(rw_dir, "config")
-values= {"tab": "\tvalue\t", "backspace": "a\bb", "quote": 'a"b', "backslash": "a\\qb"}
+values= {
+"tab": "\tvalue\t",
+"backspace": "a\bb",
+"quote": 'a"b',
+"backslash": "a\\qb",
+"hash": "value#fragment",
+"semicolon": "value;fragment",
+"leading": " value",
+"trailing": "value ",
+ }
 
 withGitConfigParser(config_path, read_only=False) asgit_config:
 forkey, valueinvalues.items():
@@ -185,11 +195,72 @@ def test_writer_escapes_special_characters_without_newline(self, rw_dir):
 stdout=subprocess.PIPE,
 check=True,
  ).stdout,
-value.encode() +b"\n",
+value.encode(defenc) +b"\n",
  )
 withopen(config_path, "rb") asconfig_file:
 self.assertNotIn(b"\x08", config_file.read())
 
+@with_rw_directory
+deftest_writer_preserves_escaped_and_non_ascii_values_safely(self, rw_dir):
+config_path=osp.join(rw_dir, "config")
+withopen(config_path, "wb") asconfig_file:
+config_file.write(
+ (
+'[section]\nnewline = "first\\nsecond"\nquote = "a\\"b"\nbackslash = "a\\\\b"\n'
+'unicode = "café\\\\path"\n'
+ ).encode(defenc)
+ )
+
+withGitConfigParser(config_path, read_only=False) asconfig:
+config.set_value("unrelated", "key", "value")
+
+expected= {
+"newline": "first\nsecond",
+"quote": 'a"b',
+"backslash": "a\\b",
+"unicode": "café\\path",
+ }
+withGitConfigParser(config_path, read_only=True) asconfig:
+forkey, valueinexpected.items():
+self.assertEqual(
+config.get_value("section", key),
+value,
+"GitPython should preserve values when rewriting unrelated entries",
+ )
+self.assertEqual(
+subprocess.run(
+ ["git", "config", "--file", config_path, "--get", "section.%s"%key],
+stdout=subprocess.PIPE,
+check=True,
+ ).stdout,
+value.encode(defenc) +b"\n",
+"git should read rewritten values with the same semantics",
+ )
+
+withopen(config_path, "rb") asconfig_file:
+contents=config_file.read()
+self.assertNotIn(b"\r", contents, "the writer should never emit carriage returns")
+self.assertNotIn(b"\x00", contents, "the writer should never emit NUL bytes")
+
+forname, valuein (("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'
+withopen(unsafe_path, "wb") asconfig_file:
+config_file.write(unsafe_contents)
+withself.assertRaisesRegex(
+ValueError,
+"CR or NUL",
+msg="unsafe existing values should abort rewrites",
+ ):
+withGitConfigParser(unsafe_path, read_only=False) asconfig:
+config.set_value("unrelated", "key", "value")
+withopen(unsafe_path, "rb") asconfig_file:
+self.assertEqual(
+config_file.read(),
+unsafe_contents,
+"rejected rewrites should leave the original file unchanged",
+ )
+
 @with_rw_directory
 deftest_set_value_rejects_config_injection(self, rw_dir):
 config_path=osp.join(rw_dir, "config")
@@ -745,15 +816,14 @@ def test_config_with_quotes_with_whitespace_outside_value(self):
 self.assertEqual(cr.get("init", "defaultBranch"), "trunk")
 
 deftest_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"')

git/config.py

@@ -462,7 +462,8 @@ def string_decode(v: str) -> str:
 v=v[:-1]
 # END cut trailing escapes to prevent decode error
 
-returnv.encode(defenc).decode("unicode_escape")
+escapes= {"b": "\b", "n": "\n", "r": "\r", "t": "\t", '"': '"', "\\": "\\"}
+returnre.sub(r"\\(.)", lambdamatch: 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:])
-elifoptval.find("\\", 1, -1) ==-1andoptval.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.
+elifre.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)
@@ -706,7 +709,7 @@ def write_section(name: str, section_dict: _OMD) -> None:
 
 forvinvalues:
 value=self._value_to_string(v)
-ifany(charinvalueforcharin'\n\t\b\\"'):
+ifany(charinvalueforcharin'\n\t\b\\"#;') orvalue[:1].isspace() orvalue[-1:].isspace():
 value=value.replace("\\", "\\\\").replace('"', '\\"')
 value='"%s\\\n"'%value.replace("\n", "\\n").replace("\t", "\\t").replace("\b", "\\b")
 fp.write(("\t%s = %s\n"% (key, value)).encode(defenc))
@@ -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_sectioninself._sections.items():
+sections.append(stored_section)
+forsectioninsections:
+forkey, valuesinsection.items_all():
+ifkey!="__name__":
+forraw_valueinvalues:
+if"\r"inself._value_to_string(raw_value) or"\x00"inself._value_to_string(raw_value):
+raiseValueError("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.

test/test_config.py

@@ -14,6 +14,7 @@
 importpytest
 
 fromgitimportGitConfigParser
+fromgit.compatimportdefenc
 fromgit.configimport_OMD, cp
 fromgit.utilimportcwd, rmfile
 fromtest.libimportSkipTest, TestCase, fixture_path, with_rw_directory
@@ -170,7 +171,16 @@ def test_rewriting_multiline_value_does_not_create_option(self, rw_dir):
 @with_rw_directory
 deftest_writer_escapes_special_characters_without_newline(self, rw_dir):
 config_path=osp.join(rw_dir, "config")
-values= {"tab": "\tvalue\t", "backspace": "a\bb", "quote": 'a"b', "backslash": "a\\qb"}
+values= {
+"tab": "\tvalue\t",
+"backspace": "a\bb",
+"quote": 'a"b',
+"backslash": "a\\qb",
+"hash": "value#fragment",
+"semicolon": "value;fragment",
+"leading": " value",
+"trailing": "value ",
+ }
 
 withGitConfigParser(config_path, read_only=False) asgit_config:
 forkey, valueinvalues.items():
@@ -185,11 +195,72 @@ def test_writer_escapes_special_characters_without_newline(self, rw_dir):
 stdout=subprocess.PIPE,
 check=True,
  ).stdout,
-value.encode() +b"\n",
+value.encode(defenc) +b"\n",
  )
 withopen(config_path, "rb") asconfig_file:
 self.assertNotIn(b"\x08", config_file.read())
 
+@with_rw_directory
+deftest_writer_preserves_escaped_and_non_ascii_values_safely(self, rw_dir):
+config_path=osp.join(rw_dir, "config")
+withopen(config_path, "wb") asconfig_file:
+config_file.write(
+ (
+'[section]\nnewline = "first\\nsecond"\nquote = "a\\"b"\nbackslash = "a\\\\b"\n'
+'unicode = "café\\\\path"\n'
+ ).encode(defenc)
+ )
+
+withGitConfigParser(config_path, read_only=False) asconfig:
+config.set_value("unrelated", "key", "value")
+
+expected= {
+"newline": "first\nsecond",
+"quote": 'a"b',
+"backslash": "a\\b",
+"unicode": "café\\path",
+ }
+withGitConfigParser(config_path, read_only=True) asconfig:
+forkey, valueinexpected.items():
+self.assertEqual(
+config.get_value("section", key),
+value,
+"GitPython should preserve values when rewriting unrelated entries",
+ )
+self.assertEqual(
+subprocess.run(
+ ["git", "config", "--file", config_path, "--get", "section.%s"%key],
+stdout=subprocess.PIPE,
+check=True,
+ ).stdout,
+value.encode(defenc) +b"\n",
+"git should read rewritten values with the same semantics",
+ )
+
+withopen(config_path, "rb") asconfig_file:
+contents=config_file.read()
+self.assertNotIn(b"\r", contents, "the writer should never emit carriage returns")
+self.assertNotIn(b"\x00", contents, "the writer should never emit NUL bytes")
+
+forname, valuein (("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'
+withopen(unsafe_path, "wb") asconfig_file:
+config_file.write(unsafe_contents)
+withself.assertRaisesRegex(
+ValueError,
+"CR or NUL",
+msg="unsafe existing values should abort rewrites",
+ ):
+withGitConfigParser(unsafe_path, read_only=False) asconfig:
+config.set_value("unrelated", "key", "value")
+withopen(unsafe_path, "rb") asconfig_file:
+self.assertEqual(
+config_file.read(),
+unsafe_contents,
+"rejected rewrites should leave the original file unchanged",
+ )
+
 @with_rw_directory
 deftest_set_value_rejects_config_injection(self, rw_dir):
 config_path=osp.join(rw_dir, "config")
@@ -745,15 +816,14 @@ def test_config_with_quotes_with_whitespace_outside_value(self):
 self.assertEqual(cr.get("init", "defaultBranch"), "trunk")
 
 deftest_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"')

git/config.py

@@ -462,7 +462,8 @@ def string_decode(v: str) -> str:
 v=v[:-1]
 # END cut trailing escapes to prevent decode error
 
-returnv.encode(defenc).decode("unicode_escape")
+escapes= {"b": "\b", "n": "\n", "r": "\r", "t": "\t", '"': '"', "\\": "\\"}
+returnre.sub(r"\\(.)", lambdamatch: 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:])
-elifoptval.find("\\", 1, -1) ==-1andoptval.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.
+elifre.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)
@@ -706,7 +709,7 @@ def write_section(name: str, section_dict: _OMD) -> None:
 
 forvinvalues:
 value=self._value_to_string(v)
-ifany(charinvalueforcharin'\n\t\b\\"'):
+ifany(charinvalueforcharin'\n\t\b\\"#;') orvalue[:1].isspace() orvalue[-1:].isspace():
 value=value.replace("\\", "\\\\").replace('"', '\\"')
 value='"%s\\\n"'%value.replace("\n", "\\n").replace("\t", "\\t").replace("\b", "\\b")
 fp.write(("\t%s = %s\n"% (key, value)).encode(defenc))
@@ -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_sectioninself._sections.items():
+sections.append(stored_section)
+forsectioninsections:
+forkey, valuesinsection.items_all():
+ifkey!="__name__":
+forraw_valueinvalues:
+if"\r"inself._value_to_string(raw_value) or"\x00"inself._value_to_string(raw_value):
+raiseValueError("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.

test/test_config.py

@@ -14,6 +14,7 @@
 importpytest
 
 fromgitimportGitConfigParser
+fromgit.compatimportdefenc
 fromgit.configimport_OMD, cp
 fromgit.utilimportcwd, rmfile
 fromtest.libimportSkipTest, TestCase, fixture_path, with_rw_directory
@@ -170,7 +171,16 @@ def test_rewriting_multiline_value_does_not_create_option(self, rw_dir):
 @with_rw_directory
 deftest_writer_escapes_special_characters_without_newline(self, rw_dir):
 config_path=osp.join(rw_dir, "config")
-values= {"tab": "\tvalue\t", "backspace": "a\bb", "quote": 'a"b', "backslash": "a\\qb"}
+values= {
+"tab": "\tvalue\t",
+"backspace": "a\bb",
+"quote": 'a"b',
+"backslash": "a\\qb",
+"hash": "value#fragment",
+"semicolon": "value;fragment",
+"leading": " value",
+"trailing": "value ",
+ }
 
 withGitConfigParser(config_path, read_only=False) asgit_config:
 forkey, valueinvalues.items():
@@ -185,11 +195,72 @@ def test_writer_escapes_special_characters_without_newline(self, rw_dir):
 stdout=subprocess.PIPE,
 check=True,
  ).stdout,
-value.encode() +b"\n",
+value.encode(defenc) +b"\n",
  )
 withopen(config_path, "rb") asconfig_file:
 self.assertNotIn(b"\x08", config_file.read())
 
+@with_rw_directory
+deftest_writer_preserves_escaped_and_non_ascii_values_safely(self, rw_dir):
+config_path=osp.join(rw_dir, "config")
+withopen(config_path, "wb") asconfig_file:
+config_file.write(
+ (
+'[section]\nnewline = "first\\nsecond"\nquote = "a\\"b"\nbackslash = "a\\\\b"\n'
+'unicode = "café\\\\path"\n'
+ ).encode(defenc)
+ )
+
+withGitConfigParser(config_path, read_only=False) asconfig:
+config.set_value("unrelated", "key", "value")
+
+expected= {
+"newline": "first\nsecond",
+"quote": 'a"b',
+"backslash": "a\\b",
+"unicode": "café\\path",
+ }
+withGitConfigParser(config_path, read_only=True) asconfig:
+forkey, valueinexpected.items():
+self.assertEqual(
+config.get_value("section", key),
+value,
+"GitPython should preserve values when rewriting unrelated entries",
+ )
+self.assertEqual(
+subprocess.run(
+ ["git", "config", "--file", config_path, "--get", "section.%s"%key],
+stdout=subprocess.PIPE,
+check=True,
+ ).stdout,
+value.encode(defenc) +b"\n",
+"git should read rewritten values with the same semantics",
+ )
+
+withopen(config_path, "rb") asconfig_file:
+contents=config_file.read()
+self.assertNotIn(b"\r", contents, "the writer should never emit carriage returns")
+self.assertNotIn(b"\x00", contents, "the writer should never emit NUL bytes")
+
+forname, valuein (("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'
+withopen(unsafe_path, "wb") asconfig_file:
+config_file.write(unsafe_contents)
+withself.assertRaisesRegex(
+ValueError,
+"CR or NUL",
+msg="unsafe existing values should abort rewrites",
+ ):
+withGitConfigParser(unsafe_path, read_only=False) asconfig:
+config.set_value("unrelated", "key", "value")
+withopen(unsafe_path, "rb") asconfig_file:
+self.assertEqual(
+config_file.read(),
+unsafe_contents,
+"rejected rewrites should leave the original file unchanged",
+ )
+
 @with_rw_directory
 deftest_set_value_rejects_config_injection(self, rw_dir):
 config_path=osp.join(rw_dir, "config")
@@ -745,15 +816,14 @@ def test_config_with_quotes_with_whitespace_outside_value(self):
 self.assertEqual(cr.get("init", "defaultBranch"), "trunk")
 
 deftest_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"')

git/config.py

@@ -462,7 +462,8 @@ def string_decode(v: str) -> str:
 v=v[:-1]
 # END cut trailing escapes to prevent decode error
 
-returnv.encode(defenc).decode("unicode_escape")
+escapes= {"b": "\b", "n": "\n", "r": "\r", "t": "\t", '"': '"', "\\": "\\"}
+returnre.sub(r"\\(.)", lambdamatch: 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:])
-elifoptval.find("\\", 1, -1) ==-1andoptval.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.
+elifre.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)
@@ -706,7 +709,7 @@ def write_section(name: str, section_dict: _OMD) -> None:
 
 forvinvalues:
 value=self._value_to_string(v)
-ifany(charinvalueforcharin'\n\t\b\\"'):
+ifany(charinvalueforcharin'\n\t\b\\"#;') orvalue[:1].isspace() orvalue[-1:].isspace():
 value=value.replace("\\", "\\\\").replace('"', '\\"')
 value='"%s\\\n"'%value.replace("\n", "\\n").replace("\t", "\\t").replace("\b", "\\b")
 fp.write(("\t%s = %s\n"% (key, value)).encode(defenc))
@@ -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_sectioninself._sections.items():
+sections.append(stored_section)
+forsectioninsections:
+forkey, valuesinsection.items_all():
+ifkey!="__name__":
+forraw_valueinvalues:
+if"\r"inself._value_to_string(raw_value) or"\x00"inself._value_to_string(raw_value):
+raiseValueError("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.

test/test_config.py

@@ -14,6 +14,7 @@
 importpytest
 
 fromgitimportGitConfigParser
+fromgit.compatimportdefenc
 fromgit.configimport_OMD, cp
 fromgit.utilimportcwd, rmfile
 fromtest.libimportSkipTest, TestCase, fixture_path, with_rw_directory
@@ -170,7 +171,16 @@ def test_rewriting_multiline_value_does_not_create_option(self, rw_dir):
 @with_rw_directory
 deftest_writer_escapes_special_characters_without_newline(self, rw_dir):
 config_path=osp.join(rw_dir, "config")
-values= {"tab": "\tvalue\t", "backspace": "a\bb", "quote": 'a"b', "backslash": "a\\qb"}
+values= {
+"tab": "\tvalue\t",
+"backspace": "a\bb",
+"quote": 'a"b',
+"backslash": "a\\qb",
+"hash": "value#fragment",
+"semicolon": "value;fragment",
+"leading": " value",
+"trailing": "value ",
+ }
 
 withGitConfigParser(config_path, read_only=False) asgit_config:
 forkey, valueinvalues.items():
@@ -185,11 +195,72 @@ def test_writer_escapes_special_characters_without_newline(self, rw_dir):
 stdout=subprocess.PIPE,
 check=True,
  ).stdout,
-value.encode() +b"\n",
+value.encode(defenc) +b"\n",
  )
 withopen(config_path, "rb") asconfig_file:
 self.assertNotIn(b"\x08", config_file.read())
 
+@with_rw_directory
+deftest_writer_preserves_escaped_and_non_ascii_values_safely(self, rw_dir):
+config_path=osp.join(rw_dir, "config")
+withopen(config_path, "wb") asconfig_file:
+config_file.write(
+ (
+'[section]\nnewline = "first\\nsecond"\nquote = "a\\"b"\nbackslash = "a\\\\b"\n'
+'unicode = "café\\\\path"\n'
+ ).encode(defenc)
+ )
+
+withGitConfigParser(config_path, read_only=False) asconfig:
+config.set_value("unrelated", "key", "value")
+
+expected= {
+"newline": "first\nsecond",
+"quote": 'a"b',
+"backslash": "a\\b",
+"unicode": "café\\path",
+ }
+withGitConfigParser(config_path, read_only=True) asconfig:
+forkey, valueinexpected.items():
+self.assertEqual(
+config.get_value("section", key),
+value,
+"GitPython should preserve values when rewriting unrelated entries",
+ )
+self.assertEqual(
+subprocess.run(
+ ["git", "config", "--file", config_path, "--get", "section.%s"%key],
+stdout=subprocess.PIPE,
+check=True,
+ ).stdout,
+value.encode(defenc) +b"\n",
+"git should read rewritten values with the same semantics",
+ )
+
+withopen(config_path, "rb") asconfig_file:
+contents=config_file.read()
+self.assertNotIn(b"\r", contents, "the writer should never emit carriage returns")
+self.assertNotIn(b"\x00", contents, "the writer should never emit NUL bytes")
+
+forname, valuein (("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'
+withopen(unsafe_path, "wb") asconfig_file:
+config_file.write(unsafe_contents)
+withself.assertRaisesRegex(
+ValueError,
+"CR or NUL",
+msg="unsafe existing values should abort rewrites",
+ ):
+withGitConfigParser(unsafe_path, read_only=False) asconfig:
+config.set_value("unrelated", "key", "value")
+withopen(unsafe_path, "rb") asconfig_file:
+self.assertEqual(
+config_file.read(),
+unsafe_contents,
+"rejected rewrites should leave the original file unchanged",
+ )
+
 @with_rw_directory
 deftest_set_value_rejects_config_injection(self, rw_dir):
 config_path=osp.join(rw_dir, "config")
@@ -745,15 +816,14 @@ def test_config_with_quotes_with_whitespace_outside_value(self):
 self.assertEqual(cr.get("init", "defaultBranch"), "trunk")
 
 deftest_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"')

git/config.py

@@ -462,7 +462,8 @@ def string_decode(v: str) -> str:
 v=v[:-1]
 # END cut trailing escapes to prevent decode error
 
-returnv.encode(defenc).decode("unicode_escape")
+escapes= {"b": "\b", "n": "\n", "r": "\r", "t": "\t", '"': '"', "\\": "\\"}
+returnre.sub(r"\\(.)", lambdamatch: 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:])
-elifoptval.find("\\", 1, -1) ==-1andoptval.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.
+elifre.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)
@@ -706,7 +709,7 @@ def write_section(name: str, section_dict: _OMD) -> None:
 
 forvinvalues:
 value=self._value_to_string(v)
-ifany(charinvalueforcharin'\n\t\b\\"'):
+ifany(charinvalueforcharin'\n\t\b\\"#;') orvalue[:1].isspace() orvalue[-1:].isspace():
 value=value.replace("\\", "\\\\").replace('"', '\\"')
 value='"%s\\\n"'%value.replace("\n", "\\n").replace("\t", "\\t").replace("\b", "\\b")
 fp.write(("\t%s = %s\n"% (key, value)).encode(defenc))
@@ -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_sectioninself._sections.items():
+sections.append(stored_section)
+forsectioninsections:
+forkey, valuesinsection.items_all():
+ifkey!="__name__":
+forraw_valueinvalues:
+if"\r"inself._value_to_string(raw_value) or"\x00"inself._value_to_string(raw_value):
+raiseValueError("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.

test/test_config.py

@@ -14,6 +14,7 @@
 importpytest
 
 fromgitimportGitConfigParser
+fromgit.compatimportdefenc
 fromgit.configimport_OMD, cp
 fromgit.utilimportcwd, rmfile
 fromtest.libimportSkipTest, TestCase, fixture_path, with_rw_directory
@@ -170,7 +171,16 @@ def test_rewriting_multiline_value_does_not_create_option(self, rw_dir):
 @with_rw_directory
 deftest_writer_escapes_special_characters_without_newline(self, rw_dir):
 config_path=osp.join(rw_dir, "config")
-values= {"tab": "\tvalue\t", "backspace": "a\bb", "quote": 'a"b', "backslash": "a\\qb"}
+values= {
+"tab": "\tvalue\t",
+"backspace": "a\bb",
+"quote": 'a"b',
+"backslash": "a\\qb",
+"hash": "value#fragment",
+"semicolon": "value;fragment",
+"leading": " value",
+"trailing": "value ",
+ }
 
 withGitConfigParser(config_path, read_only=False) asgit_config:
 forkey, valueinvalues.items():
@@ -185,11 +195,72 @@ def test_writer_escapes_special_characters_without_newline(self, rw_dir):
 stdout=subprocess.PIPE,
 check=True,
  ).stdout,
-value.encode() +b"\n",
+value.encode(defenc) +b"\n",
  )
 withopen(config_path, "rb") asconfig_file:
 self.assertNotIn(b"\x08", config_file.read())
 
+@with_rw_directory
+deftest_writer_preserves_escaped_and_non_ascii_values_safely(self, rw_dir):
+config_path=osp.join(rw_dir, "config")
+withopen(config_path, "wb") asconfig_file:
+config_file.write(
+ (
+'[section]\nnewline = "first\\nsecond"\nquote = "a\\"b"\nbackslash = "a\\\\b"\n'
+'unicode = "café\\\\path"\n'
+ ).encode(defenc)
+ )
+
+withGitConfigParser(config_path, read_only=False) asconfig:
+config.set_value("unrelated", "key", "value")
+
+expected= {
+"newline": "first\nsecond",
+"quote": 'a"b',
+"backslash": "a\\b",
+"unicode": "café\\path",
+ }
+withGitConfigParser(config_path, read_only=True) asconfig:
+forkey, valueinexpected.items():
+self.assertEqual(
+config.get_value("section", key),
+value,
+"GitPython should preserve values when rewriting unrelated entries",
+ )
+self.assertEqual(
+subprocess.run(
+ ["git", "config", "--file", config_path, "--get", "section.%s"%key],
+stdout=subprocess.PIPE,
+check=True,
+ ).stdout,
+value.encode(defenc) +b"\n",
+"git should read rewritten values with the same semantics",
+ )
+
+withopen(config_path, "rb") asconfig_file:
+contents=config_file.read()
+self.assertNotIn(b"\r", contents, "the writer should never emit carriage returns")
+self.assertNotIn(b"\x00", contents, "the writer should never emit NUL bytes")
+
+forname, valuein (("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'
+withopen(unsafe_path, "wb") asconfig_file:
+config_file.write(unsafe_contents)
+withself.assertRaisesRegex(
+ValueError,
+"CR or NUL",
+msg="unsafe existing values should abort rewrites",
+ ):
+withGitConfigParser(unsafe_path, read_only=False) asconfig:
+config.set_value("unrelated", "key", "value")
+withopen(unsafe_path, "rb") asconfig_file:
+self.assertEqual(
+config_file.read(),
+unsafe_contents,
+"rejected rewrites should leave the original file unchanged",
+ )
+
 @with_rw_directory
 deftest_set_value_rejects_config_injection(self, rw_dir):
 config_path=osp.join(rw_dir, "config")
@@ -745,15 +816,14 @@ def test_config_with_quotes_with_whitespace_outside_value(self):
 self.assertEqual(cr.get("init", "defaultBranch"), "trunk")
 
 deftest_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"')

git/config.py

@@ -462,7 +462,8 @@ def string_decode(v: str) -> str:
 v=v[:-1]
 # END cut trailing escapes to prevent decode error
 
-returnv.encode(defenc).decode("unicode_escape")
+escapes= {"b": "\b", "n": "\n", "r": "\r", "t": "\t", '"': '"', "\\": "\\"}
+returnre.sub(r"\\(.)", lambdamatch: 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:])
-elifoptval.find("\\", 1, -1) ==-1andoptval.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.
+elifre.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)
@@ -706,7 +709,7 @@ def write_section(name: str, section_dict: _OMD) -> None:
 
 forvinvalues:
 value=self._value_to_string(v)
-ifany(charinvalueforcharin'\n\t\b\\"'):
+ifany(charinvalueforcharin'\n\t\b\\"#;') orvalue[:1].isspace() orvalue[-1:].isspace():
 value=value.replace("\\", "\\\\").replace('"', '\\"')
 value='"%s\\\n"'%value.replace("\n", "\\n").replace("\t", "\\t").replace("\b", "\\b")
 fp.write(("\t%s = %s\n"% (key, value)).encode(defenc))
@@ -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_sectioninself._sections.items():
+sections.append(stored_section)
+forsectioninsections:
+forkey, valuesinsection.items_all():
+ifkey!="__name__":
+forraw_valueinvalues:
+if"\r"inself._value_to_string(raw_value) or"\x00"inself._value_to_string(raw_value):
+raiseValueError("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.

test/test_config.py

@@ -14,6 +14,7 @@
 importpytest
 
 fromgitimportGitConfigParser
+fromgit.compatimportdefenc
 fromgit.configimport_OMD, cp
 fromgit.utilimportcwd, rmfile
 fromtest.libimportSkipTest, TestCase, fixture_path, with_rw_directory
@@ -170,7 +171,16 @@ def test_rewriting_multiline_value_does_not_create_option(self, rw_dir):
 @with_rw_directory
 deftest_writer_escapes_special_characters_without_newline(self, rw_dir):
 config_path=osp.join(rw_dir, "config")
-values= {"tab": "\tvalue\t", "backspace": "a\bb", "quote": 'a"b', "backslash": "a\\qb"}
+values= {
+"tab": "\tvalue\t",
+"backspace": "a\bb",
+"quote": 'a"b',
+"backslash": "a\\qb",
+"hash": "value#fragment",
+"semicolon": "value;fragment",
+"leading": " value",
+"trailing": "value ",
+ }
 
 withGitConfigParser(config_path, read_only=False) asgit_config:
 forkey, valueinvalues.items():
@@ -185,11 +195,72 @@ def test_writer_escapes_special_characters_without_newline(self, rw_dir):
 stdout=subprocess.PIPE,
 check=True,
  ).stdout,
-value.encode() +b"\n",
+value.encode(defenc) +b"\n",
  )
 withopen(config_path, "rb") asconfig_file:
 self.assertNotIn(b"\x08", config_file.read())
 
+@with_rw_directory
+deftest_writer_preserves_escaped_and_non_ascii_values_safely(self, rw_dir):
+config_path=osp.join(rw_dir, "config")
+withopen(config_path, "wb") asconfig_file:
+config_file.write(
+ (
+'[section]\nnewline = "first\\nsecond"\nquote = "a\\"b"\nbackslash = "a\\\\b"\n'
+'unicode = "café\\\\path"\n'
+ ).encode(defenc)
+ )
+
+withGitConfigParser(config_path, read_only=False) asconfig:
+config.set_value("unrelated", "key", "value")
+
+expected= {
+"newline": "first\nsecond",
+"quote": 'a"b',
+"backslash": "a\\b",
+"unicode": "café\\path",
+ }
+withGitConfigParser(config_path, read_only=True) asconfig:
+forkey, valueinexpected.items():
+self.assertEqual(
+config.get_value("section", key),
+value,
+"GitPython should preserve values when rewriting unrelated entries",
+ )
+self.assertEqual(
+subprocess.run(
+ ["git", "config", "--file", config_path, "--get", "section.%s"%key],
+stdout=subprocess.PIPE,
+check=True,
+ ).stdout,
+value.encode(defenc) +b"\n",
+"git should read rewritten values with the same semantics",
+ )
+
+withopen(config_path, "rb") asconfig_file:
+contents=config_file.read()
+self.assertNotIn(b"\r", contents, "the writer should never emit carriage returns")
+self.assertNotIn(b"\x00", contents, "the writer should never emit NUL bytes")
+
+forname, valuein (("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'
+withopen(unsafe_path, "wb") asconfig_file:
+config_file.write(unsafe_contents)
+withself.assertRaisesRegex(
+ValueError,
+"CR or NUL",
+msg="unsafe existing values should abort rewrites",
+ ):
+withGitConfigParser(unsafe_path, read_only=False) asconfig:
+config.set_value("unrelated", "key", "value")
+withopen(unsafe_path, "rb") asconfig_file:
+self.assertEqual(
+config_file.read(),
+unsafe_contents,
+"rejected rewrites should leave the original file unchanged",
+ )
+
 @with_rw_directory
 deftest_set_value_rejects_config_injection(self, rw_dir):
 config_path=osp.join(rw_dir, "config")
@@ -745,15 +816,14 @@ def test_config_with_quotes_with_whitespace_outside_value(self):
 self.assertEqual(cr.get("init", "defaultBranch"), "trunk")
 
 deftest_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"')