Skip to content

Commit

Permalink
Add validation to ensure we don't mv a file onto itself
Browse files Browse the repository at this point in the history
Fixes aws#831
  • Loading branch information
jamesls committed Jun 30, 2014
1 parent d421dc2 commit b9857cd
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 0 deletions.
17 changes: 17 additions & 0 deletions awscli/customizations/s3/s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
21 changes: 21 additions & 0 deletions tests/integration/customizations/s3/test_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down
46 changes: 46 additions & 0 deletions tests/unit/customizations/s3/test_mv_command.py
Original file line number Diff line number Diff line change
@@ -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()

0 comments on commit b9857cd

Please sign in to comment.