Skip to content

GH-44626, GH-105476: Fix ntpath.isabs() handling of part-absolute paths #113829

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 13, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions Doc/library/os.path.rst
Original file line number Diff line number Diff line change
@@ -239,12 +239,16 @@ the :mod:`glob` module.)
.. function:: isabs(path)

Return ``True`` if *path* is an absolute pathname. On Unix, that means it
begins with a slash, on Windows that it begins with a (back)slash after chopping
off a potential drive letter.
begins with a slash, on Windows that it begins with two (back)slashes, or a
drive letter, colon, and (back)slash together.

.. versionchanged:: 3.6
Accepts a :term:`path-like object`.

.. versionchanged:: 3.13
On Windows, returns ``False`` if the given path starts with exactly one
(back)slash.


.. function:: isfile(path)

7 changes: 7 additions & 0 deletions Doc/whatsnew/3.13.rst
Original file line number Diff line number Diff line change
@@ -307,6 +307,13 @@ os
:c:func:`!posix_spawn_file_actions_addclosefrom_np`.
(Contributed by Jakub Kulik in :gh:`113117`.)

os.path
-------

* On Windows, :func:`os.path.isabs` no longer considers paths starting with
exactly one (back)slash to be absolute.
(Contributed by Barney Gale and Jon Foster in :gh:`44626`.)

pathlib
-------

13 changes: 3 additions & 10 deletions Lib/ntpath.py
Original file line number Diff line number Diff line change
@@ -77,29 +77,22 @@ def normcase(s):
return s.replace('/', '\\').lower()


# Return whether a path is absolute.
# Trivial in Posix, harder on Windows.
# For Windows it is absolute if it starts with a slash or backslash (current
# volume), or if a pathname after the volume-letter-and-colon or UNC-resource
# starts with a slash or backslash.

def isabs(s):
"""Test whether a path is absolute"""
s = os.fspath(s)
if isinstance(s, bytes):
sep = b'\\'
altsep = b'/'
colon_sep = b':\\'
double_sep = b'\\\\'
else:
sep = '\\'
altsep = '/'
colon_sep = ':\\'
double_sep = '\\\\'
s = s[:3].replace(altsep, sep)
# Absolute: UNC, device, and paths with a drive and root.
# LEGACY BUG: isabs("/x") should be false since the path has no drive.
if s.startswith(sep) or s.startswith(colon_sep, 1):
return True
return False
return s.startswith(colon_sep, 1) or s.startswith(double_sep)


# Join two (or more) paths.
6 changes: 1 addition & 5 deletions Lib/pathlib/_abc.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import functools
import ntpath
import posixpath
import sys
from errno import ENOENT, ENOTDIR, EBADF, ELOOP, EINVAL
@@ -433,10 +432,7 @@ def parents(self):
def is_absolute(self):
"""True if the path is absolute (has both a root and, if applicable,
a drive)."""
if self.pathmod is ntpath:
# ntpath.isabs() is defective - see GH-44626.
return bool(self.drive and self.root)
elif self.pathmod is posixpath:
if self.pathmod is posixpath:
# Optimization: work with raw paths on POSIX.
for path in self._raw_paths:
if path.startswith('/'):
12 changes: 10 additions & 2 deletions Lib/test/test_ntpath.py
Original file line number Diff line number Diff line change
@@ -227,10 +227,18 @@ def test_split(self):
tester('ntpath.split("//conky/mountpoint/")', ('//conky/mountpoint/', ''))

def test_isabs(self):
tester('ntpath.isabs("foo\\bar")', 0)
tester('ntpath.isabs("foo/bar")', 0)
tester('ntpath.isabs("c:\\")', 1)
tester('ntpath.isabs("c:\\foo\\bar")', 1)
tester('ntpath.isabs("c:/foo/bar")', 1)
tester('ntpath.isabs("\\\\conky\\mountpoint\\")', 1)
tester('ntpath.isabs("\\foo")', 1)
tester('ntpath.isabs("\\foo\\bar")', 1)

# gh-44626: paths with only a drive or root are not absolute.
tester('ntpath.isabs("\\foo\\bar")', 0)
tester('ntpath.isabs("/foo/bar")', 0)
tester('ntpath.isabs("c:foo\\bar")', 0)
tester('ntpath.isabs("c:foo/bar")', 0)

