Skip to content

Commit

Permalink
Remove hardcoded 's3.amazonaws.com' for virtual hosted addressing
Browse files Browse the repository at this point in the history
This removes the hard coded references to 's3.amazonaws.com' when
using the virtual hosted addressing mode of S3 and instead uses
regionalized endpoints when converting to virtual hosted
addressing.

For example, given a bucket 'foo' in us-west-2 and a key 'bar', we would
convert the URL from `s3.us-west-2.amazonaws.com/foo/bar` to
`foo.s3.amazonaws.com/bar`.  With this change we'll now convert
the URL to `foo.s3.us-west-2.amazonaws.com/bar`.

When the initial code for 's3.amazonaws.com' was first added to
botocore, it provided a number of benefits:

1. You could avoid a 301 response by using `.s3.amazonaws.com`.  This is
   because the DNS would resolve to an endpoint in the correct region,
   and the older signature version `s3` didn't include a region name
   as part of its signing process.  The end result is that a user did
   not have to correctly configure a region for an S3 bucket, they'd
   automatically get to the correct region due to the DNS resolution.
2. The 301 PermanentRedirect responses did not include any structured
   data about the correct region to use so it wasn't easy to know
   which region you _should_ be sending the request to.  As a result,
   301 responses weren't automatically handled and ended up just
   raising an exception back to the user.

Since this code was first introduced there were several things that
have changed:

1. The introduction of the `s3v4` signature version, which requires
   a correct region name as part of its signature.  Signing a request
   with the wrong region results in a 400 response.  As a result,
   it didn't matter if `foo.s3.amazonaws.com` got you to the right
   region, if the request was _signed_ with the wrong region, you'd
   get a 400 response.
2. The 301 response (as well as most responses from S3) contain
   request metadata that tell you which region a bucket is in.  This
   means that it's now possible to automatically handle 301 responses
   because we know which region to send the request to.
3. The introduction of various partitions outside of the `aws`
   partition, such as `aws-cn` meant there were other TLDs we needed
   to handle.  The "hack" put in place in botocore was to just disable
   virtual hosted addressing in certain scenarios.

Given all this, it makes sense to no longer hardcode `s3.amazonaws.com`
when converting to virtual hosted addressing.  There's already a
growing number of edge cases where we have to disable this, and
most importantly it's not needed anymore.
  • Loading branch information
jamesls committed Feb 16, 2018
1 parent a671f1b commit cfeaae6
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 23 deletions.
2 changes: 1 addition & 1 deletion botocore/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ def check_dns_name(bucket_name):


