Skip to content

Commit

Permalink
Fix S3 path-style URL for host with dots
Browse files Browse the repository at this point in the history
..for buckets that are placed in other regions that us-east-1.

According to https://docs.aws.amazon.com/AmazonS3/latest/dev/UsingBucket.html#access-bucket-intro,
`s3.amazonaws.com` can be used only for `us-east-1` region. For all
other regions `s3.aws-region.amazonaws.com` needs to be used.
  • Loading branch information
Piotr Boniecki authored and joemsak committed Mar 27, 2021
1 parent 6346da4 commit 1f2cb50
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 3 deletions.
14 changes: 11 additions & 3 deletions lib/carrierwave/storage/fog.rb
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,8 @@ def connection
end

class File
DEFAULT_S3_REGION = 'us-east-1'

include CarrierWave::Utilities::Uri

##
Expand Down Expand Up @@ -384,9 +386,15 @@ def public_url
if valid_subdomain
s3_subdomain = @uploader.fog_aws_accelerate ? "s3-accelerate" : "s3"
"#{protocol}://#{@uploader.fog_directory}.#{s3_subdomain}.amazonaws.com/#{encoded_path}"
else
# directory is not a valid subdomain, so use path style for access
"#{protocol}://s3.amazonaws.com/#{@uploader.fog_directory}/#{encoded_path}"
else # directory is not a valid subdomain, so use path style for access
region = @uploader.fog_credentials[:region].to_s
host = case region
when DEFAULT_S3_REGION, ''
's3.amazonaws.com'
else
"s3.#{region}.amazonaws.com"
end
"#{protocol}://#{host}/#{@uploader.fog_directory}/#{encoded_path}"
end
end
when 'Google'
Expand Down
18 changes: 18 additions & 0 deletions spec/storage/fog_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,24 @@ def check_file
end
end

{
nil => 's3.amazonaws.com',
'us-east-1' => 's3.amazonaws.com',
'us-east-2' => 's3.us-east-2.amazonaws.com',
'eu-central-1' => 's3.eu-central-1.amazonaws.com'
}.each do |region, expected_host|
it "should use a #{expected_host} hostname when using path style for access #{region} region" do
if @provider == 'AWS'
allow(@uploader).to receive(:fog_use_ssl_for_aws).and_return(true)
allow(@uploader).to receive(:fog_directory).and_return('foo.bar')

allow(@uploader).to receive(:fog_credentials).and_return(@uploader.fog_credentials.merge(region: region))

expect(@fog_file.public_url).to include("https://#{expected_host}/foo.bar")
end
end
end

it "should use https as a default protocol" do
if @provider == 'AWS'
expect(@fog_file.public_url).to start_with 'https://'
Expand Down

0 comments on commit 1f2cb50

Please sign in to comment.