From 426b2f5adfdc3888003361749479afe51c9c714f Mon Sep 17 00:00:00 2001
From: JordonPhillips <phjordon@amazon.com>
Date: Tue, 11 Oct 2016 10:41:58 -0700
Subject: [PATCH 1/6] Allow both dualstack and s3_accelerate

---
 botocore/client.py          | 12 ++++++---
 botocore/utils.py           | 13 ++++++++-
 tests/functional/test_s3.py | 54 +++++++++++++++++++++++++++++++++----
 tests/unit/test_utils.py    | 31 ++++++++++++++++++++-
 4 files changed, 100 insertions(+), 10 deletions(-)

diff --git a/botocore/client.py b/botocore/client.py
index 294ebcecc7..22001e0c79 100644
--- a/botocore/client.py
+++ b/botocore/client.py
@@ -29,9 +29,9 @@
 from botocore.utils import get_service_module_name
 from botocore.utils import switch_to_virtual_host_style
 from botocore.utils import switch_host_s3_accelerate
-from botocore.utils import S3_ACCELERATE_ENDPOINT
 from botocore.utils import S3RegionRedirector
 from botocore.args import ClientArgsCreator
+from botocore.compat import urlsplit
 # Keep this imported.  There's pre-existing code that uses
 # "from botocore.client import Config".
 from botocore.config import Config
@@ -421,7 +421,7 @@ def _register_s3_specific_handlers(self):
 
         # Enable accelerate if the configuration is set to to true or the
         # endpoint being used matches one of the Accelerate endpoints.
-        if s3_accelerate or S3_ACCELERATE_ENDPOINT in self._endpoint.host:
+        if s3_accelerate or self._is_accelerate_endpoint(self._endpoint.host):
             # Amazon S3 accelerate is being used then always use the virtual
             # style of addressing because it is required.
             self._force_virtual_style_s3_addressing()
@@ -435,12 +435,18 @@ def _register_s3_specific_handlers(self):
                 self._force_path_style_s3_addressing()
             elif s3_addressing_style == 'virtual':
                 self._force_virtual_style_s3_addressing()
-        elif s3_dualstack:
+        # This can't be an elif because we need it to work along with
+        # accelerate.
+        if s3_dualstack and not s3_addressing_style:
             self.meta.events.unregister('before-sign.s3', fix_s3_host)
             self.meta.events.register(
                 'before-sign.s3',
                 functools.partial(fix_s3_host, default_endpoint_url=None))
 
+    def _is_accelerate_endpoint(self, url):
+        parts = urlsplit(url).netloc.split('.')
+        return any(part == 's3-accelerate' for part in parts)
+
     def _force_path_style_s3_addressing(self):
         # Do not try to modify the host if path is specified. The
         # ``fix_s3_host`` usually switches the addresing style to virtual.
diff --git a/botocore/utils.py b/botocore/utils.py
index 6218b8ada5..620506fde9 100644
--- a/botocore/utils.py
+++ b/botocore/utils.py
@@ -46,6 +46,7 @@
     'fips-us-gov-west-1',
 ]
 S3_ACCELERATE_ENDPOINT = 's3-accelerate.amazonaws.com'
+S3_ACCELERATE_DUALSTACK_ENDPOINT = 's3-accelerate.dualstack.amazonaws.com'
 RETRYABLE_HTTP_ERRORS = (requests.Timeout, requests.ConnectionError)
 
 
@@ -775,7 +776,17 @@ def switch_host_s3_accelerate(request, operation_name, **kwargs):
     # before it gets changed to virtual. So we are not concerned with ensuring
     # that the bucket name is translated to the virtual style here and we
     # can hard code the Accelerate endpoint.
-    endpoint = 'https://' + S3_ACCELERATE_ENDPOINT
+    s3_config = request.context['client_config'].s3
+    if s3_config is None:
+        s3_config = {}
+    dualstack = s3_config.get('use_dualstack_endpoint', False)
+    if dualstack:
+        endpoint = S3_ACCELERATE_DUALSTACK_ENDPOINT
+    else:
+        endpoint = S3_ACCELERATE_ENDPOINT
+
+    endpoint = 'https://' + endpoint
+
     if operation_name in ['ListBuckets', 'CreateBucket', 'DeleteBucket']:
         return
     _switch_hosts(request, endpoint,  use_new_scheme=False)
diff --git a/tests/functional/test_s3.py b/tests/functional/test_s3.py
index 78f7c4370b..1f58fad4c6 100644
--- a/tests/functional/test_s3.py
+++ b/tests/functional/test_s3.py
@@ -322,7 +322,6 @@ def test_generate_unauthed_post(self):
         self.assertEqual(parts, expected)
 
 
-
 def test_correct_url_used_for_s3():
     # Test that given various sets of config options and bucket names,
     # we construct the expect endpoint url.
@@ -401,7 +400,6 @@ def test_correct_url_used_for_s3():
         customer_provided_endpoint='https://foo.amazonaws.com/',
         expected_url='https://foo.amazonaws.com/bucket/key')
 
