pathlib.Path: add username argument to home()

@serhiy-storchaka

BPO	42998
Nosy	@serhiy-storchaka, @eryksun, @zooba, @barneygale
PRs	bpo-42998: add 'user' parameter to pathlib.Path.home() #25271

^{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-06-13.15:51:38.442>
created_at = <Date 2021-01-22.03:57:15.818>
labels = ['type-feature', 'library']
title = 'pathlib.Path: add `username` argument to `home()`'
updated_at = <Date 2021-06-13.15:51:38.441>
user = 'https://github.com/barneygale'

bugs.python.org fields:

activity = <Date 2021-06-13.15:51:38.441>
actor = 'barneygale'
assignee = 'none'
closed = True
closed_date = <Date 2021-06-13.15:51:38.442>
closer = 'barneygale'
components = ['Library (Lib)']
creation = <Date 2021-01-22.03:57:15.818>
creator = 'barneygale'
dependencies = []
files = []
hgrepos = []
issue_num = 42998
keywords = ['patch']
message_count = 12.0
messages = ['385472', '385473', '390503', '390527', '390535', '391965', '391967', '391968', '391970', '391972', '392241', '395755']
nosy_count = 4.0
nosy_names = ['serhiy.storchaka', 'eryksun', 'steve.dower', 'barneygale']
pr_nums = ['25271']
priority = 'normal'
resolution = 'rejected'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue42998'
versions = []

The pathlib.Path.home() function looks like:

    @classmethod
    def home(cls):
        """Return a new path pointing to the user's home directory (as
        returned by os.path.expanduser('~')).
        """
        return cls(cls()._flavour.gethomedir(None))

If we add a username=None parameter and pass this to gethomedir() we can easily add a lookup of another user's home directory, so:

    import pathlib
    username = 'phil'
    pathlib.Path.home(username) == pathlib.Path('~' + username).expanduser()

I should add that this is part of a plan to spin out some Path methods into a new UserPath class. Notably, in this case, UserPath.expanduser() will call self.home() under-the-hood. This is done to reduce the surface area of abstract methods that subclasses of UserPath need implement. More discussion here: https://discuss.python.org/t/make-pathlib-extensible/3428

os.path.expanduser() has flaws.

It can return an argument unchanged if failed to determine the home directory.
On Unix it does not work for users not found in the local password database.
On Windows it only guess the home directory for other users. It does not work if you have home directory at nonstandard location (i.e. on other disk).

Until we fix these flaws it is better to not add flawed by design feature in pathlib.

Thanks for the feedback.

We can check the return value, like we do in Path.expanduser()
Seems like expected behaviour?
Worth fixing in ntpath.expanduser() I think. See Eryk Sun's comment here: https://bugs.python.org/issue39899#msg390507

Worth fixing in ntpath.expanduser()

To expand on the suggestion in msg390507, here's a demo that implements getting the profile path of a local user account, with support for the special profile directories "Default", "Public", and "ProgramData" (i.e. ALLUSERSPROFILE).

    ERROR_NONE_MAPPED = 1332

    def get_profile_path(name):
        if not isinstance(name, str):
            raise TypeError(f'name must be str, not {type(name).__name__}')
        profile_list = r'Software\Microsoft\Windows NT\CurrentVersion\ProfileList'
        if name.lower() in ('default', 'public', 'programdata'):
            subkey = profile_list
            value = name
        else:
            try:
                sid = lookup_account_name(name)[0]
            except OSError as e:
                if e.winerror != ERROR_NONE_MAPPED:
                    raise
                raise KeyError(f'name not found: {name}')
            subkey = f'{profile_list}\\{sid}'
            value = 'ProfileImagePath'
        try:
            with winreg.OpenKey(winreg.HKEY_LOCAL_MACHINE, subkey) as hkey:
                path, dtype = winreg.QueryValueEx(hkey, value)
        except FileNotFoundError:
            raise KeyError(f'name not found: {name}')
        if dtype == winreg.REG_SZ:
            return path
        if dtype == winreg.REG_EXPAND_SZ:
            return winreg.ExpandEnvironmentStrings(path)
        raise TypeError('profile path value must be a string type (1 or 2), '
                         f'not {dtype}')

For example:

    >>> print(get_profile_path('administrator'))
    C:\Users\Administrator

    >>> print(get_profile_path('default'))
    C:\Users\Default

    >>> print(get_profile_path('public'))
    C:\Users\Public

    >>> print(get_profile_path('programdata'))
    C:\ProgramData

    >>> print(get_profile_path('system'))
    C:\Windows\system32\config\systemprofile

    >>> print(get_profile_path('localservice'))
    C:\Windows\ServiceProfiles\LocalService

    >>> print(get_profile_path('networkservice'))
    C:\Windows\ServiceProfiles\NetworkService

For lookup_account_name(), _winapi.LookupAccountName(system_name, account_name) has to be implemented. It should convert the SID result to string form via ConvertSidToStringSidW() and return the tuple (sid_string, domain_name, account_type). Here's a ctypes prototype implementation of lookup_account_name():

    import ctypes

    lsalookup = ctypes.WinDLL(
        'api-ms-win-security-lsalookup-l2-1-0', use_last_error=True)
    sddl = ctypes.WinDLL('api-ms-win-security-sddl-l1-1-0', use_last_error=True)
    heap = ctypes.WinDLL('api-ms-win-core-heap-l2-1-0', use_last_error=True)

    ERROR_INSUFFICIENT_BUFFER = 122

    def lookup_account_name(name):
        sid = (ctypes.c_char * 1)()
        cb = ctypes.c_ulong()
        cch = ctypes.c_ulong()
        name_use = ctypes.c_ulong()
        lsalookup.LookupAccountNameW(None, name, sid, ctypes.byref(cb),
            None, ctypes.byref(cch), ctypes.byref(name_use))
        error = ctypes.get_last_error()
        if error != ERROR_INSUFFICIENT_BUFFER:
            raise ctypes.WinError(error)

        sid = (ctypes.c_char * cb.value)()
        domain_name = (ctypes.c_wchar * cch.value)()
        success = lsalookup.LookupAccountNameW(None, name, sid,
                        ctypes.byref(cb), domain_name, ctypes.byref(cch),
                        ctypes.byref(name_use))
        if not success:
            raise ctypes.WinError(ctypes.get_last_error())

        ssid = ctypes.c_wchar_p()
        if not sddl.ConvertSidToStringSidW(sid, ctypes.byref(ssid)):
            raise ctypes.WinError(ctypes.get_last_error())
        string_sid = ssid.value
        heap.LocalFree(ssid)

        return string_sid, domain_name.value, name_use.value

I'm not convinced this is a good change, mainly for the reasons Serhiy mentioned (and others have mentioned when I've spoken with them privately).

