Skip to content

Commit

Permalink
Merge pull request #2439 from Bonias/aws-use-proper-region-for-path-s…
Browse files Browse the repository at this point in the history
…tyle-url

Fix S3 path-style URL for host with dots for buckets that are placed in other regions than us-east-1
  • Loading branch information
mshibuya committed Dec 24, 2019
2 parents 712f405 + 5db9dcf commit 21a0061
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
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
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 21a0061

Please sign in to comment.