bpo-31904: posixpath.expanduser() handles user home of None by pxinwr · Pull Request #23530 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation8 Commits4 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
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
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.
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.
Do you really have to modify expanduser() on VxWorks?
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.
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.
Done.
| @@ -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: |
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.
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:
except KeyError:
# [bpo-10496](https://bugs.python.org/issue10496): if the user name from the path doesn't exist in the
# password database, return the path unchanged
return 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 ;-)
adorilson pushed a commit to adorilson/cpython that referenced this pull request
