Issue #27186: Update os.fspath()/PyOS_FSPath() to check the return

type of __fspath__().

As part of this change, also make sure that the pure Python
implementation of os.fspath() is tested.

7 files changed, 100 insertions, 70 deletions

diff --git a/Doc/c-api/sys.rst b/Doc/c-api/sys.rst
index f3cde8c..035cdc1 100644
--- a/Doc/c-api/sys.rst
+++ b/Doc/c-api/sys.rst
@@ -10,8 +10,9 @@ Operating System Utilities
    Return the file system representation for *path*. If the object is a
    :class:`str` or :class:`bytes` object, then its reference count is
    incremented. If the object implements the :class:`os.PathLike` interface,
-   then ``type(path).__fspath__()`` is returned. Otherwise :exc:`TypeError` is
-   raised and ``NULL`` is returned.
+   then :meth:`~os.PathLike.__fspath__` is returned as long as it is a
+   :class:`str` or :class:`bytes` object. Otherwise :exc:`TypeError` is raised
+   and ``NULL`` is returned.
 
    .. versionadded:: 3.6
 
diff --git a/Doc/library/os.rst b/Doc/library/os.rst
index 465b218..0346cc2 100644
--- a/Doc/library/os.rst
+++ b/Doc/library/os.rst
@@ -179,7 +179,8 @@ process and user.
    .. versionadded:: 3.2
 
    .. versionchanged:: 3.6
-      Support added to accept objects implementing :class:`os.PathLike`.
+      Support added to accept objects implementing the :class:`os.PathLike`
+      interface.
 
 
 .. function:: fsdecode(filename)
@@ -192,17 +193,18 @@ process and user.
    .. versionadded:: 3.2
 
    .. versionchanged:: 3.6
-      Support added to accept objects implementing :class:`os.PathLike`.
+      Support added to accept objects implementing the :class:`os.PathLike`
+      interface.
 
 
 .. function:: fspath(path)
 
    Return the file system representation of the path.
 
-   If :class:`str` or :class:`bytes` is passed in, it is returned unchanged;
-   otherwise, the result of calling ``type(path).__fspath__`` is returned
-   (which is represented by :class:`os.PathLike`). All other types raise a
-   :exc:`TypeError`.
+   If :class:`str` or :class:`bytes` is passed in, it is returned unchanged.
+   Otherwise :meth:`~os.PathLike.__fspath__` is called and its value is
+   returned as long as it is a :class:`str` or :class:`bytes` object.
+   In all other cases, :exc:`TypeError` is raised.
 
    .. versionadded:: 3.6
 
diff --git a/Lib/os.py b/Lib/os.py
index 67e1992..c31ecb2 100644
--- a/Lib/os.py
+++ b/Lib/os.py
@@ -881,14 +881,11 @@ def _fscodec():
         On Windows, use 'strict' error handler if the file system encoding is
         'mbcs' (which is the default encoding).
         """
-        filename = fspath(filename)
-        if isinstance(filename, bytes):
-            return filename
-        elif isinstance(filename, str):
+        filename = fspath(filename)  # Does type-checking of `filename`.
+        if isinstance(filename, str):
             return filename.encode(encoding, errors)
         else:
-            raise TypeError("expected str, bytes or os.PathLike object, not "
-                            + type(filename).__name__)
+            return filename
 
     def fsdecode(filename):
         """Decode filename (an os.PathLike, bytes, or str) from the filesystem
@@ -896,14 +893,11 @@ def _fscodec():
         Windows, use 'strict' error handler if the file system encoding is
         'mbcs' (which is the default encoding).
         """
-        filename = fspath(filename)
-        if isinstance(filename, str):
-            return filename
-        elif isinstance(filename, bytes):
+        filename = fspath(filename)  # Does type-checking of `filename`.
+        if isinstance(filename, bytes):
             return filename.decode(encoding, errors)
         else:
-            raise TypeError("expected str, bytes or os.PathLike object, not "
-                            + type(filename).__name__)
+            return filename
 
     return fsencode, fsdecode
 
@@ -1102,27 +1096,44 @@ def fdopen(fd, *args, **kwargs):
     import io
     return io.open(fd, *args, **kwargs)
 
-# Supply os.fspath() if not defined in C
-if not _exists('fspath'):
-    def fspath(path):
-        """Return the string representation of the path.
 
-        If str or bytes is passed in, it is returned unchanged.
-        """
-        if isinstance(path, (str, bytes)):
-            return path
+# For testing purposes, make sure the function is available when the C
+# implementation exists.
+def _fspath(path):
+    """Return the path representation of a path-like object.
 
