Skip to content

Commit

Permalink
Only set auth_path once, fixes signing errors
Browse files Browse the repository at this point in the history
Fixes aws/aws-cli#986

The bug was that the fix_s3_host always sets the auth_path
to the current path of the request.url.  However, in the case
of a retried request, we call fix_s3_host again which changes
the auth_path to the (already modified) request path.  This
generates an auth error.

The fix now is to check if auth_path is set, and if it is, then
we can immediately return.
  • Loading branch information
jamesls committed Nov 7, 2014
1 parent 82ad3a0 commit 12b8553
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 4 deletions.
5 changes: 5 additions & 0 deletions botocore/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,11 @@ def fix_s3_host(event_name, endpoint, request, auth, **kwargs):
addressing. This allows us to avoid 301 redirects for all
bucket names that can be CNAME'd.
"""
if auth.auth_path is not None:
# The auth_path has already been applied (this may be a
# retried request). We don't need to perform this
# customization again.
return
parts = urlsplit(request.url)
auth.auth_path = parts.path
path_parts = parts.path.split('/')
Expand Down
35 changes: 31 additions & 4 deletions tests/unit/test_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
import mock

import botocore.session
from botocore.awsrequest import AWSRequest
from botocore.hooks import first_non_none_response
from botocore.auth import BaseSigner
from botocore.compat import quote
from botocore import handlers

Expand Down Expand Up @@ -161,6 +163,35 @@ def test_sse_headers(self):
params['headers'][prefix + 'key-MD5'],
'N7UdGUp1E+RbVvZSTy1R8g==')

def test_fix_s3_host_initial(self):
endpoint = mock.Mock(region_name='us-west-2')
request = AWSRequest(
method='PUT',headers={},
url='https://s3-us-west-2.amazonaws.com/bucket/key.txt'
)
auth = mock.Mock()
auth.auth_path = None
handlers.fix_s3_host('foo', endpoint, request, auth)
self.assertEqual(request.url, 'https://bucket.s3.amazonaws.com/key.txt')
self.assertEqual(auth.auth_path, '/bucket/key.txt')

def test_fix_s3_host_only_applied_once(self):
endpoint = mock.Mock(region_name='us-west-2')
request = AWSRequest(
method='PUT',headers={},
url='https://s3-us-west-2.amazonaws.com/bucket/key.txt'
)
auth = mock.Mock()
auth.auth_path = None
handlers.fix_s3_host('foo', endpoint, request, auth)
# Calling the handler again should not affect the end result:
handlers.fix_s3_host('foo', endpoint, request, auth)
self.assertEqual(request.url, 'https://bucket.s3.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.
self.assertEqual(auth.auth_path, '/bucket/key.txt')


class TestRetryHandlerOrder(BaseSessionTest):
def get_handler_names(self, responses):
Expand Down Expand Up @@ -195,7 +226,3 @@ def test_s3_special_case_is_before_other_retry(self):
self.assertTrue(s3_200_handler < general_retry_handler,
"S3 200 error handler was supposed to be before "
"the general retry handler, but it was not.")


if __name__ == '__main__':
unittest.main()

0 comments on commit 12b8553

Please sign in to comment.