Merge pull request #857 from guygurari/cp-mv-bug

address issue #854 : don't allow mv/cp of multiple files to single file

Author: fviard

run-tests-minio.py

@@ -418,6 +418,32 @@ def pbucket(tail):
 ## ====== Clean up local destination dir
 test_flushdir("Clean testsuite-out/", "testsuite-out")
 
+## ====== Moving things without trailing '/'
+os.system('dd if=/dev/urandom of=testsuite-out/urandom1.bin bs=1k count=1 > /dev/null 2>&1')
+os.system('dd if=/dev/urandom of=testsuite-out/urandom2.bin bs=1k count=1 > /dev/null 2>&1')
+test_s3cmd("Put multiple files", ['put', 'testsuite-out/urandom1.bin', 'testsuite-out/urandom2.bin', '%s/' % pbucket(1)],
+           must_find = ["%s/urandom1.bin" % pbucket(1), "%s/urandom2.bin" % pbucket(1)])
+
+test_s3cmd("Move without '/'", ['mv', '%s/urandom1.bin' % pbucket(1), '%s/urandom2.bin' % pbucket(1), '%s/dir' % pbucket(1)],
+           retcode = 64,
+           must_find = ['Destination must be a directory'])
+
+test_s3cmd("Move recursive w/a '/'",
+           ['-r', 'mv', '%s/dir1' % pbucket(1), '%s/dir2' % pbucket(1)],
+           retcode = 64,
+           must_find = ['Destination must be a directory'])
+
+## ====== Moving multiple files into directory with trailing '/'
+must_find = ["'%s/urandom1.bin' -> '%s/dir/urandom1.bin'" % (pbucket(1),pbucket(1)), "'%s/urandom2.bin' -> '%s/dir/urandom2.bin'" % (pbucket(1),pbucket(1))]
+must_not_find = ["'%s/urandom1.bin' -> '%s/dir'" % (pbucket(1),pbucket(1)), "'%s/urandom2.bin' -> '%s/dir'" % (pbucket(1),pbucket(1))]
+test_s3cmd("Move multiple files",
+           ['mv', '%s/urandom1.bin' % pbucket(1), '%s/urandom2.bin' % pbucket(1), '%s/dir/' % pbucket(1)],
+           must_find = must_find,
+           must_not_find = must_not_find)
+
+## ====== Clean up local destination dir
+test_flushdir("Clean testsuite-out/", "testsuite-out")
+
 ## ====== Sync from S3
 must_find = [ "'%s/xyz/binary/random-crap.md5' -> 'testsuite-out/xyz/binary/random-crap.md5'" % pbucket(1) ]
 if have_encoding:

run-tests.py

@@ -266,7 +266,7 @@ def test_curl_HEAD(label, src_file, **kwargs):
     cmd.append(src_file)
     return test(label, cmd, **kwargs)
 
-bucket_prefix = u"%s-" % getpass.getuser()
+bucket_prefix = u"%s-" % getpass.getuser().lower()
 
 argv = sys.argv[1:]
 while argv:
@@ -415,14 +415,39 @@ def pbucket(tail):
 ## ====== Clean up local destination dir
 test_flushdir("Clean testsuite-out/", "testsuite-out")
 
+## ====== Moving things without trailing '/'
+os.system('dd if=/dev/urandom of=testsuite-out/urandom1.bin bs=1k count=1 > /dev/null 2>&1')
+os.system('dd if=/dev/urandom of=testsuite-out/urandom2.bin bs=1k count=1 > /dev/null 2>&1')
+test_s3cmd("Put multiple files", ['put', 'testsuite-out/urandom1.bin', 'testsuite-out/urandom2.bin', '%s/' % pbucket(1)],
+           must_find = ["%s/urandom1.bin" % pbucket(1), "%s/urandom2.bin" % pbucket(1)])
+
+test_s3cmd("Move without '/'", ['mv', '%s/urandom1.bin' % pbucket(1), '%s/urandom2.bin' % pbucket(1), '%s/dir' % pbucket(1)],
+           retcode = 64,
+           must_find = ['Destination must be a directory'])
+
+test_s3cmd("Move recursive w/a '/'",
+           ['-r', 'mv', '%s/dir1' % pbucket(1), '%s/dir2' % pbucket(1)],
+           retcode = 64,
+           must_find = ['Destination must be a directory'])
+
+## ====== Moving multiple files into directory with trailing '/'
+must_find = ["'%s/urandom1.bin' -> '%s/dir/urandom1.bin'" % (pbucket(1),pbucket(1)), "'%s/urandom2.bin' -> '%s/dir/urandom2.bin'" % (pbucket(1),pbucket(1))]
+must_not_find = ["'%s/urandom1.bin' -> '%s/dir'" % (pbucket(1),pbucket(1)), "'%s/urandom2.bin' -> '%s/dir'" % (pbucket(1),pbucket(1))]
+test_s3cmd("Move multiple files",
+           ['mv', '%s/urandom1.bin' % pbucket(1), '%s/urandom2.bin' % pbucket(1), '%s/dir/' % pbucket(1)],
+           must_find = must_find,
+           must_not_find = must_not_find)
+
+## ====== Clean up local destination dir
+test_flushdir("Clean testsuite-out/", "testsuite-out")
+
 ## ====== Sync from S3
 must_find = [ "'%s/xyz/binary/random-crap.md5' -> 'testsuite-out/xyz/binary/random-crap.md5'" % pbucket(1) ]
 if have_encoding:
     must_find.append(u"'%(pbucket)s/xyz/encodings/%(encoding)s/%(pattern)s' -> 'testsuite-out/xyz/encodings/%(encoding)s/%(pattern)s' " % { 'encoding' : encoding, 'pattern' : enc_pattern, 'pbucket' : pbucket(1) })
 test_s3cmd("Sync from S3", ['sync', '%s/xyz' % pbucket(1), 'testsuite-out'],
     must_find = must_find)
 
-
 ## ====== Remove 'demo' directory
 test_rmdir("Remove 'dir-test/'", "testsuite-out/xyz/dir-test/")
 

s3cmd

@@ -813,9 +813,13 @@ def subcmd_cp_mv(args, process_fce, action_str, message):
 
     info(u"Summary: %d remote files to %s" % (remote_count, action_str))
 
+    if not destination_base.endswith('/'):
+        # Trying to mv dir1/ to dir2 will not pass a test in S3.FileLists,
+        # so we don't need to test for it here.
+        if len(remote_list) > 1 or cfg.recursive:
+           raise ParameterError("Destination must be a directory and end with '/' when acting on multiple sources.")
+
     if cfg.recursive:
-        if not destination_base.endswith("/"):
-            destination_base += "/"
         for key in remote_list:
             remote_list[key]['dest_name'] = destination_base + key
     else:
@@ -1144,8 +1148,6 @@ def cmd_sync_remote2local(args):
     if not destination_base.endswith(os.path.sep):
         if fetch_source_args[0].endswith(u'/') or len(fetch_source_args) > 1:
             raise ParameterError("Destination must be a directory and end with '/' when downloading multiple sources.")
-        elif fetch_source_args[0].endswith(u'/'):
-            fetch_source_args[0] += u'/'
 
     stats_info = StatsInfo()