-        # Work from the object's type to match method resolution of other magic
-        # methods.
-        path_type = type(path)
-        try:
-            return path_type.__fspath__(path)
-        except AttributeError:
-            if hasattr(path_type, '__fspath__'):
-                raise
+    If str or bytes is passed in, it is returned unchanged. Otherwise the
+    os.PathLike interface is used to get the path representation. If the
+    path representation is not str or bytes, TypeError is raised. If the
+    provided path is not str, bytes, or os.PathLike, TypeError is raised.
+    """
+    if isinstance(path, (str, bytes)):
+        return path
+
+    # Work from the object's type to match method resolution of other magic
+    # methods.
+    path_type = type(path)
+    try:
+        path_repr = path_type.__fspath__(path)
+    except AttributeError:
+        if hasattr(path_type, '__fspath__'):
+            raise
+        else:
+            raise TypeError("expected str, bytes or os.PathLike object, "
+                            "not " + path_type.__name__)
+    if isinstance(path_repr, (str, bytes)):
+        return path_repr
+    else:
+        raise TypeError("expected {}.__fspath__() to return str or bytes, "
+                        "not {}".format(path_type.__name__,
+                                        type(path_repr).__name__))
+
+# If there is no C implementation, make the pure Python version the
+# implementation as transparently as possible.
+if not _exists('fspath'):
+    fspath = _fspath
+    fspath.__name__ = "fspath"
 
-            raise TypeError("expected str, bytes or os.PathLike object, not "
-                            + path_type.__name__)
 
 class PathLike(abc.ABC):
 
diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py
index 8581865..0bfaba9 100644
--- a/Lib/test/test_io.py
+++ b/Lib/test/test_io.py
@@ -879,7 +879,7 @@ class IOTest(unittest.TestCase):
         check_path_succeeds(PathLike(support.TESTFN.encode('utf-8')))
 
         bad_path = PathLike(TypeError)
-        with self.assertRaisesRegex(TypeError, 'invalid file'):
+        with self.assertRaises(TypeError):
             self.open(bad_path, 'w')
 
         # ensure that refcounting is correct with some error conditions
diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py
index d34f6c6..869985e 100644
--- a/Lib/test/test_os.py
+++ b/Lib/test/test_os.py
@@ -3112,55 +3112,59 @@ class TestScandir(unittest.TestCase):
 
 
 class TestPEP519(unittest.TestCase):
-    "os.fspath()"
+
+    # Abstracted so it can be overridden to test pure Python implementation
+    # if a C version is provided.
+    fspath = staticmethod(os.fspath)
+
+    class PathLike:
+        def __init__(self, path=''):
+            self.path = path
+        def __fspath__(self):
+            return self.path
 
     def test_return_bytes(self):
         for b in b'hello', b'goodbye', b'some/path/and/file':
-            self.assertEqual(b, os.fspath(b))
+            self.assertEqual(b, self.fspath(b))
 
     def test_return_string(self):
         for s in 'hello', 'goodbye', 'some/path/and/file':
-            self.assertEqual(s, os.fspath(s))
-
-    def test_fsencode_fsdecode_return_pathlike(self):
-        class PathLike:
-            def __init__(self, path):
-                self.path = path
-            def __fspath__(self):
-                return self.path
+            self.assertEqual(s, self.fspath(s))
 
+    def test_fsencode_fsdecode(self):
         for p in "path/like/object", b"path/like/object":
-            pathlike = PathLike(p)
+            pathlike = self.PathLike(p)
 
-            self.assertEqual(p, os.fspath(pathlike))
+            self.assertEqual(p, self.fspath(pathlike))
             self.assertEqual(b"path/like/object", os.fsencode(pathlike))
             self.assertEqual("path/like/object", os.fsdecode(pathlike))
 
-    def test_fspathlike(self):
-        class PathLike:
-            def __init__(self, path=''):
-                self.path = path
-            def __fspath__(self):
-                return self.path
+    def test_pathlike(self):
+        self.assertEqual('#feelthegil', self.fspath(self.PathLike('#feelthegil')))
+        self.assertTrue(issubclass(self.PathLike, os.PathLike))
+        self.assertTrue(isinstance(self.PathLike(), os.PathLike))
 
-        self.assertEqual('#feelthegil', os.fspath(PathLike('#feelthegil')))
-        self.assertTrue(issubclass(PathLike, os.PathLike))
-        self.assertTrue(isinstance(PathLike(), os.PathLike))
-
-        message = 'expected str, bytes or os.PathLike object, not'
-        for fn in (os.fsencode, os.fsdecode):
-            for obj in PathLike(None), None:
-                with self.assertRaisesRegex(TypeError, message):
-                    fn(obj)
+        with self.assertRaises(TypeError):
+            self.fspath(self.PathLike(42))
 
     def test_garbage_in_exception_out(self):
         vapor = type('blah', (), {})
         for o in int, type, os, vapor():
-            self.assertRaises(TypeError, os.fspath, o)
+            self.assertRaises(TypeError, self.fspath, o)
 
     def test_argument_required(self):
         with self.assertRaises(TypeError):
-            os.fspath()
+            self.fspath()
+
+
+# Only test if the C version is provided, otherwise TestPEP519 already tested
+# the pure Python implementation.
+if hasattr(os, "_fspath"):
+    class TestPEP519PurePython(TestPEP519):
+
+        """Explicitly test the pure Python implementation of os.fspath()."""
+
+        fspath = staticmethod(os._fspath)
 
 
 if __name__ == "__main__":
diff --git a/Misc/NEWS b/Misc/NEWS
index a87b5cb..4c9d120 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,9 @@ What's New in Python 3.6.0 alpha 3
 Library
 -------
 
+- Issue #27186: Update os.fspath()/PyOS_FSPath() to check the return value of
+  __fspath__() to be either str or bytes.
+
 - Issue #18726: All optional parameters of the dump(), dumps(),
   load() and loads() functions and JSONEncoder and JSONDecoder class
   constructors in the json module are now keyword-only.
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index 7d82490..df802cb 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -12317,12 +12317,21 @@ PyOS_FSPath(PyObject *path)
     if (NULL == func) {
         return PyErr_Format(PyExc_TypeError,
                             "expected str, bytes or os.PathLike object, "
-                            "not %S",
-                            path->ob_type);
+                            "not %.200s",
+                            Py_TYPE(path)->tp_name);
     }
 
     path_repr = PyObject_CallFunctionObjArgs(func, NULL);
     Py_DECREF(func);
+    if (!(PyUnicode_Check(path_repr) || PyBytes_Check(path_repr))) {
+        PyErr_Format(PyExc_TypeError,
+                     "expected %.200s.__fspath__() to return str or bytes, "
+                     "not %.200s", Py_TYPE(path)->tp_name,
+                     Py_TYPE(path_repr)->tp_name);
+        Py_DECREF(path_repr);
+        return NULL;
+    }
+
     return path_repr;
 }