Skip to content

Commit

Permalink
Follow-up for #2763
Browse files Browse the repository at this point in the history
- Raises an error instead of returning nil
- Reorganize spec/storage/fog_helper.rb so it doesn't have high cyclomatic complexity
  • Loading branch information
mshibuya committed Dec 1, 2024
1 parent 054bd3d commit 04bfab3
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 15 deletions.
2 changes: 1 addition & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ Metrics/ClassLength:
Enabled: false

Metrics/CyclomaticComplexity:
Max: 28
Max: 18

Metrics/MethodLength:
Enabled: false
Expand Down
11 changes: 3 additions & 8 deletions lib/carrierwave/storage/fog.rb
Original file line number Diff line number Diff line change
Expand Up @@ -376,21 +376,15 @@ def public_url
when 'AWS'
# check if some endpoint is set in fog_credentials
if @uploader.fog_credentials.has_key?(:endpoint)
if !@uploader.fog_aws_fips
"#{@uploader.fog_credentials[:endpoint]}/#{@uploader.fog_directory}/#{encoded_path}"
else
warn 'Use of options :endpoint and :fog_aws_fips=true together will fail, as FIPS endpoints do not support path-style URLs.'
nil
end
raise 'fog_aws_fips = true is incompatible with :endpoint, as FIPS endpoints do not support path-style URLs.' if @uploader.fog_aws_fips
"#{@uploader.fog_credentials[:endpoint]}/#{@uploader.fog_directory}/#{encoded_path}"
else
protocol = @uploader.fog_use_ssl_for_aws ? "https" : "http"

subdomain_regex = /^(?:[a-z]|\d(?!\d{0,2}(?:\d{1,3}){3}$))(?:[a-z0-9\.]|(?![\-])|\-(?![\.])){1,61}[a-z0-9]$/
# To use the virtual-hosted style, the bucket name needs to be representable as a subdomain
use_virtual_hosted_style = @uploader.fog_directory.to_s =~ subdomain_regex && !(protocol == 'https' && @uploader.fog_directory =~ /\./)

return nil if !use_virtual_hosted_style && @uploader.fog_aws_fips # FIPS Endpoints can only be used with Virtual Hosted-Style addressing.

region = @uploader.fog_credentials[:region].to_s
regional_host = 's3.amazonaws.com' # used for DEFAULT_S3_REGION or no region set
if @uploader.fog_aws_fips
Expand All @@ -403,6 +397,7 @@ def public_url
regional_host = 's3-accelerate.amazonaws.com' if @uploader.fog_aws_accelerate
"#{protocol}://#{@uploader.fog_directory}.#{regional_host}/#{encoded_path}"
else # directory is not a valid subdomain, so use path style for access
raise 'FIPS Endpoints can only be used with Virtual Hosted-Style addressing.' if @uploader.fog_aws_fips
"#{protocol}://#{regional_host}/#{@uploader.fog_directory}/#{encoded_path}"
end
end
Expand Down
10 changes: 5 additions & 5 deletions spec/storage/fog_helper.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
def fog_tests(fog_credentials)
shared_examples "Fog storage" do |fog_credentials|
describe "with #{fog_credentials[:provider]} provider", with_retry: !Fog.mocking do
before do
WebMock.disable! unless Fog.mocking?
Expand Down Expand Up @@ -506,11 +506,11 @@ def check_file
expect(@fog_file.public_url).to include("https://#{CARRIERWAVE_DIRECTORY}.s3-accelerate.amazonaws.com")
end

it 'returns nil when both :endpoint and :fog_aws_fips=true' do
it 'raises an error when both :endpoint and :fog_aws_fips=true' do
allow(@uploader).to receive(:fog_credentials).and_return(@uploader.fog_credentials.merge(endpoint: 'https://custom-endpoint.example.com'))
allow(@uploader).to receive(:fog_directory).and_return('SiteAssets')
allow(@uploader).to receive(:fog_aws_fips).and_return(true)
expect(@fog_file.url).to be nil
expect { @fog_file.url }.to raise_error RuntimeError, /incompatible/
end

it 'returns endpoint+bucket when :endpoint and !:fog_aws_fips' do
Expand Down Expand Up @@ -562,13 +562,13 @@ def check_file
'us-east-2',
'us-gov-west-1'
].each do |region|
it "public_url should be nil" do
it "raises an error" do
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_aws_fips).and_return(true)
allow(@uploader).to receive(:fog_credentials).and_return(@uploader.fog_credentials.merge(region: region))

expect(@fog_file.public_url).to be_nil
expect { @fog_file.url }.to raise_error RuntimeError, /Virtual Hosted-Style/
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/storage/fog_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

describe CarrierWave::Storage::Fog do
FOG_CREDENTIALS.each do |credential|
fog_tests(credential)
include_examples 'Fog storage', credential
end

describe '.eager_load' do
Expand Down

0 comments on commit 04bfab3

Please sign in to comment.