Merge pull request #8704 from senny/remove_regexp_global_from_url_for

replace regexp global in #url_for

2 files changed, 9 insertions, 2 deletions

diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb
index 43f26d696d..e9ef9ee9f2 100644
--- a/actionpack/lib/action_dispatch/http/url.rb
+++ b/actionpack/lib/action_dispatch/http/url.rb
@@ -32,8 +32,12 @@ module ActionDispatch
           params.reject! { |_,v| v.to_param.nil? }
 
           result = build_host_url(options)
-          if options[:trailing_slash] && !path.ends_with?('/')
-            result << path.sub(/(\?|\z)/) { "/" + $& }
+          if options[:trailing_slash]
+            if path.include?('?')
+              result << path.sub(/\?/, '/\&')
+            else
+              result << path.sub(/[^\/]\z|\A\z/, '\&/')
+            end
           else
             result << path
           end
diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb
index 8a01b29340..91810864d5 100644
--- a/actionpack/test/dispatch/request_test.rb
+++ b/actionpack/test/dispatch/request_test.rb
@@ -13,6 +13,9 @@ class RequestTest < ActiveSupport::TestCase
 
     assert_equal '/books', url_for(:only_path => true, :path => '/books')
 
+    assert_equal 'http://www.example.com/books/?q=code', url_for(trailing_slash: true, path: '/books?q=code')
+    assert_equal 'http://www.example.com/books/?spareslashes=////', url_for(trailing_slash: true, path: '/books?spareslashes=////')
+
     assert_equal 'http://www.example.com',  url_for
     assert_equal 'http://api.example.com',  url_for(:subdomain => 'api')
     assert_equal 'http://example.com',      url_for(:subdomain => false)