Undocumented KeyError from os.path.expanduser

[acdha]

BPO	20164
Nosy	@terryjreedy, @vstinner, @serhiy-storchaka
Superseder	bpo-10496: Python startup should not require passwd entry

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2018-12-16.21:59:17.788>
created_at = <Date 2014-01-07.16:15:27.081>
labels = ['type-bug', 'library']
title = 'Undocumented KeyError from os.path.expanduser'
updated_at = <Date 2018-12-16.21:59:17.786>
user = 'https://bugs.python.org/acdha'

bugs.python.org fields:

activity = <Date 2018-12-16.21:59:17.786>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2018-12-16.21:59:17.788>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2014-01-07.16:15:27.081>
creator = 'acdha'
dependencies = []
files = []
hgrepos = []
issue_num = 20164
keywords = []
message_count = 7.0
messages = ['207552', '207555', '207579', '227746', '227953', '331924', '331938']
nosy_count = 6.0
nosy_names = ['terry.reedy', 'vstinner', 'docs@python', 'serhiy.storchaka', 'acdha', 'zaytsev']
pr_nums = []
priority = 'normal'
resolution = 'duplicate'
stage = 'resolved'
status = 'closed'
superseder = '10496'
type = 'behavior'
url = 'https://bugs.python.org/issue20164'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

[acdha]

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:

cadams@Io:~ $ sudo -H python
Python 2.7.5 (default, Aug 25 2013, 00:04:04) 
[GCC 4.2.1 Compatible Apple LLVM 5.0 (clang-500.0.68)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
>>> import posix
>>> posix.setuid(1001)
>>> posix.seteuid(1001)
>>> os.environ.pop('HOME')
'/var/root'
>>> os.path.expanduser("~")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/posixpath.py", line 269, in expanduser
    userhome = pwd.getpwuid(os.getuid()).pw_dir
KeyError: 'getpwuid(): uid not found: 1001'

Context:

• https://github.com/toastdriven/django-haystack/issues/924
• https://github.com/kennethreitz/requests/issues/1846.

[zaytsev]

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.

What do you think?

[acdha]

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]

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.

    if 'HOME' in os.environ:
        userhome = os.environ['HOME']
    elif 'USERPROFILE' in os.environ:
        userhome = os.environ['USERPROFILE']
    elif not 'HOMEPATH' in os.environ:
        return path
    <create userhome from HOMEDRIVE and HOMEPATH

--
The pwd doc already says " KeyError is raised if the entry asked for cannot be found.", so this is not an issue.

[acdha]

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]

The behavior was changed in f2f4555 as a part of bpo-10496.

[vstinner]

Even if it's not exactly a duplicate of bpo-10496, the reported bug has been fixed. I close the issue.