get_netrc_auth should handle os.path.expanduser failing #1846
Conversation
|
Upstream Python bug report: http://bugs.python.org/issue20164 |
|
Well this makes me very sad. Thanks for raising this! Given that upstream is not going to fix this on 2.6 for sure, even if they do implement a fix elsewhere, I think we need to handle this. The correct behaviour is that I'm also inclined to be cautious, so we should restrict the scope of the |
os.path.expanduser can raise a KeyError when $HOME is not set and the POSIX getpwuid() call fails, which can happen when running under a UID which is not in /etc/passwd or when the password file cannot be read. The upstream bug report http://bugs.python.org/issue20164 is unlikely to be backported to Python 2.x even if fixed so this change handles KeyError by skipping netrc authentication. Closes psf#1846
| # getpwuid fails. See http://bugs.python.org/issue20164 & | ||
| # https://github.com/kennethreitz/requests/issues/1846 | ||
| return | ||
|
|
||
| if os.path.exists(loc) and not netrc_path: | ||
| netrc_path = loc |
There was a problem hiding this comment.
While we're changing this function, can we avoid this and not netrc_path test by breaking from this loop instead? Saves us some iterations, probably.
There was a problem hiding this comment.
I just pushed out a80bd77 which does this and also changes the next block to a more obvious bare return rather than returning a variable which can only be None
* Stop iterating as soon as we find a netrc file * More obvious return None
|
Alright, I'm happy with this. You can ignore TravisCI (I have no idea why it's still running), the relevant CI results are these (all of which passed): http://ci.kennethreitz.org/job/requests-pr/PYTHON=2.6/167/ Waiting on @sigmavirus24 to review. |
|
Great - thanks for reviewing it! |
|
WHY IS TRAVIS RUNNING AGAIN OH MY GOD |
get_netrc_auth should handle os.path.expanduser failing
|
I'm very sad that this requires so many lines of code, but so be it. |
|
This will at some point become unnecessary once the upstream fix has been widely deployed – hopefully in less than another 5 years: |
https://github.com/toastdriven/django-haystack/issues/924 has a problem report from a user who appears to be running inside a process which does not have
$HOMEdefined and is running under a UID which is either not in /etc/passwd or does not have permission to access it.This causes utils.get_netrc_auth to raise an unexpected
KeyError. The easiest fix would be to simply add that to the except block at the bottom but that's probably too dangerous – I'd probably handle theKeyErrorright at the source until this can be fixed upstream.