Skip to content
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

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

Merged
merged 3 commits into from
Jan 13, 2024
Merged
Show file tree
Hide file tree
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
Expand Up @@ -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)

Expand Down
7 changes: 7 additions & 0 deletions Doc/whatsnew/3.13.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------

Expand Down
13 changes: 3 additions & 10 deletions Lib/ntpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
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
Expand Down Expand Up @@ -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('/'):
Expand Down
12 changes: 10 additions & 2 deletions Lib/test/test_ntpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions Lib/test/test_pathlib/test_pathlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions Lib/test/test_unittest/test_program.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 13 additions & 12 deletions Lib/test/test_zoneinfo/test_zoneinfo.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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:
Expand All @@ -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",),
],
]

Expand All @@ -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 = [
Expand Down Expand Up @@ -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
Expand Down
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.
Loading