Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Avoid calling URI.escape in #url method, and use Paperclip::Interpola…

…tedString

Paperclip::InterpolatedString will ensure that we're not double escape the string, which was the cause of the escaped filename problem in #path method.
  • Loading branch information...
commit 56e413590c9e9d7fc62311863295b779b55be071 1 parent db60516
@sikachu sikachu authored
View
7 lib/paperclip/attachment.rb
@@ -131,7 +131,9 @@ def assign uploaded_file
def url(style_name = default_style, use_timestamp = @options.use_timestamp)
default_url = @options.default_url.is_a?(Proc) ? @options.default_url.call(self) : @options.default_url
url = original_filename.nil? ? interpolate(default_url, style_name) : interpolate(@options.url, style_name)
- URI.escape(use_timestamp && updated_at ? [url, updated_at].compact.join(url.include?("?") ? "&" : "?") : url)
+
+ url << (url.include?("?") ? "&" : "?") + updated_at.to_s if use_timestamp && updated_at
+ url.respond_to?(:escape) ? url.escape : URI.escape(url)
end
# Returns the path of the attachment as defined by the :path option. If the
@@ -139,7 +141,8 @@ def url(style_name = default_style, use_timestamp = @options.use_timestamp)
# on disk. If the file is stored in S3, the path is the "key" part of the
# URL, and the :bucket option refers to the S3 bucket.
def path(style_name = default_style)
- original_filename.nil? ? nil : interpolate(@options.path, style_name)
+ path = original_filename.nil? ? nil : interpolate(@options.path, style_name)
+ path.respond_to?(:unescape) ? path.unescape : path
end
# Alias to +url+
View
6 lib/paperclip/interpolations.rb
@@ -1,3 +1,5 @@
+require 'paperclip/interpolated_string'
+
module Paperclip
# This module contains all the methods that are available for interpolation
# in paths and urls. To add your own (or override an existing one), you
@@ -29,11 +31,13 @@ def self.all
# an interpolation pattern for Paperclip to use.
def self.interpolate pattern, *args
pattern = args.first.instance.send(pattern) if pattern.kind_of? Symbol
- all.reverse.inject( pattern.dup ) do |result, tag|
+ interpolated_string = all.reverse.inject(InterpolatedString.new(pattern)) do |result, tag|
result.gsub(/:#{tag}/) do |match|
send( tag, *args )
end
end
+ interpolated_string.force_escape if pattern =~ /:url/
+ interpolated_string
end
# Returns the filename, the same way as ":basename.:extension" would.
View
4 test/storage/filesystem_test.rb
@@ -44,6 +44,10 @@ class FileSystemTest < Test::Unit::TestCase
assert File.exists?(@dummy.avatar.path)
end
+ should "store the path unescaped" do
+ assert_match /\/spaced file\.png/, @dummy.avatar.path
+ end
+
should "return an escaped version of URL" do
assert_match /\/spaced%20file\.png/, @dummy.avatar.url
end
View
39 test/storage/s3_live_test.rb
@@ -25,7 +25,10 @@ class S3LiveTest < Test::Unit::TestCase
@dummy.avatar = @file
end
- teardown { @file.close }
+ teardown do
+ @file.close
+ @dummy.destroy
+ end
should "still return a Tempfile when sent #to_file" do
assert_equal Paperclip::Tempfile, @dummy.avatar.to_file.class
@@ -47,5 +50,39 @@ class S3LiveTest < Test::Unit::TestCase
end
end
end
+
+ context "An attachment that uses S3 for storage and has spaces in file name" do
+ setup do
+ rebuild_model :styles => { :thumb => "100x100", :square => "32x32#" },
+ :storage => :s3,
+ :bucket => ENV["S3_TEST_BUCKET"],
+ :s3_credentials => File.new(File.join(File.dirname(__FILE__), "..", "s3.yml"))
+
+ Dummy.delete_all
+ @dummy = Dummy.new
+ @dummy.avatar = File.new(File.join(File.dirname(__FILE__), '..', 'fixtures', 'spaced file.png'), 'rb')
+ @dummy.save
+ end
+
+ teardown { @dummy.destroy }
+
+ should "return an unescaped version for path" do
+ assert_match /.+\/spaced file\.png/, @dummy.avatar.path
+ end
+
+ should "return an escaped version for url" do
+ assert_match /.+\/spaced%20file\.png/, @dummy.avatar.url
+ end
+
+ should "be accessible" do
+ assert_match /200 OK/, `curl -I #{@dummy.avatar.url}`
+ end
+
+ should "be destoryable" do
+ url = @dummy.avatar.url
+ @dummy.destroy
+ assert_match /404 Not Found/, `curl -I #{url}`
+ end
+ end
end
end
View
7 test/storage/s3_test.rb
@@ -116,7 +116,6 @@ def rails_env(env)
rebuild_model :styles => { :large => ['500x500#', :jpg] },
:storage => :s3,
:bucket => "bucket",
- :path => ":attachment/:basename.:extension",
:s3_credentials => {
'access_key_id' => "12345",
'secret_access_key' => "54321"
@@ -126,7 +125,11 @@ def rails_env(env)
@dummy.avatar = File.new(File.join(File.dirname(__FILE__), '..', 'fixtures', 'spaced file.png'), 'rb')
end
- should "return an escaped version of url" do
+ should "return an unescaped version for path" do
+ assert_match /.+\/spaced file\.png/, @dummy.avatar.path
+ end
+
+ should "return an escaped version for url" do
assert_match /.+\/spaced%20file\.png/, @dummy.avatar.url
end
end
Please sign in to comment.
Something went wrong with that request. Please try again.