Default S3 protocol to empty string

Default `s3_protocol` used to be `http` when `:s3_permissions` are
`:public_read` (default), and `https` when `:s3_permissions` are
anything else.

With an empty String as default, if the page is served over HTTPS,
attachment URL will be HTTPS, and if the page is served over HTTP,
attachment url will be HTTP.

`public-read` is an authorization concept, independent form the
encryption of the HTTP connection used to read the file. As such, the
one shouldn't define the other.

[fixes #2038]

lib/paperclip/storage/s3.rb

@@ -50,10 +50,9 @@ module Storage
     #   Or globally:
     #     :s3_permissions => :private
     #
-    # * +s3_protocol+: The protocol for the URLs generated to your S3 assets. Can be either
-    #   'http', 'https', or an empty string to generate protocol-relative URLs. Defaults to 'http'
-    #   when your :s3_permissions are :public_read (the default), and 'https' when your
-    #   :s3_permissions are anything else.
+    # * +s3_protocol+: The protocol for the URLs generated to your S3 assets.
+    #   Can be either 'http', 'https', or an empty string to generate
+    #   protocol-relative URLs. Defaults to empty string.
     # * +s3_headers+: A hash of headers or a Proc. You may specify a hash such as
     #   {'Expires' => 1.year.from_now.httpdate}. If you use a Proc, headers are determined at
     #   runtime. Paperclip will call that Proc with attachment as the only argument.
@@ -131,12 +130,7 @@ def self.extended base
         base.instance_eval do
           @s3_options     = @options[:s3_options]     || {}
           @s3_permissions = set_permissions(@options[:s3_permissions])
-          @s3_protocol    = @options[:s3_protocol]    ||
-            Proc.new do |style, attachment|
-              permission  = (@s3_permissions[style.to_s.to_sym] || @s3_permissions[:default])
-              permission  = permission.call(attachment, style) if permission.respond_to?(:call)
-              (permission == :"public-read") ? 'http'.freeze : 'https'.freeze
-            end
+          @s3_protocol    = @options[:s3_protocol] || "".freeze
           @s3_metadata = @options[:s3_metadata] || {}
           @s3_headers = {}
           merge_s3_headers(@options[:s3_headers], @s3_headers, @s3_metadata)

spec/paperclip/storage/s3_spec.rb

@@ -114,7 +114,7 @@ def aws2_add_region
     end
 
     it "returns a url based on an S3 path" do
-      assert_match %r{^http://s3.amazonaws.com/bucket/avatars/data[^\.]}, @dummy.avatar.url
+      assert_match %r{^//s3.amazonaws.com/bucket/avatars/data[^\.]}, @dummy.avatar.url
     end
 
     it "uses the correct bucket" do
@@ -252,7 +252,7 @@ def aws2_add_region
     end
 
     it "returns a url based on an :s3_host_name path" do
-      assert_match %r{^http://s3-ap-northeast-1.amazonaws.com/bucket/avatars/data[^\.]}, @dummy.avatar.url
+      assert_match %r{^//s3-ap-northeast-1.amazonaws.com/bucket/avatars/data[^\.]}, @dummy.avatar.url
     end
 
     it "uses the S3 bucket with the correct host name" do
@@ -278,7 +278,7 @@ class << @dummy
 
     it "uses s3_host_name as a proc if available" do
       @dummy.value = "s3.something.com"
-      assert_equal "http://s3.something.com/bucket/avatars/data", @dummy.avatar.url(:original, timestamp: false)
+      assert_equal "//s3.something.com/bucket/avatars/data", @dummy.avatar.url(:original, timestamp: false)
     end
   end
 
@@ -487,7 +487,7 @@ def original_filename
     end
 
     it "returns a url based on an S3 subdomain" do
-      assert_match %r{^http://bucket.s3.amazonaws.com/avatars/data[^\.]}, @dummy.avatar.url
+      assert_match %r{^//bucket.s3.amazonaws.com/avatars/data[^\.]}, @dummy.avatar.url
     end
   end
 
@@ -510,7 +510,7 @@ def original_filename
     end
 
     it "returns a url based on the host_alias" do
-      assert_match %r{^http://something.something.com/avatars/data[^\.]}, @dummy.avatar.url
+      assert_match %r{^//something.something.com/avatars/data[^\.]}, @dummy.avatar.url
     end
   end
 
@@ -534,8 +534,8 @@ def counter
     end
 
     it "returns a url based on the host_alias" do
-      assert_match %r{^http://cdn1.example.com/avatars/data[^\.]}, @dummy.avatar.url
-      assert_match %r{^http://cdn2.example.com/avatars/data[^\.]}, @dummy.avatar.url
+      assert_match %r{^//cdn1.example.com/avatars/data[^\.]}, @dummy.avatar.url
+      assert_match %r{^//cdn2.example.com/avatars/data[^\.]}, @dummy.avatar.url
     end
 
     it "still returns the bucket name" do
@@ -789,7 +789,7 @@ def counter
       it "does not get a bucket to get a URL" do
         @dummy.avatar.expects(:s3).never
         @dummy.avatar.expects(:s3_bucket).never
-        assert_match %r{^http://s3\.amazonaws\.com/testing/avatars/original/5k\.png}, @dummy.avatar.url
+        assert_match %r{^//s3\.amazonaws\.com/testing/avatars/original/5k\.png}, @dummy.avatar.url
       end
 
       it "is rewound after flush_writes" do
@@ -1526,29 +1526,6 @@ class DummyCredentialProvider; end
           }
         )
       end
-
-      context "when assigned" do
-        before do
-          @file = File.new(fixture_file('5k.png'), 'rb')
-          @dummy = Dummy.new
-          @dummy.stubs(:private_attachment? => true)
-          @dummy.avatar = @file
-        end
-
-        after { @file.close }
-
-        context "and saved" do
-          before do
-            @dummy.save
-          end
-
-          it "succeeds" do
-            assert @dummy.avatar.url().include?       "https://"
-            assert @dummy.avatar.url(:thumb).include? "http://"
-          end
-        end
-      end
-
     end
   end