-
-
Notifications
You must be signed in to change notification settings - Fork 32k
bpo-31904: posixpath.expanduser() handles user home of None #23530
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
user home directory is None. * Skip test_expanduser and test_expanduser_pwd on VxWorks
As a person using this API I'd probably prefer an exception instead of getting the input string returned without me noticing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really have to modify expanduser() on VxWorks?
I would prefer with a first PR to only skip the two tests on VxWorks: copy your skip into a new PR. You don't have to write a NEWS entry, I can add "skip news" label.
If no user home directory set for certain user, the variable userhome will result in None. And further os.fsencode(userhome) and userhome.rstrip(root) will run into exception.
Done. |
Lib/posixpath.py
Outdated
@@ -262,6 +262,9 @@ def expanduser(path): | |||
# password database, return the path unchanged | |||
return path | |||
userhome = pwent.pw_dir | |||
# if the current user has no home directory, return the path unchanged | |||
if userhome is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not comfortable to change the behavior on any platform.
I would be ok if the change is specific to VxWorks, if you add:
if userhome is None: | |
if userhome is None and sys.platform == "vxworks": |
Please update the comment and the NEWS entry to mention that only VxWorks is impacted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modified.
I don't expect None as the home directory on platforms other than VxWorks, and so I prefer to get an exception. I agree with @jstasiak: #23530 (comment) Ok, now it leaves the behavior unchanged on other platforms. Thanks! I merged your PR. |
@vstinner I have to go back on my previous comment a little bit. I just read the surrounding code and it seems that there are already cases where the path is returned unchanged so there's precedent established. |
I wrote this code path:
This case is well specified. But for me, if the user exists, its pwd.getpwnam(name).pw_dir should not be None. The current code worked well for 30 years, but I don't feel the need to change it on all platforms just because of VxWorks ;-) |
On VxWorks, no user home directory at all. So posixpath.expanduser() returns the input path unchanged if user home directory is None. And skip test_expanduser and test_expanduser_pwd on VxWorks.
https://bugs.python.org/issue31904