tahoe cp: ignore trailing slash on source arguments

This avoids an error case where an empty child name resulted in a
duplicate mkdir. It adds a precondition check to guard against empty
child names, and some test cases. It also cleans up a funny redundancy
noticed earlier (refs ticket:2329).

diff --git a/src/allmydata/scripts/tahoe_cp.py b/src/allmydata/scripts/tahoe_cp.py
index 46244e87ed2d5d5df038cecc9c8090e4b0096697..a5cfe16a1a25b7b65e9ca9d9856256457b7da9a4 100644 (file)
--- a/src/allmydata/scripts/tahoe_cp.py
+++ b/src/allmydata/scripts/tahoe_cp.py
@@ -151,6 +151,7 @@ class LocalDirectoryTarget:
 
     def get_child_target(self, name):
         precondition(isinstance(name, unicode), name)
+        precondition(len(name), name) # don't want ""
         if self.children is None:
             self.populate(recurse=False)
         if name in self.children:
@@ -637,6 +638,8 @@ class Copier:
             url = self.nodeurl + "uri/%s" % urllib.quote(rootcap)
             name = None
             if path:
+                if path.endswith("/"):
+                    path = path[:-1]
                 url += "/" + escape_path(path)
                 last_slash = path.rfind(u"/")
                 name = path
@@ -659,13 +662,6 @@ class Copier:
                 writecap = to_str(d.get("rw_uri"))
                 readcap = to_str(d.get("ro_uri"))
                 mutable = d.get("mutable", False) # older nodes don't provide it
-
-                last_slash = source_spec.rfind(u"/")
-                if last_slash != -1:
-                    # TODO: this looks funny and redundant with the 'name'
-                    # assignment above. cf #2329
-                    name = source_spec[last_slash+1:]
-
                 t = TahoeFileSource(self.nodeurl, mutable, writecap, readcap, name)
         return t
 

diff --git a/src/allmydata/test/test_cli_cp.py b/src/allmydata/test/test_cli_cp.py
index 80be26819e96eddcfaedb0aaf2f369ecbdebc8ea..24087a4f49f00325b194dbb77746e02ef92430da 100644 (file)
--- a/src/allmydata/test/test_cli_cp.py
+++ b/src/allmydata/test/test_cli_cp.py
@@ -658,7 +658,9 @@ starting copy, 2 files, 1 directories
 
 # these test cases come from ticket #2329 comment 40
 # trailing slash on target *directory* should not matter, test both
-# trailing slash on files should cause error
+# trailing slash on target files should cause error
+# trailing slash on source directory should not matter, test a few
+# ignore trailing slash on source file, that's easiest
 
 COPYOUT_TESTCASES = """
 cp    $FILECAP          to/existing-file : to/existing-file
@@ -680,16 +682,22 @@ cp    $FILECAP $DIRCAP  to/existing-file/ : E4-NEED-R
 cp -r $FILECAP $DIRCAP  to/existing-file/ : E7-BADSLASH
 
 # single source to a (present) target directory
-cp    $FILECAP       to : E2-DESTNAME
-cp -r $FILECAP       to : E2-DESTNAME
-cp    $DIRCAP/file   to : to/file
-cp -r $DIRCAP/file   to : to/file
-cp    $PARENTCAP/dir to : E4-NEED-R
-cp -r $PARENTCAP/dir to : to/dir/file
-cp    $DIRCAP        to : E4-NEED-R
-cp -r $DIRCAP        to : to/file
-cp    $DIRALIAS      to : E4-NEED-R
-cp -r $DIRALIAS      to : to/file
+cp    $FILECAP        to : E2-DESTNAME
+cp -r $FILECAP        to : E2-DESTNAME
+cp    $DIRCAP/file    to : to/file
+cp -r $DIRCAP/file    to : to/file
+# these two should behave like the two above: ignore trailing slash
+cp    $DIRCAP/file/   to : to/file
+cp -r $DIRCAP/file/   to : to/file
+cp    $PARENTCAP/dir  to : E4-NEED-R
+cp -r $PARENTCAP/dir  to : to/dir/file
+# these two should ignore the trailing source slash too
+cp    $PARENTCAP/dir/ to : E4-NEED-R
+cp -r $PARENTCAP/dir/ to : to/dir/file
+cp    $DIRCAP         to : E4-NEED-R
+cp -r $DIRCAP         to : to/file
+cp    $DIRALIAS       to : E4-NEED-R
+cp -r $DIRALIAS       to : to/file
 
 cp    $FILECAP       to/ : E2-DESTNAME
 cp -r $FILECAP       to/ : E2-DESTNAME