gh-122400: Handle ValueError in filecmp (GH-122401)

Author: picnixz

Lib/filecmp.py

@@ -164,12 +164,14 @@ def phase2(self): # Distinguish files, directories, funnies
             ok = True
             try:
                 a_stat = os.stat(a_path)
-            except OSError:
+            except (OSError, ValueError):
+                # See https://github.com/python/cpython/issues/122400
+                # for the rationale for protecting against ValueError.
                 # print('Can\'t stat', a_path, ':', why.args[1])
                 ok = False
             try:
                 b_stat = os.stat(b_path)
-            except OSError:
+            except (OSError, ValueError):
                 # print('Can\'t stat', b_path, ':', why.args[1])
                 ok = False
 
@@ -285,12 +287,12 @@ def cmpfiles(a, b, common, shallow=True):
 # Return:
 #       0 for equal
 #       1 for different
-#       2 for funny cases (can't stat, etc.)
+#       2 for funny cases (can't stat, NUL bytes, etc.)
 #
 def _cmp(a, b, sh, abs=abs, cmp=cmp):
     try:
         return not abs(cmp(a, b, sh))
-    except OSError:
+    except (OSError, ValueError):
         return 2
 
 

Lib/test/test_filecmp.py

@@ -156,6 +156,39 @@ def test_cmpfiles(self):
                     (['file'], ['file2'], []),
                     "Comparing mismatched directories fails")
 
+    def test_cmpfiles_invalid_names(self):
+        # See https://github.com/python/cpython/issues/122400.
+        for file, desc in [
+            ('\x00', 'NUL bytes filename'),
+            (__file__ + '\x00', 'filename with embedded NUL bytes'),
+            ("\uD834\uDD1E.py", 'surrogate codes (MUSICAL SYMBOL G CLEF)'),
+            ('a' * 1_000_000, 'very long filename'),
+        ]:
+            for other_dir in [self.dir, self.dir_same, self.dir_diff]:
+                with self.subTest(f'cmpfiles: {desc}', other_dir=other_dir):
+                    res = filecmp.cmpfiles(self.dir, other_dir, [file])
+                    self.assertTupleEqual(res, ([], [], [file]))
+
+    def test_dircmp_invalid_names(self):
+        for bad_dir, desc in [
+            ('\x00', 'NUL bytes dirname'),
+            (f'Top{os.sep}Mid\x00', 'dirname with embedded NUL bytes'),
+            ("\uD834\uDD1E", 'surrogate codes (MUSICAL SYMBOL G CLEF)'),
+            ('a' * 1_000_000, 'very long dirname'),
+        ]:
+            d1 = filecmp.dircmp(self.dir, bad_dir)
+            d2 = filecmp.dircmp(bad_dir, self.dir)
+            for target in [
+                # attributes where os.listdir() raises OSError or ValueError
+                'left_list', 'right_list',
+                'left_only', 'right_only', 'common',
+            ]:
+                with self.subTest(f'dircmp(ok, bad): {desc}', target=target):
+                    with self.assertRaises((OSError, ValueError)):
+                        getattr(d1, target)
+                with self.subTest(f'dircmp(bad, ok): {desc}', target=target):
+                    with self.assertRaises((OSError, ValueError)):
+                        getattr(d2, target)
 
     def _assert_lists(self, actual, expected):
         """Assert that two lists are equal, up to ordering."""

Misc/NEWS.d/next/Library/2024-07-29-16-47-08.gh-issue-122400.fM0YSv.rst

@@ -0,0 +1,3 @@
+Handle :exc:`ValueError`\s raised by :func:`os.stat` in
+:class:`filecmp.dircmp` and :func:`filecmp.cmpfiles`.
+Patch by Bénédikt Tran.