-
     # S3 accelerate
     use_accelerate = {'use_accelerate_endpoint': True}
     yield t.case(
@@ -445,19 +443,22 @@ def test_correct_url_used_for_s3():
         region='us-west-2', bucket='bucket', key='key',
         s3_config=use_dualstack,
         # Still default to virtual hosted when possible.
-        expected_url='https://bucket.s3.dualstack.us-west-2.amazonaws.com/key')
+        expected_url=(
+            'https://bucket.s3.dualstack.us-west-2.amazonaws.com/key'))
     # Non DNS compatible buckets use path style for dual stack.
     yield t.case(
         region='us-west-2', bucket='bucket.dot', key='key',
         s3_config=use_dualstack,
         # Still default to virtual hosted when possible.
-        expected_url='https://s3.dualstack.us-west-2.amazonaws.com/bucket.dot/key')
+        expected_url=(
+            'https://s3.dualstack.us-west-2.amazonaws.com/bucket.dot/key'))
     # Supports is_secure (use_ssl=False in create_client()).
     yield t.case(
         region='us-west-2', bucket='bucket.dot', key='key', is_secure=False,
         s3_config=use_dualstack,
         # Still default to virtual hosted when possible.
-        expected_url='http://s3.dualstack.us-west-2.amazonaws.com/bucket.dot/key')
+        expected_url=(
+            'http://s3.dualstack.us-west-2.amazonaws.com/bucket.dot/key'))
 
     # Is path style is requested, we should use it, even if the bucket is
     # DNS compatible.
@@ -471,6 +472,49 @@ def test_correct_url_used_for_s3():
         # Still default to virtual hosted when possible.
         expected_url='https://s3.dualstack.us-west-2.amazonaws.com/bucket/key')
 
+    # Accelerate + dual stack
+    use_accelerate_dualstack = {
+        'use_accelerate_endpoint': True,
+        'use_dualstack_endpoint': True,
+    }
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        s3_config=use_accelerate_dualstack,
+        expected_url=(
+            'https://bucket.s3-accelerate.dualstack.amazonaws.com/key'))
+    yield t.case(
+        # region is ignored with S3 accelerate.
+        region='us-west-2', bucket='bucket', key='key',
+        s3_config=use_accelerate_dualstack,
+        expected_url=(
+            'https://bucket.s3-accelerate.dualstack.amazonaws.com/key'))
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        s3_config=use_dualstack,
+        customer_provided_endpoint='https://s3-accelerate.amazonaws.com',
+        expected_url=(
+            'https://bucket.s3-accelerate.dualstack.amazonaws.com/key'))
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        s3_config=use_dualstack,
+        customer_provided_endpoint='http://s3-accelerate.amazonaws.com',
+        expected_url=(
+            'http://bucket.s3-accelerate.dualstack.amazonaws.com/key'))
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        s3_config=use_accelerate_dualstack, is_secure=False,
+        # Note we're using http://  because is_secure=False.
+        expected_url=(
+            'http://bucket.s3-accelerate.dualstack.amazonaws.com/key'))
+    # Use virtual even if path is specified for s3 accelerate because
+    # path style will not work with S3 accelerate.
+    use_accelerate_dualstack['addressing_style'] = 'path'
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        s3_config=use_accelerate_dualstack,
+        expected_url=(
+            'https://bucket.s3-accelerate.dualstack.amazonaws.com/key'))
+
 
 class S3AddressingCases(object):
     def __init__(self, verify_function):
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
index 5d5cd852c7..a7fa3a15ee 100644
--- a/tests/unit/test_utils.py
+++ b/tests/unit/test_utils.py
@@ -51,6 +51,7 @@
 from botocore.utils import ContainerMetadataFetcher
 from botocore.model import DenormalizedStructureBuilder
 from botocore.model import ShapeResolver
+from botocore.config import Config
 
 
 class TestURINormalization(unittest.TestCase):
@@ -973,6 +974,8 @@ def setUp(self):
             method='PUT', headers={},
             url=self.original_url
         )
+        self.client_config = Config()
+        self.request.context['client_config'] = self.client_config
 
     def test_switch_host(self):
         switch_host_s3_accelerate(self.request, 'PutObject')
@@ -989,7 +992,7 @@ def test_do_not_switch_black_listed_operations(self):
             'CreateBucket'
         ]
         for op_name in blacklist_ops:
-            switch_host_s3_accelerate(self.request, 'ListBuckets')
+            switch_host_s3_accelerate(self.request, op_name)
             self.assertEqual(self.request.url, self.original_url)
 
     def test_uses_original_endpoint_scheme(self):
@@ -999,6 +1002,32 @@ def test_uses_original_endpoint_scheme(self):
             self.request.url,
             'http://s3-accelerate.amazonaws.com/foo/key.txt')
 
+    def test_uses_dualstack(self):
+        self.client_config.s3 = {'use_dualstack_endpoint': True}
+        self.original_url = 'https://s3.dualstack.amazonaws.com/foo/key.txt'
+        self.request = AWSRequest(
+            method='PUT', headers={},
+            url=self.original_url
+        )
+        self.request.context['client_config'] = self.client_config
+        switch_host_s3_accelerate(self.request, 'PutObject')
+        self.assertEqual(
+            self.request.url,
+            'https://s3-accelerate.dualstack.amazonaws.com/foo/key.txt')
+
+    def test_does_not_use_dualstack_unless_explicitly_enabled(self):
+        self.client_config.s3 = {'use_dualstack_endpoint': False}
+        self.original_url = 'https://s3.dualstack.amazonaws.com/foo/key.txt'
+        self.request = AWSRequest(
+            method='PUT', headers={},
+            url=self.original_url
+        )
+        self.request.context['client_config'] = self.client_config
+        switch_host_s3_accelerate(self.request, 'PutObject')
+        self.assertEqual(
+            self.request.url,
+            'https://s3-accelerate.amazonaws.com/foo/key.txt')
+
 
 class TestS3RegionRedirector(unittest.TestCase):
     def setUp(self):