What would you do with your custom subclass if home() did not have this argument?

Thanks very much for taking a look. I can understand the view that, given the unreliability of os.path.expanduser('~foo'), we shouldn't be making that functionality more available.

My argument for this being a reasonable change is as follows:

Firstly, os.getcwd() is a more fundamental operation than os.path.absolute(). In both pathlib and os.path, the absolute() implementation is built on the getcwd() implementation. The process is: call the more fundamental operation, and append whatever else was passed as an argument.

Analagously, gethomedir() is a more fundamental operation than expanduser(). In the same way, expanduser() can call a more fundamental operation and then append the path passed in.

Secondly, for a pathlib.AbstractPath implementation, it's a much cleaner API to have getcwd() and gethomedir() as abstract methods with identical signatures, and absolute() and expanduser() with default implementations.

Thirdly, if there's are issues with the implementation for retrieving other user's home directories, those should be treated as individual issues with their own bugs, and we should add suitable warnings in the docs in the short term. My patch should make these cases *easier* to solve as we no longer have two competing implementations of the same thing.

Finally, I don't think anyone is going to see home('barney') in the docs and decided to retrieve a user's home directory on a whim. If someone does need to retrieve another user's home directory, they're going to use Path('~' + user).expanduser() if Path.home(user) isn't available.

My argument mostly comes from the similarities I perceive between getcwd/absolute and gethomedir/expanduser. We don't expect users to os.path.absolute('.') to get the current working directory, after all! But perhaps I've overestimated the similarities here. Any thoughts?

Thanks again for taking a look.

In the previous comment, I was referring to bpo-39899 when I referred to "my patch". Long day! :D

Yeah, that's all reasonable.

But why wouldn't we go the other way and say "getting a user directory for a different user is not supported and all existing ways to do it are now deprecated"?

I know you think that's not a relevant argument right now, but it is a related one, and any change to the status quo justifies bringing it back up. So now it's up, we want to decide on it before we make another change that could run counter to the overall direction.

So I think, given the unlikelihood of us being able to properly fix the functionality, that we should deprecate getting another user's directory completely and, hence, not add new entry points to deprecated API.

Convince me I'm wrong :)

@eryksun

Totally valid!

I suppose it hinges on the relatively likelihood/unlikelihood of us being able to make expanduser('~foo') reliable in future. On this question I'm relieved to defer to the experts!

@eryksun: penny for your thoughts?

So far I've been keen to keep the Path and AbstractPath APIs the same, but perhaps this case warrants an exception.

What if AbstractPath lacked both home() and expanduser(), and those methods only existed on Path? Of all the methods on Path, these two are the *least* useful for non-local filesystems.

We can then circle back round to this issue when the fate of expanduser() becomes clearer.

Does that sound any good?

Per discussion, expanduser('~other') is considered unreliable, and we shouldn't add new functions that call through to it. Resolving as 'rejected'.

Navigation Menu

Repository navigation

pathlib.Path: add `username` argument to `home()` #87164

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

pathlib.Path: add username argument to home() #87164

Description

Activity

barneygale commented on Jan 22, 2021

barneygale commented on Jan 22, 2021

serhiy-storchaka commented on Apr 8, 2021

barneygale commented on Apr 8, 2021

eryksun commented on Apr 8, 2021

zooba commented on Apr 26, 2021

barneygale commented on Apr 26, 2021

barneygale commented on Apr 26, 2021

zooba commented on Apr 26, 2021

barneygale commented on Apr 26, 2021

barneygale commented on Apr 28, 2021

barneygale commented on Jun 13, 2021

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

pathlib.Path: add `username` argument to `home()` #87164