From 2100e3e4d2149f770554a67c13678310bdd8e3b4 Mon Sep 17 00:00:00 2001 From: barneygale Date: Mon, 8 Jan 2024 21:07:27 +0000 Subject: [PATCH 1/3] GH-44626, GH-105476: Fix `ntpath.isabs()` handling of part-absolute paths. On Windows, `os.path.isabs()` now returns `False` when given a path that starts with exactly one (back)slash. This is more compatible with other functions in `os.path`, and with Microsoft's own documentation. Also adjust `pathlib.PureWindowsPath.is_absolute()` to call `ntpath.isabs()`, which corrects its handling of partial UNC/device paths like `//foo`. Co-authored-by: Jon Foster --- Doc/library/os.path.rst | 8 ++++++-- Doc/whatsnew/3.13.rst | 7 +++++++ Lib/ntpath.py | 13 +++---------- Lib/pathlib/_abc.py | 6 +----- Lib/test/test_ntpath.py | 12 ++++++++++-- Lib/test/test_pathlib/test_pathlib.py | 4 ++++ .../2024-01-08-21-15-48.gh-issue-44626.DRq-PR.rst | 5 +++++ 7 files changed, 36 insertions(+), 19 deletions(-) create mode 100644 Misc/NEWS.d/next/Tests/2024-01-08-21-15-48.gh-issue-44626.DRq-PR.rst diff --git a/Doc/library/os.path.rst b/Doc/library/os.path.rst index 95933f56d50542..3cab7a260df008 100644 --- a/Doc/library/os.path.rst +++ b/Doc/library/os.path.rst @@ -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) diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index 59b9281e6d2b89..05b9b87a63252f 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -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 ------- diff --git a/Lib/ntpath.py b/Lib/ntpath.py index 3061a4a5ef4c56..aa0e018eb668c2 100644 --- a/Lib/ntpath.py +++ b/Lib/ntpath.py @@ -77,12 +77,6 @@ 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) @@ -90,16 +84,15 @@ def isabs(s): 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. diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py index be22ecef4d214e..58288a5e2c2f46 100644 --- a/Lib/pathlib/_abc.py +++ b/Lib/pathlib/_abc.py @@ -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('/'): diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py index bf990ed36fbcae..aefcb98f1c30eb 100644 --- a/Lib/test/test_ntpath.py +++ b/Lib/test/test_ntpath.py @@ -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) diff --git a/Lib/test/test_pathlib/test_pathlib.py b/Lib/test/test_pathlib/test_pathlib.py index 6e42122212632d..b1cc241af2afb6 100644 --- a/Lib/test/test_pathlib/test_pathlib.py +++ b/Lib/test/test_pathlib/test_pathlib.py @@ -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 diff --git a/Misc/NEWS.d/next/Tests/2024-01-08-21-15-48.gh-issue-44626.DRq-PR.rst b/Misc/NEWS.d/next/Tests/2024-01-08-21-15-48.gh-issue-44626.DRq-PR.rst new file mode 100644 index 00000000000000..3fa304be0fa6d4 --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2024-01-08-21-15-48.gh-issue-44626.DRq-PR.rst @@ -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. From f6f0e35a33fd55b00fbef53ec9dacdb0a1ba818f Mon Sep 17 00:00:00 2001 From: barneygale Date: Mon, 8 Jan 2024 21:58:36 +0000 Subject: [PATCH 2/3] Attempt to fix some test failures. --- Lib/test/test_unittest/test_program.py | 4 ++-- Lib/test/test_zoneinfo/test_zoneinfo.py | 23 ++++++++++++----------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_unittest/test_program.py b/Lib/test/test_unittest/test_program.py index f6d52f93e4a25f..8dacd48034a891 100644 --- a/Lib/test/test_unittest/test_program.py +++ b/Lib/test/test_unittest/test_program.py @@ -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'] + # '//...' is considered absolute by os.path.abspath on all platforms. + argv = ['progname', '//foo/bar/baz.py', '//green/red.py'] self._patch_isfile(argv) program.createTests = lambda: None diff --git a/Lib/test/test_zoneinfo/test_zoneinfo.py b/Lib/test/test_zoneinfo/test_zoneinfo.py index 7b6b69d0109d88..dacd4569674b14 100644 --- a/Lib/test/test_zoneinfo/test_zoneinfo.py +++ b/Lib/test/test_zoneinfo/test_zoneinfo.py @@ -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,7 +1728,7 @@ 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",)) @@ -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 From bb7abe5918acc0f34bdae2d2e303c72839059aa0 Mon Sep 17 00:00:00 2001 From: barneygale Date: Mon, 8 Jan 2024 22:15:40 +0000 Subject: [PATCH 3/3] Fix tests, attempt #2 --- Lib/test/test_unittest/test_program.py | 4 ++-- Lib/test/test_zoneinfo/test_zoneinfo.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_unittest/test_program.py b/Lib/test/test_unittest/test_program.py index 8dacd48034a891..d8e8969687f937 100644 --- a/Lib/test/test_unittest/test_program.py +++ b/Lib/test/test_unittest/test_program.py @@ -447,8 +447,8 @@ def _join(name): def testParseArgsAbsolutePathsThatCannotBeConverted(self): program = self.program - # '//...' is considered absolute by os.path.abspath on all platforms. - 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 diff --git a/Lib/test/test_zoneinfo/test_zoneinfo.py b/Lib/test/test_zoneinfo/test_zoneinfo.py index dacd4569674b14..18eab5b33540c9 100644 --- a/Lib/test/test_zoneinfo/test_zoneinfo.py +++ b/Lib/test/test_zoneinfo/test_zoneinfo.py @@ -1730,7 +1730,7 @@ def test_env_variable_relative_paths(self): def test_reset_tzpath_kwarg(self): 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 = [