Repository navigation
Commit cd72d78
committed
Merge pull request #46 from ereOn/master
Incorrect handling of backslashes and quotes in GitConfigParser
Steps to reproduce the issue:
import git
config = git.Repo().config_writer()
config.add_section('test')
config.set_value('test', 'test', r'some\data')
Now if you try to read this value using a regular (non Python) git config, Git complains that the configuration file is invalid:
fatal: bad config file line 11 in .git/config
Indeed, if you open .git/config you can see that the value is written as:
[test]
test = some\data
While the git-config configuration states that:
String values may be entirely or partially enclosed in double quotes. You need to enclose variable values in double quotes if you want to preserve leading or trailing whitespace, or if the variable value contains comment characters (i.e. it contains # or ;). Double quote " and backslash \ characters in variable values must be escaped: use \" for " and \ for .
That is, the backslashes are not escaped in the configuration file.
This also causes issues while reading, because values are not un-escaped.
This pull request fixes both those issues and also fixes unescaped quotes pairs.
A test-case has been provided along with the fixes.3 files changed
Lines changed: 56 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -245,8 +245,28 @@ def _read(self, fp, fpname): | |||
| 245 | 245 | if pos != -1 and optval[pos-1].isspace(): | |
| 246 | 246 | optval = optval[:pos] | |
| 247 | 247 | optval = optval.strip() | |
| 248 | - if optval == '""': | ||
| 249 | - optval = '' | ||
| 248 | + | ||
| 249 | + # Remove paired unescaped-quotes | ||
| 250 | + unquoted_optval = '' | ||
| 251 | + escaped = False | ||
| 252 | + in_quote = False | ||
| 253 | + for c in optval: | ||
| 254 | + if not escaped and c == '"': | ||
| 255 | + in_quote = not in_quote | ||
| 256 | + else: | ||
| 257 | + escaped = (c == '\\') and not escaped | ||
| 258 | + unquoted_optval += c | ||
| 259 | + | ||
| 260 | + if in_quote: | ||
| 261 | + if not e: | ||
| 262 | + e = cp.ParsingError(fpname) | ||
| 263 | + e.append(lineno, repr(line)) | ||
| 264 | + | ||
| 265 | + optval = unquoted_optval | ||
| 266 | + | ||
| 267 | + optval = optval.replace('\\\\', '\\') # Unescape backslashes | ||
| 268 | + optval = optval.replace(r'\"', '"') # Unescape quotes | ||
| 269 | + | ||
| 250 | 270 | optname = self.optionxform(optname.rstrip()) | |
| 251 | 271 | cursect[optname] = optval | |
| 252 | 272 | else: | |
@@ -303,7 +323,11 @@ def write_section(name, section_dict): | |||
| 303 | 323 | fp.write("[%s]\n" % name) | |
| 304 | 324 | for (key, value) in section_dict.items(): | |
| 305 | 325 | if key != "__name__": | |
| 306 | - fp.write("\t%s = %s\n" % (key, str(value).replace('\n', '\n\t'))) | ||
| 326 | + value = str(value) | ||
| 327 | + value = value.replace('\\', '\\\\') # Escape backslashes | ||
| 328 | + value = value.replace('"', r'\"') # Escape quotes | ||
| 329 | + value = value.replace('\n', '\n\t') | ||
| 330 | + fp.write("\t%s = %s\n" % (key, value)) | ||
| 307 | 331 | # END if key is not __name__ | |
| 308 | 332 | # END section writing | |
| 309 | 333 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,4 @@ | |||
| 1 | + [values] | ||
| 2 | + backslash = some\\data | ||
| 3 | + quote = this is a \"quoted value\" | ||
| 4 | + quoted = "all" "your \"quotes\" a"re bel"ong to """"us" | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -101,4 +101,28 @@ def test_base(self): | |||
| 101 | 101 | # it raises if there is no default though | |
| 102 | 102 | self.failUnlessRaises(NoSectionError, r_config.get_value, "doesnt", "exist") | |
| 103 | 103 | ||
| 104 | - | ||
| 104 | + def test_values(self): | ||
| 105 | + file_obj = self._to_memcache(fixture_path("git_config_values")) | ||
| 106 | + w_config = GitConfigParser(file_obj, read_only = False) | ||
| 107 | + w_config.write() # enforce writing | ||
| 108 | + orig_value = file_obj.getvalue() | ||
| 109 | + | ||
| 110 | + # Reading must unescape backslashes | ||
| 111 | + backslash = w_config.get('values', 'backslash') | ||
| 112 | + assert backslash == r'some\data' | ||
| 113 | + | ||
| 114 | + # Reading must unescape quotes | ||
| 115 | + quote = w_config.get('values', 'quote') | ||
| 116 | + assert quote == 'this is a "quoted value"' | ||
| 117 | + | ||
| 118 | + # Reading must remove surrounding quotes | ||
| 119 | + quoted = w_config.get('values', 'quoted') | ||
| 120 | + assert quoted == 'all your "quotes" are belong to us' | ||
| 121 | + | ||
| 122 | + # Writing must escape backslashes and quotes | ||
| 123 | + w_config.set('values', 'backslash', backslash) | ||
| 124 | + w_config.set('values', 'quote', quote) | ||
| 125 | + w_config.write() # enforce writing | ||
| 126 | + | ||
| 127 | + # Contents shouldn't differ | ||
| 128 | + assert file_obj.getvalue() == orig_value | ||
0 commit comments