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

@@ -38,7 +38,7 @@ module Paperclip
 
 
     def timestamp_as_needed(url, options)
     def timestamp_as_needed(url, options)
       if options[:timestamp] && timestamp_possible?
       if options[:timestamp] && timestamp_possible?
-        delimiter_char = url.include?('?') ? '&' : '?'
+        delimiter_char = url.match(/\?.+=/) ? '&' : '?'
         "#{url}#{delimiter_char}#{@attachment.updated_at.to_s}"
         "#{url}#{delimiter_char}#{@attachment.updated_at.to_s}"
       else
       else
         url
         url
@@ -58,7 +58,7 @@ module Paperclip
     end
     end
 
 
     def escape_url(url)
     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
   end
 end
 end

test/url_generator_test.rb

@@ -146,8 +146,8 @@ class UrlGeneratorTest < Test::Unit::TestCase
     assert_equal "#{expected}?#{updated_at}", result
     assert_equal "#{expected}?#{updated_at}", result
   end
   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
     updated_at = 1231231234
     mock_interpolator = MockInterpolator.new(:result => expected)
     mock_interpolator = MockInterpolator.new(:result => expected)
     mock_attachment = MockAttachment.new(:updated_at => updated_at)
     mock_attachment = MockAttachment.new(:updated_at => updated_at)