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 Paperclip
     #   Or globally:
     #   Or globally:
     #     :s3_permissions => :private
     #     :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
     # * +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
     #   {'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.
     #   runtime. Paperclip will call that Proc with attachment as the only argument.
@@ -131,12 +130,7 @@ module Paperclip
         base.instance_eval do
         base.instance_eval do
           @s3_options     = @options[:s3_options]     || {}
           @s3_options     = @options[:s3_options]     || {}
           @s3_permissions = set_permissions(@options[:s3_permissions])
           @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_metadata = @options[:s3_metadata] || {}
           @s3_headers = {}
           @s3_headers = {}
           merge_s3_headers(@options[:s3_headers], @s3_headers, @s3_metadata)
           merge_s3_headers(@options[:s3_headers], @s3_headers, @s3_metadata)

spec/paperclip/storage/s3_spec.rb

@@ -114,7 +114,7 @@ describe Paperclip::Storage::S3 do
     end
     end
 
 
     it "returns a url based on an S3 path" do
     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
     end
 
 
     it "uses the correct bucket" do
     it "uses the correct bucket" do
@@ -252,7 +252,7 @@ describe Paperclip::Storage::S3 do
     end
     end
 
 
     it "returns a url based on an :s3_host_name path" do
     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
     end
 
 
     it "uses the S3 bucket with the correct host name" do
     it "uses the S3 bucket with the correct host name" do
@@ -278,7 +278,7 @@ describe Paperclip::Storage::S3 do
 
 
     it "uses s3_host_name as a proc if available" do
     it "uses s3_host_name as a proc if available" do
       @dummy.value = "s3.something.com"
       @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
   end
   end
 
 
@@ -487,7 +487,7 @@ describe Paperclip::Storage::S3 do
     end
     end
 
 
     it "returns a url based on an S3 subdomain" do
     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
   end
   end
 
 
@@ -510,7 +510,7 @@ describe Paperclip::Storage::S3 do
     end
     end
 
 
     it "returns a url based on the host_alias" do
     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
   end
   end
 
 
@@ -534,8 +534,8 @@ describe Paperclip::Storage::S3 do
     end
     end
 
 
     it "returns a url based on the host_alias" do
     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
     end
 
 
     it "still returns the bucket name" do
     it "still returns the bucket name" do
@@ -789,7 +789,7 @@ describe Paperclip::Storage::S3 do
       it "does not get a bucket to get a URL" do
       it "does not get a bucket to get a URL" do
         @dummy.avatar.expects(:s3).never
         @dummy.avatar.expects(:s3).never
         @dummy.avatar.expects(:s3_bucket).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
       end
 
 
       it "is rewound after flush_writes" do
       it "is rewound after flush_writes" do
@@ -1526,29 +1526,6 @@ describe Paperclip::Storage::S3 do
           }
           }
         )
         )
       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
   end
   end