# gh-96290: normal UNC paths and device paths without trailing backslashes
tester('ntpath.isabs("\\\\conky\\mountpoint")', 1)
4 changes: 4 additions & 0 deletions Lib/test/test_pathlib/test_pathlib.py
Original file line number Diff line number Diff line change
@@ -970,10 +970,14 @@ def test_is_absolute(self):
self.assertTrue(P('c:/a').is_absolute())
self.assertTrue(P('c:/a/b/').is_absolute())
# UNC paths are absolute by definition.
self.assertTrue(P('//').is_absolute())
self.assertTrue(P('//a').is_absolute())
self.assertTrue(P('//a/b').is_absolute())
self.assertTrue(P('//a/b/').is_absolute())
self.assertTrue(P('//a/b/c').is_absolute())
self.assertTrue(P('//a/b/c/d').is_absolute())
self.assertTrue(P('//?/UNC/').is_absolute())
self.assertTrue(P('//?/UNC/spam').is_absolute())

def test_join(self):
P = self.cls
4 changes: 2 additions & 2 deletions Lib/test/test_unittest/test_program.py
Original file line number Diff line number Diff line change
@@ -447,8 +447,8 @@ def _join(name):

def testParseArgsAbsolutePathsThatCannotBeConverted(self):
program = self.program
# even on Windows '/...' is considered absolute by os.path.abspath
argv = ['progname', '/foo/bar/baz.py', '/green/red.py']
drive = os.path.splitdrive(os.getcwd())[0]
argv = ['progname', f'{drive}/foo/bar/baz.py', f'{drive}/green/red.py']
self._patch_isfile(argv)

program.createTests = lambda: None
25 changes: 13 additions & 12 deletions Lib/test/test_zoneinfo/test_zoneinfo.py
Original file line number Diff line number Diff line change
@@ -36,6 +36,7 @@
TEMP_DIR = None
DATA_DIR = pathlib.Path(__file__).parent / "data"
ZONEINFO_JSON = DATA_DIR / "zoneinfo_data.json"
DRIVE = os.path.splitdrive('x:')[0]

# Useful constants
ZERO = timedelta(0)
@@ -1679,8 +1680,8 @@ def test_env_variable(self):
"""Tests that the environment variable works with reset_tzpath."""
new_paths = [
("", []),
("/etc/zoneinfo", ["/etc/zoneinfo"]),
(f"/a/b/c{os.pathsep}/d/e/f", ["/a/b/c", "/d/e/f"]),
(f"{DRIVE}/etc/zoneinfo", [f"{DRIVE}/etc/zoneinfo"]),
(f"{DRIVE}/a/b/c{os.pathsep}{DRIVE}/d/e/f", [f"{DRIVE}/a/b/c", f"{DRIVE}/d/e/f"]),
]

for new_path_var, expected_result in new_paths:
@@ -1694,22 +1695,22 @@ def test_env_variable_relative_paths(self):
test_cases = [
[("path/to/somewhere",), ()],
[
("/usr/share/zoneinfo", "path/to/somewhere",),
("/usr/share/zoneinfo",),
(f"{DRIVE}/usr/share/zoneinfo", "path/to/somewhere",),
(f"{DRIVE}/usr/share/zoneinfo",),
],
[("../relative/path",), ()],
[
("/usr/share/zoneinfo", "../relative/path",),
("/usr/share/zoneinfo",),
(f"{DRIVE}/usr/share/zoneinfo", "../relative/path",),
(f"{DRIVE}/usr/share/zoneinfo",),
],
[("path/to/somewhere", "../relative/path",), ()],
[
(
"/usr/share/zoneinfo",
f"{DRIVE}/usr/share/zoneinfo",
"path/to/somewhere",
"../relative/path",
),
("/usr/share/zoneinfo",),
(f"{DRIVE}/usr/share/zoneinfo",),
],
]

@@ -1727,9 +1728,9 @@ def test_env_variable_relative_paths(self):
self.assertSequenceEqual(tzpath, expected_paths)

def test_reset_tzpath_kwarg(self):
self.module.reset_tzpath(to=["/a/b/c"])
self.module.reset_tzpath(to=[f"{DRIVE}/a/b/c"])

self.assertSequenceEqual(self.module.TZPATH, ("/a/b/c",))
self.assertSequenceEqual(self.module.TZPATH, (f"{DRIVE}/a/b/c",))

def test_reset_tzpath_relative_paths(self):
bad_values = [
@@ -1758,8 +1759,8 @@ def test_tzpath_type_error(self):
self.module.reset_tzpath(bad_value)

def test_tzpath_attribute(self):
tzpath_0 = ["/one", "/two"]
tzpath_1 = ["/three"]
tzpath_0 = [f"{DRIVE}/one", f"{DRIVE}/two"]
tzpath_1 = [f"{DRIVE}/three"]

with self.tzpath_context(tzpath_0):
query_0 = self.module.TZPATH
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Fix :func:`os.path.isabs` incorrectly returning ``True`` when given a path
that starts with exactly one (back)slash on Windows.

Fix :meth:`pathlib.PureWindowsPath.is_absolute` incorrectly returning
``False`` for some paths beginning with two (back)slashes.