gh-150557: Fix os.path.ismount() for UNC paths with no share on Windows#150572
Closed
amruthamodela06 wants to merge 1 commit into
Closed
gh-150557: Fix os.path.ismount() for UNC paths with no share on Windows#150572amruthamodela06 wants to merge 1 commit into
amruthamodela06 wants to merge 1 commit into
Conversation
Author
|
The failing Windows CI checks appear unrelated to this change — test_ntpath passes on the Win32 builder. The 3 failures are in unrelated subsystems: test_msvcrt.test_kbhit — console keyboard input |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
On Windows,
os.path.ismount()(ntpath.ismount()) reports a UNC path that names a server but no share as a mount point. A UNC mount point requires both a server and a share (\\server\share); a bare server name is not one.Genuine share roots are correct and stay unchanged:
Regression
This is partly a regression from 3.11, where
os.path.ismount(r"\\?\UNC\server")returnedFalse. Thesplitroot()-based rewrite ofismount()(gh-101000) changed it toTrue. (That same rewrite correctly fixed\\?\UNC\server\share, which was wronglyFalseon 3.11.)Root cause
ismount()treats the UNC/device branch as a mount point whenever the path equals its drive with no trailing component:But
splitroot()packs an incomplete, one-component UNC entirely intodrive:So
restis empty and the check passes despite there being no share.Fix
Require a UNC
driveto name both a server and a share - i.e. a separator with non-empty text on both sides must appear after the leading\\(or after the\\?\UNC\prefix for extended UNC):Device paths (
\\.\dev,\\?\dev), extended drive-letter paths (\\?\c:), volume GUID paths, and drive-letter roots all keep their current behavior.Scope & safety
ismount()is a leaf function: nothing inntpathcalls it, andjoin()/normpath()/split()/splitdrive()are all driven bysplitroot(), notismount(). Its only stdlib consumer ispathlib.Path.is_mount().splitroot()is not modified, so no other path operation changes. This also aligns with the existing docs, which already state that "a drive letter root and a share UNC are always mount points" - a bare server is not a share UNC.Tests
News entry added via
blurb. Regression tests added totest_ntpath.test_ismount:\\server->False\\server\(trailing separator) ->False\\?\UNC\serverand\\?\UNC\server\->Falseb"\\server"andb"\\?\UNC\server"->False\\server\shareand\\?\UNC\server\share(str and bytes) -> stillTrueThe existing assertions for
\\localhost\c$(with/without trailing slash, str and bytes) continue to pass. Verified locally on Windows after rebuild:test_ntpathpasses, and the full test suite passes with no new failures.Backport
The bug exists on every branch since 3.12 (where the
splitroot()rewrite landed), so this looks like a backport candidate for the active maintenance branches - deferring to maintainers on exactly which.Split off from #139916 per discussion in #150557.