From 37999b65780651878ff80c4a4041e80273c901a9 Mon Sep 17 00:00:00 2001
From: Prem Sichanugrist <s@sikachu.com>
Date: Tue, 3 Jul 2012 22:23:54 -0400
Subject: [PATCH] Fix regression on `s3_protocol`

`s3_protocol` now returns the protocol without a colon. If you need a
colon, you can pass in `true` as a second argument.

This might not be a best solution, so any better patch that does not
introduce a regression is welcome.

Fixes #921
---
 lib/paperclip/storage/s3.rb | 22 +++++++++++-----------
 test/storage/s3_test.rb     | 15 +++++++++++++++
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/lib/paperclip/storage/s3.rb b/lib/paperclip/storage/s3.rb
index bd7542907..6094c7542 100644
--- a/lib/paperclip/storage/s3.rb
+++ b/lib/paperclip/storage/s3.rb
@@ -150,13 +150,13 @@ def sanitize_hash(hash)
         end
 
         Paperclip.interpolates(:s3_alias_url) do |attachment, style|
-          "#{attachment.s3_protocol(style)}//#{attachment.s3_host_alias}/#{attachment.path(style).gsub(%r{^/}, "")}"
+          "#{attachment.s3_protocol(style, true)}//#{attachment.s3_host_alias}/#{attachment.path(style).gsub(%r{^/}, "")}"
         end unless Paperclip::Interpolations.respond_to? :s3_alias_url
         Paperclip.interpolates(:s3_path_url) do |attachment, style|
-          "#{attachment.s3_protocol(style)}//#{attachment.s3_host_name}/#{attachment.bucket_name}/#{attachment.path(style).gsub(%r{^/}, "")}"
+          "#{attachment.s3_protocol(style, true)}//#{attachment.s3_host_name}/#{attachment.bucket_name}/#{attachment.path(style).gsub(%r{^/}, "")}"
         end unless Paperclip::Interpolations.respond_to? :s3_path_url
         Paperclip.interpolates(:s3_domain_url) do |attachment, style|
-          "#{attachment.s3_protocol(style)}//#{attachment.bucket_name}.#{attachment.s3_host_name}/#{attachment.path(style).gsub(%r{^/}, "")}"
+          "#{attachment.s3_protocol(style, true)}//#{attachment.bucket_name}.#{attachment.s3_host_name}/#{attachment.path(style).gsub(%r{^/}, "")}"
         end unless Paperclip::Interpolations.respond_to? :s3_domain_url
         Paperclip.interpolates(:asset_host) do |attachment, style|
           "#{attachment.path(style).gsub(%r{^/}, "")}"
@@ -276,15 +276,15 @@ def s3_permissions(style = default_style)
         s3_permissions
       end
 
-      def s3_protocol(style = default_style)
-        protocol = if @s3_protocol.respond_to?(:call)
-          @s3_protocol.call(style, self).to_s
+      def s3_protocol(style = default_style, with_colon = false)
+        protocol = @s3_protocol
+        protocol = protocol.call(style, self) if protocol.respond_to?(:call)
+
+        if with_colon && !protocol.empty?
+          "#{protocol}:"
         else
-          @s3_protocol.to_s
+          protocol.to_s
         end
-
-        protocol = protocol.split(":").first + ":" unless protocol.empty?
-        protocol
       end
 
       def create_bucket
@@ -359,7 +359,7 @@ def find_credentials creds
       end
 
       def use_secure_protocol?(style_name)
-        s3_protocol(style_name) == "https:"
+        s3_protocol(style_name) == "https"
       end
     end
   end
diff --git a/test/storage/s3_test.rb b/test/storage/s3_test.rb
index 02087ff7f..5f77a68a8 100644
--- a/test/storage/s3_test.rb
+++ b/test/storage/s3_test.rb
@@ -125,7 +125,22 @@ def teardown
     should "use the correct key" do
       assert_equal "avatars/stringio.txt", @dummy.avatar.s3_object.key
     end
+  end
+
+  context "s3_protocol" do
+    ["http", :http, ""].each do |protocol|
+      context "as #{protocol.inspect}" do
+        setup do
+          rebuild_model :storage => :s3, :s3_protocol => protocol
 
+          @dummy = Dummy.new
+        end
+
+        should "return the s3_protocol in string" do
+          assert_equal protocol.to_s, @dummy.avatar.s3_protocol
+        end
+      end
+    end
   end
 
   context ":s3_protocol => 'https'" do