From a451699ce6605b422b4c0af65c1b2f78f968622c Mon Sep 17 00:00:00 2001 From: Nineteendo Date: Thu, 28 Mar 2024 20:09:23 +0100 Subject: [PATCH 1/4] Handle non-iterables for `ntpath.commonpath` --- Lib/ntpath.py | 65 ++++++++++++++++++++--------------------- Lib/test/test_ntpath.py | 3 ++ 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/Lib/ntpath.py b/Lib/ntpath.py index f1c48ecd1e5e2a..cd991a0421a087 100644 --- a/Lib/ntpath.py +++ b/Lib/ntpath.py @@ -831,24 +831,24 @@ def relpath(path, start=None): raise -# Return the longest common sub-path of the sequence of paths given as input. +# Return the longest common sub-path of the iterable of paths given as input. # The function is case-insensitive and 'separator-insensitive', i.e. if the # only difference between two paths is the use of '\' versus '/' as separator, # they are deemed to be equal. # # However, the returned path will have the standard '\' separator (even if the # given paths had the alternative '/' separator) and will have the case of the -# first path given in the sequence. Additionally, any trailing separator is +# first path given in the iterable. Additionally, any trailing separator is # stripped from the returned path. def commonpath(paths): - """Given a sequence of path names, returns the longest common sub-path.""" - + """Given an iterable of path names, returns the longest common sub-path.""" + paths = tuple(map(os.fspath, paths)) if not paths: - raise ValueError('commonpath() arg is an empty sequence') + raise ValueError('commonpath() arg is an empty iterable') - paths = tuple(map(os.fspath, paths)) - if isinstance(paths[0], bytes): + path = paths[0] + if isinstance(path, bytes): sep = b'\\' altsep = b'/' curdir = b'.' @@ -858,37 +858,34 @@ def commonpath(paths): curdir = '.' try: - drivesplits = [splitroot(p.replace(altsep, sep).lower()) for p in paths] - split_paths = [p.split(sep) for d, r, p in drivesplits] - - if len({r for d, r, p in drivesplits}) != 1: - raise ValueError("Can't mix absolute and relative paths") - - # Check that all drive letters or UNC paths match. The check is made only - # now otherwise type errors for mixing strings and bytes would not be - # caught. - if len({d for d, r, p in drivesplits}) != 1: - raise ValueError("Paths don't have the same drive") - - drive, root, path = splitroot(paths[0].replace(altsep, sep)) - common = path.split(sep) - common = [c for c in common if c and c != curdir] - - split_paths = [[c for c in s if c and c != curdir] for s in split_paths] - s1 = min(split_paths) - s2 = max(split_paths) - for i, c in enumerate(s1): - if c != s2[i]: - common = common[:i] - break - else: - common = common[:len(s1)] - - return drive + root + sep.join(common) + rootsplits = [splitroot(p.replace(altsep, sep).lower()) for p in paths] except (TypeError, AttributeError): genericpath._check_arg_types('commonpath', *paths) raise + # Check that all drive letters or UNC paths match. The check is made only + # now otherwise type errors for mixing strings and bytes would not be + # caught. + if len({drt[0] for drt in rootsplits}) != 1: + raise ValueError("Paths don't have the same drive") + + if len({drt[1] for drt in rootsplits}) != 1: + raise ValueError("Can't mix absolute and relative paths") + + drive, root, tail = splitroot(path.replace(altsep, sep)) + common = [c for c in tail.split(sep) if c and c != curdir] + split_paths = [ + [c for c in drt[2].split(sep) if c and c != curdir] + for drt in rootsplits + ] + s1 = min(split_paths) + s2 = max(split_paths) + for i, c in enumerate(s1): + if c != s2[i]: + return drive + root + sep.join(common[:i]) + + return drive + root + sep.join(common[:len(s1)]) + try: # The isdir(), isfile(), islink() and exists() implementations in diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py index 9cb03e3cd5de8d..c816f99e7e9f1b 100644 --- a/Lib/test/test_ntpath.py +++ b/Lib/test/test_ntpath.py @@ -871,11 +871,14 @@ def check_error(exc, paths): self.assertRaises(exc, ntpath.commonpath, [os.fsencode(p) for p in paths]) + self.assertRaises(TypeError, ntpath.commonpath, None) self.assertRaises(ValueError, ntpath.commonpath, []) + self.assertRaises(ValueError, ntpath.commonpath, iter([])) check_error(ValueError, ['C:\\Program Files', 'Program Files']) check_error(ValueError, ['C:\\Program Files', 'C:Program Files']) check_error(ValueError, ['\\Program Files', 'Program Files']) check_error(ValueError, ['Program Files', 'C:\\Program Files']) + check(['C:\\Program Files'], 'C:\\Program Files') check(['C:\\Program Files', 'C:\\Program Files'], 'C:\\Program Files') check(['C:\\Program Files\\', 'C:\\Program Files'], From 921816a1c30b0c1ac6388e1a9dff39a6b883f134 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Thu, 28 Mar 2024 19:13:20 +0000 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2024-03-28-19-13-20.gh-issue-117335.d6uKJu.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-03-28-19-13-20.gh-issue-117335.d6uKJu.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-03-28-19-13-20.gh-issue-117335.d6uKJu.rst b/Misc/NEWS.d/next/Core and Builtins/2024-03-28-19-13-20.gh-issue-117335.d6uKJu.rst new file mode 100644 index 00000000000000..e419b2e97f3886 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-03-28-19-13-20.gh-issue-117335.d6uKJu.rst @@ -0,0 +1 @@ +Raise TypeError for non-sequences for :func:`ntpath.commonpath`. From 7f6902948acb09e25f144ff5ebf2d49d7cd75cc7 Mon Sep 17 00:00:00 2001 From: Nineteendo Date: Thu, 28 Mar 2024 21:23:11 +0100 Subject: [PATCH 3/4] Revert unnecessary changes --- Lib/ntpath.py | 49 ++++++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/Lib/ntpath.py b/Lib/ntpath.py index cd991a0421a087..21019799e27b51 100644 --- a/Lib/ntpath.py +++ b/Lib/ntpath.py @@ -847,8 +847,7 @@ def commonpath(paths): if not paths: raise ValueError('commonpath() arg is an empty iterable') - path = paths[0] - if isinstance(path, bytes): + if isinstance(paths[0], bytes): sep = b'\\' altsep = b'/' curdir = b'.' @@ -859,33 +858,33 @@ def commonpath(paths): try: rootsplits = [splitroot(p.replace(altsep, sep).lower()) for p in paths] + + # Check that all drive letters or UNC paths match. The check is made + # only now otherwise type errors for mixing strings and bytes would not + # be caught. + if len({d for d, _, _ in rootsplits}) != 1: + raise ValueError("Paths don't have the same drive") + + if len({r for _, r, _ in rootsplits}) != 1: + raise ValueError("Can't mix absolute and relative paths") + + drive, root, tail = splitroot(paths[0].replace(altsep, sep)) + common = [c for c in tail.split(sep) if c and c != curdir] + + split_paths = [ + [c for c in t.split(sep) if c and c != curdir] + for _, _, t in rootsplits + ] + s1 = min(split_paths) + s2 = max(split_paths) + for i, c in enumerate(s1): + if c != s2[i]: + return drive + root + sep.join(common[:i]) + return drive + root + sep.join(common[:len(s1)]) except (TypeError, AttributeError): genericpath._check_arg_types('commonpath', *paths) raise - # Check that all drive letters or UNC paths match. The check is made only - # now otherwise type errors for mixing strings and bytes would not be - # caught. - if len({drt[0] for drt in rootsplits}) != 1: - raise ValueError("Paths don't have the same drive") - - if len({drt[1] for drt in rootsplits}) != 1: - raise ValueError("Can't mix absolute and relative paths") - - drive, root, tail = splitroot(path.replace(altsep, sep)) - common = [c for c in tail.split(sep) if c and c != curdir] - split_paths = [ - [c for c in drt[2].split(sep) if c and c != curdir] - for drt in rootsplits - ] - s1 = min(split_paths) - s2 = max(split_paths) - for i, c in enumerate(s1): - if c != s2[i]: - return drive + root + sep.join(common[:i]) - - return drive + root + sep.join(common[:len(s1)]) - try: # The isdir(), isfile(), islink() and exists() implementations in From 331ceb479d89ed3057cb6caaccb9c52b04e3a6c8 Mon Sep 17 00:00:00 2001 From: Nineteendo Date: Thu, 28 Mar 2024 21:42:41 +0100 Subject: [PATCH 4/4] Revert all refactoring --- Lib/ntpath.py | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/Lib/ntpath.py b/Lib/ntpath.py index 21019799e27b51..ecfc7d48dbb192 100644 --- a/Lib/ntpath.py +++ b/Lib/ntpath.py @@ -857,30 +857,33 @@ def commonpath(paths): curdir = '.' try: - rootsplits = [splitroot(p.replace(altsep, sep).lower()) for p in paths] + drivesplits = [splitroot(p.replace(altsep, sep).lower()) for p in paths] + split_paths = [p.split(sep) for d, r, p in drivesplits] - # Check that all drive letters or UNC paths match. The check is made - # only now otherwise type errors for mixing strings and bytes would not - # be caught. - if len({d for d, _, _ in rootsplits}) != 1: - raise ValueError("Paths don't have the same drive") - - if len({r for _, r, _ in rootsplits}) != 1: + if len({r for d, r, p in drivesplits}) != 1: raise ValueError("Can't mix absolute and relative paths") - drive, root, tail = splitroot(paths[0].replace(altsep, sep)) - common = [c for c in tail.split(sep) if c and c != curdir] + # Check that all drive letters or UNC paths match. The check is made only + # now otherwise type errors for mixing strings and bytes would not be + # caught. + if len({d for d, r, p in drivesplits}) != 1: + raise ValueError("Paths don't have the same drive") + + drive, root, path = splitroot(paths[0].replace(altsep, sep)) + common = path.split(sep) + common = [c for c in common if c and c != curdir] - split_paths = [ - [c for c in t.split(sep) if c and c != curdir] - for _, _, t in rootsplits - ] + split_paths = [[c for c in s if c and c != curdir] for s in split_paths] s1 = min(split_paths) s2 = max(split_paths) for i, c in enumerate(s1): if c != s2[i]: - return drive + root + sep.join(common[:i]) - return drive + root + sep.join(common[:len(s1)]) + common = common[:i] + break + else: + common = common[:len(s1)] + + return drive + root + sep.join(common) except (TypeError, AttributeError): genericpath._check_arg_types('commonpath', *paths) raise