From d1a9e071cdb1cf942f0668ef6a46ac3648c9e1d0 Mon Sep 17 00:00:00 2001
From: JordonPhillips <phjordon@amazon.com>
Date: Wed, 12 Oct 2016 10:58:51 -0700
Subject: [PATCH 2/6] Simplify setting addressing style

---
 botocore/args.py          |   9 ---
 botocore/client.py        | 126 ++++++++++++++++++++------------------
 botocore/handlers.py      |   1 -
 tests/unit/test_client.py |  48 +++++++++------
 4 files changed, 97 insertions(+), 87 deletions(-)

diff --git a/botocore/args.py b/botocore/args.py
index 2b9366d8a3..da6383f6ae 100644
--- a/botocore/args.py
+++ b/botocore/args.py
@@ -20,7 +20,6 @@
 import logging
 
 import botocore.serialize
-from botocore.utils import fix_s3_host
 from botocore.signers import RequestSigner
 from botocore.config import Config
 from botocore.endpoint import EndpointCreator
@@ -59,10 +58,7 @@ def get_client_args(self, service_model, region_name, is_secure,
             endpoint_config['signature_version'],
             credentials, event_emitter)
 
-        # Add any additional s3 configuration for client
         config_kwargs['s3'] = s3_config
-        self._conditionally_unregister_fix_s3_host(endpoint_url, event_emitter)
-
         new_config = Config(**config_kwargs)
         endpoint_creator = EndpointCreator(event_emitter)
 
@@ -177,11 +173,6 @@ def compute_s3_config(self, scoped_config, client_config):
 
         return s3_configuration
 
-    def _conditionally_unregister_fix_s3_host(self, endpoint_url, emitter):
-        # If the user is providing a custom endpoint, we should not alter it.
-        if endpoint_url is not None:
-            emitter.unregister('before-sign.s3', fix_s3_host)
-
     def _convert_config_to_bool(self, config_dict, keys):
         # Make sure any further modifications to this section of the config
         # will not affect the scoped config by making a copy of it.
diff --git a/botocore/client.py b/botocore/client.py
index 22001e0c79..f85c42cf6d 100644
--- a/botocore/client.py
+++ b/botocore/client.py
@@ -25,11 +25,11 @@
 from botocore.model import ServiceModel
 from botocore.paginate import Paginator
 from botocore.utils import CachedProperty
-from botocore.utils import fix_s3_host
 from botocore.utils import get_service_module_name
-from botocore.utils import switch_to_virtual_host_style
 from botocore.utils import switch_host_s3_accelerate
 from botocore.utils import S3RegionRedirector
+from botocore.utils import fix_s3_host
+from botocore.utils import switch_to_virtual_host_style
 from botocore.args import ClientArgsCreator
 from botocore.compat import urlsplit
 # Keep this imported.  There's pre-existing code that uses
@@ -67,7 +67,7 @@ def create_client(self, service_name, region_name, is_secure=True,
             service_model, region_name, is_secure, endpoint_url,
             verify, credentials, scoped_config, client_config, endpoint_bridge)
         service_client = cls(**client_args)
-        self._create_s3_redirector(service_client, endpoint_bridge)
+        self._register_s3_events(service_client, endpoint_bridge, endpoint_url)
         return service_client
 
     def create_client_class(self, service_name, api_version=None):
@@ -114,10 +114,73 @@ def _register_retries(self, service_model):
         self._event_emitter.register('needs-retry.%s' % endpoint_prefix,
                                      handler, unique_id=unique_id)
 
-    def _create_s3_redirector(self, client, endpoint_bridge):
+    def _register_s3_events(self, client, endpoint_bridge, endpoint_url):
         if client.meta.service_model.service_name != 's3':
             return
         S3RegionRedirector(endpoint_bridge, client).register()
