gh-100809: Fix handling of drive-relative paths in pathlib.Path.absolute()

[barneygale]

If it's available, use nt._getfullpathname() to retrieve an absolute path. This allows paths such as 'X:' to be made absolute even when os.getcwd() returns a path on another drive. It follows the behaviour of os.path.abspath(), except that no path normalisation is performed.

• Issue: pathlib.Path.absolute() mishandles drive-relative Windows paths  #100809

[eryksun]

Are you planning to also fix this in 3.10 and 3.11? That requires fixing _from_parts().

>>> pathlib.WindowsPath._from_parts(['C:\\spam', 'C:eggs'])
WindowsPath('C:eggs')

Maybe the fix for _from_parts() should be a separate PR.

[barneygale]

Are you planning to also fix this in 3.10 and 3.11? That requires fixing _from_parts().

>>> pathlib.WindowsPath._from_parts(['C:\\spam', 'C:eggs'])
WindowsPath('C:eggs')

Maybe the fix for _from_parts() should be a separate PR.

I fixed this in #95450 and we decided not to backport because it was seemed like quite an obscure bug. I'd like to do the same here as the bug seems equally obscure (there are no user-logged bugs on the tracker that I could find), but I don't mind backporting both if we collectively feel that it's important.

[barneygale]

With fresher eyes I realise my tests don't exercise per-drive working directories.

One way to address this would be to add a subst() context manager to Lib/test/support/os_helper.py and use it from test_absolute(). The context manager would call subst to allocate a new drive letter on __enter__(), and subst /d on __exit__(). But perhaps there's a better way, @eryksun?

[eryksun]

I fixed this in #95450 and we decided not to backport because it was seemed like quite an obscure bug.

Maybe skip fixing this in 3.10, but absolute() is a documented method in 3.11. It should be able to resolve relative drive paths such as "Z:spam", which date back to MS-DOS in the 1980s. Maybe there's a more surgical fix for handling relative drive paths in _from_parts() that doesn't require using ntpath.join().

[barneygale]

Maybe, but the bug is observable just from Path('C:\\foo') / 'C:', and constructors have been documented the entire time. I don't think I can make the fix for #94909 much more surgical than #95450 because pathlib's original algorithm for joining segments (right-to-left scanning) doesn't lend itself well to the problem. Any fix I attemped resembled os.path.join().

I don't think the presence of this bug in 3.11 makes absolute() so defective as to not warrant documentation.

Does that seem reasonable?

[eryksun]

With fresher eyes I realise my tests don't exercise per-drive working directories.

BASE is os.path.realpath(TESTFN), which is in the initial current working directory. The _getfullpathname() branch is tested as long as this resolves to a real path on a drive-letter drive.

One way to address this would be to add a subst() context manager to Lib/test/support/os_helper.py and use it from test_absolute(). The context manager would call subst

The "subst.exe" app calls DefineDosDeviceW() without the flag DDD_NO_BROADCAST_SYSTEM. Without this flag, a WM_DEVICECHANGE message for either DBT_DEVICEARRIVAL or DBT_DEVICEREMOVECOMPLETE is broadcasted to the top-level windows of applications in the current session via BroadCastSystemMessageW().

I wouldn't want to announce the new drive in the current session (i.e. DBT_DEVICEARRIVAL), particularly not to the desktop shell, Explorer¹. Instead, QueryDosDeviceW() and DefineDosDeviceW() could be added to _winapi, or called using ctypes. For example:

try:
    import ctypes
except ImportError:
    def subst_drive(path):
        raise NotImplementedError('ctypes is not available')
else:
    import contextlib
    import string

    ERROR_FILE_NOT_FOUND = 2
    DDD_REMOVE_DEFINITION = 2
    DDD_EXACT_MATCH_ON_REMOVE = 4
    DDD_NO_BROADCAST_SYSTEM = 8

    kernel32 = ctypes.WinDLL('kernel32', use_last_error=True)

    @contextlib.contextmanager
    def subst_drive(path):
        """Temporarily yield a substitute drive for a given path."""
        for c in reversed(string.ascii_uppercase):
            drive = f'{c}:'
            if (not kernel32.QueryDosDeviceW(drive, None, 0) and
                    ctypes.get_last_error() == ERROR_FILE_NOT_FOUND):
                break
        else:
            raise FileExistsError('no available logical drive')
        if not kernel32.DefineDosDeviceW(
                DDD_NO_BROADCAST_SYSTEM, drive, path):
            raise ctypes.WinError(ctypes.get_last_error())
        try:
            yield drive
        finally:
            if not kernel32.DefineDosDeviceW(
                    DDD_REMOVE_DEFINITION | DDD_EXACT_MATCH_ON_REMOVE,
                    drive, path):
                raise ctypes.WinError(ctypes.get_last_error())

