Add validation to ensure we don't mv a file onto itself

jamesls · jamesls · commit b9857cd98ece · 2014-06-30T16:40:00.000-07:00
Fixes aws#831
diff --git a/awscli/customizations/s3/s3.py b/awscli/customizations/s3/s3.py
@@ -662,6 +662,23 @@ def add_paths(self, paths):
             self.parameters['dest'] = paths[1]
         elif len(paths) == 1:
             self.parameters['dest'] = paths[0]
+        self._validate_path_args()
+
+    def _validate_path_args(self):
+        # If we're using a mv command, you can't copy the object onto itself.
+        params = self.parameters
+        if self.cmd == 'mv' and self._same_path(params['src'], params['dest']):
+            raise ValueError("Cannot mv a file onto itself: '%s' - '%s'" % (
+                params['src'], params['dest']))
+
+    def _same_path(self, src, dest):
+        if not self.parameters['paths_type'] == 's3s3':
+            return False
+        elif src == dest:
+            return True
+        elif dest.endswith('/'):
+            src_base = os.path.basename(src)
+            return src == os.path.join(dest, src_base)
 
     def _normalize_s3_trailing_slash(self, paths):
         for i, path in enumerate(paths):
diff --git a/tests/integration/customizations/s3/test_plugin.py b/tests/integration/customizations/s3/test_plugin.py
@@ -247,6 +247,27 @@ def test_mv_to_nonexistent_bucket(self):
         p = aws('s3 mv %s s3://bad-noexist-13143242/foo.txt' % (full_path,))
         self.assertEqual(p.rc, 1)
 
+    def test_cant_move_file_onto_itself_small_file(self):
+        # We don't even need a remote file in this case.  We can
+        # immediately validate that we can't move a file onto itself.
+        bucket_name = self.create_bucket()
+        self.put_object(bucket_name, key_name='key.txt', contents='foo')
+        p = aws('s3 mv s3://%s/key.txt s3://%s/key.txt' % (bucket_name, bucket_name))
+        self.assertEqual(p.rc, 255)
+        self.assertIn('Cannot mv a file onto itself', p.stderr)
+
+    def test_cant_move_large_file_onto_itself(self):
+        # At the API level, you can multipart copy an object onto itself,
+        # but a mv command doesn't make sense because a mv is just a
+        # cp + an rm of the src file.  We should be consistent and
+        # not allow large files to be mv'd onto themselves.
+        file_contents = six.BytesIO(b'a' * (1024 * 1024 * 10))
+        bucket_name = self.create_bucket()
+        self.put_object(bucket_name, key_name='key.txt', contents=file_contents)
+        p = aws('s3 mv s3://%s/key.txt s3://%s/key.txt' % (bucket_name, bucket_name))
+        self.assertEqual(p.rc, 255)
+        self.assertIn('Cannot mv a file onto itself', p.stderr)
+
 
 class TestRm(BaseS3CLICommand):
     @unittest.skipIf(platform.system() not in ['Darwin', 'Linux'],
diff --git a/tests/unit/customizations/s3/test_mv_command.py b/tests/unit/customizations/s3/test_mv_command.py
@@ -0,0 +1,46 @@
+#!/usr/bin/env python
+# Copyright 2014 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"). You
+# may not use this file except in compliance with the License. A copy of
+# the License is located at
+#
+#     http://aws.amazon.com/apache2.0/
+#
+# or in the "license" file accompanying this file. This file is
+# distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF
+# ANY KIND, either express or implied. See the License for the specific
+# language governing permissions and limitations under the License.
+from awscli.testutils import BaseAWSCommandParamsTest, FileCreator
+import re
+
+import mock
+import six
+
+
+class TestMvCommand(BaseAWSCommandParamsTest):
+
+    prefix = 's3 mv '
+
+    def setUp(self):
+        super(TestMvCommand, self).setUp()
+        self.files = FileCreator()
+
+    def tearDown(self):
+        super(TestMvCommand, self).tearDown()
+        self.files.remove_all()
+
+    def test_cant_mv_object_onto_itself(self):
+        cmdline = '%s s3://bucket/key s3://bucket/key' % self.prefix
+        stderr = self.run_cmd(cmdline, expected_rc=255)[1]
+        self.assertIn('Cannot mv a file onto itself', stderr)
+
+    def test_cant_mv_object_with_implied_name(self):
+        # The "key" key name is implied in the dst argument.
+        cmdline = '%s s3://bucket/key s3://bucket/' % self.prefix
+        stderr = self.run_cmd(cmdline, expected_rc=255)[1]
+        self.assertIn('Cannot mv a file onto itself', stderr)
+
+
+if __name__ == "__main__":
+    unittest.main()