+        self._set_s3_addressing_style(
+            endpoint_url, client.meta.config.s3, client.meta.events)
+        # Enable accelerate if the configuration is set to to true or the
+        # endpoint being used matches one of the accelerate endpoints.
+        if self._is_s3_accelerate(endpoint_url, client.meta.config.s3):
+            # Also make sure that the hostname gets switched to
+            # s3-accelerate.amazonaws.com
+            client.meta.events.register_first(
+                'request-created.s3', switch_host_s3_accelerate)
+
+    def _set_s3_addressing_style(self, endpoint_url, s3_config, event_emitter):
+        if s3_config is None:
+            s3_config = {}
+
+        style = self._get_s3_addressing_style(endpoint_url, s3_config)
+        handler = self._get_s3_addressing_handler(
+            endpoint_url, s3_config, style)
+        if handler is not None:
+            event_emitter.register('before-sign.s3', handler)
+
+    def _get_s3_addressing_style(self, endpoint_url, s3_config):
+        # Use virtual host style addressing if accelerate is enabled or if
+        # the given endpoint url is an accelerate endpoint.
+        accelerate = s3_config.get('use_accelerate_endpoint', False)
+        if accelerate or self._is_s3_accelerate(endpoint_url, s3_config):
+            return 'virtual'
+
+        # If a particular style is configured, use it.
+        configured_style = s3_config.get('addressing_style')
+        if configured_style:
+            return configured_style
+
+    def _get_s3_addressing_handler(self, endpoint_url, s3_config, style):
+        # If virtual host style was configured, use it regardless of whether
+        # or not the bucket looks dns compatible.
+        if style == 'virtual':
+            return switch_to_virtual_host_style
+
+        # If path style is configured, no additional steps are needed. If
+        # endpoint_url was specified, don't default to virtual. We could
+        # potentially default provided endpoint urls to virtual hosted
+        # style, but for now it is avoided.
+        if style == 'path' or endpoint_url is not None:
+            return None
+
+        # For dual stack mode, we need to clear the default endpoint url in
+        # order to use the existing netloc if the bucket is dns compatible.
+        if s3_config.get('use_dualstack_endpoint', False):
+            return functools.partial(
+                fix_s3_host, default_endpoint_url=None)
+
+        # By default, try to use virtual style with path fallback.
+        return fix_s3_host
+
+    def _is_s3_accelerate(self, endpoint_url, s3_config):
+        if s3_config is not None and s3_config.get('use_accelerate_endpoint'):
+            return True
+
+        if endpoint_url is None:
+            return False
+
+        parts = urlsplit(endpoint_url).netloc.split('.')
+        return any(part == 's3-accelerate' for part in parts)
 
     def _get_client_args(self, service_model, region_name, is_secure,
                          endpoint_url, verify, credentials,
@@ -406,61 +469,6 @@ def _register_handlers(self):
                                   self.meta.service_model.endpoint_prefix,
                                   self._request_signer.handler)
 
-        self._register_s3_specific_handlers()
-
-    def _register_s3_specific_handlers(self):
-        # Register all of the s3 specific handlers
-        if self.meta.config.s3 is None:
-            s3_addressing_style = None
-            s3_accelerate = None
-            s3_dualstack = None
-        else:
-            s3_addressing_style = self.meta.config.s3.get('addressing_style')
-            s3_accelerate = self.meta.config.s3.get('use_accelerate_endpoint')
-            s3_dualstack = self.meta.config.s3.get('use_dualstack_endpoint')
-
-        # Enable accelerate if the configuration is set to to true or the
-        # endpoint being used matches one of the Accelerate endpoints.
-        if s3_accelerate or self._is_accelerate_endpoint(self._endpoint.host):
-            # Amazon S3 accelerate is being used then always use the virtual
-            # style of addressing because it is required.
-            self._force_virtual_style_s3_addressing()
-            # Also make sure that the hostname gets switched to
-            # s3-accelerate.amazonaws.com
-            self.meta.events.register_first(
-                'request-created.s3', switch_host_s3_accelerate)
-        elif s3_addressing_style:
-            # Otherwise go ahead with the style the user may have specified.
-            if s3_addressing_style == 'path':
-                self._force_path_style_s3_addressing()
-            elif s3_addressing_style == 'virtual':
-                self._force_virtual_style_s3_addressing()
-        # This can't be an elif because we need it to work along with
-        # accelerate.
-        if s3_dualstack and not s3_addressing_style:
-            self.meta.events.unregister('before-sign.s3', fix_s3_host)
-            self.meta.events.register(
-                'before-sign.s3',
-                functools.partial(fix_s3_host, default_endpoint_url=None))
-
-    def _is_accelerate_endpoint(self, url):
-        parts = urlsplit(url).netloc.split('.')
-        return any(part == 's3-accelerate' for part in parts)
-
-    def _force_path_style_s3_addressing(self):
-        # Do not try to modify the host if path is specified. The
-        # ``fix_s3_host`` usually switches the addresing style to virtual.
-        self.meta.events.unregister('before-sign.s3', fix_s3_host)
-
-    def _force_virtual_style_s3_addressing(self):
-        # If the virtual host addressing style is being forced,
-        # switch the default fix_s3_host handler for the more general
-        # switch_to_virtual_host_style handler that does not have opt out
-        # cases (other than throwing an error if the name is DNS incompatible)
-        self.meta.events.unregister('before-sign.s3', fix_s3_host)
-        self.meta.events.register(
-            'before-sign.s3', switch_to_virtual_host_style)
-
     @property
     def _service_model(self):
         return self.meta.service_model
diff --git a/botocore/handlers.py b/botocore/handlers.py
index 431b08af4c..9298c1947a 100644
--- a/botocore/handlers.py
+++ b/botocore/handlers.py
@@ -818,7 +818,6 @@ def _replace_content(self, section):
     ('choose-signer.cognito-idp.SetUserSettings', disable_signing),
     ('choose-signer.cognito-idp.GetJWKS', disable_signing),
     ('choose-signer.cognito-idp.DeleteUserAttributes', disable_signing),
-    ('before-sign.s3', utils.fix_s3_host),
     ('before-parameter-build.s3.HeadObject', sse_md5),
     ('before-parameter-build.s3.GetObject', sse_md5),
     ('before-parameter-build.s3.PutObject', sse_md5),
diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py
index 6f74adfeea..e97097ab8a 100644
--- a/tests/unit/test_client.py
+++ b/tests/unit/test_client.py
@@ -1162,32 +1162,32 @@ def test_client_s3_addressing_style_config_overrides_scoped_config(self):
     def test_client_s3_addressing_style_default_registers_correctly(self):
         event_emitter = mock.Mock()
         creator = self.create_client_creator(event_emitter=event_emitter)
