[pathlib] issues with Windows device paths

[eryksun]

BPO	33898
Nosy	@pfmoore, @pitrou, @tjguk, @zware, @eryksun, @zooba
PRs	bpo-33898: Fix pathlib issues with Windows device paths #8671

^{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 = None
created_at = <Date 2018-06-18.22:40:29.969>
labels = ['type-bug', '3.8', '3.9', '3.7', 'library', 'OS-windows']
title = 'pathlib issues with Windows device paths'
updated_at = <Date 2020-04-17.17:59:50.770>
user = 'https://github.com/eryksun'

bugs.python.org fields:

activity = <Date 2020-04-17.17:59:50.770>
actor = 'pitrou'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)', 'Windows']
creation = <Date 2018-06-18.22:40:29.969>
creator = 'eryksun'
dependencies = []
files = []
hgrepos = []
issue_num = 33898
keywords = ['patch']
message_count = 2.0
messages = ['319921', '337405']
nosy_count = 6.0
nosy_names = ['paul.moore', 'pitrou', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower']
pr_nums = ['8671']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue33898'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

Linked PRs

• GH-78079: Fix UNC device path root normalization in pathlib #102003

[eryksun]

For \\?\ extended device paths, pathlib removes the trailing slash for the root directory if the device isn't UNC or a logical (A-Z) drive.

Correct:

    >>> str(Path('//?/UNC/'))
    '\\\\?\\UNC\\'

    >>> str(Path('//?/Z:/'))
    '\\\\?\\Z:\\'

Incorrect:

    >>> str(Path('//?/BootPartition/'))
    '\\\\?\\BootPartition'

    >>> str(Path('//?/Volume{}/'))
    '\\\\?\\Volume{}'

    >>> str(Path('//?/Global/Z:/'))
    '\\\\?\\Global\\Z:'

It keeps the trailing slash for some \\.\ paths, but not all.

Correct:

    >>> str(Path('//./BootPartition/'))
    '\\\\.\\BootPartition\\'

Incorrect:

    >>> str(Path('//./Global/Z:/'))
    '\\\\.\\Global\\Z:'

It adds a root directory to \\.\ device paths where none was specified.

Incorrect:

    >>> str(Path('//./nul'))
    '\\\\.\\nul\\'

    >>> str(Path('//./PhysicalDrive0'))
    '\\\\.\\PhysicalDrive0\\'

    >>> str(Path('//./C:'))
    '\\\\.\\C:\\'

"\\\\.\\C:" refers to the volume device, whereas "\\\\.\\C:\\" is the root directory in the file system.

pathlib should parse \\?\ and \\.\ device paths the same way with respect to the drive and root. The difference in practice is only how Windows does (\\.\) or does not (\\?\) canonicalize the path.

Additionally, pathlib fails to identify the drive correctly in these cases.

Incorrect:

    >>> Path('//?/Global/Z:/').drive
    '\\\\?\\'

    >>> Path('//?/BootPartition/Temp').drive
    '\\\\?\\'

Except for "UNC" and "Global" paths, the drive should be the first component after the local-device prefix. The "UNC" device also includes subsequent server and share components, if any. For the reserved "Global" symlink, it should look to the next component. For example, r'\\?\Global\UNC\server\share' is a drive.

There's also the "GlobalRoot" symlink (or "Global\\GlobalRoot" to be pedantic), but that can't be handled generically.

[zooba]

Eryk - I've got the PR ready as far as I'm concerned. Have you had a look to see if you're happy with its logic for these cases?

[barneygale]

@eryksun I suppose that any remaining pathlib device path normalization issues are down to the if condition here:

cpython/Lib/pathlib.py

Lines 284 to 286 in 666c084

	if drv.startswith(sep):
	# pathlib assumes that UNC paths always have a root.
	root = sep

Can you recommend a better condition? :)

[zooba]

I think this is the issue that requires splitroot to be correct (which it might be by now?) and then pathlib has to be very deliberate about the meaning of a trailing slash in the drive part vs a leading slash in the rest of the path.

The current PR doesn't seem to be easily resolvable, but is probably well out of date anyway because of other work that's been done in pathlib and ntpath.

The existing test suite and the examples Eryk posted above are probably the best way to figure out the right semantics.

[barneygale]

Based on Eryk's test cases, ntpath.splitroot() gets //?/Global and //./Global paths wrong:

>>> ntpath.splitroot('//./Global/Z:/')
('//./Global', '/', 'Z:/')
>>> ntpath.splitroot('//?/Global/Z:/')
('//?/Global', '/', 'Z:/')

I believe that all the other failing test cases can be put down to pathlib erroneously adding roots to paths like //./C:, //./nul, //./PhysicalDrive0 and //?/UNC/.

[zooba]

I believe that all the other failing test cases can be put down to pathlib erroneously adding roots to paths

Yeah, this is what I expected with "deliberate about the meaning of ... leading slash in the rest of the path". I know we've encountered these consistency issues before, but I don't know whether they were all resolved.

[barneygale]

I've opened a PR that fixes all of Eryk's test cases except //./Global and //?/Global: #102003

[barneygale]

Hey @eryksun, do you think we should leave this issue open to cover the remaining "Global" and "GLOBALROOT" issues? The problem there lies mostly in ntpath.splitroot(), so I might remove the expert-pathlib label, or even create a new issue.

[zooba]

I vote for a new issue (with a more descriptive title), and would also say it's not a high priority. If you can describe it well enough, might even be able to justify tagging it "easy" and offer it up for someone at sprints.

[barneygale]

Logged here:

• os.path.splitroot() splits "Global" and "GLOBALROOT" device paths incorrectly #103593

I don't understand the rules for "Global" and "GLOBALROOT" so it's not very well described :)

Resolving this issue! Thanks all.