Skip to content

pathlib.Path.expanduser() does not call os.path.expanduser() #84080

Closed
Closed
pathlib.Path.expanduser() does not call os.path.expanduser()#84080
Labels
3.10 (EOL)end of lifestdlibStandard Library Python modules in the Lib/ directory

Description

@barneygale
barneygale
mannequin
opened on Mar 8, 2020
BPO 39899
Nosy @serhiy-storchaka, @eryksun, @zooba, @barneygale, @Fidget-Spinner, @akulakov
PRs
  • bpo-39899: Make pathlib use os.path.expanduser() to expand home directories #18841
  • bpo-39899: ntpath.expanduser(): don't check the basename of the directory matches the username if we're requesting the current user's home directory #25277
    		•

    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 2021-04-09.22:03:46.934>
created_at = <Date 2020-03-08.05:06:12.759>
labels = ['library', '3.10']
title = '`pathlib.Path.expanduser()` does not call `os.path.expanduser()`'
updated_at = <Date 2021-09-06.17:52:12.404>
user = 'https://github.com/barneygale'

    bugs.python.org fields:

    activity = <Date 2021-09-06.17:52:12.404>
actor = 'andrei.avk'
assignee = 'none'
closed = True
closed_date = <Date 2021-04-09.22:03:46.934>
closer = 'steve.dower'
components = ['Library (Lib)']
creation = <Date 2020-03-08.05:06:12.759>
creator = 'barneygale'
dependencies = []
files = []
hgrepos = []
issue_num = 39899
keywords = ['patch']
message_count = 19.0
messages = ['363635', '363636', '363637', '363639', '363644', '363647', '363652', '390449', '390452', '390461', '390477', '390494', '390497', '390504', '390507', '390528', '390669', '401167', '401168']
nosy_count = 6.0
nosy_names = ['serhiy.storchaka', 'eryksun', 'steve.dower', 'barneygale', 'kj', 'andrei.avk']
pr_nums = ['18841', '25277']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue39899'
versions = ['Python 3.10']

    Activity

    1. barneygale commented on Mar 8, 2020

      @barneygale
      barneygalemannequin
      on Mar 8, 2020
      MannequinAuthor

      pathlib.Path.expanduser() does not call os.path.expanduser(), but instead re-implements it. The implementations look pretty similar and I can't see a good reason for the duplication. The only difference is that pathlib.Path.expanduser() raises RuntimeError when a home directory cannot be resolved, whereas os.path.expanduser() returns the path unchanged.

    2. barneygale
      added
      3.9 (EOL)end of life
      stdlibStandard Library Python modules in the Lib/ directory
      on Mar 8, 2020

    3. serhiy-storchaka commented on Mar 8, 2020

      @serhiy-storchaka
      serhiy-storchaka
      on Mar 8, 2020
      Member

      There are two reasons:

      1. os.path.expanduser() returns the path unchanged when a home directory cannot be resolved, pathlib.Path.expanduser() raises an error. The latter behavior looks more robust, but we can't change os.path.expanduser().

      2. os.path.expanduser() needs to split the path on components while pathlib.Path.expanduser() already has ready components. In some cases it may be more efficient.

    4. barneygale commented on Mar 8, 2020

      @barneygale
      barneygalemannequin
      on Mar 8, 2020
      MannequinAuthor

      We can check whether os.path.expanduser() returned a path beginning with "~" and raise a RuntimeError if so, right? On point #2, I'm not sure this optimization alone justifies the duplication. PR incoming...

    5. serhiy-storchaka commented on Mar 8, 2020

      @serhiy-storchaka
      serhiy-storchaka
      on Mar 8, 2020
      Member

      I see no reason to change the current code.

    6. serhiy-storchaka
      closed this as completedon Mar 8, 2020
    7. serhiy-storchaka
      added
      invalid
      on Mar 8, 2020
    8. serhiy-storchaka
      closed this as completedon Mar 8, 2020
    9. serhiy-storchaka
      added
      invalid
      on Mar 8, 2020

    10. barneygale commented on Mar 8, 2020

      @barneygale
      barneygalemannequin
      on Mar 8, 2020
      MannequinAuthor

      I see no reason for the duplication, and I can point to one concrete bug affecting your re-implementation of expanduser that doesn't affect the original, i.e. that a KeyError is raised on Windows when "USERNAME" is not present in os.environ, whereas all similar cases raise RuntimeError. These sorts of issues sneak in when you duplicate code - better to stick with the battle-hardened version rather than an inherently risky rewrite.

    11. serhiy-storchaka commented on Mar 8, 2020

      @serhiy-storchaka
      serhiy-storchaka
      on Mar 8, 2020
      Member

      It was discussed in bpo-19776.

      Having separate implementation we can avoid design flaws of os.path.expanduser().

    12. barneygale commented on Mar 8, 2020

      @barneygale
      barneygalemannequin
      on Mar 8, 2020
      MannequinAuthor

      The only design flaw mentioned in that thread is that os.path.expanduser() returns the input unchanged if expansion fails, which is not very pythonic. However, such a problem doesn't necessitate a rewrite of os.path.expanduser(). Checking result[:1] is enough.

      I think there's also a difference in the Windows heuristic in that pathlib checks whether basename(%HOMEPATH%) == %USERNAME% whereas ntpath.expanduser doesn't. But if that's really an issue it should probably be fixed in ntpath IMO, rather than having divergent implementations.

    13. 29 remaining items

    14. zooba
      closed this as completedon Apr 7, 2021
    15. zooba
      closed this as completedon Apr 7, 2021

    16. eryksun commented on Apr 7, 2021

      @eryksun
      eryksun
      on Apr 7, 2021
      Contributor

      For a "~user" path, the value of userhome should always be used if target_user == current_user. If for some reason the USERPROFILE environment variable isn't defined, the fallback "%HOMEDRIVE%%HOMEPATH%" does not necessarily end in the user's name. Example rewrite:

          if i != 1: #~user
        target_user = path[1:i]
        if isinstance(target_user, bytes):
            target_user = os.fsdecode(target_user)
        current_user = os.environ.get('USERNAME')

        if target_user != current_user:
            # Try to guess user home directory.  By default all user
            # profile directories are located in the same place and are
            # named by corresponding usernames.  If userhome isn't a
            # normal profile directory, this guess is likely wrong,
            # so we bail out.
            if current_user != basename(userhome):
                return path
            userhome = join(dirname(userhome), target_user)
    17. eryksun
      reopened this on Apr 7, 2021
    18. eryksun
      reopened this on Apr 7, 2021

    19. serhiy-storchaka commented on Apr 8, 2021

      @serhiy-storchaka
      serhiy-storchaka
      on Apr 8, 2021
      Member

      AFAIK you can set arbitrary path as user home directory. So home directories of different users can even not be on the same disk, and the last component of the path can be different from the user name.

      os.path.expanduser() has many flaws, and it just "guess" the home directory for other users. It is difficult to fix os.path.expanduser() due to backward compatibility, but we should do better in pathlib from start.

    20. eryksun commented on Apr 8, 2021

      @eryksun
      eryksun
      on Apr 8, 2021
      Contributor

      os.path.expanduser() has many flaws, and it just "guess" the
      home directory for other users.

      I'm fine with not guessing another user's profile directory (or home directory) in some cases, or even always. But the code that got committed bails out even if the target user is the same as the current user, according to the USERNAME environment variable. It shouldn't do that.

      ---

      If we want something better than guessing, it's not very hard to get the real profile directory or home directory for another user on the current system. The profile directory is configured as the "ProfileImagePath" value in a subkey of "HKLM\Software\Microsoft\Windows NT\CurrentVersion\ProfileList". The subkey name is the user SID in string form. The SID can be looked up with LookupAccountNameW(NULL, target_user, &sid, ...) and converted to string form with ConvertSidToStringSidW(&sid, string_sid). If there's no local profile directory, try looking up the user's configured home_dir and/or home_dir_drive (a mapped drive for a remote home directory) via NetUserGetInfo(NULL, target_user, 4, &info).

    21. barneygale commented on Apr 8, 2021

      @barneygale
      barneygalemannequin
      on Apr 8, 2021
      MannequinAuthor

      Good spot Eryk - I've put in another PR to address it.

    22. zooba commented on Apr 9, 2021

      @zooba
      zooba
      on Apr 9, 2021
      Member

      New changeset ba1db57 by Barney Gale in branch 'master':
      bpo-39899: Don't double-check directory name if we're requesting the current user's home directory in ntpath.expanduser() (GH-25277)
      ba1db57

    23. zooba
      closed this as completedon Apr 9, 2021
    24. zooba
      closed this as completedon Apr 9, 2021

    25. akulakov commented on Sep 6, 2021

      @akulakov
      akulakov
      on Sep 6, 2021
      Contributor

      Note this change also fixes https://bugs.python.org/issue41082 . I'm guessing it's too much of an edge case to backport this fix to 3.9, so I've put up a possible fix via docs update on that issue.

    26. akulakov commented on Sep 6, 2021

      @akulakov
      akulakov
      on Sep 6, 2021
      Contributor

      To be more precise, this change fixes https://bugs.python.org/issue41082 by raising RuntimeError instead of KeyError and also by documenting it, which means matplotlib can fix it by either using os.path.expanduser or catching RuntimeError, whichever might work better in their case.

      I will let them know once we sort this out.

    27. ezio-melotti
      transferred this issue fromon Apr 10, 2022

    28. zahlman commented on Nov 4, 2025

      @zahlman
      zahlman
      on Nov 4, 2025

      Relatedly: Path('./~').expanduser() (or with any number of leading ./ components) will expand, where os.path.expanduser('./~') gives back './~'. This is because the . components are dropped while creating the Path object. (It doesn't matter what the current working directory is; the path isn't being resolved here.) Not a bug (at least IMHO), but noteworthy for anyone migrating code who expected to conditionally disable path expansion by supplying a value like that.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      3.10 (EOL)end of lifestdlibStandard Library Python modules in the Lib/ directory

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions