Merge pull request rails#43287 from Shopify/append-slash

Append trailing slash when the path is generated

Author: byroot

actionpack/lib/action_dispatch/http/url.rb

@@ -71,7 +71,8 @@ def path_for(options)
           path = options[:script_name].to_s.chomp("/")
           path << options[:path] if options.key?(:path)
 
-          add_trailing_slash(path) if options[:trailing_slash]
+          path = "/" if options[:trailing_slash] && path.blank?
+
           add_params(path, options[:params]) if options.key?(:params)
           add_anchor(path, options[:anchor]) if options.key?(:anchor)
 
@@ -101,14 +102,6 @@ def extract_subdomains_from(host, tld_length)
             parts[0..-(tld_length + 2)]
           end
 
-          def add_trailing_slash(path)
-            if path.include?("?")
-              path.sub!(/\?/, '/\&')
-            elsif !path.include?(".")
-              path.sub!(/[^\/]\z|\A\z/, '\&/')
-            end
-          end
-
           def build_host_url(host, port, protocol, options, path)
             if match = host.match(HOST_REGEXP)
               protocol ||= match[1] unless protocol == false

actionpack/lib/action_dispatch/routing/route_set.rb

@@ -820,6 +820,11 @@ def url_for(options, route_name = nil, url_strategy = UNKNOWN, method_name = nil
 
         route_with_params = generate(route_name, path_options, recall)
         path = route_with_params.path(method_name)
+
+        if options[:trailing_slash] && !options[:format] && !path.end_with?("/")
+          path += "/"
+        end
+
         params = route_with_params.params
 
         if options.key? :params

actionpack/test/dispatch/request_test.rb

@@ -42,9 +42,6 @@ class RequestUrlFor < BaseRequestTest
 
     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)

actionpack/test/dispatch/url_generation_test.rb

@@ -12,11 +12,16 @@ class ::MyRouteGeneratingController < ActionController::Base
       def index
         render plain: foo_path
       end
+
+      def add_trailing_slash
+        render plain: url_for(trailing_slash: true, params: request.query_parameters, format: params[:format])
+      end
     end
 
     Routes.draw do
       get "/foo", to: "my_route_generating#index", as: :foo
       get "(/optional/:optional_id)/baz", to: "my_route_generating#index", as: :baz
+      get "/add_trailing_slash", to: "my_route_generating#add_trailing_slash", as: :add_trailing_slash
 
       resources :bars
 
@@ -156,7 +161,42 @@ def app
       assert_equal "http://www.example.com/baz", baz_url("")
     end
 
+    test "generating the current URL with a trailing slashes" do
+      get "/add_trailing_slash"
+      assert_equal "http://www.example.com/add_trailing_slash/", response.body
+    end
+
+    test "generating the current URL with a trailing slashes and query string" do
+      get "/add_trailing_slash?a=b"
+      assert_equal "http://www.example.com/add_trailing_slash/?a=b", response.body
+    end
+
+    test "generating the current URL with a trailing slashes and format indicator" do
+      get "/add_trailing_slash.json"
+      assert_equal "http://www.example.com/add_trailing_slash.json", response.body
+    end
+
     test "generating URLs with trailing slashes" do
+      assert_equal "/bars/", bars_path(
+        trailing_slash: true,
+      )
+    end
+
+    test "generating URLs with trailing slashes and dot including param" do
+      assert_equal "/bars/hax0r.json/", bar_path(
+        "hax0r.json",
+        trailing_slash: true,
+      )
+    end
+
+    test "generating URLs with trailing slashes and query string" do
+      assert_equal "/bars/?a=b", bars_path(
+        trailing_slash: true,
+        a: "b"
+      )
+    end
+
+    test "generating URLs with trailing slashes and format" do
       assert_equal "/bars.json", bars_path(
         trailing_slash: true,
         format: "json"