You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Undocumented KeyError from os.path.expanduser #64363
This is a more general version of bpo-10496: os.path.expanduser is documented as returning the unmodified string if it cannot be expanded (http://docs.python.org/3/library/os.path.html#os.path.expanduser) but there's one edge case where this can fail: when running without HOME in the environment:
In this case, pwd.getpwuid is called and although it's not documented as such, it raises a KeyError anytime the underlying POSIX getpwuid() call fails:
Amusingly, it's also the case on BG/Q compute nodes. Only this morning, I cooked up the following patch:
--- a/Lib/posixpath.py+++ b/Lib/posixpath.py@@ -237,7 +237,11 @@ def expanduser(path):
if i == 1:
if 'HOME' not in os.environ:
import pwd
- userhome = pwd.getpwuid(os.getuid()).pw_dir+ try:+ userhome = pwd.getpwuid(os.getuid()).pw_dir+ except KeyError:+ import warnings+ warnings.warn('Unable to auto-detect $HOME, export it or functionality such as user-installed modules will not work!', RuntimeWarning)
else:
userhome = os.environ['HOME']
else:
I think it's a bit harsh to fail completely, but returning silently also doesn't sound like a good idea, so I decided to issue a warning instead; the wording should be probably tweaked by a native speaker of English though.
Other than hoisting the warnings import to the top (PEP-8) that seems entirely reasonable to me. The user report which we got was confusing because it was non-obvious where it came from - a warning or other pointer would have helped the original reporter get an idea as to what was failing earlier.
The other two changes I'd suggest would be documentation updates:
os.path.expanduser noting that KeyError may be raised when HOME is undefined and pwd.getpwuid fails for the active user
pwd.getpwuid noting that KeyErrors will be raised when the underlying POSIX call fails
I think the doc should be considered correct and posixpath buggy. The ntpath code checks for the existence of HOME and substitutes before returning the arg unchanged, as documented.
I agree that making the code match the docs would be preferable – an unexpected KeyError might be easier to track down that way but it'd still surprise most developers.
re:pwd docs, the formatting in https://hg.python.org/cpython/file/8e9df3414185/Doc/library/pwd.rst makes it easy to miss the KeyError note – it's at the end of a paragraph after a table and since "entry" is a generic term it's possible to misread it as applying to fields within the record rather than the record itself. I think it would at least be worth adding a newline and perhaps explicitly noting that it applies to any function which returns a password database entry.
Given that there are only two functions which it applies to, it might be best to simply add a note to each of them for people who use "pydoc pwd.getpwduid" or an IDE and don't see the module-level docs but that's getting a bit contrived.
Activity
acdha commentedon Jan 7, 2014
This is a more general version of bpo-10496: os.path.expanduser is documented as returning the unmodified string if it cannot be expanded (http://docs.python.org/3/library/os.path.html#os.path.expanduser) but there's one edge case where this can fail: when running without HOME in the environment:
https://github.com/python/cpython/blob/3.3/Lib/posixpath.py#L259-L263
In this case, pwd.getpwuid is called and although it's not documented as such, it raises a KeyError anytime the underlying POSIX getpwuid() call fails:
https://github.com/python/cpython/blob/3.3/Modules/pwdmodule.c#L119-L120
Reproducing this deliberately:
Context:
zaytsev commentedon Jan 7, 2014
Amusingly, it's also the case on BG/Q compute nodes. Only this morning, I cooked up the following patch:
I think it's a bit harsh to fail completely, but returning silently also doesn't sound like a good idea, so I decided to issue a warning instead; the wording should be probably tweaked by a native speaker of English though.
What do you think?
acdha commentedon Jan 7, 2014
Other than hoisting the warnings import to the top (PEP-8) that seems entirely reasonable to me. The user report which we got was confusing because it was non-obvious where it came from - a warning or other pointer would have helped the original reporter get an idea as to what was failing earlier.
The other two changes I'd suggest would be documentation updates:
os.path.expanduser noting that KeyError may be raised when HOME is undefined and pwd.getpwuid fails for the active user
pwd.getpwuid noting that KeyErrors will be raised when the underlying POSIX call fails
terryjreedy commentedon Sep 28, 2014
I think the doc should be considered correct and posixpath buggy. The ntpath code checks for the existence of HOME and substitutes before returning the arg unchanged, as documented.
--
The pwd doc already says " KeyError is raised if the entry asked for cannot be found.", so this is not an issue.
acdha commentedon Sep 30, 2014
I agree that making the code match the docs would be preferable – an unexpected KeyError might be easier to track down that way but it'd still surprise most developers.
re:pwd docs, the formatting in https://hg.python.org/cpython/file/8e9df3414185/Doc/library/pwd.rst makes it easy to miss the KeyError note – it's at the end of a paragraph after a table and since "entry" is a generic term it's possible to misread it as applying to fields within the record rather than the record itself. I think it would at least be worth adding a newline and perhaps explicitly noting that it applies to any function which returns a password database entry.
Given that there are only two functions which it applies to, it might be best to simply add a note to each of them for people who use "pydoc pwd.getpwduid" or an IDE and don't see the module-level docs but that's getting a bit contrived.
serhiy-storchaka commentedon Dec 16, 2018
The behavior was changed in f2f4555 as a part of bpo-10496.
vstinner commentedon Dec 16, 2018
Even if it's not exactly a duplicate of bpo-10496, the reported bug has been fixed. I close the issue.