def fix_s3_host(request, signature_version, region_name,
default_endpoint_url='s3.amazonaws.com', **kwargs):
default_endpoint_url=None, **kwargs):
"""
This handler looks at S3 requests just before they are signed.
If there is a bucket name on the path (true for everything except
Expand Down
20 changes: 10 additions & 10 deletions tests/functional/test_s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,11 +330,11 @@ def test_resign_request_with_region_when_needed(self):

self.assertEqual(self.http_session_send_mock.call_count, 2)
calls = [c[0][0] for c in self.http_session_send_mock.call_args_list]
initial_url = ('https://foo.s3.amazonaws.com/'
initial_url = ('https://foo.s3.us-west-2.amazonaws.com/'
'?encoding-type=url')
self.assertEqual(calls[0].url, initial_url)

fixed_url = ('https://foo.s3.amazonaws.com/'
fixed_url = ('https://foo.s3.eu-central-1.amazonaws.com/'
'?encoding-type=url')
self.assertEqual(calls[1].url, fixed_url)

Expand All @@ -349,15 +349,15 @@ def test_generate_unauthed_url(self):
'Bucket': 'foo',
'Key': 'bar'
})
self.assertEqual(url, 'https://foo.s3.amazonaws.com/bar')
self.assertEqual(url, 'https://foo.s3.us-west-2.amazonaws.com/bar')

def test_generate_unauthed_post(self):
config = Config(signature_version=botocore.UNSIGNED)
client = self.session.create_client('s3', self.region, config=config)
parts = client.generate_presigned_post(Bucket='foo', Key='bar')
expected = {
'fields': {'key': 'bar'},
'url': 'https://foo.s3.amazonaws.com/'
'url': 'https://foo.s3.us-west-2.amazonaws.com/'
}
self.assertEqual(parts, expected)

Expand Down Expand Up @@ -446,33 +446,33 @@ def test_correct_url_used_for_s3():
# The default behavior for sigv2. DNS compatible buckets
yield t.case(region='us-west-2', bucket='bucket', key='key',
signature_version='s3',
expected_url='https://bucket.s3.amazonaws.com/key')
expected_url='https://bucket.s3.us-west-2.amazonaws.com/key')
yield t.case(region='us-east-1', bucket='bucket', key='key',
signature_version='s3',
expected_url='https://bucket.s3.amazonaws.com/key')
yield t.case(region='us-west-1', bucket='bucket', key='key',
signature_version='s3',
expected_url='https://bucket.s3.amazonaws.com/key')
expected_url='https://bucket.s3.us-west-1.amazonaws.com/key')
yield t.case(region='us-west-1', bucket='bucket', key='key',
signature_version='s3', is_secure=False,
expected_url='http://bucket.s3.amazonaws.com/key')
expected_url='http://bucket.s3.us-west-1.amazonaws.com/key')

# Virtual host addressing is independent of signature version.
yield t.case(region='us-west-2', bucket='bucket', key='key',
signature_version='s3v4',
expected_url=(
'https://bucket.s3.amazonaws.com/key'))
'https://bucket.s3.us-west-2.amazonaws.com/key'))
yield t.case(region='us-east-1', bucket='bucket', key='key',
signature_version='s3v4',
expected_url='https://bucket.s3.amazonaws.com/key')
yield t.case(region='us-west-1', bucket='bucket', key='key',
signature_version='s3v4',
expected_url=(
'https://bucket.s3.amazonaws.com/key'))
'https://bucket.s3.us-west-1.amazonaws.com/key'))
yield t.case(region='us-west-1', bucket='bucket', key='key',
signature_version='s3v4', is_secure=False,
expected_url=(
'http://bucket.s3.amazonaws.com/key'))
'http://bucket.s3.us-west-1.amazonaws.com/key'))

# Regions outside of the 'aws' partition.
# These should still default to virtual hosted addressing
Expand Down
14 changes: 7 additions & 7 deletions tests/integration/test_s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ def test_presign_sigv4(self):
'get_object', Params={'Bucket': self.bucket_name, 'Key': self.key})
self.assertTrue(
presigned_url.startswith(
'https://s3.amazonaws.com/%s/%s' % (
'https://%s.s3.amazonaws.com/%s' % (
self.bucket_name, self.key)),
"Host was suppose to be the us-east-1 endpoint, instead "
"got: %s" % presigned_url)
Expand Down Expand Up @@ -647,7 +647,7 @@ def test_presign_post_sigv4(self):
# Make sure the correct endpoint is being used
self.assertTrue(
post_args['url'].startswith(
'https://s3.amazonaws.com/%s' % self.bucket_name),
'https://%s.s3.amazonaws.com/' % self.bucket_name),
"Host was suppose to use us-east-1 endpoint, instead "
"got: %s" % post_args['url'])

Expand All @@ -671,8 +671,8 @@ def test_presign_sigv2(self):
'get_object', Params={'Bucket': self.bucket_name, 'Key': self.key})
self.assertTrue(
presigned_url.startswith(
'https://%s.s3.amazonaws.com/%s' % (
self.bucket_name, self.key)),
'https://%s.s3.%s.amazonaws.com/%s' % (
self.bucket_name, self.region, self.key)),
"Host was suppose to use DNS style, instead "
"got: %s" % presigned_url)
# Try to retrieve the object using the presigned url.
Expand All @@ -687,7 +687,7 @@ def test_presign_sigv4(self):

self.assertTrue(
presigned_url.startswith(
'https://s3.us-west-2.amazonaws.com/%s/%s' % (
'https://%s.s3.us-west-2.amazonaws.com/%s' % (
self.bucket_name, self.key)),
"Host was suppose to be the us-west-2 endpoint, instead "
"got: %s" % presigned_url)
Expand Down Expand Up @@ -715,7 +715,7 @@ def test_presign_post_sigv2(self):
# Make sure the correct endpoint is being used
self.assertTrue(
post_args['url'].startswith(
'https://%s.s3.amazonaws.com' % self.bucket_name),
'https://%s.s3.us-west-2.amazonaws.com' % self.bucket_name),
"Host was suppose to use DNS style, instead "
"got: %s" % post_args['url'])

Expand Down Expand Up @@ -748,7 +748,7 @@ def test_presign_post_sigv4(self):
# Make sure the correct endpoint is being used
self.assertTrue(
post_args['url'].startswith(
'https://s3.us-west-2.amazonaws.com/%s' % self.bucket_name),
'https://%s.s3.us-west-2.amazonaws.com/' % self.bucket_name),
"Host was suppose to use DNS style, instead "
"got: %s" % post_args['url'])

Expand Down
4 changes: 2 additions & 2 deletions tests/unit/test_s3_addressing.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def test_list_objects_dns_name_non_classic(self):
prepared_request = self.get_prepared_request('list_objects', params,
force_hmacv1=True)
self.assertEqual(prepared_request.url,
'https://safename.s3.amazonaws.com/')
'https://safename.s3.us-west-2.amazonaws.com/')

def test_list_objects_unicode_query_string_eu_central_1(self):
self.region_name = 'eu-central-1'
Expand All @@ -81,7 +81,7 @@ def test_list_objects_unicode_query_string_eu_central_1(self):
prepared_request = self.get_prepared_request('list_objects', params)
self.assertEqual(
prepared_request.url,
('https://safename.s3.amazonaws.com/'
('https://safename.s3.eu-central-1.amazonaws.com/'
'?marker=%C3%A4%C3%B6%C3%BC-01.txt')
)

Expand Down
6 changes: 3 additions & 3 deletions tests/unit/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -677,13 +677,13 @@ def test_fix_s3_host_initial(self):
request=request, signature_version=signature_version,
region_name=region_name)
self.assertEqual(request.url,
'https://bucket.s3.amazonaws.com/key.txt')
'https://bucket.s3-us-west-2.amazonaws.com/key.txt')
self.assertEqual(request.auth_path, '/bucket/key.txt')

def test_fix_s3_host_only_applied_once(self):
request = AWSRequest(
method='PUT', headers={},
url='https://s3-us-west-2.amazonaws.com/bucket/key.txt'
url='https://s3.us-west-2.amazonaws.com/bucket/key.txt'
)
region_name = 'us-west-2'
signature_version = 's3'
Expand All @@ -695,7 +695,7 @@ def test_fix_s3_host_only_applied_once(self):
request=request, signature_version=signature_version,
region_name=region_name)
self.assertEqual(request.url,
'https://bucket.s3.amazonaws.com/key.txt')
'https://bucket.s3.us-west-2.amazonaws.com/key.txt')
# This was a bug previously. We want to make sure that
# calling fix_s3_host() again does not alter the auth_path.
# Otherwise we'll get signature errors.
Expand Down

0 comments on commit cfeaae6

Please sign in to comment.