Footnotes

1. That said, we can't prevent the new drive from being visible to processes in the current logon session that call GetLogicalDrives() or GetLogicalDriveStringsW(). This is the case if an Explorer window is manually refreshed by pressing F5. ↩

[barneygale]

I wouldn't want to announce the new drive in the current session (i.e. DBT_DEVICEARRIVAL), particularly not to the desktop shell, Explorer

Could you expand on why this is a bad idea please? Thank you!

[eryksun]

Could you expand on why this is a bad idea please?

This is a drive that we're creating for our own temporary use. I don't think it's appropriate to have Explorer windows automatically updated to include it. I also wouldn't want unknown complications from what any other application might do when sent a WM_DEVICECHANGE message for DBT_DEVICEARRIVAL.

Also, it's more efficient to directly call QueryDosDeviceW() in a loop when searching for an available drive than it is to repeatedly spawn a "subst.exe" process.

[barneygale]

Gotcha, thanks.

Is there a time-of-check to time-of-use issue with your code above? I wonder if we could skip the QueryDosDeviceW checks and instead call DefineDosDeviceW with different drive letters until it succeeds?

[eryksun]

Is there a time-of-check to time-of-use issue with your code above? I wonder if we could skip the QueryDosDeviceW checks and instead call DefineDosDeviceW with different drive letters until it succeeds?

I don't know how to avoid the small race condition without resorting to the NT API. "subst.exe" doesn't handle this any better.

DefineDosDeviceW() tries to be clever. The device names are defined as symbolic link objects in NT, e.g. "\??\Z:" -> "\??\C:\Windows". The target path of the symbolic link is a counted string with a current length and a maximum length, like most strings in NT. DefineDosDeviceW() uses this to implement a scheme that allows defining a device multiple times. The most recent definition is the active one, and each definition can be individually removed from the target buffer. For example:

>>> flags = DDD_NO_BROADCAST_SYSTEM
>>> kernel32.DefineDosDeviceW(flags, 'Z:', 'C:\\Windows')
1
>>> kernel32.DefineDosDeviceW(flags, 'Z:', 'C:\\ProgramData')
1
>>> kernel32.DefineDosDeviceW(flags, 'Z:', 'C:\\Temp')
1
>>> os.path.samefile('Z:', 'C:\\Temp')
True
>>> flags = DDD_REMOVE_DEFINITION | DDD_EXACT_MATCH_ON_REMOVE
>>> kernel32.DefineDosDeviceW(flags, 'Z:', 'C:\\Temp')
1
>>> os.path.samefile('Z:', 'C:\\ProgramData')
True
>>> kernel32.DefineDosDeviceW(flags, 'Z:', 'C:\\ProgramData')
1
>>> os.path.samefile('Z:', 'C:\\Windows')
True

I think this is bad behavior. The new drive definition affects all processes in the current logon session (i.e. the local device context). I wouldn't want to mask an existing drive definition. At least redefining a system drive requires administrator access.

[zooba]

The macOS failure is known and unrelated, so this is good to merge if the rest of CI agrees.

[miss-islington]

Thanks @barneygale for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

[miss-islington]

Thanks @barneygale for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

[miss-islington]

Sorry, @barneygale and @zooba, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 072011b3c38f871cdc3ab62630ea2234d09456d1 3.10

[miss-islington]

Sorry @barneygale and @zooba, I had trouble checking out the 3.11 backport branch.
Please retry by removing and re-adding the "needs backport to 3.11" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 072011b3c38f871cdc3ab62630ea2234d09456d1 3.11

[miss-islington]

Thanks @barneygale for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

[miss-islington]

Sorry, @barneygale and @zooba, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 072011b3c38f871cdc3ab62630ea2234d09456d1 3.11

[zooba]

@barneygale Can I leave the backports with you?

[bedevere-bot]

GH-101993 is a backport of this pull request to the 3.11 branch.

[barneygale]

@zooba looks like this won't backport unless we also backport #95450. OK to skip?

[zooba]

Yeah, skip it.