Try to escape each part of a path redirect route correctly

A path redirect may contain any and all parts of a url which have different
escaping rules for each part. This commit tries to escape each part correctly
by splitting the string into three chunks - path (which may also include a host),
query and fragment; then it applies the correct escape pattern to each part.

Whilst using `URI.parse` would be better, unfortunately the possible presence
of %{name} parameters in the path redirect string prevents us from using it so
we have to use a regular expression instead.

Fixes #13110.

actionpack/CHANGELOG.md

+*   Try to escape each part of a url correctly when using a redirect route.
+
+    Fixes #13110.
+
+    *Andrew White*
+
 *   Better error message for typos in assert_response argument.
 
     When the response type argument to `assert_response` is not a known

actionpack/lib/action_dispatch/routing/redirection.rb

@@ -57,11 +57,33 @@ def inspect
         def relative_path?(path)
           path && !path.empty? && path[0] != '/'
         end
+
+        def escape(params)
+          Hash[params.map{ |k,v| [k, Rack::Utils.escape(v)] }]
+        end
+
+        def escape_fragment(params)
+          Hash[params.map{ |k,v| [k, Journey::Router::Utils.escape_fragment(v)] }]
+        end
+
+        def escape_path(params)
+          Hash[params.map{ |k,v| [k, Journey::Router::Utils.escape_path(v)] }]
+        end
     end
 
     class PathRedirect < Redirect
+      URL_PARTS = /\A([^?]+)?(\?[^#]+)?(#.+)?\z/
+
       def path(params, request)
-        (params.empty? || !block.match(/%\{\w*\}/)) ? block : (block % escape(params))
+        if block.match(URL_PARTS)
+          path     = interpolation_required?($1, params) ? $1 % escape_path(params)     : $1
+          query    = interpolation_required?($2, params) ? $2 % escape(params)          : $2
+          fragment = interpolation_required?($3, params) ? $3 % escape_fragment(params) : $3
+
+          "#{path}#{query}#{fragment}"
+        else
+          interpolation_required?(block, params) ? block % escape(params) : block
+        end
       end
 
       def inspect
@@ -69,8 +91,8 @@ def inspect
       end
 
       private
-        def escape(params)
-          Hash[params.map{ |k,v| [k, Rack::Utils.escape(v)] }]
+        def interpolation_required?(string, params)
+          !params.empty? && string && string.match(/%\{\w*\}/)
         end
     end
 
@@ -101,11 +123,6 @@ def path(params, request)
       def inspect
         "redirect(#{status}, #{options.map{ |k,v| "#{k}: #{v}" }.join(', ')})"
       end
-
-      private
-        def escape_path(params)
-          Hash[params.map{ |k,v| [k, URI.parser.escape(v)] }]
-        end
     end
 
     module Redirection

actionpack/test/dispatch/routing_test.rb

@@ -3235,7 +3235,9 @@ class TestRedirectInterpolation < ActionDispatch::IntegrationTest
 
       get "/foo/:id" => redirect("/foo/bar/%{id}")
       get "/bar/:id" => redirect(:path => "/foo/bar/%{id}")
+      get "/baz/:id" => redirect("/baz?id=%{id}&foo=?&bar=1#id-%{id}")
       get "/foo/bar/:id" => ok
+      get "/baz" => ok
     end
   end
 
@@ -3251,6 +3253,14 @@ def app; Routes end
     verify_redirect "http://www.example.com/foo/bar/1%3E"
   end
 
+  test "path redirect escapes interpolated parameters correctly" do
+    get "/foo/1%201"
+    verify_redirect "http://www.example.com/foo/bar/1%201"
+
+    get "/baz/1%201"
+    verify_redirect "http://www.example.com/baz?id=1+1&foo=?&bar=1#id-1%201"
+  end
+
 private
   def verify_redirect(url, status=301)
     assert_equal status, @response.status