-        client = creator.create_client('myservice', 'us-west-2')
-        self.assertNotIn(
+        client = creator.create_client('s3', 'us-west-2')
+        self.assertIn(
             mock.call('before-sign.s3', utils.fix_s3_host),
-            client.meta.events.unregister.call_args_list
+            client.meta.events.register.call_args_list
         )
 
     def test_client_s3_addressing_style_auto_registers_correctly(self):
         event_emitter = mock.Mock()
         creator = self.create_client_creator(event_emitter=event_emitter)
         client = creator.create_client(
-            'myservice', 'us-west-2',
+            's3', 'us-west-2',
             scoped_config={'s3': {'addressing_style': 'auto'}}
         )
-        self.assertNotIn(
+        self.assertIn(
             mock.call('before-sign.s3', utils.fix_s3_host),
-            client.meta.events.unregister.call_args_list
+            client.meta.events.register.call_args_list
         )
 
     def test_client_s3_addressing_style_virtual_registers_correctly(self):
         event_emitter = mock.Mock()
         creator = self.create_client_creator(event_emitter=event_emitter)
         client = creator.create_client(
-            'myservice', 'us-west-2',
+            's3', 'us-west-2',
             scoped_config={'s3': {'addressing_style': 'virtual'}}
         )
-        self.assertIn(
+        self.assertNotIn(
             mock.call('before-sign.s3', utils.fix_s3_host),
             client.meta.events.unregister.call_args_list
         )
@@ -1200,7 +1200,7 @@ def test_client_s3_addressing_style_path_registers_correctly(self):
         event_emitter = mock.Mock()
         creator = self.create_client_creator(event_emitter=event_emitter)
         client = creator.create_client(
-            'myservice', 'us-west-2',
+            's3', 'us-west-2',
             scoped_config={'s3': {'addressing_style': 'path'}}
         )
         self.assertNotIn(
@@ -1212,26 +1212,38 @@ def test_client_s3_addressing_style_path_registers_correctly(self):
             client.meta.events.register.call_args_list
         )
 
-    def test_custom_endpoint_unregisters_fix_s3_host(self):
+    def test_custom_endpoint_uses_path_style(self):
         event_emitter = mock.Mock()
         creator = self.create_client_creator(event_emitter=event_emitter)
 
-        # It shouldn't be unregistered if we don't provide a url
-        client = creator.create_client('myservice', 'us-west-2')
-        self.assertNotIn(
+        # fix_s3_host should be registered if we don't provide a url
+        client = creator.create_client('s3', 'us-west-2')
+        self.assertIn(
             mock.call('before-sign.s3', utils.fix_s3_host),
-            client.meta.events.unregister.call_args_list
+            client.meta.events.register.call_args_list
         )
 
-        # If we do provide a url, it must be unregistered
+        # If we do provide a url, fix_s3_host should not be registered
         event_emitter.reset_mock()
         client = creator.create_client(
-            'myservice', 'us-west-2',
+            's3', 'us-west-2',
             endpoint_url="foo.com"
         )
+        self.assertNotIn(
+            mock.call('before-sign.s3', mock.ANY),
+            client.meta.events.register.call_args_list
+        )
+
+    def test_custom_accelerate_url_forces_virtual_host(self):
+        event_emitter = mock.Mock()
+        creator = self.create_client_creator(event_emitter=event_emitter)
+        client = creator.create_client(
+            's3', 'us-west-2',
+            endpoint_url='https://s3-accelerate.amazonaws.com'
+        )
         self.assertIn(
-            mock.call('before-sign.s3', utils.fix_s3_host),
-            client.meta.events.unregister.call_args_list
+            mock.call('before-sign.s3', utils.switch_to_virtual_host_style),
+            client.meta.events.register.call_args_list
         )
 
     def test_client_payload_signing_from_scoped_config(self):

From dc842fe13c8069e58aa94c2c301c08163509cbe4 Mon Sep 17 00:00:00 2001
From: JordonPhillips <phjordon@amazon.com>
Date: Wed, 12 Oct 2016 14:33:49 -0700
Subject: [PATCH 3/6] switch to whitelisting

---
 botocore/utils.py           | 16 ++++------------
 tests/functional/test_s3.py |  9 ++-------
 tests/unit/test_utils.py    | 13 -------------
 3 files changed, 6 insertions(+), 32 deletions(-)

diff --git a/botocore/utils.py b/botocore/utils.py
index 620506fde9..8ed08bab53 100644
--- a/botocore/utils.py
+++ b/botocore/utils.py
@@ -45,8 +45,6 @@
     'us-gov-west-1',
     'fips-us-gov-west-1',
 ]
-S3_ACCELERATE_ENDPOINT = 's3-accelerate.amazonaws.com'
-S3_ACCELERATE_DUALSTACK_ENDPOINT = 's3-accelerate.dualstack.amazonaws.com'
 RETRYABLE_HTTP_ERRORS = (requests.Timeout, requests.ConnectionError)
 
 
@@ -776,16 +774,10 @@ def switch_host_s3_accelerate(request, operation_name, **kwargs):
     # before it gets changed to virtual. So we are not concerned with ensuring
     # that the bucket name is translated to the virtual style here and we
     # can hard code the Accelerate endpoint.
-    s3_config = request.context['client_config'].s3
-    if s3_config is None:
-        s3_config = {}
-    dualstack = s3_config.get('use_dualstack_endpoint', False)
-    if dualstack:
-        endpoint = S3_ACCELERATE_DUALSTACK_ENDPOINT
-    else:
-        endpoint = S3_ACCELERATE_ENDPOINT
-
-    endpoint = 'https://' + endpoint
+    whitelist = ['dualstack', 'amazonaws', 'com']
+    parts = urlsplit(request.url).netloc.split('.')
+    parts = [p for p in parts if p in whitelist]
+    endpoint = 'https://s3-accelerate.' + '.'.join(parts)
 
     if operation_name in ['ListBuckets', 'CreateBucket', 'DeleteBucket']:
         return
diff --git a/tests/functional/test_s3.py b/tests/functional/test_s3.py
index 1f58fad4c6..09e805aff6 100644
--- a/tests/functional/test_s3.py
+++ b/tests/functional/test_s3.py
@@ -488,18 +488,13 @@ def test_correct_url_used_for_s3():
         s3_config=use_accelerate_dualstack,
         expected_url=(
             'https://bucket.s3-accelerate.dualstack.amazonaws.com/key'))
+    # Only s3-accelerate overrides a customer endpoint
     yield t.case(
         region='us-east-1', bucket='bucket', key='key',
         s3_config=use_dualstack,
         customer_provided_endpoint='https://s3-accelerate.amazonaws.com',
         expected_url=(
-            'https://bucket.s3-accelerate.dualstack.amazonaws.com/key'))
-    yield t.case(
-        region='us-east-1', bucket='bucket', key='key',
-        s3_config=use_dualstack,
-        customer_provided_endpoint='http://s3-accelerate.amazonaws.com',
-        expected_url=(
-            'http://bucket.s3-accelerate.dualstack.amazonaws.com/key'))
+            'https://bucket.s3-accelerate.amazonaws.com/key'))
     yield t.case(
         region='us-east-1', bucket='bucket', key='key',
         s3_config=use_accelerate_dualstack, is_secure=False,
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
index a7fa3a15ee..2ef8588a0d 100644
--- a/tests/unit/test_utils.py
+++ b/tests/unit/test_utils.py
@@ -1015,19 +1015,6 @@ def test_uses_dualstack(self):
             self.request.url,
             'https://s3-accelerate.dualstack.amazonaws.com/foo/key.txt')
 
