Skip to content

Don't remove quotes if \ or " are present inside #2048

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+41 −4
Open

Don't remove quotes if \ or " are present inside #2048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c8e4aa0
Refactor quote parsing, to prepare for more checks
EliahKagan Jun 8, 2025
7bcea08
Add tests of ConfigParser with `"..."` with `"` or `\` inside
EliahKagan Jun 8, 2025
f2b8041
Don't remove quotes if `\` or `"` are present inside
EliahKagan Jun 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions git/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,18 +496,26 @@ def string_decode(v: str) -> str:
if mo:
# We might just have handled the last line, which could contain a quotation we want to remove.
optname, vi, optval = mo.group("option", "vi", "value")
optname = self.optionxform(optname.rstrip())

if vi in ("=", ":") and ";" in optval and not optval.strip().startswith('"'):
pos = optval.find(";")
if pos != -1 and optval[pos - 1].isspace():
optval = optval[:pos]
optval = optval.strip()
optname = self.optionxform(optname.rstrip())
if len(optval) > 1 and optval[0] == '"' and optval[-1] != '"':

if len(optval) < 2 or optval[0] != '"':
# Does not open quoting.
pass
elif optval[-1] != '"':
# Opens quoting and does not close: appears to start multi-line quoting.
is_multi_line = True
optval = string_decode(optval[1:])
elif len(optval) > 1 and optval[0] == '"' and 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]
# END handle multi-line
# TODO: Handle other quoted content, especially well-formed backslash escapes.

# Preserves multiple values for duplicate optnames.
cursect.add(optname, optval)
else:
Expand Down
9 changes: 9 additions & 0 deletions test/fixtures/git_config_with_quotes_escapes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[custom]
hasnewline = "first\nsecond"
hasbackslash = "foo\\bar"
hasquote = "ab\"cd"
hastrailingbackslash = "word\\"
hasunrecognized = "p\qrs"
hasunescapedquotes = "ab"cd"e"
ordinary = "hello world"
unquoted = good evening
20 changes: 20 additions & 0 deletions test/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,26 @@ def test_config_with_quotes_with_whitespace_outside_value(self):
cr = GitConfigParser(fixture_path("git_config_with_quotes_whitespace_outside"), read_only=True)
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."""
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"')

# It is less obvious whether and what to eventually do with this.
self.assertEqual(cr.get("custom", "hasunescapedquotes"), '"ab"cd"e"')

# Cases where quote removal is clearly safe should happen even after those.
self.assertEqual(cr.get("custom", "ordinary"), "hello world")

# Cases without quotes should still parse correctly even after those, too.
self.assertEqual(cr.get("custom", "unquoted"), "good evening")

def test_get_values_works_without_requiring_any_other_calls_first(self):
file_obj = self._to_memcache(fixture_path("git_config_multiple"))
cr = GitConfigParser(file_obj, read_only=True)
Expand Down
Loading