search

gitpython-developers / GitPython

GitPython is a python library used to interact with Git repositories.
http://gitpython.readthedocs.org
BSD 3-Clause "New" or "Revised" License
4.6k stars 905 forks source link

Fix `IndexError` in `GitConfigParser` When a Quoted Config Value Contains a Trailing New Line #1908

Closed DaveLak closed 5 months ago

DaveLak commented 5 months ago

Fixes: https://github.com/gitpython-developers/GitPython/issues/1887

Improve the guarding if check in GitConfigParser's string_decode function to safely handle empty strings and prevent IndexErrors 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.

Byron commented 5 months ago

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 gitoxide version of this I can say that this implementation here is more hole than cheese (if that makes sense :)), but it seems to work well enough for the common cases, still. Since GitPython is in maintenance mode, I don't think there is any need to try to improve it beyond fixing bugs.