diff --git a/Lib/genericpath.py b/Lib/genericpath.py index ca4a5108fd4..671406197a7 100644 --- a/Lib/genericpath.py +++ b/Lib/genericpath.py @@ -130,3 +130,16 @@ def _splitext(p, sep, altsep, extsep): filenameIndex += 1 return p, p[:0] + +def _check_arg_types(funcname, *args): + hasstr = hasbytes = False + for s in args: + if isinstance(s, str): + hasstr = True + elif isinstance(s, bytes): + hasbytes = True + else: + raise TypeError('%s() argument must be str or bytes, not %r' % + (funcname, s.__class__.__name__)) from None + if hasstr and hasbytes: + raise TypeError("Can't mix strings and bytes in path components") from None diff --git a/Lib/macpath.py b/Lib/macpath.py index 5ca00977c6d..dbcf3684684 100644 --- a/Lib/macpath.py +++ b/Lib/macpath.py @@ -50,20 +50,24 @@ def isabs(s): def join(s, *p): - colon = _get_colon(s) - path = s - for t in p: - if (not path) or isabs(t): - path = t - continue - if t[:1] == colon: - t = t[1:] - if colon not in path: - path = colon + path - if path[-1:] != colon: - path = path + colon - path = path + t - return path + try: + colon = _get_colon(s) + path = s + for t in p: + if (not path) or isabs(t): + path = t + continue + if t[:1] == colon: + t = t[1:] + if colon not in path: + path = colon + path + if path[-1:] != colon: + path = path + colon + path = path + t + return path + except (TypeError, AttributeError, BytesWarning): + genericpath._check_arg_types('join', s, *p) + raise def split(s): diff --git a/Lib/ntpath.py b/Lib/ntpath.py index f6b5cd7b0ce..8f5dc55bec5 100644 --- a/Lib/ntpath.py +++ b/Lib/ntpath.py @@ -80,32 +80,36 @@ def join(path, *paths): sep = '\\' seps = '\\/' colon = ':' - result_drive, result_path = splitdrive(path) - for p in paths: - p_drive, p_path = splitdrive(p) - if p_path and p_path[0] in seps: - # Second path is absolute - if p_drive or not result_drive: - result_drive = p_drive - result_path = p_path - continue - elif p_drive and p_drive != result_drive: - if p_drive.lower() != result_drive.lower(): - # Different drives => ignore the first path entirely - result_drive = p_drive + try: + result_drive, result_path = splitdrive(path) + for p in paths: + p_drive, p_path = splitdrive(p) + if p_path and p_path[0] in seps: + # Second path is absolute + if p_drive or not result_drive: + result_drive = p_drive result_path = p_path continue - # Same drive in different case - result_drive = p_drive - # Second path is relative to the first - if result_path and result_path[-1] not in seps: - result_path = result_path + sep - result_path = result_path + p_path - ## add separator between UNC and non-absolute path - if (result_path and result_path[0] not in seps and - result_drive and result_drive[-1:] != colon): - return result_drive + sep + result_path - return result_drive + result_path + elif p_drive and p_drive != result_drive: + if p_drive.lower() != result_drive.lower(): + # Different drives => ignore the first path entirely + result_drive = p_drive + result_path = p_path + continue + # Same drive in different case + result_drive = p_drive + # Second path is relative to the first + if result_path and result_path[-1] not in seps: + result_path = result_path + sep + result_path = result_path + p_path + ## add separator between UNC and non-absolute path + if (result_path and result_path[0] not in seps and + result_drive and result_drive[-1:] != colon): + return result_drive + sep + result_path + return result_drive + result_path + except (TypeError, AttributeError, BytesWarning): + genericpath._check_arg_types('join', path, *paths) + raise # Split a path in a drive specification (a drive letter followed by a @@ -558,27 +562,31 @@ def relpath(path, start=None): if not path: raise ValueError("no path specified") - start_abs = abspath(normpath(start)) - path_abs = abspath(normpath(path)) - start_drive, start_rest = splitdrive(start_abs) - path_drive, path_rest = splitdrive(path_abs) - if normcase(start_drive) != normcase(path_drive): - raise ValueError("path is on mount %r, start on mount %r" % ( - path_drive, start_drive)) + try: + start_abs = abspath(normpath(start)) + path_abs = abspath(normpath(path)) + start_drive, start_rest = splitdrive(start_abs) + path_drive, path_rest = splitdrive(path_abs) + if normcase(start_drive) != normcase(path_drive): + raise ValueError("path is on mount %r, start on mount %r" % ( + path_drive, start_drive)) - start_list = [x for x in start_rest.split(sep) if x] - path_list = [x for x in path_rest.split(sep) if x] - # Work out how much of the filepath is shared by start and path. - i = 0 - for e1, e2 in zip(start_list, path_list): - if normcase(e1) != normcase(e2): - break - i += 1 + start_list = [x for x in start_rest.split(sep) if x] + path_list = [x for x in path_rest.split(sep) if x] + # Work out how much of the filepath is shared by start and path. + i = 0 + for e1, e2 in zip(start_list, path_list): + if normcase(e1) != normcase(e2): + break + i += 1 - rel_list = [pardir] * (len(start_list)-i) + path_list[i:] - if not rel_list: - return curdir - return join(*rel_list) + rel_list = [pardir] * (len(start_list)-i) + path_list[i:] + if not rel_list: + return curdir + return join(*rel_list) + except (TypeError, ValueError, AttributeError, BytesWarning): + genericpath._check_arg_types('relpath', path, start) + raise # determine if two files are in fact the same file diff --git a/Lib/posixpath.py b/Lib/posixpath.py index f08c9310d4e..ce5f7928cc7 100644 --- a/Lib/posixpath.py +++ b/Lib/posixpath.py @@ -82,13 +82,9 @@ def join(a, *p): path += b else: path += sep + b - except (TypeError, AttributeError): - for s in (a,) + p: - if not isinstance(s, (str, bytes)): - raise TypeError('join() argument must be str or bytes, not %r' % - s.__class__.__name__) from None - # Must have a mixture of text and binary data - raise TypeError("Can't mix strings and bytes in path components") from None + except (TypeError, AttributeError, BytesWarning): + genericpath._check_arg_types('join', a, *p) + raise return path @@ -446,13 +442,16 @@ def relpath(path, start=None): if start is None: start = curdir - start_list = [x for x in abspath(start).split(sep) if x] - path_list = [x for x in abspath(path).split(sep) if x] + try: + start_list = [x for x in abspath(start).split(sep) if x] + path_list = [x for x in abspath(path).split(sep) if x] + # Work out how much of the filepath is shared by start and path. + i = len(commonprefix([start_list, path_list])) - # Work out how much of the filepath is shared by start and path. - i = len(commonprefix([start_list, path_list])) - - rel_list = [pardir] * (len(start_list)-i) + path_list[i:] - if not rel_list: - return curdir - return join(*rel_list) + rel_list = [pardir] * (len(start_list)-i) + path_list[i:] + if not rel_list: + return curdir + return join(*rel_list) + except (TypeError, AttributeError, BytesWarning): + genericpath._check_arg_types('relpath', path, start) + raise diff --git a/Lib/test/test_genericpath.py b/Lib/test/test_genericpath.py index e59ed4d21c6..2e31fe42362 100644 --- a/Lib/test/test_genericpath.py +++ b/Lib/test/test_genericpath.py @@ -434,6 +434,39 @@ class CommonTest(GenericTest): with support.temp_cwd(name): self.test_abspath() + def test_join_errors(self): + # Check join() raises friendly TypeErrors. + with support.check_warnings(('', BytesWarning), quiet=True): + errmsg = "Can't mix strings and bytes in path components" + with self.assertRaisesRegex(TypeError, errmsg): + self.pathmodule.join(b'bytes', 'str') + with self.assertRaisesRegex(TypeError, errmsg): + self.pathmodule.join('str', b'bytes') + # regression, see #15377 + errmsg = r'join\(\) argument must be str or bytes, not %r' + with self.assertRaisesRegex(TypeError, errmsg % 'int'): + self.pathmodule.join(42, 'str') + with self.assertRaisesRegex(TypeError, errmsg % 'int'): + self.pathmodule.join('str', 42) + with self.assertRaisesRegex(TypeError, errmsg % 'bytearray'): + self.pathmodule.join(bytearray(b'foo'), bytearray(b'bar')) + + def test_relpath_errors(self): + # Check relpath() raises friendly TypeErrors. + with support.check_warnings(('', BytesWarning), quiet=True): + errmsg = "Can't mix strings and bytes in path components" + with self.assertRaisesRegex(TypeError, errmsg): + self.pathmodule.relpath(b'bytes', 'str') + with self.assertRaisesRegex(TypeError, errmsg): + self.pathmodule.relpath('str', b'bytes') + errmsg = r'relpath\(\) argument must be str or bytes, not %r' + with self.assertRaisesRegex(TypeError, errmsg % 'int'): + self.pathmodule.relpath(42, 'str') + with self.assertRaisesRegex(TypeError, errmsg % 'int'): + self.pathmodule.relpath('str', 42) + with self.assertRaisesRegex(TypeError, errmsg % 'bytearray'): + self.pathmodule.relpath(bytearray(b'foo'), bytearray(b'bar')) + if __name__=="__main__": unittest.main() diff --git a/Lib/test/test_macpath.py b/Lib/test/test_macpath.py index 22f84919bb9..80bec7a799a 100644 --- a/Lib/test/test_macpath.py +++ b/Lib/test/test_macpath.py @@ -142,6 +142,8 @@ class MacPathTestCase(unittest.TestCase): class MacCommonTest(test_genericpath.CommonTest, unittest.TestCase): pathmodule = macpath + test_relpath_errors = None + if __name__ == "__main__": unittest.main() diff --git a/Lib/test/test_posixpath.py b/Lib/test/test_posixpath.py index d5a12a31dd7..b2454794e5b 100644 --- a/Lib/test/test_posixpath.py +++ b/Lib/test/test_posixpath.py @@ -57,22 +57,6 @@ class PosixPathTest(unittest.TestCase): self.assertEqual(posixpath.join(b"/foo/", b"bar/", b"baz/"), b"/foo/bar/baz/") - def test_join_errors(self): - # Check posixpath.join raises friendly TypeErrors. - errmsg = "Can't mix strings and bytes in path components" - with self.assertRaisesRegex(TypeError, errmsg): - posixpath.join(b'bytes', 'str') - with self.assertRaisesRegex(TypeError, errmsg): - posixpath.join('str', b'bytes') - # regression, see #15377 - errmsg = r'join\(\) argument must be str or bytes, not %r' - with self.assertRaisesRegex(TypeError, errmsg % 'NoneType'): - posixpath.join(None, 'str') - with self.assertRaisesRegex(TypeError, errmsg % 'NoneType'): - posixpath.join('str', None) - with self.assertRaisesRegex(TypeError, errmsg % 'bytearray'): - posixpath.join(bytearray(b'foo'), bytearray(b'bar')) - def test_split(self): self.assertEqual(posixpath.split("/foo/bar"), ("/foo", "bar")) self.assertEqual(posixpath.split("/"), ("/", "")) diff --git a/Misc/NEWS b/Misc/NEWS index 6decc6051e3..48bdc575084 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -162,6 +162,9 @@ Core and Builtins Library ------- +- Issue #21883: os.path.join() and os.path.relpath() now raise a TypeError with + more helpful error message for unsupported or mismatched types of arguments. + - Issue #22219: The zipfile module CLI now adds entries for directories (including empty directories) in ZIP file.