diff --git a/.changes/next-release/bugfix-s3-68869.json b/.changes/next-release/bugfix-s3-68869.json new file mode 100644 index 0000000000..91e05b6918 --- /dev/null +++ b/.changes/next-release/bugfix-s3-68869.json @@ -0,0 +1,5 @@ +{ + "type": "bugfix", + "category": "s3", + "description": "Fix s3 presigned URLs for operations with query components (`#2962 `__)" +} diff --git a/botocore/handlers.py b/botocore/handlers.py index 55087f6749..e3eb5c886b 100644 --- a/botocore/handlers.py +++ b/botocore/handlers.py @@ -1049,6 +1049,10 @@ def remove_bucket_from_url_paths_from_model(params, model, context, **kwargs): bucket_path = '/{Bucket}' if req_uri.startswith(bucket_path): model.http['requestUri'] = req_uri[len(bucket_path) :] + # Strip query off the requestUri before using as authPath. The + # HmacV1Auth signer will append query params to the authPath during + # signing. + req_uri = req_uri.split('?')[0] # If the request URI is ONLY a bucket, the auth_path must be # terminated with a '/' character to generate a signature that the # server will accept. diff --git a/tests/unit/test_handlers.py b/tests/unit/test_handlers.py index 08a315a78a..2701c32efe 100644 --- a/tests/unit/test_handlers.py +++ b/tests/unit/test_handlers.py @@ -1680,3 +1680,50 @@ def test_remove_arn_from_signing_path(auth_path_in, auth_path_expected): request=request, some='other', kwarg='values' ) assert request.auth_path == auth_path_expected + + +@pytest.mark.parametrize( + 'request_uri_before, request_uri_after, auth_path', + [ + ('/{Bucket}', '', '/{Bucket}/'), + ('/{Bucket}?query', '?query', '/{Bucket}/'), + ('/{Bucket}/123', '/123', '/{Bucket}/123'), + ('/{Bucket}/123?query', '/123?query', '/{Bucket}/123'), + ], +) +def test_remove_bucket_from_url_paths_from_model( + request_uri_before, request_uri_after, auth_path +): + operation_def = { + 'name': 'TestOp', + 'http': { + 'method': 'GET', + 'requestUri': request_uri_before, + 'responseCode': 200, + }, + 'input': {'shape': 'TestOpInput'}, + } + service_def = { + 'metadata': {}, + 'shapes': { + 'TestOpInput': { + 'type': 'structure', + 'required': ['Bucket'], + 'members': { + 'Bucket': { + 'shape': 'String', + 'contextParam': {'name': 'Bucket'}, + 'location': 'uri', + 'locationName': 'Bucket', + }, + }, + }, + }, + } + model = OperationModel(operation_def, ServiceModel(service_def)) + # the handler modifies ``model`` in place + handlers.remove_bucket_from_url_paths_from_model( + params=None, model=model, context=None + ) + assert model.http['requestUri'] == request_uri_after + assert model.http['authPath'] == auth_path