Merge pull request #909 from kyleknap/sync-encoding

Fixed s3 to s3 sync url decoding pagniation bug.

CHANGELOG.rst

@@ -23,6 +23,10 @@ Next Release (TBD)
 * bugfix:Endpoint URL: Provide a better error message when
   an invalid ``--endpoint-url`` is provided
   (`issue 899 <https://github.com/aws/aws-cli/issues/899>`__)
+* bugfix:``aws s3``: Fix issue when keys do not get properly
+  url decoded when syncing from a bucket that requires pagination
+  to a bucket that requires less pagination
+  (`issue 909 <https://github.com/aws/aws-cli/pull/909>`__)
 
 
 1.4.2

awscli/customizations/s3/utils.py

@@ -350,7 +350,8 @@ def list_objects(self, bucket, prefix=None, page_size=None):
         with ScopedEventHandler(self._operation.session,
                                 'after-call.s3.ListObjects',
                                 self._decode_keys,
-                                'BucketListerDecodeKeys'):
+                                'BucketListerDecodeKeys',
+                                True):
             pages = self._operation.paginate(self._endpoint, **kwargs)
             for response, page in pages:
                 contents = page['Contents']
@@ -368,18 +369,22 @@ def _decode_keys(self, parsed, **kwargs):
 class ScopedEventHandler(object):
     """Register an event callback for the duration of a scope."""
 
-    def __init__(self, session, event_name, handler, unique_id=None):
+    def __init__(self, session, event_name, handler, unique_id=None,
+                 unique_id_uses_count=False):
         self._session = session
         self._event_name = event_name
         self._handler = handler
         self._unique_id = unique_id
+        self._unique_id_uses_count = unique_id_uses_count
 
     def __enter__(self):
-        self._session.register(self._event_name, self._handler, self._unique_id)
+        self._session.register(self._event_name, self._handler, self._unique_id,
+                               self._unique_id_uses_count)
 
     def __exit__(self, exc_type, exc_value, traceback):
         self._session.unregister(self._event_name, self._handler,
-                                 self._unique_id)
+                                 self._unique_id,
+                                 self._unique_id_uses_count)
 
 
 class PrintTask(namedtuple('PrintTask',

tests/integration/customizations/s3/test_plugin.py

@@ -490,20 +490,29 @@ def test_sync_with_plus_chars_paginate(self):
         self.assertNotIn('upload:', p2.stdout)
         self.assertEqual('', p2.stdout)
 
-    def test_s3_to_s3_sync_with_plus_char(self):
-        self.files.create_file('foo+.txt', contents="foo")
+    def test_s3_to_s3_sync_with_plus_char_paginate(self):
+        keynames = []
+        for i in range(4):
+            keyname = 'foo+%d' % i
+            keynames.append(keyname)
+            self.files.create_file(keyname, contents='')
+
         bucket_name = self.create_bucket()
         bucket_name_2 = self.create_bucket()
 
         p = aws('s3 sync %s s3://%s' % (self.files.rootdir, bucket_name))
         self.assert_no_errors(p)
-        self.assertTrue(self.key_exists(bucket_name, 'foo+.txt'))
+        for key in keynames:
+            self.assertTrue(self.key_exists(bucket_name, key))
 
-        p = aws('s3 sync s3://%s/ s3://%s/' % (bucket_name, bucket_name_2))
+        p = aws('s3 sync s3://%s/ s3://%s/ --page-size 2' % 
+                (bucket_name, bucket_name_2))
         self.assert_no_errors(p)
-        self.assertTrue(self.key_exists(bucket_name_2, 'foo+.txt'))
+        for key in keynames:
+            self.assertTrue(self.key_exists(bucket_name_2, key))
 
-        p2 = aws('s3 sync s3://%s/ s3://%s/' % (bucket_name, bucket_name_2))
+        p2 = aws('s3 sync s3://%s/ s3://%s/ --page-size 2' %
+                 (bucket_name, bucket_name_2))
         self.assertNotIn('copy:', p2.stdout)
         self.assertEqual('', p2.stdout)
 

tests/unit/customizations/s3/fake_session.py

@@ -70,10 +70,12 @@ def emit(self, *args, **kwargs):
     def emit_first_non_none_response(self, *args, **kwargs):
         pass
 
-    def register(self, name, handler, unique_id=None):
+    def register(self, name, handler, unique_id=None,
+                 unique_id_uses_call=False):
         pass
 
-    def unregister(self, name, handler, unique_id=None):
+    def unregister(self, name, handler, unique_id=None,
+                   unique_id_uses_call=False):
         pass
 
 

tests/unit/customizations/s3/test_utils.py

@@ -297,15 +297,19 @@ def test_scoped_session_handler(self):
         session = mock.Mock()
         scoped = ScopedEventHandler(session, 'eventname', 'handler')
         with scoped:
-            session.register.assert_called_with('eventname', 'handler', None)
-        session.unregister.assert_called_with('eventname', 'handler', None)
+            session.register.assert_called_with('eventname', 'handler', None,
+                                                False)
+        session.unregister.assert_called_with('eventname', 'handler', None,
+                                              False)
 
     def test_scoped_session_unique(self):
         session = mock.Mock()
         scoped = ScopedEventHandler(session, 'eventname', 'handler', 'unique')
         with scoped:
-            session.register.assert_called_with('eventname', 'handler', 'unique')
-        session.unregister.assert_called_with('eventname', 'handler', 'unique')
+            session.register.assert_called_with('eventname', 'handler',
+                                                'unique', False)
+        session.unregister.assert_called_with('eventname', 'handler', 'unique',
+                                              False)
 
 
 class TestGetFileStat(unittest.TestCase):