From c78ca1e044b7ca4c1764bb3670196e72351d4467 Mon Sep 17 00:00:00 2001 From: Brett Cannon Date: Fri, 24 Jun 2016 12:03:43 -0700 Subject: 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. --- Doc/c-api/sys.rst | 5 ++-- Doc/library/os.rst | 14 +++++----- Lib/os.py | 71 +++++++++++++++++++++++++++++---------------------- Lib/test/test_io.py | 2 +- Lib/test/test_os.py | 62 +++++++++++++++++++++++--------------------- Misc/NEWS | 3 +++ Modules/posixmodule.c | 13 ++++++++-- 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; } -- cgit v0.12