Make URL escaping more consistent

1. Escape '%' characters in URLs - only unescaped data
   should be passed to URL helpers

2. Add an `escape_segment` helper to `Router::Utils`
   that escapes '/' characters

3. Use `escape_segment` rather than `escape_fragment`
   in optimized URL generation

4. Use `escape_segment` rather than `escape_path`
   in URL generation

For point 4 there are two exceptions. Firstly, when a route uses wildcard
segments (e.g. *foo) then we use `escape_path` as the value may contain '/'
characters. This means that wildcard routes can't be optimized. Secondly,
if a `:controller` segment is used in the path then this uses `escape_path`
as the controller may be namespaced.

Fixes #14629, #14636 and #14070.

1 files changed, 17 insertions, 4 deletions

diff --git a/actionpack/lib/action_dispatch/journey/visitors.rb b/actionpack/lib/action_dispatch/journey/visitors.rb
index daade5bb74..d9f634623d 100644
--- a/actionpack/lib/action_dispatch/journey/visitors.rb
+++ b/actionpack/lib/action_dispatch/journey/visitors.rb
@@ -114,19 +114,26 @@ module ActionDispatch
         end
 
         private
+          def escape_path(value)
+            Router::Utils.escape_path(value)
+          end
+
+          def escape_segment(value)
+            Router::Utils.escape_segment(value)
+          end
 
           def visit(node, optional = false)
             case node.type
             when :LITERAL, :SLASH, :DOT
               node.left
             when :STAR
-              visit(node.left)
+              visit_STAR(node.left)
             when :GROUP
               visit(node.left, true)
             when :CAT
               visit_CAT(node, optional)
             when :SYMBOL
-              visit_SYMBOL(node)
+              visit_SYMBOL(node, node.to_sym)
             end
           end
 
@@ -141,9 +148,15 @@ module ActionDispatch
             end
           end
 
-          def visit_SYMBOL(node)
+          def visit_STAR(node)
             if value = options[node.to_sym]
-              Router::Utils.escape_path(value)
+              escape_path(value)
+            end
+          end
+
+          def visit_SYMBOL(node, name)
+            if value = options[name]
+              name == :controller ? escape_path(value) : escape_segment(value)
             end
           end
       end