-    def test_does_not_use_dualstack_unless_explicitly_enabled(self):
-        self.client_config.s3 = {'use_dualstack_endpoint': False}
-        self.original_url = 'https://s3.dualstack.amazonaws.com/foo/key.txt'
-        self.request = AWSRequest(
-            method='PUT', headers={},
-            url=self.original_url
-        )
-        self.request.context['client_config'] = self.client_config
-        switch_host_s3_accelerate(self.request, 'PutObject')
-        self.assertEqual(
-            self.request.url,
-            'https://s3-accelerate.amazonaws.com/foo/key.txt')
-
 
 class TestS3RegionRedirector(unittest.TestCase):
     def setUp(self):

From 12980a18fc174e73e8028042b415f1d7f4874b05 Mon Sep 17 00:00:00 2001
From: JordonPhillips <phjordon@amazon.com>
Date: Thu, 13 Oct 2016 12:02:47 -0700
Subject: [PATCH 4/6] style -> addressing_style

---
 botocore/client.py | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/botocore/client.py b/botocore/client.py
index f85c42cf6d..86d6eda00c 100644
--- a/botocore/client.py
+++ b/botocore/client.py
@@ -132,9 +132,10 @@ def _set_s3_addressing_style(self, endpoint_url, s3_config, event_emitter):
         if s3_config is None:
             s3_config = {}
 
-        style = self._get_s3_addressing_style(endpoint_url, s3_config)
+        addressing_style = self._get_s3_addressing_style(
+            endpoint_url, s3_config)
         handler = self._get_s3_addressing_handler(
-            endpoint_url, s3_config, style)
+            endpoint_url, s3_config, addressing_style)
         if handler is not None:
             event_emitter.register('before-sign.s3', handler)
 
@@ -145,22 +146,23 @@ def _get_s3_addressing_style(self, endpoint_url, s3_config):
         if accelerate or self._is_s3_accelerate(endpoint_url, s3_config):
             return 'virtual'
 
-        # If a particular style is configured, use it.
-        configured_style = s3_config.get('addressing_style')
-        if configured_style:
-            return configured_style
+        # If a particular addressing style is configured, use it.
+        configured_addressing_style = s3_config.get('addressing_style')
+        if configured_addressing_style:
+            return configured_addressing_style
 
-    def _get_s3_addressing_handler(self, endpoint_url, s3_config, style):
+    def _get_s3_addressing_handler(self, endpoint_url, s3_config,
+                                   addressing_style):
         # If virtual host style was configured, use it regardless of whether
         # or not the bucket looks dns compatible.
-        if style == 'virtual':
+        if addressing_style == 'virtual':
             return switch_to_virtual_host_style
 
         # If path style is configured, no additional steps are needed. If
         # endpoint_url was specified, don't default to virtual. We could
         # potentially default provided endpoint urls to virtual hosted
         # style, but for now it is avoided.
-        if style == 'path' or endpoint_url is not None:
+        if addressing_style == 'path' or endpoint_url is not None:
             return None
 
         # For dual stack mode, we need to clear the default endpoint url in

From 8ab453fdfc1461e497ff84a1fd8c9e99db429d0d Mon Sep 17 00:00:00 2001
From: JordonPhillips <phjordon@amazon.com>
Date: Thu, 13 Oct 2016 13:13:14 -0700
Subject: [PATCH 5/6] More robust accelerate detection

