Restore interpolation of path option in redirect routes

2 files changed, 50 insertions, 1 deletions

diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb
index 866a2f5602..dc28389360 100644
--- a/actionpack/lib/action_dispatch/routing/redirection.rb
+++ b/actionpack/lib/action_dispatch/routing/redirection.rb
@@ -1,4 +1,5 @@
 require 'action_dispatch/http/request'
+require 'active_support/core_ext/uri'
 require 'rack/utils'
 
 module ActionDispatch
@@ -47,8 +48,17 @@ module ActionDispatch
           :params   => request.query_parameters
         }.merge options
 
+        if !params.empty? && url_options[:path].match(/%\{\w*\}/)
+          url_options[:path] = (url_options[:path] % escape_path(params))
+        end
+
         ActionDispatch::Http::URL.url_for url_options
       end
+
+      private
+        def escape_path(params)
+          Hash[params.map{ |k,v| [k, URI.parser.escape(v)] }]
+        end
     end
 
     module Redirection
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index 9e522d44fa..82bfa084f1 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -63,8 +63,13 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
       match 'secure', :to => redirect("/secure/login")
 
       match 'mobile', :to => redirect(:subdomain => 'mobile')
+      match 'documentation', :to => redirect(:domain => 'example-documentation.com', :path => '')
+      match 'new_documentation', :to => redirect(:path => '/documentation/new')
       match 'super_new_documentation', :to => redirect(:host => 'super-docs.com')
 
+      match 'stores/:name',        :to => redirect(:subdomain => 'stores', :path => '/%{name}')
+      match 'stores/:name(*rest)', :to => redirect(:subdomain => 'stores', :path => '/%{name}%{rest}')
+
       match 'youtube_favorites/:youtube_id/:name', :to => redirect(YoutubeFavoritesRedirector)
 
       constraints(lambda { |req| true }) do
@@ -727,6 +732,20 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     end
   end
 
+  def test_redirect_hash_with_domain_and_path
+    with_test_routes do
+      get '/documentation'
+      verify_redirect 'http://www.example-documentation.com'
+    end
+  end
+
+  def test_redirect_hash_with_path
+    with_test_routes do
+      get '/new_documentation'
+      verify_redirect 'http://www.example.com/documentation/new'
+    end
+  end
+
   def test_redirect_hash_with_host
     with_test_routes do
       get '/super_new_documentation?section=top'
@@ -734,6 +753,20 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     end
   end
 
+  def test_redirect_hash_path_substitution
+    with_test_routes do
+      get '/stores/iernest'
+      verify_redirect 'http://stores.example.com/iernest'
+    end
+  end
+
+  def test_redirect_hash_path_substitution_with_catch_all
+    with_test_routes do
+      get '/stores/iernest/products'
+      verify_redirect 'http://stores.example.com/iernest/products'
+    end
+  end
+
   def test_redirect_class
     with_test_routes do
       get '/youtube_favorites/oHg5SJYRHA0/rick-rolld'
@@ -2641,17 +2674,23 @@ class TestRedirectInterpolation < ActionDispatch::IntegrationTest
       ok = lambda { |env| [200, { 'Content-Type' => 'text/plain' }, []] }
 
       get "/foo/:id" => redirect("/foo/bar/%{id}")
+      get "/bar/:id" => redirect(:path => "/foo/bar/%{id}")
       get "/foo/bar/:id" => ok
     end
   end
 
   def app; Routes end
 
-  test "redirect escapes interpolated parameters" do
+  test "redirect escapes interpolated parameters with redirect proc" do
     get "/foo/1%3E"
     verify_redirect "http://www.example.com/foo/bar/1%3E"
   end
 
+  test "redirect escapes interpolated parameters with option proc" do
+    get "/bar/1%3E"
+    verify_redirect "http://www.example.com/foo/bar/1%3E"
+  end
+
 private
   def verify_redirect(url, status=301)
     assert_equal status, @response.status