In url_for, never append ? when the query string is empty anyway.

It used to behave like this:

    url_for(controller: 'x', action: 'y', q: {})
    # -> "/x/y?"

We previously avoided empty query strings in most cases by removing
nil values, then checking whether params was empty.  But as you can
see above, even non-empty params can yield an empty query string.  So
I changed the code to just directly check whether the query string
ended up empty.

(To make everything more consistent, the "removing nil values"
functionality should probably move to ActionPack's Hash#to_query, the
place where empty hashes and arrays get removed.  However, this would
change a lot more behavior.)

1 files changed, 2 insertions, 1 deletions

diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb
index 92b10b6d3b..37f41ae988 100644
--- a/actionpack/lib/action_dispatch/http/url.rb
+++ b/actionpack/lib/action_dispatch/http/url.rb
@@ -81,7 +81,8 @@ module ActionDispatch
         def add_params(path, params)
           params = { params: params } unless params.is_a?(Hash)
           params.reject! { |_,v| v.to_param.nil? }
-          path << "?#{params.to_query}" unless params.empty?
+          query = params.to_query
+          path << "?#{query}" unless query.empty?
         end
 
         def add_anchor(path, anchor)