---
 botocore/client.py          | 33 +++++++++++++++++++++++--
 tests/functional/test_s3.py | 49 +++++++++++++++++++++++++++++++++++--
 2 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/botocore/client.py b/botocore/client.py
index 86d6eda00c..62577a05f5 100644
--- a/botocore/client.py
+++ b/botocore/client.py
@@ -156,6 +156,7 @@ def _get_s3_addressing_handler(self, endpoint_url, s3_config,
         # If virtual host style was configured, use it regardless of whether
         # or not the bucket looks dns compatible.
         if addressing_style == 'virtual':
+            logger.debug("Using S3 virtual host style addressing.")
             return switch_to_virtual_host_style
 
         # If path style is configured, no additional steps are needed. If
@@ -163,8 +164,12 @@ def _get_s3_addressing_handler(self, endpoint_url, s3_config,
         # potentially default provided endpoint urls to virtual hosted
         # style, but for now it is avoided.
         if addressing_style == 'path' or endpoint_url is not None:
+            logger.debug("Using S3 path style addressing.")
             return None
 
+        logger.debug("Defaulting to S3 virtual host style addressing with "
+                     "path style addressing fallback.")
+
         # For dual stack mode, we need to clear the default endpoint url in
         # order to use the existing netloc if the bucket is dns compatible.
         if s3_config.get('use_dualstack_endpoint', False):
@@ -175,14 +180,38 @@ def _get_s3_addressing_handler(self, endpoint_url, s3_config,
         return fix_s3_host
 
     def _is_s3_accelerate(self, endpoint_url, s3_config):
+        # Accelerate has been explicitly configured
         if s3_config is not None and s3_config.get('use_accelerate_endpoint'):
             return True
 
+        # Accelerate mode is turned on automatically if an endpoint url is
+        # provided that matches the accelerate scheme.
         if endpoint_url is None:
             return False
 
-        parts = urlsplit(endpoint_url).netloc.split('.')
-        return any(part == 's3-accelerate' for part in parts)
+        # Accelerate is only valid for Amazon endpoints.
+        netloc = urlsplit(endpoint_url).netloc
+        if not netloc.endswith('amazonaws.com'):
+            return False
+
+        # The first part of the url should always be s3-accelerate
+        parts = netloc.split('.')
+        if parts[0] != 's3-accelerate':
+            return False
+
+        # There should not be more than two components between 's3-accelerate'
+        # and 'amazonaws.com'
+        feature_parts = parts[1:-2]
+        if len(feature_parts) > 2:
+            return False
+
+        # There should be no duplicates
+        if len(feature_parts) != len(set(feature_parts)):
+            return False
+
+        # Remaining parts must be in the whitelist.
+        whitelist = ['dualstack']
+        return all(p in whitelist for p in feature_parts)
 
     def _get_client_args(self, service_model, region_name, is_secure,
                          endpoint_url, verify, credentials,
diff --git a/tests/functional/test_s3.py b/tests/functional/test_s3.py
index 09e805aff6..59c0079a91 100644
--- a/tests/functional/test_s3.py
+++ b/tests/functional/test_s3.py
@@ -411,6 +411,7 @@ def test_correct_url_used_for_s3():
         region='us-west-2', bucket='bucket', key='key',
         s3_config=use_accelerate,
         expected_url='https://bucket.s3-accelerate.amazonaws.com/key')
+    # Provided endpoints still get recognized as accelerate endpoints.
     yield t.case(
         region='us-east-1', bucket='bucket', key='key',
         customer_provided_endpoint='https://s3-accelerate.amazonaws.com',
@@ -424,6 +425,21 @@ def test_correct_url_used_for_s3():
         s3_config=use_accelerate, is_secure=False,
         # Note we're using http://  because is_secure=False.
         expected_url='http://bucket.s3-accelerate.amazonaws.com/key')
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        # s3-accelerate must be the first part of the url.
+        customer_provided_endpoint='https://foo.s3-accelerate.amazonaws.com',
+        expected_url='https://foo.s3-accelerate.amazonaws.com/bucket/key')
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        # The endpoint must be an Amazon endpoint.
+        customer_provided_endpoint='https://s3-accelerate.notamazon.com',
+        expected_url='https://s3-accelerate.notamazon.com/bucket/key')
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        # Extra components must be whitelisted.
+        customer_provided_endpoint='https://s3-accelerate.foo.amazonaws.com',
+        expected_url='https://s3-accelerate.foo.amazonaws.com/bucket/key')
     # Use virtual even if path is specified for s3 accelerate because
     # path style will not work with S3 accelerate.
     yield t.case(
@@ -483,18 +499,47 @@ def test_correct_url_used_for_s3():
         expected_url=(
             'https://bucket.s3-accelerate.dualstack.amazonaws.com/key'))
     yield t.case(
-        # region is ignored with S3 accelerate.
+        # Region is ignored with S3 accelerate.
         region='us-west-2', bucket='bucket', key='key',
         s3_config=use_accelerate_dualstack,
         expected_url=(
             'https://bucket.s3-accelerate.dualstack.amazonaws.com/key'))
