Merge pull request #14986 from dlangevin/trailing-slash-url-generation

Fixes URL generation with trailing_slash: true

Conflicts:
	actionpack/lib/action_dispatch/http/url.rb

3 files changed, 37 insertions, 7 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 92e94ee463..86e98c011c 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   Fix URL generation with `:trailing_slash` such that it does not add
+    a trailing slash after `.:format`
+
+    *Dan Langevin*
+
 *   Build full URI as string when processing path in integration tests for
     performance reasons.
 
@@ -131,4 +136,5 @@
 
     *Tony Wooster*
 
+
 Please check [4-1-stable](https://github.com/rails/rails/blob/4-1-stable/actionpack/CHANGELOG.md) for previous changes.
diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb
index a6c17f50a5..4cba4f5f37 100644
--- a/actionpack/lib/action_dispatch/http/url.rb
+++ b/actionpack/lib/action_dispatch/http/url.rb
@@ -37,13 +37,7 @@ module ActionDispatch
           path  = options[:script_name].to_s.chomp("/")
           path << options[:path].to_s
 
-          if options[:trailing_slash]
-            if path.include?('?')
-              path.sub!(/\?/, '/\&')
-            else
-              path.sub!(/[^\/]\z|\A\z/, '\&/')
-            end
-          end
+          add_trailing_slash(path) if options[:trailing_slash]
 
           result = path
 
@@ -66,6 +60,18 @@ module ActionDispatch
 
         private
 
+        def add_trailing_slash(path)
+          # includes querysting
+          if path.include?('?')
+            path.sub!(/\?/, '/\&')
+          # does not have a .format
+          elsif !path.include?(".")
+            path.sub!(/[^\/]\z|\A\z/, '\&/')
+          end
+
+          path
+        end
+
         def build_host_url(options)
           if match = options[:host].match(HOST_REGEXP)
             options[:protocol] ||= match[1] unless options[:protocol] == false
diff --git a/actionpack/test/dispatch/url_generation_test.rb b/actionpack/test/dispatch/url_generation_test.rb
index 910ff8a80f..a4dfd0a63d 100644
--- a/actionpack/test/dispatch/url_generation_test.rb
+++ b/actionpack/test/dispatch/url_generation_test.rb
@@ -15,6 +15,8 @@ module TestUrlGeneration
     Routes.draw do
       get "/foo", :to => "my_route_generating#index", :as => :foo
 
+      resources :bars
+
       mount MyRouteGeneratingController.action(:index), at: '/bar'
     end
 
@@ -109,6 +111,22 @@ module TestUrlGeneration
     test "omit subdomain when key is blank" do
       assert_equal "http://example.com/foo", foo_url(subdomain: "")
     end
+
+    test "generating URLs with trailing slashes" do
+      assert_equal "/bars.json", bars_path(
+        trailing_slash: true,
+        format: 'json'
+      )
+    end
+
+    test "generating URLS with querystring and trailing slashes" do
+      assert_equal "/bars.json?a=b", bars_path(
+        trailing_slash: true,
+        a: 'b',
+        format: 'json'
+      )
+    end
+
   end
 end