[Python-checkins] cpython: Issue #27186: Update os.fspath()/PyOS_FSPath() to check the return

Fri Jun 24 15:03:48 EDT 2016

https://hg.python.org/cpython/rev/ea7b6a7827a4
changeset: 102160:ea7b6a7827a4
parent: 102158:7d5bc8a3c7fc
user: Brett Cannon <brett at python.org>
date: Fri Jun 24 12:03:43 2016 -0700
summary:
 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.
files:
 Doc/c-api/sys.rst | 5 +-
 Doc/library/os.rst | 14 +++--
 Lib/os.py | 73 +++++++++++++++++-------------
 Lib/test/test_io.py | 2 +-
 Lib/test/test_os.py | 68 +++++++++++++++-------------
 Misc/NEWS | 3 +
 Modules/posixmodule.c | 13 ++++-
 7 files changed, 104 insertions(+), 74 deletions(-)

diff --git a/Doc/c-api/sys.rst b/Doc/c-api/sys.rst
--- a/Doc/c-api/sys.rst
+++ b/Doc/c-api/sys.rst
@@ -10,8 +10,9 @@
 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
--- a/Doc/library/os.rst
+++ b/Doc/library/os.rst
@@ -179,7 +179,8 @@
 .. 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 @@
 .. 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
--- a/Lib/os.py
+++ b/Lib/os.py
@@ -881,14 +881,11 @@
 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 @@
 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 @@
 import io
 return io.open(fd, *args, **kwargs)
 
-# Supply os.fspath() if not defined in C
+
+# 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.
+
+ 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'):
- def fspath(path):
- """Return the string representation of the path.
+ fspath = _fspath
+ fspath.__name__ = "fspath"
 
- If str or bytes is passed in, it is returned unchanged.
- """
- 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:
- return path_type.__fspath__(path)
- except AttributeError:
- if hasattr(path_type, '__fspath__'):
- raise
-
- 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
--- a/Lib/test/test_io.py
+++ b/Lib/test/test_io.py
@@ -879,7 +879,7 @@
 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
--- a/Lib/test/test_os.py
+++ b/Lib/test/test_os.py
@@ -3112,55 +3112,59 @@
 
 
 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)
-
- self.assertEqual(p, os.fspath(pathlike))
+ pathlike = self.PathLike(p)
+
+ 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
-
- 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)
+ 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))
+
+ 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
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,9 @@
 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
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -12317,12 +12317,21 @@
 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;
 }
 
-- 
Repository URL: https://hg.python.org/cpython
