Skip to content

Commit 85c8155

Browse files
authored
Merge pull request #2048 from EliahKagan/conservative-quote-removal
Don't remove quotes if `\` or `"` are present inside
2 parents 17d53b0 + f2b8041 commit 85c8155

File tree

3 files changed

+41
-4
lines changed

3 files changed

+41
-4
lines changed

git/config.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -496,18 +496,26 @@ def string_decode(v: str) -> str:
496496
if mo:
497497
# We might just have handled the last line, which could contain a quotation we want to remove.
498498
optname, vi, optval = mo.group("option", "vi", "value")
499+
optname = self.optionxform(optname.rstrip())
500+
499501
if vi in ("=", ":") and ";" in optval and not optval.strip().startswith('"'):
500502
pos = optval.find(";")
501503
if pos != -1 and optval[pos - 1].isspace():
502504
optval = optval[:pos]
503505
optval = optval.strip()
504-
optname = self.optionxform(optname.rstrip())
505-
if len(optval) > 1 and optval[0] == '"' and optval[-1] != '"':
506+
507+
if len(optval) < 2 or optval[0] != '"':
508+
# Does not open quoting.
509+
pass
510+
elif optval[-1] != '"':
511+
# Opens quoting and does not close: appears to start multi-line quoting.
506512
is_multi_line = True
507513
optval = string_decode(optval[1:])
508-
elif len(optval) > 1 and optval[0] == '"' and optval[-1] == '"':
514+
elif optval.find("\\", 1, -1) == -1 and optval.find('"', 1, -1) == -1:
515+
# Opens and closes quoting. Single line, and all we need is quote removal.
509516
optval = optval[1:-1]
510-
# END handle multi-line
517+
# TODO: Handle other quoted content, especially well-formed backslash escapes.
518+
511519
# Preserves multiple values for duplicate optnames.
512520
cursect.add(optname, optval)
513521
else:
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
[custom]
2+
hasnewline = "first\nsecond"
3+
hasbackslash = "foo\\bar"
4+
hasquote = "ab\"cd"
5+
hastrailingbackslash = "word\\"
6+
hasunrecognized = "p\qrs"
7+
hasunescapedquotes = "ab"cd"e"
8+
ordinary = "hello world"
9+
unquoted = good evening

test/test_config.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,26 @@ def test_config_with_quotes_with_whitespace_outside_value(self):
428428
cr = GitConfigParser(fixture_path("git_config_with_quotes_whitespace_outside"), read_only=True)
429429
self.assertEqual(cr.get("init", "defaultBranch"), "trunk")
430430

431+
def test_config_with_quotes_containing_escapes(self):
432+
"""For now just suppress quote removal. But it would be good to interpret most of these."""
433+
cr = GitConfigParser(fixture_path("git_config_with_quotes_escapes"), read_only=True)
434+
435+
# These can eventually be supported by substituting the represented character.
436+
self.assertEqual(cr.get("custom", "hasnewline"), R'"first\nsecond"')
437+
self.assertEqual(cr.get("custom", "hasbackslash"), R'"foo\\bar"')
438+
self.assertEqual(cr.get("custom", "hasquote"), R'"ab\"cd"')
439+
self.assertEqual(cr.get("custom", "hastrailingbackslash"), R'"word\\"')
440+
self.assertEqual(cr.get("custom", "hasunrecognized"), R'"p\qrs"')
441+
442+
# It is less obvious whether and what to eventually do with this.
443+
self.assertEqual(cr.get("custom", "hasunescapedquotes"), '"ab"cd"e"')
444+
445+
# Cases where quote removal is clearly safe should happen even after those.
446+
self.assertEqual(cr.get("custom", "ordinary"), "hello world")
447+
448+
# Cases without quotes should still parse correctly even after those, too.
449+
self.assertEqual(cr.get("custom", "unquoted"), "good evening")
450+
431451
def test_get_values_works_without_requiring_any_other_calls_first(self):
432452
file_obj = self._to_memcache(fixture_path("git_config_multiple"))
433453
cr = GitConfigParser(file_obj, read_only=True)

0 commit comments

Comments
 (0)