From 0fafcd4f99fcb835183b1e16381d4067d47b4e74 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 14 Jun 2018 17:55:19 +0300 Subject: [PATCH 1/5] bpo-33721: Make some os.path functions be tolerant to invalid paths. Such functions as os.path.exists(), os.path.lexists(), os.path.isdir(), os.path.isfile(), os.path.islink(), and os.path.ismount() now return False instead of raising ValueError or its subclasses UnicodeEncodeError and UnicodeDecodeError for paths that contain characters or bytes unrepresntative at the OS level. --- Doc/whatsnew/3.8.rst | 12 ++++++ Lib/genericpath.py | 6 +-- Lib/macpath.py | 2 +- Lib/ntpath.py | 6 +-- Lib/posixpath.py | 8 ++-- Lib/test/test_genericpath.py | 40 ++++++++++++++----- Lib/test/test_posixpath.py | 12 ++++++ .../2018-06-14-17-53-30.bpo-33721.8i9_9A.rst | 7 ++++ Modules/clinic/posixmodule.c.h | 23 +---------- Modules/posixmodule.c | 18 +++++++-- 10 files changed, 88 insertions(+), 46 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-06-14-17-53-30.bpo-33721.8i9_9A.rst diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst index 10fa182faeb3bf..b3c7575464d5aa 100644 --- a/Doc/whatsnew/3.8.rst +++ b/Doc/whatsnew/3.8.rst @@ -90,6 +90,18 @@ New Modules Improved Modules ================ +os.path +------- + +:mod:`os.path` functions that return the boolean result like +:func:`~os.path.exists`, :func:`~os.path.lexists`, :func:`~os.path.isdir`, +:func:`~os.path.isfile`, :func:`~os.path.islink`, and :func:`~os.path.ismount` +now return ``False`` instead of raising :exc:`ValueError` or its subclasses +:exc:`UnicodeEncodeError` and :exc:`UnicodeDecodeError` for paths that contain +characters or bytes unrepresntative at the OS level. +(Contributed by Serhiy Storchaka in :issue:`33721`.) + + Optimizations ============= diff --git a/Lib/genericpath.py b/Lib/genericpath.py index 303b3b349a9f40..5dd703d736c509 100644 --- a/Lib/genericpath.py +++ b/Lib/genericpath.py @@ -17,7 +17,7 @@ def exists(path): """Test whether a path exists. Returns False for broken symbolic links""" try: os.stat(path) - except OSError: + except (OSError, ValueError): return False return True @@ -28,7 +28,7 @@ def isfile(path): """Test whether a path is a regular file""" try: st = os.stat(path) - except OSError: + except (OSError, ValueError): return False return stat.S_ISREG(st.st_mode) @@ -40,7 +40,7 @@ def isdir(s): """Return true if the pathname refers to an existing directory.""" try: st = os.stat(s) - except OSError: + except (OSError, ValueError): return False return stat.S_ISDIR(st.st_mode) diff --git a/Lib/macpath.py b/Lib/macpath.py index aacf7235b011fd..9a12d2feee3527 100644 --- a/Lib/macpath.py +++ b/Lib/macpath.py @@ -138,7 +138,7 @@ def lexists(path): try: st = os.lstat(path) - except OSError: + except (OSError, ValueError): return False return True diff --git a/Lib/ntpath.py b/Lib/ntpath.py index 2182ec776cc503..b0203ccf69abda 100644 --- a/Lib/ntpath.py +++ b/Lib/ntpath.py @@ -229,7 +229,7 @@ def islink(path): """ try: st = os.lstat(path) - except (OSError, AttributeError): + except (OSError, ValueError, AttributeError): return False return stat.S_ISLNK(st.st_mode) @@ -239,7 +239,7 @@ def lexists(path): """Test whether a path exists. Returns True for broken symbolic links""" try: st = os.lstat(path) - except OSError: + except (OSError, ValueError): return False return True @@ -521,7 +521,7 @@ def abspath(path): path = os.fspath(path) try: path = _getfullpathname(path) - except OSError: + except (OSError, ValueError): pass # Bad path - return unchanged. elif isinstance(path, bytes): path = os.getcwdb() diff --git a/Lib/posixpath.py b/Lib/posixpath.py index e92186c64e0ddb..7e3f3db4b6d017 100644 --- a/Lib/posixpath.py +++ b/Lib/posixpath.py @@ -169,7 +169,7 @@ def islink(path): """Test whether a path is a symbolic link""" try: st = os.lstat(path) - except (OSError, AttributeError): + except (OSError, ValueError, AttributeError): return False return stat.S_ISLNK(st.st_mode) @@ -179,7 +179,7 @@ def lexists(path): """Test whether a path exists. Returns True for broken symbolic links""" try: os.lstat(path) - except OSError: + except (OSError, ValueError): return False return True @@ -191,7 +191,7 @@ def ismount(path): """Test whether a path is a mount point""" try: s1 = os.lstat(path) - except OSError: + except (OSError, ValueError): # It doesn't exist -- so not a mount point. :-) return False else: @@ -206,7 +206,7 @@ def ismount(path): parent = realpath(parent) try: s2 = os.lstat(parent) - except OSError: + except (OSError, ValueError): return False dev1 = s1.st_dev diff --git a/Lib/test/test_genericpath.py b/Lib/test/test_genericpath.py index 8b291cda689ce1..321fc7dcb1efd5 100644 --- a/Lib/test/test_genericpath.py +++ b/Lib/test/test_genericpath.py @@ -138,10 +138,20 @@ def test_exists(self): self.assertIs(self.pathmodule.exists(filename), True) self.assertIs(self.pathmodule.exists(bfilename), True) + self.assertIs(self.pathmodule.exists(filename + '\udfff'), False) + self.assertIs(self.pathmodule.exists(bfilename + b'\xff'), False) + self.assertIs(self.pathmodule.exists(filename + '\x00'), False) + self.assertIs(self.pathmodule.exists(bfilename + b'\x00'), False) + if self.pathmodule is not genericpath: self.assertIs(self.pathmodule.lexists(filename), True) self.assertIs(self.pathmodule.lexists(bfilename), True) + self.assertIs(self.pathmodule.lexists(filename + '\udfff'), False) + self.assertIs(self.pathmodule.lexists(bfilename + b'\xff'), False) + self.assertIs(self.pathmodule.lexists(filename + '\x00'), False) + self.assertIs(self.pathmodule.lexists(bfilename + b'\x00'), False) + @unittest.skipUnless(hasattr(os, "pipe"), "requires os.pipe()") def test_exists_fd(self): r, w = os.pipe() @@ -158,6 +168,11 @@ def test_isdir(self): self.assertIs(self.pathmodule.isdir(filename), False) self.assertIs(self.pathmodule.isdir(bfilename), False) + self.assertIs(self.pathmodule.isdir(filename + '\udfff'), False) + self.assertIs(self.pathmodule.isdir(bfilename + b'\xff'), False) + self.assertIs(self.pathmodule.isdir(filename + '\x00'), False) + self.assertIs(self.pathmodule.isdir(bfilename + b'\x00'), False) + try: create_file(filename) self.assertIs(self.pathmodule.isdir(filename), False) @@ -178,6 +193,11 @@ def test_isfile(self): self.assertIs(self.pathmodule.isfile(filename), False) self.assertIs(self.pathmodule.isfile(bfilename), False) + self.assertIs(self.pathmodule.isfile(filename + '\udfff'), False) + self.assertIs(self.pathmodule.isfile(bfilename + b'\xff'), False) + self.assertIs(self.pathmodule.isfile(filename + '\x00'), False) + self.assertIs(self.pathmodule.isfile(bfilename + b'\x00'), False) + try: create_file(filename) self.assertIs(self.pathmodule.isfile(filename), True) @@ -294,22 +314,24 @@ class TestGenericTest(GenericTest, unittest.TestCase): def test_invalid_paths(self): for attr in GenericTest.common_attributes: # os.path.commonprefix doesn't raise ValueError - if attr == 'commonprefix': + if attr in 'commonprefix': continue func = getattr(self.pathmodule, attr) with self.subTest(attr=attr): - try: + if attr in ('exists', 'isdir', 'isfile'): func('/tmp\udfffabcds') - except (OSError, UnicodeEncodeError): - pass - try: func(b'/tmp\xffabcds') - except (OSError, UnicodeDecodeError): - pass - with self.assertRaisesRegex(ValueError, 'embedded null'): func('/tmp\x00abcds') - with self.assertRaisesRegex(ValueError, 'embedded null'): func(b'/tmp\x00abcds') + else: + with self.assertRaises((OSError, UnicodeEncodeError)): + func('/tmp\udfffabcds') + with self.assertRaises((OSError, UnicodeDecodeError)): + func(b'/tmp\xffabcds') + with self.assertRaisesRegex(ValueError, 'embedded null'): + func('/tmp\x00abcds') + with self.assertRaisesRegex(ValueError, 'embedded null'): + func(b'/tmp\x00abcds') # Following TestCase is not supposed to be run from test_genericpath. # It is inherited by other test modules (macpath, ntpath, posixpath). diff --git a/Lib/test/test_posixpath.py b/Lib/test/test_posixpath.py index 9476ede5319397..ae59ef5927be84 100644 --- a/Lib/test/test_posixpath.py +++ b/Lib/test/test_posixpath.py @@ -153,9 +153,11 @@ def test_dirname(self): def test_islink(self): self.assertIs(posixpath.islink(support.TESTFN + "1"), False) self.assertIs(posixpath.lexists(support.TESTFN + "2"), False) + with open(support.TESTFN + "1", "wb") as f: f.write(b"foo") self.assertIs(posixpath.islink(support.TESTFN + "1"), False) + if support.can_symlink(): os.symlink(support.TESTFN + "1", support.TESTFN + "2") self.assertIs(posixpath.islink(support.TESTFN + "2"), True) @@ -164,6 +166,11 @@ def test_islink(self): self.assertIs(posixpath.exists(support.TESTFN + "2"), False) self.assertIs(posixpath.lexists(support.TESTFN + "2"), True) + self.assertIs(posixpath.islink(support.TESTFN + "\udfff"), False) + self.assertIs(posixpath.islink(os.fsencode(support.TESTFN) + b"\xff"), False) + self.assertIs(posixpath.islink(support.TESTFN + "\x00"), False) + self.assertIs(posixpath.islink(os.fsencode(support.TESTFN) + b"\x00"), False) + def test_ismount(self): self.assertIs(posixpath.ismount("/"), True) self.assertIs(posixpath.ismount(b"/"), True) @@ -177,6 +184,11 @@ def test_ismount_non_existent(self): finally: safe_rmdir(ABSTFN) + self.assertIs(posixpath.ismount('/\udfff'), False) + self.assertIs(posixpath.ismount(b'/\xff'), False) + self.assertIs(posixpath.ismount('/\x00'), False) + self.assertIs(posixpath.ismount(b'/\x00'), False) + @unittest.skipUnless(support.can_symlink(), "Test requires symlink support") def test_ismount_symlinks(self): diff --git a/Misc/NEWS.d/next/Library/2018-06-14-17-53-30.bpo-33721.8i9_9A.rst b/Misc/NEWS.d/next/Library/2018-06-14-17-53-30.bpo-33721.8i9_9A.rst new file mode 100644 index 00000000000000..a074be204a5472 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-06-14-17-53-30.bpo-33721.8i9_9A.rst @@ -0,0 +1,7 @@ +:mod:`os.path` functions that return the boolean result like +:func:`~os.path.exists`, :func:`~os.path.lexists`, :func:`~os.path.isdir`, +:func:`~os.path.isfile`, :func:`~os.path.islink`, and +:func:`~os.path.ismount` now return ``False`` instead of raising +:exc:`ValueError` or its subclasses :exc:`UnicodeEncodeError` and +:exc:`UnicodeDecodeError` for paths that contain characters or bytes +unrepresntative at the OS level. diff --git a/Modules/clinic/posixmodule.c.h b/Modules/clinic/posixmodule.c.h index c41d1314037831..810480c3ffdd98 100644 --- a/Modules/clinic/posixmodule.c.h +++ b/Modules/clinic/posixmodule.c.h @@ -1005,27 +1005,6 @@ PyDoc_STRVAR(os__isdir__doc__, #define OS__ISDIR_METHODDEF \ {"_isdir", (PyCFunction)os__isdir, METH_O, os__isdir__doc__}, -static PyObject * -os__isdir_impl(PyObject *module, path_t *path); - -static PyObject * -os__isdir(PyObject *module, PyObject *arg) -{ - PyObject *return_value = NULL; - path_t path = PATH_T_INITIALIZE("_isdir", "path", 0, 0); - - if (!PyArg_Parse(arg, "O&:_isdir", path_converter, &path)) { - goto exit; - } - return_value = os__isdir_impl(module, &path); - -exit: - /* Cleanup for path */ - path_cleanup(&path); - - return return_value; -} - #endif /* defined(MS_WINDOWS) */ #if defined(MS_WINDOWS) @@ -6627,4 +6606,4 @@ os_getrandom(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject #ifndef OS_GETRANDOM_METHODDEF #define OS_GETRANDOM_METHODDEF #endif /* !defined(OS_GETRANDOM_METHODDEF) */ -/*[clinic end generated code: output=b5d1ec71bc6f0651 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=4f02a7b88410d154 input=a9049054013a1b77]*/ diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 24d8be66665e82..c6ed7708cc87a6 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3854,22 +3854,32 @@ os__getfinalpathname_impl(PyObject *module, path_t *path) /*[clinic input] os._isdir - path: path_t + path as arg: object / Return true if the pathname refers to an existing directory. [clinic start generated code]*/ static PyObject * -os__isdir_impl(PyObject *module, path_t *path) -/*[clinic end generated code: output=75f56f32720836cb input=5e0800149c0ad95f]*/ +os__isdir(PyObject *module, PyObject *arg) +/*[clinic end generated code: output=404f334d85d4bf25 input=36cb6785874d479e]*/ { DWORD attributes; + path_t path = PATH_T_INITIALIZE("_isdir", "path", 0, 0); + + if (!path_converter(arg, &path)) { + if (PyErr_ExceptionMatches(PyExc_ValueError)) { + PyErr_Clear(); + Py_RETURN_FALSE; + } + return NULL; + } Py_BEGIN_ALLOW_THREADS - attributes = GetFileAttributesW(path->wide); + attributes = GetFileAttributesW(path.wide); Py_END_ALLOW_THREADS + path_cleanup(&path); if (attributes == INVALID_FILE_ATTRIBUTES) Py_RETURN_FALSE; From dba3f82e22e8144dbcdcadd08102011f65b578cb Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 15 Jun 2018 07:02:19 +0300 Subject: [PATCH 2/5] Fix test_site. --- Lib/test/test_site.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_site.py b/Lib/test/test_site.py index e3c9deebf08c02..742be1ec03d416 100644 --- a/Lib/test/test_site.py +++ b/Lib/test/test_site.py @@ -159,13 +159,11 @@ def test_addpackage_import_bad_pth_file(self): # Issue 5258 pth_dir, pth_fn = self.make_pth("abc\x00def\n") with captured_stderr() as err_out: - site.addpackage(pth_dir, pth_fn, set()) - self.assertRegex(err_out.getvalue(), "line 1") - self.assertRegex(err_out.getvalue(), - re.escape(os.path.join(pth_dir, pth_fn))) - # XXX: ditto previous XXX comment. - self.assertRegex(err_out.getvalue(), 'Traceback') - self.assertRegex(err_out.getvalue(), 'ValueError') + self.assertFalse(site.addpackage(pth_dir, pth_fn, set())) + self.assertEqual(err_out.getvalue(), "") + for path in sys.path: + if isinstance(path, str): + self.assertNotIn("abc\x00def", path) def test_addsitedir(self): # Same tests for test_addpackage since addsitedir() essentially just From c2d91b50642c790f136c22c62305d90e8175dc79 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 16 Jun 2018 13:32:59 +0300 Subject: [PATCH 3/5] Revert unintentional change. --- Lib/test/test_genericpath.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_genericpath.py b/Lib/test/test_genericpath.py index 321fc7dcb1efd5..08e1c12101c7f8 100644 --- a/Lib/test/test_genericpath.py +++ b/Lib/test/test_genericpath.py @@ -314,7 +314,7 @@ class TestGenericTest(GenericTest, unittest.TestCase): def test_invalid_paths(self): for attr in GenericTest.common_attributes: # os.path.commonprefix doesn't raise ValueError - if attr in 'commonprefix': + if attr == 'commonprefix': continue func = getattr(self.pathmodule, attr) with self.subTest(attr=attr): From 80c1f7f276be396da61bb13bb59aaab3348538a4 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 19 Aug 2018 10:03:14 +0300 Subject: [PATCH 4/5] Fix wording. --- Doc/whatsnew/3.8.rst | 4 ++-- .../next/Library/2018-06-14-17-53-30.bpo-33721.8i9_9A.rst | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst index 0a9e5b744a7d9d..f9e2f209b40b7d 100644 --- a/Doc/whatsnew/3.8.rst +++ b/Doc/whatsnew/3.8.rst @@ -107,12 +107,12 @@ Improved Modules os.path ------- -:mod:`os.path` functions that return the boolean result like +:mod:`os.path` functions that return a boolean result like :func:`~os.path.exists`, :func:`~os.path.lexists`, :func:`~os.path.isdir`, :func:`~os.path.isfile`, :func:`~os.path.islink`, and :func:`~os.path.ismount` now return ``False`` instead of raising :exc:`ValueError` or its subclasses :exc:`UnicodeEncodeError` and :exc:`UnicodeDecodeError` for paths that contain -characters or bytes unrepresntative at the OS level. +characters or bytes unrepresentable at the OS level. (Contributed by Serhiy Storchaka in :issue:`33721`.) diff --git a/Misc/NEWS.d/next/Library/2018-06-14-17-53-30.bpo-33721.8i9_9A.rst b/Misc/NEWS.d/next/Library/2018-06-14-17-53-30.bpo-33721.8i9_9A.rst index a074be204a5472..499315417534df 100644 --- a/Misc/NEWS.d/next/Library/2018-06-14-17-53-30.bpo-33721.8i9_9A.rst +++ b/Misc/NEWS.d/next/Library/2018-06-14-17-53-30.bpo-33721.8i9_9A.rst @@ -1,7 +1,7 @@ -:mod:`os.path` functions that return the boolean result like +:mod:`os.path` functions that return a boolean result like :func:`~os.path.exists`, :func:`~os.path.lexists`, :func:`~os.path.isdir`, :func:`~os.path.isfile`, :func:`~os.path.islink`, and :func:`~os.path.ismount` now return ``False`` instead of raising :exc:`ValueError` or its subclasses :exc:`UnicodeEncodeError` and :exc:`UnicodeDecodeError` for paths that contain characters or bytes -unrepresntative at the OS level. +unrepresentable at the OS level. From 473ef99940fc7d0d04e6373ff42f86c283e27a5d Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sun, 19 Aug 2018 10:48:04 +0300 Subject: [PATCH 5/5] Make same changes in pathlib. --- Doc/library/os.path.rst | 8 +++++ Doc/library/pathlib.rst | 12 ++++++- Doc/whatsnew/3.8.rst | 13 +++++++ Lib/pathlib.py | 34 +++++-------------- Lib/test/test_pathlib.py | 29 +++++++++++++++- .../2018-06-14-17-53-30.bpo-33721.8i9_9A.rst | 9 +++-- 6 files changed, 76 insertions(+), 29 deletions(-) diff --git a/Doc/library/os.path.rst b/Doc/library/os.path.rst index 36bb21c222ed28..5a0b178b9d04b1 100644 --- a/Doc/library/os.path.rst +++ b/Doc/library/os.path.rst @@ -55,6 +55,14 @@ the :mod:`glob` module.) * :mod:`macpath` for old-style MacOS paths +.. versionchanged:: 3.8 + + :func:`exists`, :func:`lexists`, :func:`isdir`, :func:`isfile`, + :func:`islink`, and :func:`ismount` now return ``False`` instead of + raising an exception for paths that contain characters or bytes + unrepresentable at the OS level. + + .. function:: abspath(path) Return a normalized absolutized version of the pathname *path*. On most diff --git a/Doc/library/pathlib.rst b/Doc/library/pathlib.rst index ec604f6815937d..fc193000ba698f 100644 --- a/Doc/library/pathlib.rst +++ b/Doc/library/pathlib.rst @@ -638,7 +638,17 @@ Methods Concrete paths provide the following methods in addition to pure paths methods. Many of these methods can raise an :exc:`OSError` if a system -call fails (for example because the path doesn't exist): +call fails (for example because the path doesn't exist). + +.. versionchanged:: 3.8 + + :meth:`~Path.exists()`, :meth:`~Path.is_dir()`, :meth:`~Path.is_file()`, + :meth:`~Path.is_mount()`, :meth:`~Path.is_symlink()`, + :meth:`~Path.is_block_device()`, :meth:`~Path.is_char_device()`, + :meth:`~Path.is_fifo()`, :meth:`~Path.is_socket()` now return ``False`` + instead of raising an exception for paths that contain characters + unrepresentable at the OS level. + .. classmethod:: Path.cwd() diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst index f9e2f209b40b7d..bbb3eb13623184 100644 --- a/Doc/whatsnew/3.8.rst +++ b/Doc/whatsnew/3.8.rst @@ -115,6 +115,19 @@ now return ``False`` instead of raising :exc:`ValueError` or its subclasses characters or bytes unrepresentable at the OS level. (Contributed by Serhiy Storchaka in :issue:`33721`.) +pathlib +------- + +:mod:`pathlib.Path` methods that return a boolean result like +:meth:`~pathlib.Path.exists()`, :meth:`~pathlib.Path.is_dir()`, +:meth:`~pathlib.Path.is_file()`, :meth:`~pathlib.Path.is_mount()`, +:meth:`~pathlib.Path.is_symlink()`, :meth:`~pathlib.Path.is_block_device()`, +:meth:`~pathlib.Path.is_char_device()`, :meth:`~pathlib.Path.is_fifo()`, +:meth:`~pathlib.Path.is_socket()` now return ``False`` instead of raising +:exc:`ValueError` or its subclass :exc:`UnicodeEncodeError` for paths that +contain characters unrepresentable at the OS level. +(Contributed by Serhiy Storchaka in :issue:`33721`.) + Optimizations ============= diff --git a/Lib/pathlib.py b/Lib/pathlib.py index 6be372ed320a2c..0b050ed52ec08a 100644 --- a/Lib/pathlib.py +++ b/Lib/pathlib.py @@ -7,7 +7,7 @@ import re import sys from _collections_abc import Sequence -from errno import EINVAL, ENOENT, ENOTDIR +from errno import EINVAL from operator import attrgetter from stat import S_ISDIR, S_ISLNK, S_ISREG, S_ISSOCK, S_ISBLK, S_ISCHR, S_ISFIFO from urllib.parse import quote_from_bytes as urlquote_from_bytes @@ -1318,9 +1318,7 @@ def exists(self): """ try: self.stat() - except OSError as e: - if e.errno not in (ENOENT, ENOTDIR): - raise + except (FileNotFoundError, NotADirectoryError, ValueError): return False return True @@ -1330,9 +1328,7 @@ def is_dir(self): """ try: return S_ISDIR(self.stat().st_mode) - except OSError as e: - if e.errno not in (ENOENT, ENOTDIR): - raise + except (FileNotFoundError, NotADirectoryError, ValueError): # Path doesn't exist or is a broken symlink # (see https://bitbucket.org/pitrou/pathlib/issue/12/) return False @@ -1344,9 +1340,7 @@ def is_file(self): """ try: return S_ISREG(self.stat().st_mode) - except OSError as e: - if e.errno not in (ENOENT, ENOTDIR): - raise + except (FileNotFoundError, NotADirectoryError, ValueError): # Path doesn't exist or is a broken symlink # (see https://bitbucket.org/pitrou/pathlib/issue/12/) return False @@ -1378,9 +1372,7 @@ def is_symlink(self): """ try: return S_ISLNK(self.lstat().st_mode) - except OSError as e: - if e.errno not in (ENOENT, ENOTDIR): - raise + except (FileNotFoundError, NotADirectoryError, ValueError): # Path doesn't exist return False @@ -1390,9 +1382,7 @@ def is_block_device(self): """ try: return S_ISBLK(self.stat().st_mode) - except OSError as e: - if e.errno not in (ENOENT, ENOTDIR): - raise + except (FileNotFoundError, NotADirectoryError, ValueError): # Path doesn't exist or is a broken symlink # (see https://bitbucket.org/pitrou/pathlib/issue/12/) return False @@ -1403,9 +1393,7 @@ def is_char_device(self): """ try: return S_ISCHR(self.stat().st_mode) - except OSError as e: - if e.errno not in (ENOENT, ENOTDIR): - raise + except (FileNotFoundError, NotADirectoryError, ValueError): # Path doesn't exist or is a broken symlink # (see https://bitbucket.org/pitrou/pathlib/issue/12/) return False @@ -1416,9 +1404,7 @@ def is_fifo(self): """ try: return S_ISFIFO(self.stat().st_mode) - except OSError as e: - if e.errno not in (ENOENT, ENOTDIR): - raise + except (FileNotFoundError, NotADirectoryError, ValueError): # Path doesn't exist or is a broken symlink # (see https://bitbucket.org/pitrou/pathlib/issue/12/) return False @@ -1429,9 +1415,7 @@ def is_socket(self): """ try: return S_ISSOCK(self.stat().st_mode) - except OSError as e: - if e.errno not in (ENOENT, ENOTDIR): - raise + except (FileNotFoundError, NotADirectoryError, ValueError): # Path doesn't exist or is a broken symlink # (see https://bitbucket.org/pitrou/pathlib/issue/12/) return False diff --git a/Lib/test/test_pathlib.py b/Lib/test/test_pathlib.py index ae7c75deb0e6b7..99a2857535a97a 100644 --- a/Lib/test/test_pathlib.py +++ b/Lib/test/test_pathlib.py @@ -1341,6 +1341,8 @@ def test_exists(self): self.assertIs(False, (p / 'linkA' / 'bah').exists()) self.assertIs(False, (p / 'foo').exists()) self.assertIs(False, P('/xyzzy').exists()) + self.assertIs(False, P(BASE + '\udfff').exists()) + self.assertIs(False, P(BASE + '\x00').exists()) def test_open_common(self): p = self.cls(BASE) @@ -1865,7 +1867,9 @@ def test_is_dir(self): if support.can_symlink(): self.assertFalse((P / 'linkA').is_dir()) self.assertTrue((P / 'linkB').is_dir()) - self.assertFalse((P/ 'brokenLink').is_dir()) + self.assertFalse((P/ 'brokenLink').is_dir(), False) + self.assertIs((P / 'dirA\udfff').is_dir(), False) + self.assertIs((P / 'dirA\x00').is_dir(), False) def test_is_file(self): P = self.cls(BASE) @@ -1877,6 +1881,8 @@ def test_is_file(self): self.assertTrue((P / 'linkA').is_file()) self.assertFalse((P / 'linkB').is_file()) self.assertFalse((P/ 'brokenLink').is_file()) + self.assertIs((P / 'fileA\udfff').is_file(), False) + self.assertIs((P / 'fileA\x00').is_file(), False) @only_posix def test_is_mount(self): @@ -1889,6 +1895,8 @@ def test_is_mount(self): self.assertTrue(R.is_mount()) if support.can_symlink(): self.assertFalse((P / 'linkA').is_mount()) + self.assertIs(self.cls('/\udfff').is_mount(), False) + self.assertIs(self.cls('/\x00').is_mount(), False) def test_is_symlink(self): P = self.cls(BASE) @@ -1900,6 +1908,11 @@ def test_is_symlink(self): self.assertTrue((P / 'linkA').is_symlink()) self.assertTrue((P / 'linkB').is_symlink()) self.assertTrue((P/ 'brokenLink').is_symlink()) + self.assertIs((P / 'fileA\udfff').is_file(), False) + self.assertIs((P / 'fileA\x00').is_file(), False) + if support.can_symlink(): + self.assertIs((P / 'linkA\udfff').is_file(), False) + self.assertIs((P / 'linkA\x00').is_file(), False) def test_is_fifo_false(self): P = self.cls(BASE) @@ -1907,6 +1920,8 @@ def test_is_fifo_false(self): self.assertFalse((P / 'dirA').is_fifo()) self.assertFalse((P / 'non-existing').is_fifo()) self.assertFalse((P / 'fileA' / 'bah').is_fifo()) + self.assertIs((P / 'fileA\udfff').is_fifo(), False) + self.assertIs((P / 'fileA\x00').is_fifo(), False) @unittest.skipUnless(hasattr(os, "mkfifo"), "os.mkfifo() required") def test_is_fifo_true(self): @@ -1918,6 +1933,8 @@ def test_is_fifo_true(self): self.assertTrue(P.is_fifo()) self.assertFalse(P.is_socket()) self.assertFalse(P.is_file()) + self.assertIs(self.cls(BASE, 'myfifo\udfff').is_fifo(), False) + self.assertIs(self.cls(BASE, 'myfifo\x00').is_fifo(), False) def test_is_socket_false(self): P = self.cls(BASE) @@ -1925,6 +1942,8 @@ def test_is_socket_false(self): self.assertFalse((P / 'dirA').is_socket()) self.assertFalse((P / 'non-existing').is_socket()) self.assertFalse((P / 'fileA' / 'bah').is_socket()) + self.assertIs((P / 'fileA\udfff').is_socket(), False) + self.assertIs((P / 'fileA\x00').is_socket(), False) @unittest.skipUnless(hasattr(socket, "AF_UNIX"), "Unix sockets required") def test_is_socket_true(self): @@ -1940,6 +1959,8 @@ def test_is_socket_true(self): self.assertTrue(P.is_socket()) self.assertFalse(P.is_fifo()) self.assertFalse(P.is_file()) + self.assertIs(self.cls(BASE, 'mysock\udfff').is_socket(), False) + self.assertIs(self.cls(BASE, 'mysock\x00').is_socket(), False) def test_is_block_device_false(self): P = self.cls(BASE) @@ -1947,6 +1968,8 @@ def test_is_block_device_false(self): self.assertFalse((P / 'dirA').is_block_device()) self.assertFalse((P / 'non-existing').is_block_device()) self.assertFalse((P / 'fileA' / 'bah').is_block_device()) + self.assertIs((P / 'fileA\udfff').is_block_device(), False) + self.assertIs((P / 'fileA\x00').is_block_device(), False) def test_is_char_device_false(self): P = self.cls(BASE) @@ -1954,6 +1977,8 @@ def test_is_char_device_false(self): self.assertFalse((P / 'dirA').is_char_device()) self.assertFalse((P / 'non-existing').is_char_device()) self.assertFalse((P / 'fileA' / 'bah').is_char_device()) + self.assertIs((P / 'fileA\udfff').is_char_device(), False) + self.assertIs((P / 'fileA\x00').is_char_device(), False) def test_is_char_device_true(self): # Under Unix, /dev/null should generally be a char device @@ -1963,6 +1988,8 @@ def test_is_char_device_true(self): self.assertTrue(P.is_char_device()) self.assertFalse(P.is_block_device()) self.assertFalse(P.is_file()) + self.assertIs(self.cls('/dev/null\udfff').is_char_device(), False) + self.assertIs(self.cls('/dev/null\x00').is_char_device(), False) def test_pickling_common(self): p = self.cls(BASE, 'fileA') diff --git a/Misc/NEWS.d/next/Library/2018-06-14-17-53-30.bpo-33721.8i9_9A.rst b/Misc/NEWS.d/next/Library/2018-06-14-17-53-30.bpo-33721.8i9_9A.rst index 499315417534df..987bf4696d568c 100644 --- a/Misc/NEWS.d/next/Library/2018-06-14-17-53-30.bpo-33721.8i9_9A.rst +++ b/Misc/NEWS.d/next/Library/2018-06-14-17-53-30.bpo-33721.8i9_9A.rst @@ -1,7 +1,12 @@ :mod:`os.path` functions that return a boolean result like :func:`~os.path.exists`, :func:`~os.path.lexists`, :func:`~os.path.isdir`, -:func:`~os.path.isfile`, :func:`~os.path.islink`, and -:func:`~os.path.ismount` now return ``False`` instead of raising +:func:`~os.path.isfile`, :func:`~os.path.islink`, and :func:`~os.path.ismount`, +and :mod:`pathlib.Path` methods that return a boolean result like +:meth:`~pathlib.Path.exists()`, :meth:`~pathlib.Path.is_dir()`, +:meth:`~pathlib.Path.is_file()`, :meth:`~pathlib.Path.is_mount()`, +:meth:`~pathlib.Path.is_symlink()`, :meth:`~pathlib.Path.is_block_device()`, +:meth:`~pathlib.Path.is_char_device()`, :meth:`~pathlib.Path.is_fifo()`, +:meth:`~pathlib.Path.is_socket()` now return ``False`` instead of raising :exc:`ValueError` or its subclasses :exc:`UnicodeEncodeError` and :exc:`UnicodeDecodeError` for paths that contain characters or bytes unrepresentable at the OS level.