Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix S3 path-style URL for host with dots for buckets that are placed in other regions than us-east-1 #2439

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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