From 128d664c1143d119922393fde342431c1523535e Mon Sep 17 00:00:00 2001 From: Prem Sichanugrist Date: Mon, 12 Dec 2011 00:57:55 -0500 Subject: [PATCH] URLGenerator should disregard a ? which isn't followed by an = In some case, we seriously don't want to prepend the timestamp with & if there wasn't a query string there. If the original URL doesn't have any query string, we should add the ?. --- lib/paperclip/url_generator.rb | 4 ++-- test/url_generator_test.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/paperclip/url_generator.rb b/lib/paperclip/url_generator.rb index a80f24653..a8b3d684f 100644 --- a/lib/paperclip/url_generator.rb +++ b/lib/paperclip/url_generator.rb @@ -38,7 +38,7 @@ def most_appropriate_url def timestamp_as_needed(url, options) if options[:timestamp] && timestamp_possible? - delimiter_char = url.include?('?') ? '&' : '?' + delimiter_char = url.match(/\?.+=/) ? '&' : '?' "#{url}#{delimiter_char}#{@attachment.updated_at.to_s}" else url @@ -58,7 +58,7 @@ def escape_url_as_needed(url, options) end def escape_url(url) - url.respond_to?(:escape) ? url.escape : URI.escape(url) + (url.respond_to?(:escape) ? url.escape : URI.escape(url)).gsub(/(\/.+)\?(.+\.)/, '\1%3F\2') end end end diff --git a/test/url_generator_test.rb b/test/url_generator_test.rb index b37182b38..76ea85657 100644 --- a/test/url_generator_test.rb +++ b/test/url_generator_test.rb @@ -146,8 +146,8 @@ def escape assert_equal "#{expected}?#{updated_at}", result end - should "produce URLs with the updated_at when it exists, separated with a & if a ? already exists" do - expected = "the?expected result" + should "produce URLs with the updated_at when it exists, separated with a & if a ? follow by = already exists" do + expected = "the?expected=result" updated_at = 1231231234 mock_interpolator = MockInterpolator.new(:result => expected) mock_attachment = MockAttachment.new(:updated_at => updated_at)