Fix IndexError in GitConfigParser When a Quoted Config Value Contains a Trailing New Line
#1908
Conversation
Improve the guarding `if` check in `GitConfigParser`'s `string_decode` function to safely handle empty strings and prevent `IndexError`s when accessing string elements. This resolves an IndexError in the `GitConfigParser`'s `.read()` method when the config file contains a quoted value containing a trailing new line. Fixes: gitpython-developers#1887
There was a problem hiding this comment.
I've suggested a small change that I think improves clarity.
Separately from the changes here but relevant to the logic they are a part of, I'm a bit worried about the case where there are two (or more) trailing backslashes, so that removing one of them still leaves a trailing backslash. However, that was present before, and this PR definitely need not be expanded to address it! I'm also not certain I'm right to be worried about that; maybe it is less important to cover, or a part of a separate class of malformed configuration file inputs that are intended to trigger decoding errors.
Thanks for taking another look - admittedly I forgot to merge this PR. Cygwin is the bane of this CI as it takes unnatural amounts of time :/. Having worked on the |
| @@ -452,7 +452,7 @@ def _read(self, fp: Union[BufferedReader, IO[bytes]], fpname: str) -> None: | |||
| e = None # None, or an exception. | |||
|
|
|||
| def string_decode(v: str) -> str: | |||
| if v[-1] == "\\": | |||
| if v and v.endswith("\\"): | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Sorry, for some reason I didn't see that comment! That addresses this proactively and my above comment can be disregarded entirely.
If the code is modified further in the future then the str annotation, which is inconsistent with a None value, could be examined and potentially changed.
There was a problem hiding this comment.
By the way, I tried to enable branch protections which should enable auto-merge, and thus resolve the issue with long-running CI sometimes preventing a merge in addition to me forgetting to do it later. It didn't yet work for this PR, but I will see for the next one.
Fixes: #1887
Improve the guarding
ifcheck inGitConfigParser'sstring_decodefunction to safely handle empty strings and preventIndexErrors when accessing string elements.This resolves an IndexError in the
GitConfigParser's.read()method when the config file contains a quoted value ending with a new line.