Correct the use of params options when given to url_for

Merge url for tests and add changelog entry for #8233.

3 files changed, 16 insertions, 10 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 0f1e1f303f..04a3bf8697 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,5 +1,17 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   Fix error when using a non-hash query argument named "params" in `url_for`.
+
+    Before:
+
+        url_for(params: "") # => undefined method `reject!' for "":String
+
+    After:
+
+        url_for(params: "") # => http://www.example.com?params=
+
+    *tumayun + Carlos Antonio da Silva*
+
 *   Render every partial with a new `ActionView::PartialRenderer`. This resolves
     issues when rendering nested partials.
     Fix #8197
diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb
index 7183939934..bced7d84c0 100644
--- a/actionpack/lib/action_dispatch/http/url.rb
+++ b/actionpack/lib/action_dispatch/http/url.rb
@@ -28,7 +28,7 @@ module ActionDispatch
           path  = options.delete(:script_name).to_s.chomp("/")
           path << options.delete(:path).to_s
 
-          params = options[:params].is_a?(Hash) ? options[:params] : {}
+          params = options[:params].is_a?(Hash) ? options[:params] : options.slice(:params)
           params.reject! { |_,v| v.to_param.nil? }
 
           result = build_host_url(options)
diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb
index 08d1c6fe73..f2bacf3e20 100644
--- a/actionpack/test/dispatch/request_test.rb
+++ b/actionpack/test/dispatch/request_test.rb
@@ -3,7 +3,7 @@ require 'abstract_unit'
 class RequestTest < ActiveSupport::TestCase
 
   def url_for(options = {})
-    options.reverse_merge!(:host => 'www.example.com')
+    options = { host: 'www.example.com' }.merge!(options)
     ActionDispatch::Http::URL.url_for(options)
   end
 
@@ -25,6 +25,8 @@ class RequestTest < ActiveSupport::TestCase
     assert_equal 'http://www.example.com/',       url_for(:trailing_slash => true)
     assert_equal 'http://dhh:supersecret@www.example.com', url_for(:user => 'dhh', :password => 'supersecret')
     assert_equal 'http://www.example.com?search=books',    url_for(:params => { :search => 'books' })
+    assert_equal 'http://www.example.com?params=',  url_for(:params => '')
+    assert_equal 'http://www.example.com?params=1', url_for(:params => 1)
   end
 
   test "remote ip" do
@@ -799,14 +801,6 @@ class RequestTest < ActiveSupport::TestCase
     end
   end
 
-  test "url_for options[:params]" do
-    assert_equal 'http://www.example.com?params=', url_for(:params => '')
-    assert_equal 'http://www.example.com?params=1', url_for(:params => 1)
-    assert_equal 'http://www.example.com', url_for
-    assert_equal 'http://www.example.com', url_for(:params => {})
-    assert_equal 'http://www.example.com?name=tumayun', url_for(:params => { :name => 'tumayun' })
-  end
-
 protected
 
   def stub_request(env = {})