GeezerVison is showing github.com — support "UNC" device paths in ntpath.splitdrive · Issue #81790 · python/cpython
support "UNC" device paths in ntpath.splitdrive #81790Description
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 2019-07-17.11:26:34.050>
labels = ['type-bug', 'library', '3.10', 'OS-windows']
title = 'support "UNC" device paths in ntpath.splitdrive'
updated_at = <Date 2022-03-06.18:47:25.283>
user = 'https://github.com/eryksun' bugs.python.org fields:
activity = <Date 2022-03-06.18:47:25.283>
actor = 'steve.dower'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)', 'Windows']
creation = <Date 2019-07-17.11:26:34.050>
creator = 'eryksun'
dependencies = []
files = ['49541']
hgrepos = []
issue_num = 37609
keywords = ['patch']
message_count = 9.0
messages = ['348055', '348099', '348206', '352110', '352355', '379780', '390375', '414604', '414621']
nosy_count = 8.0
nosy_names = ['paul.moore', 'tim.golden', 'zach.ware', 'serhiy.storchaka', 'eryksun', 'steve.dower', 'nsiregar', 'barneygale']
pr_nums = ['14841', '25261', '31702']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue37609'
versions = ['Python 3.10'] Activityeryksun commented on Jul 17, 2019 ContributorAuthor More actions Windows Python includes UNC shares such as "//server/spam" in its definition of a drive. This is natural because Windows supports setting a UNC path as the working directory and handles the share component as the working drive when resolving rooted paths such as "/eggs". For the sake of generality when working with \\?\ extended paths, Python should expand its definition of a UNC drive to include "UNC" device paths.
A practical example is calling glob.glob with a "//?/UNC" device path.
>>> import os, sys, glob
>>> sys.addaudithook(lambda s,a: print('#', a[0]) if s == 'glob.glob' else None)
regular UNC path:
>>> glob.glob('//localhost/C$/Sys*')
# //localhost/C$/Sys*
['//localhost/C$/System Volume Information']"UNC" device path:
>>> glob.glob('//?/UNC/localhost/C$/Sys*')
# //?/UNC/localhost/C$/Sys*
# //?/UNC/localhost/C$
# //?/UNC/localhost
# //?/UNC/
[]Since the magic character "?" is in the path (IMO, the drive should be excluded from this check, but that's a separate issue), the internal function glob._iglob calls itself recursively until it reaches the base case of dirname == pathname, where dirname is from os.path.split(pathname). The problem here is that ntpath.split doesn't stop at the proper base case of "//?/UNC/localhost/C$". This is due to ntpath.splitdrive. For example:
>>> os.path.splitdrive('//?/UNC/localhost/C$/Sys*')
('//?/UNC', '/localhost/C$/Sys*')
>>> os.path.splitdrive('//./UNC/localhost/C$/Sys*')
('//./UNC', '/localhost/C$/Sys*')The results should be "//?/UNC/localhost/C$" and "//./UNC/localhost/C$".
In other cases, returning a device as the drive is fine, if not exactly meaningful (e.g. "//./NUL"). I don't think this needs to change. serhiy-storchaka commented on Jul 18, 2019 Do you want to create a PR Eryk? nsiregar commented on Jul 20, 2019 I was unsure about implementation in the patch, do you have UNC format specification? zooba commented on Sep 12, 2019 For clarity, given Eryk's examples above, both "\\?\UNC\" and "//?/UNC/" are okay (as are any combination of forward and backslashes in the prefix, as normalization will be applied for any except the "\\?\" version). "UNC" is also case-insensitive. eryksun commented on Sep 13, 2019 ContributorAuthor More actions Please consult the attached file "splitdrive.py". I redesigned splitdrive() to support "UNC" and "GLOBAL" junctions in device paths. I relaxed the design to allow repeated separators everywhere except for the UNC root. IIRC, Windows has supported this since XP. For example:
>>> print(nt._getfullpathname('//server///share'))
\\server\share
>>> print(nt._getfullpathname(r'\\server\\\share'))
\\server\shareThere are also a couple of minor behavior changes in the new implementation.
The old implementation would split "//server/" as ('//server/', ''). Since there's no share, this should not count as a drive. The new implementation splits it as ('', '//server/'). Similarly it splits '//?/UNC/server/' as ('', '//?/UNC/server/').
The old implementation also allowed any character as a drive 'letter'. For example, it would split '/:/spam' as ('/:', '/spam'). The new implementation ensures that the drive letter in a DOS drive is alphabetic.
I also extended test_splitdrive to use a list of test cases in order to avoid having to define each case twice. It calls tester() a second time for each case, with slash and backslash swapped. eryksun commented on Oct 27, 2020 ContributorAuthor More actions I'm attaching a rewrite of splitdrive() from msg352355. This version uses an internal _next() function to get the indices of the next path component, ignoring repeated separators. It also flattens the nested structure of the previous implementation by adding multiple return statements. zooba commented on Apr 6, 2021 Once bpo-43105 is merged, I've got a fairly simple implementation for this using the (new) nt._path_splitroot native method, as well as improved tests that cover both the native and emulated calculations. barneygale commented on Mar 6, 2022 I'd like to pick this up, as it would allow us to remove a duplicate implementation in pathlib with its own shortcomings.
If using native functionality if difficult to get right, could I put @eryksun's splitdrive.py implementation up for review? zooba commented on Mar 6, 2022 If you can build this on top of nt._path_splitroot then it could save a decent amount of work, though at the same time I think it's worthwhile having a pure Python implementation which is cross-platform.
Haven't looked at the PR yet (or Eryk's implementation recently), but at the very least it would be nice to have tests that verify consistency with nt._path_splitroot. That way at least we'll discover if the native version changes. barneygale commented on May 20, 2022 I have a patch for this issue here: #91882
Could a core dev please review? Thank you.
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|