bpo-33721: Make some os.path functions and pathlib.Path methods be tolerant to invalid paths. (#7695)

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
unrepresentative at the OS level.

4 files changed, 75 insertions, 16 deletions

diff --git a/Lib/test/test_genericpath.py b/Lib/test/test_genericpath.py
index 8b291cd..08e1c12 100644
--- a/Lib/test/test_genericpath.py
+++ b/Lib/test/test_genericpath.py
@@ -138,10 +138,20 @@ class GenericTest:
         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 @@ class GenericTest:
         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 @@ class GenericTest:
         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)
@@ -298,18 +318,20 @@ class TestGenericTest(GenericTest, unittest.TestCase):
                 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_pathlib.py b/Lib/test/test_pathlib.py
index e436db9..876eecc 100644
--- a/Lib/test/test_pathlib.py
+++ b/Lib/test/test_pathlib.py
@@ -1342,6 +1342,8 @@ class _BasePathTest(object):
             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)
@@ -1866,7 +1868,9 @@ class _BasePathTest(object):
         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)
@@ -1878,6 +1882,8 @@ class _BasePathTest(object):
             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):
@@ -1890,6 +1896,8 @@ class _BasePathTest(object):
         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)
@@ -1901,6 +1909,11 @@ class _BasePathTest(object):
             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)
@@ -1908,6 +1921,8 @@ class _BasePathTest(object):
         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):
@@ -1919,6 +1934,8 @@ class _BasePathTest(object):
         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)
@@ -1926,6 +1943,8 @@ class _BasePathTest(object):
         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):
@@ -1941,6 +1960,8 @@ class _BasePathTest(object):
         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)
@@ -1948,6 +1969,8 @@ class _BasePathTest(object):
         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)
@@ -1955,6 +1978,8 @@ class _BasePathTest(object):
         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
@@ -1964,6 +1989,8 @@ class _BasePathTest(object):
         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/Lib/test/test_posixpath.py b/Lib/test/test_posixpath.py
index 9476ede..ae59ef5 100644
--- a/Lib/test/test_posixpath.py
+++ b/Lib/test/test_posixpath.py
@@ -153,9 +153,11 @@ class PosixPathTest(unittest.TestCase):
     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 @@ class PosixPathTest(unittest.TestCase):
             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 @@ class PosixPathTest(unittest.TestCase):
         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/Lib/test/test_site.py b/Lib/test/test_site.py
index e3c9dee..742be1e 100644
--- a/Lib/test/test_site.py
+++ b/Lib/test/test_site.py
@@ -159,13 +159,11 @@ class HelperFunctionsTests(unittest.TestCase):
         # 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