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.

7 files changed, 134 insertions, 91 deletions

diff --git a/Lib/genericpath.py b/Lib/genericpath.py
index ca4a510..6714061 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 5ca0097..dbcf368 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 f6b5cd7..8f5dc55 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))
-
-    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
+    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
 
-    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 f08c931..ce5f792 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]
-
-    # 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)
+    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]))
+
+        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 e59ed4d..2e31fe4 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 22f8491..80bec7a 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 d5a12a3..b2454794 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("/"), ("/", ""))