Permalink
Browse files

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.
  • Loading branch information...
sikachu committed Sep 23, 2011
1 parent 0ca98d1 commit 23cb822de3807404589706ead964f2ef0d759ec9
@@ -1,4 +1,6 @@
# encoding: utf-8
+require 'uri'
+
module Paperclip
# The Attachment class manages the files for a given attachment. It saves
# when the model saves, deletes when the model is destroyed, and processes
@@ -155,7 +157,7 @@ def assign uploaded_file
def url(style_name = default_style, use_timestamp = @use_timestamp)
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)
- 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)

This comment has been minimized.

Show comment
Hide comment
@oparrish

oparrish Sep 28, 2011

Thanks for making this change! I wish I knew that it happened though as the result was some broken enclosure links in my RSS feed because they were getting double encoded.

@oparrish

oparrish Sep 28, 2011

Thanks for making this change! I wish I knew that it happened though as the result was some broken enclosure links in my RSS feed because they were getting double encoded.

This comment has been minimized.

Show comment
Hide comment
@filipegiusti

filipegiusti Oct 11, 2011

We are 2 with double encoded urls.

@filipegiusti

filipegiusti Oct 11, 2011

We are 2 with double encoded urls.

end
# Returns the path of the attachment as defined by the :path option. If the
View
@@ -918,7 +918,22 @@ def do_after_all; 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
context "when trying a nonexistant storage type" do
Binary file not shown.
@@ -30,5 +30,23 @@ class FileSystemTest < Test::Unit::TestCase
@dummy.save!
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
View
@@ -110,6 +110,27 @@ def rails_env(env)
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
setup do
AWS::S3::Base.stubs(:establish_connection!)

0 comments on commit 23cb822

Please sign in to comment.