pathlib ABCs: check filename validity when copying

Use `target_path.joinpath('_').with_name(source_name)` to generate child
paths in a target directory when copying. This formula ensures `ValueError`
is raised if the filename is invalid for the target path type (e.g. empty
or contains a path separator.)

No change of public behaviour because `pathlib.Path` is the only exposed
implementation of `ReadablePath`, and validity issues arise only when the
source and target path parsers differ.

Author: barneygale

Lib/pathlib/__init__.py

@@ -1101,13 +1101,9 @@ def copy_into(self, target_dir, **kwargs):
         """
         Copy this file or directory tree into the given existing directory.
         """
-        name = self.name
-        if not name:
-            raise ValueError(f"{self!r} has an empty name")
-        elif hasattr(target_dir, 'with_segments'):
-            target = target_dir / name
-        else:
-            target = self.with_segments(target_dir, name)
+        if not hasattr(target_dir, 'with_segments'):
+            target_dir = self.with_segments(target_dir)
+        target = target_dir.joinpath('_').with_name(self.name)
         return self.copy(target, **kwargs)
 
     def _copy_from(self, source, follow_symlinks=True, preserve_metadata=False):
@@ -1120,7 +1116,7 @@ def _copy_from(self, source, follow_symlinks=True, preserve_metadata=False):
             children = source.iterdir()
             os.mkdir(self)
             for child in children:
-                self.joinpath(child.name)._copy_from(
+                self.joinpath('_').with_name(child.name)._copy_from(
                     child, follow_symlinks, preserve_metadata)
             if preserve_metadata:
                 copy_info(source.info, self)

Lib/pathlib/types.py

@@ -152,7 +152,7 @@ def stem(self):
     def with_name(self, name):
         """Return a new path with the file name changed."""
         split = self.parser.split
-        if split(name)[0]:
+        if not name or split(name)[0]:
             raise ValueError(f"Invalid name {name!r}")
         path = vfspath(self)
         path = path.removesuffix(split(path)[1]) + name
@@ -363,10 +363,8 @@ def copy_into(self, target_dir, **kwargs):
         """
         Copy this file or directory tree into the given existing directory.
         """
-        name = self.name
-        if not name:
-            raise ValueError(f"{self!r} has an empty name")
-        return self.copy(target_dir / name, **kwargs)
+        target = target_dir.joinpath('_').with_name(self.name)
+        return self.copy(target, **kwargs)
 
 
 class _WritablePath(_JoinablePath):
@@ -436,7 +434,7 @@ def _copy_from(self, source, follow_symlinks=True):
                 children = src.iterdir()
                 dst.mkdir()
                 for child in children:
-                    stack.append((child, dst.joinpath(child.name)))
+                    stack.append((child, dst.joinpath('_').with_name(child.name)))
             else:
                 ensure_different_files(src, dst)
                 with vfsopen(src, 'rb') as source_f:

Lib/test/test_pathlib/support/local_path.py

@@ -26,6 +26,7 @@
 
 class LocalPathGround:
     can_symlink = can_symlink
+    seps = os.path.sep + (os.path.altsep or '')
 
     def __init__(self, path_cls):
         self.path_cls = path_cls

Lib/test/test_pathlib/support/zip_path.py

@@ -24,6 +24,7 @@
 
 class ZipPathGround:
     can_symlink = True
+    seps = '/'
 
     def __init__(self, path_cls):
         self.path_cls = path_cls

Lib/test/test_pathlib/test_copy.py

@@ -130,6 +130,16 @@ def test_copy_dir_into_itself(self):
         self.assertRaises(OSError, source.copy, target)
         self.assertRaises(OSError, source.copy, target, follow_symlinks=False)
 
+    def test_copy_dir_backslash_name(self):
+        if '\\' in self.source_ground.seps:
+            self.skipTest('backslash is a source path separator')
+        elif '\\' not in self.target_ground.seps:
+            self.skipTest('backslash is not a target path separator')
+        source = self.source_root / 'dirC'
+        target = self.target_root / 'copyC'
+        self.source_ground.create_file(source / 'back\\slash', b'')
+        self.assertRaises(ValueError, source.copy, target)
+
     def test_copy_into(self):
         source = self.source_root / 'fileA'
         target_dir = self.target_root / 'dirA'
@@ -146,6 +156,15 @@ def test_copy_into_empty_name(self):
         self.target_ground.create_dir(target_dir)
         self.assertRaises(ValueError, source.copy_into, target_dir)
 
+    def test_copy_into_backslash_name(self):
+        if '\\' in self.source_ground.seps:
+            self.skipTest('backslash is a source path separator')
+        elif '\\' not in self.target_ground.seps:
+            self.skipTest('backslash is not a target path separator')
+        source = self.source_root / 'back\\slash'
+        target_dir = self.target_root / 'dirA'
+        self.assertRaises(ValueError, source.copy_into, target_dir)
+
 
 class ZipToZipPathCopyTest(CopyTestBase, unittest.TestCase):
     source_ground = ZipPathGround(ReadableZipPath)

Lib/test/test_pathlib/test_join.py

@@ -308,6 +308,7 @@ def test_with_name(self):
         self.assertEqual(P('/a/b.py').with_name('d.xml'), P('/a/d.xml'))
         self.assertEqual(P('a/Dot ending.').with_name('d.xml'), P('a/d.xml'))
         self.assertEqual(P('/a/Dot ending.').with_name('d.xml'), P('/a/d.xml'))
+        self.assertRaises(ValueError, P('a/b').with_name, '')
         self.assertRaises(ValueError, P('a/b').with_name, '/c')
         self.assertRaises(ValueError, P('a/b').with_name, 'c/')
         self.assertRaises(ValueError, P('a/b').with_name, 'c/d')