Repository navigation
fix(config): decode escapes in unquoted config values - #2297
Open
JingHao-Leon wants to merge 1 commit into
Open
JingHao-Leon wants to merge 1 commit into
JingHao-Leon wants to merge 1 commit into
Conversation
JingHao-Leon
marked this pull request as draft
October 19, 2026 11:07
Git decodes the standard escape sequences in unquoted config values, but `GitConfigParser._read` only called `parse_value()` when a value continued onto a following line. A single-line unquoted value therefore kept its escapes literal, so `b = first\nsecond` read back as `first\nsecond` rather than `first<LF>second`. The same value also decoded differently depending only on whether it was wrapped: `b = first\nsecond \` continued onto a second line already went through `parse_value()` and was decoded. The missing decode made writes unsafe. The parsed value held no control character, so the writer's CR/LF/NUL guard did not trigger, and saving an unrelated key emitted a value that git then read differently than before. With the escape decoded, the existing guard rejects the value instead of corrupting the file. A value is decoded only when every backslash in it introduces a known escape. Git rejects an unknown escape outright, so GitPython never had to handle one; it tolerates them instead, which is what keeps a Windows path such as `C:\Users\me` usable. Decoding unconditionally would have broken that: the `\t` of `\Temp\test_foo` would become a tab while the `\U` of `\Users` stayed literal, mangling the path. Guarding on the whole value decodes `a\tb` and `foo\\bar` as git does and leaves the path alone. An existing case in `test_backslash_line_continuation` asserted the undecoded spelling `val\\` for `k = val\\` followed by a newline. Git yields `val\` there, since the second backslash escapes the first, so that expectation is corrected to match. Checked against git 2.50.1 (Apple Git-155). Co-Authored-By: Codex <codex@openai.com>
JingHao-Leon
force-pushed
the
fix/config-unquoted-escape-parsing
branch
from
October 19, 2026 11:16
fd7027c to
983e182
Compare
JingHao-Leon
marked this pull request as ready for review
October 19, 2026 12:02
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
GitConfigParserleaves backslash escapes literal in unquoted config values, so it disagrees with Git on the same file. Quoted values are decoded correctly; unquoted ones are not.git config --get a.bb = first\nsecondfirst<LF>secondfirst\nsecondb = first\tsecondfirst<TAB>secondfirst\tsecondb = foo\\barfoo\barfoo\\barb = ab\"cdab"cdab\"cdThe value also decoded differently depending only on whether it was wrapped, which is the inconsistency that identifies the cause:
b = first\nsecondstayed literal, whileb = first\nsecond \continued onto a second line was decoded.This also made read-modify-write corrupt config files. The parsed value held no control character, so the writer's CR/LF/NUL guard never triggered, and saving an unrelated key changed what Git read:
After the fix GitPython reads
x<LF>ylike Git does, and the existing writer guard rejects the control character instead of corrupting the file.Root cause
git/config.py:624—parse_value()was called only whencontinuedwas set:parse_value()is what decodes escapes, strips unquoted comments and trims whitespace. Gating it oncontinuedmade the decode depend on line wrapping, so a single-line unquoted value skipped it while a continued one got it.Why the decode is conditional on the escapes being known
Git decodes the standard escape sequences in unquoted values as well, but it is not accurate to say it treats unquoted values exactly as it treats quoted ones: for an escape it does not recognise, git rejects the line outright.
So
path = C:\Users\meis not a line git can read at all. GitPython is deliberately more tolerant and keeps such backslashes literal, which is what lets a Windows path survive a round-trip, and existing tests depend on it.Decoding unconditionally regresses that tolerance. In
C:\Users\RUNNER~1\AppData\Local\Temp\test_foo, the\tof\Tempand\testbecame tabs while the\Uof\Usersstayed literal, so the path no longer matched. That is not hypothetical: it turned four Windows jobs red on the first push of this branch —I did not touch those tests to match the new behaviour, since that would only hide the regression. Instead
parse_value()now runs only when every backslash in the value introduces a known escape:first\nsecond,a\tb,foo\\bar— all backslashes known, so decoded as git does.C:\Users\...\Temp\test_foo— contains the unknown\U, so the whole value stays literal and the path is preserved.The escape table moved to a module-level
UNQUOTED_ESCAPESso the decoder and the newescapes_are_known()guard cannot drift apart. A trailing backslash is treated as known, since git drops it rather than treating it as an unknown escape; without that, a value ending in\after a continuation would stop being comment-stripped.One existing expectation moves with this:
test_backslash_line_continuationassertedk = val\\followed by a newline reads asval\\. Git yieldsval\, since the second backslash escapes the first and there is no continuation, so that case now asserts the value Git actually produces.Validation
All figures are from this branch against git 2.50.1 (Apple Git-155). Every expected value in the new fixture was first read out of real
git config --getrather than assumed.New regression test, red before the fix:
Config suite after the fix:
(before the change: 53 passed, 2 skipped — the extra test is the new one.)
Full suite, same tree, before and after:
The +1 is the new test; skipped and error counts are identical. The 913 errors are environmental and not related to this change: the sandbox clone is blobless, so
test/lib/helper.py::TestBase.setUpClasscannot materialise historical blobs for its two dependency fixtures andgit gcaborts. Those same 913 errors are present on unmodifiedmain, and the lazy-fetch path was disabled on both runs so neither hung.Lint, using the rule set from
pyproject.toml(E,W,F,B,C4,TC004):What I could not verify locally
I am on macOS, so the Windows path handling that drove the design above is not reproducible here — POSIX paths contain no backslashes. I checked the mechanism by feeding the exact Windows temp paths through
GitConfigParserdirectly on this machine and confirming they now round-trip unchanged:All three round-trip exactly. That exercises the same
escapes_are_known()branch the four failing tests depend on, but it is not a substitute for running the Windows jobs, so the Windows CI result on this push is the real check.No changelog entry:
CONTRIBUTING.mddoes not ask for one,doc/source/changes.rsthas no unreleased section, and the comparable config fixes (#2213, #2245, #2227, #2211) did not touch it.Fixes #2296
This change was prepared with the assistance of an AI coding agent.