diff --git a/AGENTS.md b/AGENTS.md index cdffa4764..a308591ea 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,6 +5,10 @@ including the [Prevent agent impersonation](CONTRIBUTING.md#prevent-agent-impers section governing agent identification and separation of unaltered user statements when communicating through a person's account. +[Prefer pull requests](CONTRIBUTING.md#prefer-pull-requests): a PR can serve as an +issue with a fix attached. Do not create an issue solely to follow it immediately +with a PR that fixes it; describe the problem in the PR instead. + # Commit messages Follow Conventional Commits for every commit. Every commit must have a diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1a64a701d..8a155dc0d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -9,6 +9,16 @@ The following is a short step-by-step rundown of what one typically would do to - Feel free to add yourself to AUTHORS file. - Create a pull request. +## Prefer pull requests + +Pull requests are preferred when you have a fix or improvement to contribute: a PR +can be treated as an issue with a fix attached. Describe the problem and proposed +solution in the PR itself; a separate issue is not required. + +Please avoid opening an issue solely to follow it immediately with a PR that fixes +it. Open the PR directly instead. Issues are still welcome for problems without a +fix or for changes that need discussion before implementation. + ## Quality expectations Contributions must be made with care and meet the quality bar of the surrounding code. diff --git a/README.md b/README.md index 029b4fa00..8d1b590b2 100644 --- a/README.md +++ b/README.md @@ -201,8 +201,12 @@ Please have a look at the [contributions file][contributing]. - [Questions and Answers](http://stackexchange.com/filters/167317/gitpython) - Please post on Stack Overflow and use the `gitpython` tag - [Issue Tracker](https://github.com/gitpython-developers/GitPython/issues) - - Post reproducible bugs and feature requests as a new issue. - Please be sure to provide the following information if posting bugs: + - Post reproducible bugs and feature requests as a new issue when you do not have + a fix or need discussion before implementation. If you have a fix or improvement, + [open a pull request directly](CONTRIBUTING.md#prefer-pull-requests) and describe + the problem there; you do not need to create an issue first. + Please be sure to provide the following information when reporting bugs in an + issue or PR: - GitPython version (e.g. `import git; git.__version__`) - Python version (e.g. `python --version`) - The encountered stack-trace, if applicable diff --git a/git/config.py b/git/config.py index 7109c09c3..732d3658b 100644 --- a/git/config.py +++ b/git/config.py @@ -69,6 +69,13 @@ VALID_CONFIG_OPTION_NAME_RE = re.compile(r"^[A-Za-z0-9_.-]+$") """Pattern for option names that can be written without changing config syntax.""" +UNQUOTED_ESCAPES = {"b": "\b", "n": "\n", "t": "\t", '"': '"', "\\": "\\"} +"""Escape sequences git recognizes in an unquoted value. + +Any other backslash escape is an error for git, which is why a value holding one is +left undecoded rather than rejected. +""" + class MetaParserBuilder(abc.ABCMeta): # noqa: B024 """Utility class wrapping base-class methods into decorators that assure read-only @@ -530,10 +537,9 @@ def parse_value(value: str) -> str: parsed: List[str] = [] whitespace: List[str] = [] quoted = escaped = False - escapes = {"b": "\b", "n": "\n", "t": "\t", '"': '"', "\\": "\\"} for char in value: if escaped: - parsed.append(escapes.get(char, "\\" + char)) + parsed.append(UNQUOTED_ESCAPES.get(char, "\\" + char)) escaped = False continue if char.isspace() and not quoted: @@ -552,6 +558,26 @@ def parse_value(value: str) -> str: parsed.append(char) return "".join(parsed) + def escapes_are_known(value: str) -> bool: + """Whether every backslash in `value` introduces an escape git recognises. + + Git rejects an unquoted value carrying an unknown escape outright. GitPython + tolerates such values, because that is what lets a Windows path such as + ``C:\\Users\\me`` stay literal, so a value is decoded only when every one + of its backslashes is part of a known escape. Decoding part of a value would + otherwise turn the ``\\t`` of ``\\Temp\\test`` into a tab while leaving the + ``\\U`` of ``\\Users`` alone. + """ + index = value.find("\\") + while index >= 0: + # A trailing backslash has nothing to escape and git drops it, so + # only a backslash followed by an unknown escape disqualifies the + # value from being decoded. + if index + 1 < len(value) and value[index + 1] not in UNQUOTED_ESCAPES: + return False + index = value.find("\\", index + 2) + return True + while True: # We assume to read binary! raw_line = fp.readline() @@ -606,23 +632,30 @@ def parse_value(value: str) -> str: # next line is appended before the complete value is # parsed. An even number means the last backslash is # escaped and the value ends there. - continued = False + eof_continuation = False while True: if not is_line_continuation(optval): break continuation = fp.readline() if not continuation: - # Backslash at end of file: git drops it. - optval = optval[:-1] + # Keep the backslash until parsing so preceding + # whitespace is significant even though git drops it. + eof_continuation = True break lineno = lineno + 1 joined = continuation.decode(defenc) while joined.endswith("\n") or joined.endswith("\r"): joined = joined[:-1] optval = optval[:-1] + joined - continued = True - if continued: + # Git decodes the standard escape sequences in unquoted + # values, but only a value in which every backslash is part + # of a known escape is decoded, so that a Windows path keeps + # its backslashes instead of losing them one escape at a time. + optval, _ = strip_inline_comment(optval) + if escapes_are_known(optval): optval = parse_value(optval) + elif eof_continuation: + optval = optval[:-1] elif quote_open: # Opens quoting and does not close: appears to start multi-line quoting. is_multi_line = True diff --git a/test/fixtures/git_config_unquoted_escapes b/test/fixtures/git_config_unquoted_escapes new file mode 100644 index 000000000..e146fff97 --- /dev/null +++ b/test/fixtures/git_config_unquoted_escapes @@ -0,0 +1,12 @@ +[custom] + hasnewline = first\nsecond + hastab = first\tsecond + hasbackslash = foo\\bar + hasquote = ab\"cd + hastrailingbackslash = word\\ + ordinary = hello world + withcolon = host:port + withequals = key=value + withhash = value#notcomment + hasunknownescape = foo\qbar + haswindowspath = C:\Users\RUNNER~1\AppData\Local\Temp\test_foo diff --git a/test/test_config.py b/test/test_config.py index e3c30421c..49b8535c7 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -365,7 +365,7 @@ def test_backslash_line_continuation(self): (b"[a]\n\tk = one\\\n two ; ignored\n", "one two"), (b'[a]\n\tk = one\\\n "two"\n', "one two"), (b"[a]\n\tk = one\\\n two\\tthree\n", "one two\tthree"), - (b"[a]\n\tk = val\\\\\n next\n", "val\\\\"), + (b"[a]\n\tk = val\\\\\n next\n", "val\\"), (b"[a]\n\tk = end\\\n", "end"), (b"[alias]\n\tco = checkout \\\n\t\t-v\n", "checkout \t\t-v"), ] @@ -378,6 +378,33 @@ def test_backslash_line_continuation(self): key = "co" if section == "alias" else "k" self.assertEqual(config.get_value(section, key), expected) + @with_rw_directory + def test_continued_values_preserve_comments_and_eof_whitespace(self, rw_dir): + cases = [ + (b"one\\\n two # C:\\Users\n", "one two"), + (b"one\\\n two ; C:\\Users\n", "one two"), + (b"one\\\n two\\tthree # C:\\Users\n", "one two\tthree"), + (b'one\\\n "two # three" # C:\\Users\n', "one two # three"), + (b"value \\", "value "), + (b"value \\\n", "value "), + (b"value \t\\\n", "value \t"), + (b"one\\\n two \\\n", "one two "), + (b"C:\\Users \\\n", "C:\\Users "), + ] + config_path = osp.join(rw_dir, "config") + for value, expected in cases: + with self.subTest(value=value): + with open(config_path, "wb") as config_file: + config_file.write(b"[a]\n\tk = " + value) + + with GitConfigParser(config_path, read_only=False) as config: + self.assertEqual(config.get_value("a", "k"), expected) + config.set_value("other", "value", "updated") + + with GitConfigParser(config_path) as config: + self.assertEqual(config.get_value("a", "k"), expected) + self.assertEqual(config.get_value("other", "value"), "updated") + def test_get_value_reads_git_boolean_spellings(self): # git accepts yes/no and on/off as well as true/false, and getboolean on # this class already did. get_value returned them as strings, so "no" and @@ -1209,6 +1236,30 @@ def test_config_with_quotes_containing_escapes(self): # Cases without quotes should still parse correctly even after those, too. self.assertEqual(cr.get("custom", "unquoted"), "good evening") + def test_config_with_unquoted_escapes(self): + """Decode git's escape sequences in unquoted values, but only when every + backslash in the value is part of a known escape.""" + cr = GitConfigParser(fixture_path("git_config_unquoted_escapes"), read_only=True) + + self.assertEqual(cr.get("custom", "hasnewline"), "first\nsecond") + self.assertEqual(cr.get("custom", "hastab"), "first\tsecond") + self.assertEqual(cr.get("custom", "hasbackslash"), R"foo\bar") + self.assertEqual(cr.get("custom", "hasquote"), 'ab"cd') + self.assertEqual(cr.get("custom", "hastrailingbackslash"), "word\\") + + # Git rejects an unknown escape rather than decoding it. GitPython keeps + # such a value literal instead of rejecting it, which is what keeps a + # Windows path usable, so no backslash in one may be decoded: doing so + # would turn the \t of \Temp\test into a tab while leaving \U alone. + self.assertEqual(cr.get("custom", "hasunknownescape"), R"foo\qbar") + self.assertEqual(cr.get("custom", "haswindowspath"), R"C:\Users\RUNNER~1\AppData\Local\Temp\test_foo") + + # Values without escapes keep parsing as before. + self.assertEqual(cr.get("custom", "ordinary"), "hello world") + self.assertEqual(cr.get("custom", "withcolon"), "host:port") + self.assertEqual(cr.get("custom", "withequals"), "key=value") + self.assertEqual(cr.get("custom", "withhash"), "value") + 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)