Skip to content

Commit 2d4f068

Browse files
Byroncodex
andcommitted
fix: reject comments after implicit boolean config keys
Note that this is just a fixup, on a huge hack which is the native git-config parsing. Let's just hope this holds up until v4. <!-- agent --> GitConfigParser accepted entries such as "enabled # comment" and "enabled ; comment" even though Git rejects them. The comment became part of the option name, and an equals sign or colon inside the comment could make the entry look like an assignment. Silently stripping the comment would also accept configuration that Git considers invalid. Exclude both comment markers from the shared option-name expression and require a full-line match for valueless options. The assignment pattern cannot cross a comment marker, and the valueless fallback cannot accept just the valid-looking prefix. Such lines now raise the existing ParsingError during reading or an attempted edit. Ordinary bare keys retain their implicit true value and round-trip behavior. Add six regression cases covering both markers, spaces, tabs, adjacent comments, and assignment delimiters inside comments. Compare rejection with git config, check both getboolean and an unrelated edit raise ParsingError, and verify that the failed edit leaves the original bytes untouched. All six cases failed before full-line matching was added. Git reference: checkout 1630431f326e15fcde608827b5ff38422528eb59, config.c:get_value. Without an assignment, that parser requires the line to end after the key and optional whitespace. Runtime comparisons used Git 2.50.1 (Apple Git-155), which rejected all six inputs with exit status 128. Validation on Python 3.12.14: 42 configuration tests and six regression subtests passed, with two existing skips. Ruff lint and formatting and git diff --check passed. Assisted-by: GPT 6.0 Co-authored-by: GPT 6.0 <codex@openai.com>
1 parent d171e34 commit 2d4f068

2 files changed

Lines changed: 35 additions & 2 deletions

File tree

git/config.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ class GitConfigParser(cp.RawConfigParser, metaclass=MetaParserBuilder):
310310
re_comment = re.compile(r"^\s*[#;]")
311311
# } END configuration
312312

313-
optvalueonly_source = r"\s*(?P<option>[^:=\s][^:=]*)"
313+
optvalueonly_source = r"\s*(?P<option>[^:=\s#;][^:=#;]*)"
314314

315315
OPTVALUEONLY = re.compile(optvalueonly_source)
316316

@@ -594,7 +594,7 @@ def parse_value(value: str) -> str:
594594
cursect.add(optname, optval)
595595
else:
596596
# A valueless option is an implicit boolean true, not an empty value.
597-
mo = self.OPTVALUEONLY.match(line)
597+
mo = self.OPTVALUEONLY.fullmatch(line)
598598
if mo:
599599
optname = self.optionxform(mo.group("option").rstrip())
600600
cursect.add(optname, None)

test/test_config.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -917,6 +917,39 @@ def test_implicit_boolean_round_trip(self, rw_dir):
917917
self.assertIs(config.getboolean("flag", "empty"), False)
918918
self.assertEqual(dict(config.items_all("flag"))["multiple"], ["false", None, "", None])
919919

920+
@with_rw_directory
921+
def test_implicit_boolean_rejects_comments_like_git(self, rw_dir):
922+
config_path = osp.join(rw_dir, "config")
923+
comments = (
924+
" # comment",
925+
" ; comment",
926+
"# comment = value",
927+
"; comment = value",
928+
"\t# comment: value",
929+
"\t; comment: value",
930+
)
931+
for comment in comments:
932+
with self.subTest(comment=comment):
933+
content = ("[flag]\n\tenabled%s\n" % comment).encode(defenc)
934+
with open(config_path, "wb") as config_file:
935+
config_file.write(content)
936+
937+
result = subprocess.run(
938+
["git", "config", "--file", config_path, "--bool", "flag.enabled"],
939+
stdout=subprocess.PIPE,
940+
stderr=subprocess.PIPE,
941+
)
942+
self.assertEqual(result.returncode, 128, result.stderr)
943+
with GitConfigParser(config_path) as config:
944+
with self.assertRaises(cp.ParsingError):
945+
config.getboolean("flag", "enabled")
946+
with GitConfigParser(config_path, read_only=False) as config:
947+
with self.assertRaises(cp.ParsingError):
948+
config.set_value("other", "value", "updated")
949+
950+
with open(config_path, "rb") as config_file:
951+
self.assertEqual(config_file.read(), content)
952+
920953
def test_config_with_quotes(self):
921954
cr = GitConfigParser(fixture_path("git_config_with_quotes"), read_only=True)
922955

0 commit comments

Comments
 (0)