Merge pull request #2359 from ransombriggs/rbriggs-url-fix

Fix issue with subdomains and buckets with periods

lib/carrierwave/storage/fog.rb

@@ -365,8 +365,12 @@ def public_url
                 "#{@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]$/
+                valid_subdomain = @uploader.fog_directory.to_s =~ subdomain_regex && !(protocol == 'https' && @uploader.fog_directory =~ /\./)
+
                 # if directory is a valid subdomain, use that style for access
-                if @uploader.fog_directory.to_s =~ /^(?:[a-z]|\d(?!\d{0,2}(?:\d{1,3}){3}$))(?:[a-z0-9\.]|(?![\-])|\-(?![\.])){1,61}[a-z0-9]$/
+                if valid_subdomain
                   s3_subdomain = @uploader.fog_aws_accelerate ? "s3-accelerate" : "s3"
                   "#{protocol}://#{@uploader.fog_directory}.#{s3_subdomain}.amazonaws.com/#{encoded_path}"
                 else

spec/storage/fog_helper.rb

@@ -110,19 +110,19 @@ class FogSpec#{fog_credentials[:provider]}Uploader < CarrierWave::Uploader::Base
 
             context "directory is a valid subdomain" do
               before do
-                allow(@uploader).to receive(:fog_directory).and_return('assets.site.com')
+                allow(@uploader).to receive(:fog_directory).and_return('assets-site-com')
               end
 
               it "should use a subdomain URL for AWS" do
                 if @provider == 'AWS'
-                  expect(@fog_file.public_url).to include('https://assets.site.com.s3.amazonaws.com')
+                  expect(@fog_file.public_url).to include('https://assets-site-com.s3.amazonaws.com')
                 end
               end
 
               it "should use accelerate domain if fog_aws_accelerate is true" do
                 if @provider == 'AWS'
                   allow(@uploader).to receive(:fog_aws_accelerate).and_return(true)
-                  expect(@fog_file.public_url).to include('https://assets.site.com.s3-accelerate.amazonaws.com')
+                  expect(@fog_file.public_url).to include('https://assets-site-com.s3-accelerate.amazonaws.com')
                 end
               end
             end
@@ -134,6 +134,22 @@ class FogSpec#{fog_credentials[:provider]}Uploader < CarrierWave::Uploader::Base
               end
             end
 
+            it "should not use a subdomain URL for AWS if https && the directory is not accessible over https as a virtual hosted bucket" 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')
+                expect(@fog_file.public_url).to include('https://s3.amazonaws.com/foo.bar')
+              end
+            end
+
+            it "should use a subdomain URL for AWS if http && the directory is not accessible over https as a virtual hosted bucket" do
+              if @provider == 'AWS'
+                allow(@uploader).to receive(:fog_use_ssl_for_aws).and_return(false)
+                allow(@uploader).to receive(:fog_directory).and_return('foo.bar')
+                expect(@fog_file.public_url).to include('http://foo.bar.s3.amazonaws.com/')
+              end
+            end
+
             it "should use https as a default protocol" do
               if @provider == 'AWS'
                 expect(@fog_file.public_url).to start_with 'https://'