Return a URI.escape'd URL from attachment

It's Paperclip's responsibility to escape special characters from the URL to make sure that it's comply with standard.

Closes #577, Closes #563, and reverse my judgement on #482.

lib/paperclip/attachment.rb

 # encoding: utf-8
 # encoding: utf-8
+require 'uri'
+
 module Paperclip
 module Paperclip
   # The Attachment class manages the files for a given attachment. It saves
   # The Attachment class manages the files for a given attachment. It saves
   # when the model saves, deletes when the model is destroyed, and processes
   # when the model saves, deletes when the model is destroyed, and processes
@@ -155,7 +157,7 @@ module Paperclip
     def url(style_name = default_style, use_timestamp = @use_timestamp)
     def url(style_name = default_style, use_timestamp = @use_timestamp)
       default_url = @default_url.is_a?(Proc) ? @default_url.call(self) : @default_url
       default_url = @default_url.is_a?(Proc) ? @default_url.call(self) : @default_url
       url = original_filename.nil? ? interpolate(default_url, style_name) : interpolate(@url, style_name)
       url = original_filename.nil? ? interpolate(default_url, style_name) : interpolate(@url, style_name)
-      use_timestamp && updated_at ? [url, updated_at].compact.join(url.include?("?") ? "&" : "?") : url
+      URI.escape(use_timestamp && updated_at ? [url, updated_at].compact.join(url.include?("?") ? "&" : "?") : url)
     end
     end
 
 
     # Returns the path of the attachment as defined by the :path option. If the
     # Returns the path of the attachment as defined by the :path option. If the

test/attachment_test.rb

@@ -918,7 +918,22 @@ class AttachmentTest < Test::Unit::TestCase
           end
           end
         end
         end
       end
       end
+    end
 
 
+    context "with a file that has space in file name" do
+      setup do
+        @attachment.stubs(:instance_read).with(:file_name).returns("spaced file.png")
+        @attachment.stubs(:instance_read).with(:content_type).returns("image/png")
+        @attachment.stubs(:instance_read).with(:file_size).returns(12345)
+        dtnow = DateTime.now
+        @now = Time.now
+        Time.stubs(:now).returns(@now)
+        @attachment.stubs(:instance_read).with(:updated_at).returns(dtnow)
+      end
+
+      should "returns an escaped version of the URL" do
+        assert_match /\/spaced%20file\.png/, @attachment.url
+      end
     end
     end
 
 
     context "when trying a nonexistant storage type" do
     context "when trying a nonexistant storage type" do

test/storage/filesystem_test.rb

@@ -30,5 +30,23 @@ class FileSystemTest < Test::Unit::TestCase
 
 
       @dummy.save!
       @dummy.save!
     end
     end
+
+    context "with file that has space in file name" do
+      setup do
+        rebuild_model :styles => { :thumbnail => "25x25#" }
+        @dummy = Dummy.create!
+
+        @dummy.avatar = File.open(File.join(File.dirname(__FILE__), "..", "fixtures", "spaced file.png"))
+        @dummy.save
+      end
+
+      should "store the file" do
+        assert File.exists?(@dummy.avatar.path)
+      end
+
+      should "return an escaped version of URL" do
+        assert_match /\/spaced%20file\.png/, @dummy.avatar.url
+      end
+    end
   end
   end
 end
 end

test/storage/s3_test.rb

@@ -110,6 +110,27 @@ class S3Test < Test::Unit::TestCase
     end
     end
   end
   end
 
 
+  context "An attachment that uses S3 for storage and has spaces in file name" do
+    setup do
+      AWS::S3::Base.stubs(:establish_connection!)
+      rebuild_model :styles  => { :large => ['500x500#', :jpg] },
+                    :storage => :s3,
+                    :bucket  => "bucket",
+                    :path => ":attachment/:basename.:extension",
+                    :s3_credentials => {
+                      'access_key_id' => "12345",
+                      'secret_access_key' => "54321"
+                    }
+
+      @dummy = Dummy.new
+      @dummy.avatar = File.new(File.join(File.dirname(__FILE__), '..', 'fixtures', 'spaced file.png'), 'rb')
+    end
+
+    should "return an escaped version of url" do
+      assert_match /.+\/spaced%20file\.png/, @dummy.avatar.url
+    end
+  end
+
   context "" do
   context "" do
     setup do
     setup do
       AWS::S3::Base.stubs(:establish_connection!)
       AWS::S3::Base.stubs(:establish_connection!)