Skip to content

Commit

Permalink
Fix for Aws::S3::Client#head_bucket with improper region.
Browse files Browse the repository at this point in the history
Resolved the following issues:

  s3 = Aws::S3::Client.new(region: 'us-west-1')
  s3.head_bucket(bucket: 'us-west-2-bucket')
  #=> raises Aws::S3::Errors::BadRequest (without a message)

If the bucket exists, but you are configured for the incorrect region,
the SDK fails to extract the actual bucket region from the 400 response.
This results in an empty error message. Added support for loading the
bucket region from the "x-amz-bucket-region" header, which is necessary
for all HEAD responses.

Fixes #1161
  • Loading branch information
trevorrowe committed Apr 18, 2016
1 parent 0960da2 commit fd52fa5
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 6 deletions.
6 changes: 6 additions & 0 deletions aws-sdk-core/features/s3/buckets.feature
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ Feature: S3 Buckets
When I delete the bucket
Then the bucket should not exist

Scenario: HEAD bucket works with improper region
Given I am using the S3 "us-west-2" region
And I create a bucket
When I am using the S3 "us-east-1" region
Then I should be able to HEAD the bucket

Scenario: CRUD buckets using a regional endpoint
Given I am using the S3 "us-west-2" region
When I create a bucket
Expand Down
4 changes: 4 additions & 0 deletions aws-sdk-core/features/s3/step_definitions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ def create_bucket(options = {})
expect { @client.get_bucket_location(bucket: @bucket_name) }.not_to raise_error
end

Then(/^I should be able to HEAD the bucket$/) do
expect { @client.head_bucket(bucket: @bucket_name) }.not_to raise_error
end

When(/^I delete the bucket$/) do
@client.delete_bucket(bucket: @bucket_name)
@created_buckets.delete(@bucket_name)
Expand Down
18 changes: 12 additions & 6 deletions aws-sdk-core/lib/aws-sdk-core/plugins/s3_request_signer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,8 @@ def handle_region_errors(response)
end

def get_region_and_retry(context)
actual_region = region_from_body(context)
if actual_region.nil? || actual_region == ""
raise "Couldn't get region from body: #{context.body}"
end
actual_region = context.http_response.headers['x-amz-bucket-region']
actual_region ||= region_from_body(context.http_response.body_contents)
update_bucket_cache(context, actual_region)
log_warning(context, actual_region)
update_region_header(context, actual_region)
Expand All @@ -116,7 +114,10 @@ def update_bucket_cache(context, actual_region)

def wrong_sigv4_region?(resp)
resp.context.http_response.status_code == 400 &&
(
resp.context.http_response.headers['x-amz-bucket-region'] ||
resp.context.http_response.body_contents.match(/<Region>.+?<\/Region>/)
)
end

def update_region_header(context, region)
Expand All @@ -128,8 +129,13 @@ def update_region_header(context, region)
signer.sign(context.http_request)
end

def region_from_body(context)
context.http_response.body_contents.match(/<Region>(.+?)<\/Region>/)[1]
def region_from_body(body)
region = body.match(/<Region>(.+?)<\/Region>/)[1]
if region.nil? || region == ""
raise "couldn't get region from body: #{body}"
else
region
end
end

def log_warning(context, actual_region)
Expand Down

0 comments on commit fd52fa5

Please sign in to comment.