fix: restore config parser compatibility

The Python package matrix failed across POSIX runners because a submodule test wrote dotted option names inside a submodule section, syntax that Git rejects and the new parser correctly disallows. Write the test identity through the submodule repository config instead.

Windows matrix jobs additionally failed while parsing manually assembled include files containing unescaped native paths. Serialize those test values and conditional subsection patterns with Git-compatible quoting so backslashes remain data rather than invalid escape prefixes.

Match Git value-setter behavior by accepting CR and LF, which the canonical writer can safely quote and round-trip, while continuing to reject NUL in setters and parsed files. The newline injection regression now verifies escaped data and absence of an injected section.

Author: codex

git/config.py

@@ -77,7 +77,10 @@
 """
 
 UNSAFE_CONFIG_CHARS_RE = re.compile(r"[\r\n\x00]")
-"""Characters that cannot be safely written in config names or values."""
+"""Characters that cannot be safely written in config names."""
+
+UNSAFE_CONFIG_VALUE_CHARS_RE = re.compile(r"\x00")
+"""Characters that cannot be represented in Git config values."""
 
 
 def _escape_section_subsection(value: str) -> str:
@@ -187,6 +190,8 @@ def _parse_value(self, line_number: int) -> str:
                 if quoted:
                     self._error(line_number)
                 return "".join(value)
+            if char == "\x00":
+                self._error(line_number)
             if comment:
                 continue
             if self._is_space(char) and not quoted:
@@ -984,8 +989,8 @@ def _value_to_string(self, value: Union[str, bytes, int, float, bool]) -> str:
 
     def _value_to_string_safe(self, value: Union[str, bytes, int, float, bool]) -> str:
         value_str = self._value_to_string(value)
-        if UNSAFE_CONFIG_CHARS_RE.search(value_str):
-            raise ValueError("Git config values must not contain CR, LF, or NUL")
+        if UNSAFE_CONFIG_VALUE_CHARS_RE.search(value_str):
+            raise ValueError("Git config values must not contain NUL")
         return value_str
 
     def _assure_config_name_safe(self, name: "cp._SectionName", label: str) -> None:

test/test_config.py

@@ -13,7 +13,7 @@
 import pytest
 
 from git import Git, GitConfigParser
-from git.config import _OMD, cp
+from git.config import _OMD, _escape_config_value, _escape_section_subsection, cp
 from git.util import rmfile
 
 from test.lib import SkipTest, TestCase, fixture_path, with_rw_directory
@@ -169,6 +169,7 @@ def test_git_parser_rejects_syntax_rejected_by_git(self):
             b"[core]\nkey: value\n",
             b'[core]\nkey = "unterminated\n',
             b'[core]\nkey = "bad\\q"\n',
+            b'[core]\nkey = "bad\x00value"\n',
         )
         for config_content in invalid_configs:
             with self.subTest(config_content=config_content):
@@ -236,16 +237,15 @@ def test_writer_escapes_quoted_subsection_names(self, rw_dir):
         self.assertEqual(Git().config("--file", config_path, "--get", 'submodule.docs"archive\\part].path'), "docs")
 
     @with_rw_directory
-    def test_set_value_rejects_config_injection(self, rw_dir):
+    def test_set_value_escapes_config_injection(self, rw_dir):
         config_path = osp.join(rw_dir, "config")
         payload = "foo\n[core]\nhooksPath=/tmp/hooks"
 
         with GitConfigParser(config_path, read_only=False) as git_config:
-            with pytest.raises(ValueError, match="CR, LF, or NUL"):
-                git_config.set_value("user", "name", payload)
+            git_config.set_value("user", "name", payload)
 
         with GitConfigParser(config_path, read_only=True) as git_config:
-            self.assertFalse(git_config.has_section("user"))
+            self.assertEqual(git_config.get_value("user", "name"), payload)
             self.assertFalse(git_config.has_section("core"))
 
     @with_rw_directory
@@ -321,24 +321,26 @@ def test_writer_rejects_unquoted_section_terminators(self, rw_dir):
             self.assertFalse(git_config.has_section("other"), "an unsafe section name injected an [other] section")
 
     @with_rw_directory
-    def test_set_and_add_value_reject_unsafe_value_characters(self, rw_dir):
+    def test_set_and_add_value_match_git_control_character_handling(self, rw_dir):
         config_path = osp.join(rw_dir, "config")
-        bad_values = ("foo\rbar", "foo\nbar", "foo\x00bar", b"foo\nbar")
 
         with GitConfigParser(config_path, read_only=False) as git_config:
             git_config.add_section("user")
-            for bad_value in bad_values:
-                with pytest.raises(ValueError, match="CR, LF, or NUL"):
-                    git_config.set("user", "name", bad_value)
-                with pytest.raises(ValueError, match="CR, LF, or NUL"):
-                    git_config.set_value("user", "name", bad_value)
-                with pytest.raises(ValueError, match="CR, LF, or NUL"):
-                    git_config.add_value("user", "name", bad_value)
+            git_config.set("user", "carriage-return", "foo\rbar")
+            git_config.set_value("user", "line-feed", "foo\nbar")
+            git_config.add_value("user", "bytes-line-feed", b"foo\nbar")
 
-            git_config.set_value("user", "name", "safe")
+            for setter in (git_config.set, git_config.set_value, git_config.add_value):
+                with pytest.raises(ValueError, match="must not contain NUL"):
+                    setter("user", "name", "foo\x00bar")
 
         with GitConfigParser(config_path, read_only=True) as git_config:
-            self.assertEqual(git_config.get_value("user", "name"), "safe")
+            self.assertEqual(git_config.get_value("user", "carriage-return"), "foo\rbar")
+            self.assertEqual(git_config.get_value("user", "line-feed"), "foo\nbar")
+            self.assertEqual(git_config.get_value("user", "bytes-line-feed"), "foo\nbar")
+
+        self.assertEqual(Git().config("--file", config_path, "--get", "user.carriage-return"), "foo\rbar")
+        self.assertEqual(Git().config("--file", config_path, "--get", "user.line-feed"), "foo\nbar")
 
     def test_base(self):
         path_repo = fixture_path("git_config")
@@ -482,8 +484,8 @@ def test_multiple_include_paths_with_same_key(self, rw_dir):
         # We write it manually because set_value would overwrite the key.
         with open(fp_main, "w") as f:
             f.write("[include]\n")
-            f.write(f"\tpath = {fp_inc1}\n")
-            f.write(f"\tpath = {fp_inc2}\n")
+            f.write(f"\tpath = {_escape_config_value(fp_inc1)}\n")
+            f.write(f"\tpath = {_escape_config_value(fp_inc2)}\n")
 
         with GitConfigParser(fp_main, read_only=True) as cr:
             # Both included files should be loaded.
@@ -509,8 +511,15 @@ def test_conditional_includes_from_git_dir(self, rw_dir):
         path2 = osp.join(rw_dir, "config2")
         template = '[includeIf "{}:{}"]\n    path={}\n'
 
+        def include_config(condition, pattern):
+            return template.format(
+                condition,
+                _escape_section_subsection(pattern),
+                _escape_config_value(path2),
+            )
+
         with open(path1, "w") as stream:
-            stream.write(template.format("gitdir", git_dir, path2))
+            stream.write(include_config("gitdir", git_dir))
 
         # Ensure that config is ignored if no repo is set.
         with GitConfigParser(path1) as config:
@@ -524,39 +533,39 @@ def test_conditional_includes_from_git_dir(self, rw_dir):
 
         # Ensure that config is ignored if case is incorrect.
         with open(path1, "w") as stream:
-            stream.write(template.format("gitdir", git_dir.upper(), path2))
+            stream.write(include_config("gitdir", git_dir.upper()))
 
         with GitConfigParser(path1, repo=repo) as config:
             assert not config._has_includes()
             assert config._included_paths() == []
 
         # Ensure that config is included if case is ignored.
         with open(path1, "w") as stream:
-            stream.write(template.format("gitdir/i", git_dir.upper(), path2))
+            stream.write(include_config("gitdir/i", git_dir.upper()))
 
         with GitConfigParser(path1, repo=repo) as config:
             assert config._has_includes()
             assert config._included_paths() == [("path", path2)]
 
         # Ensure that config is included with path using glob pattern.
         with open(path1, "w") as stream:
-            stream.write(template.format("gitdir", "**/repo1", path2))
+            stream.write(include_config("gitdir", "**/repo1"))
 
         with GitConfigParser(path1, repo=repo) as config:
             assert config._has_includes()
             assert config._included_paths() == [("path", path2)]
 
         # Ensure that config is ignored if path is not matching git_dir.
         with open(path1, "w") as stream:
-            stream.write(template.format("gitdir", "incorrect", path2))
+            stream.write(include_config("gitdir", "incorrect"))
 
         with GitConfigParser(path1, repo=repo) as config:
             assert not config._has_includes()
             assert config._included_paths() == []
 
         # Ensure that config is included if path in hierarchy.
         with open(path1, "w") as stream:
-            stream.write(template.format("gitdir", "target1/", path2))
+            stream.write(include_config("gitdir", "target1/"))
 
         with GitConfigParser(path1, repo=repo) as config:
             assert config._has_includes()
@@ -578,23 +587,23 @@ def test_conditional_includes_from_branch_name(self, rw_dir):
 
         # Ensure that config is included is branch is correct.
         with open(path1, "w") as stream:
-            stream.write(template.format("/foo/branch", path2))
+            stream.write(template.format("/foo/branch", _escape_config_value(path2)))
 
         with GitConfigParser(path1, repo=repo) as config:
             assert config._has_includes()
             assert config._included_paths() == [("path", path2)]
 
         # Ensure that config is included is branch is incorrect.
         with open(path1, "w") as stream:
-            stream.write(template.format("incorrect", path2))
+            stream.write(template.format("incorrect", _escape_config_value(path2)))
 
         with GitConfigParser(path1, repo=repo) as config:
             assert not config._has_includes()
             assert config._included_paths() == []
 
         # Ensure that config is included with branch using glob pattern.
         with open(path1, "w") as stream:
-            stream.write(template.format("/foo/**", path2))
+            stream.write(template.format("/foo/**", _escape_config_value(path2)))
 
         with GitConfigParser(path1, repo=repo) as config:
             assert config._has_includes()
@@ -630,23 +639,23 @@ def test_conditional_includes_remote_url(self, rw_dir):
 
         # Ensure that config with hasconfig and full url is correct.
         with open(path1, "w") as stream:
-            stream.write(template.format("https://github.com/foo/repo", path2))
+            stream.write(template.format("https://github.com/foo/repo", _escape_config_value(path2)))
 
         with GitConfigParser(path1, repo=repo) as config:
             assert config._has_includes()
             assert config._included_paths() == [("path", path2)]
 
         # Ensure that config with hasconfig and incorrect url is incorrect.
         with open(path1, "w") as stream:
-            stream.write(template.format("incorrect", path2))
+            stream.write(template.format("incorrect", _escape_config_value(path2)))
 
         with GitConfigParser(path1, repo=repo) as config:
             assert not config._has_includes()
             assert config._included_paths() == []
 
         # Ensure that config with hasconfig and url using glob pattern is correct.
         with open(path1, "w") as stream:
-            stream.write(template.format("**/**github.com*/**", path2))
+            stream.write(template.format("**/**github.com*/**", _escape_config_value(path2)))
 
         with GitConfigParser(path1, repo=repo) as config:
             assert config._has_includes()

test/test_submodule.py

@@ -987,10 +987,10 @@ def test_git_submodules_and_add_sm_with_new_commit(self, rwdir):
 
         parent.index.commit("moved submodules")
 
-        with sm.config_writer() as writer:
-            writer.set_value("user.email", "example@example.com")
-            writer.set_value("user.name", "me")
         smm = sm.module()
+        with smm.config_writer() as writer:
+            writer.set_value("user", "email", "example@example.com")
+            writer.set_value("user", "name", "me")
         fp = osp.join(smm.working_tree_dir, "empty-file")
         with open(fp, "w"):
             pass