-    # Only s3-accelerate overrides a customer endpoint
+    # Only s3-accelerate overrides a customer endpoint.
     yield t.case(
         region='us-east-1', bucket='bucket', key='key',
         s3_config=use_dualstack,
         customer_provided_endpoint='https://s3-accelerate.amazonaws.com',
         expected_url=(
             'https://bucket.s3-accelerate.amazonaws.com/key'))
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        # Dualstack is whitelisted.
+        customer_provided_endpoint=(
+            'https://s3-accelerate.dualstack.amazonaws.com'),
+        expected_url=(
+            'https://bucket.s3-accelerate.dualstack.amazonaws.com/key'))
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        # Even whitelisted parts cannot be duplicated.
+        customer_provided_endpoint=(
+            'https://s3-accelerate.dualstack.dualstack.amazonaws.com'),
+        expected_url=(
+            'https://s3-accelerate.dualstack.dualstack'
+            '.amazonaws.com/bucket/key'))
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        # More than two extra parts is not allowed.
+        customer_provided_endpoint=(
+            'https://s3-accelerate.dualstack.dualstack.dualstack'
+            '.amazonaws.com'),
+        expected_url=(
+            'https://s3-accelerate.dualstack.dualstack.dualstack.amazonaws.com'
+            '/bucket/key'))
+    yield t.case(
+        region='us-east-1', bucket='bucket', key='key',
+        # Extra components must be whitelisted.
+        customer_provided_endpoint='https://s3-accelerate.foo.amazonaws.com',
+        expected_url='https://s3-accelerate.foo.amazonaws.com/bucket/key')
     yield t.case(
         region='us-east-1', bucket='bucket', key='key',
         s3_config=use_accelerate_dualstack, is_secure=False,

From e9f2dbf806f34320dc76396ec7c7166abc1766c9 Mon Sep 17 00:00:00 2001
From: JordonPhillips <phjordon@amazon.com>
Date: Thu, 13 Oct 2016 15:22:32 -0700
Subject: [PATCH 6/6] Use static whitelist

---
 botocore/client.py | 18 ++++++++----------
 botocore/utils.py  |  9 ++++++---
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/botocore/client.py b/botocore/client.py
index 62577a05f5..33de501e62 100644
--- a/botocore/client.py
+++ b/botocore/client.py
@@ -30,6 +30,7 @@
 from botocore.utils import S3RegionRedirector
 from botocore.utils import fix_s3_host
 from botocore.utils import switch_to_virtual_host_style
+from botocore.utils import S3_ACCELERATE_WHITELIST
 from botocore.args import ClientArgsCreator
 from botocore.compat import urlsplit
 # Keep this imported.  There's pre-existing code that uses
@@ -180,7 +181,7 @@ def _get_s3_addressing_handler(self, endpoint_url, s3_config,
         return fix_s3_host
 
     def _is_s3_accelerate(self, endpoint_url, s3_config):
-        # Accelerate has been explicitly configured
+        # Accelerate has been explicitly configured.
         if s3_config is not None and s3_config.get('use_accelerate_endpoint'):
             return True
 
@@ -194,24 +195,21 @@ def _is_s3_accelerate(self, endpoint_url, s3_config):
         if not netloc.endswith('amazonaws.com'):
             return False
 
-        # The first part of the url should always be s3-accelerate
+        # The first part of the url should always be s3-accelerate.
         parts = netloc.split('.')
         if parts[0] != 's3-accelerate':
             return False
 
-        # There should not be more than two components between 's3-accelerate'
-        # and 'amazonaws.com'
+        # Url parts between 's3-accelerate' and 'amazonaws.com' which
+        # represent different url features.
         feature_parts = parts[1:-2]
-        if len(feature_parts) > 2:
-            return False
 
-        # There should be no duplicates
+        # There should be no duplicate url parts.
         if len(feature_parts) != len(set(feature_parts)):
             return False
 
-        # Remaining parts must be in the whitelist.
-        whitelist = ['dualstack']
-        return all(p in whitelist for p in feature_parts)
+        # Remaining parts must all be in the whitelist.
+        return all(p in S3_ACCELERATE_WHITELIST for p in feature_parts)
 
     def _get_client_args(self, service_model, region_name, is_secure,
                          endpoint_url, verify, credentials,
diff --git a/botocore/utils.py b/botocore/utils.py
index 8ed08bab53..0f09568eae 100644
--- a/botocore/utils.py
+++ b/botocore/utils.py
@@ -46,6 +46,7 @@
     'fips-us-gov-west-1',
 ]
 RETRYABLE_HTTP_ERRORS = (requests.Timeout, requests.ConnectionError)
+S3_ACCELERATE_WHITELIST = ['dualstack']
 
 
 class _RetriesExceededError(Exception):
@@ -774,10 +775,12 @@ def switch_host_s3_accelerate(request, operation_name, **kwargs):
     # before it gets changed to virtual. So we are not concerned with ensuring
     # that the bucket name is translated to the virtual style here and we
     # can hard code the Accelerate endpoint.
-    whitelist = ['dualstack', 'amazonaws', 'com']
     parts = urlsplit(request.url).netloc.split('.')
-    parts = [p for p in parts if p in whitelist]
-    endpoint = 'https://s3-accelerate.' + '.'.join(parts)
+    parts = [p for p in parts if p in S3_ACCELERATE_WHITELIST]
+    endpoint = 'https://s3-accelerate.'
+    if len(parts) > 0:
+        endpoint += '.'.join(parts) + '.'
+    endpoint += 'amazonaws.com'
 
     if operation_name in ['ListBuckets', 'CreateBucket', 'DeleteBucket']:
         return