Permalink
Browse files

Escape interpolated params when redirecting - fixes #5688

  • Loading branch information...
1 parent f00ab1d commit 78c181b701d57a4960521b974510a84752e61af0 @pixeltrix pixeltrix committed Apr 29, 2012
Showing with 47 additions and 2 deletions.
  1. +12 −2 actionpack/lib/action_dispatch/routing/redirection.rb
  2. +35 −0 actionpack/test/dispatch/routing_test.rb
@@ -1,4 +1,6 @@
require 'action_dispatch/http/request'
+require 'active_support/core_ext/uri'
+require 'rack/utils'
module ActionDispatch
module Routing
@@ -43,7 +45,7 @@ def redirect(*args, &block)
path = args.shift
path_proc = if path.is_a?(String)
- proc { |params| (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % params) }
+ proc { |params| (params.empty? || !path.match(/%\{\w*\}/)) ? path : (path % escape(params)) }
elsif options.any?
options_proc(options)
elsif path.respond_to?(:call)
@@ -66,7 +68,7 @@ def options_proc(options)
elsif params.empty? || !options[:path].match(/%\{\w*\}/)
options.delete(:path)
else
- (options.delete(:path) % params)
+ (options.delete(:path) % escape_path(params))
end
default_options = {
@@ -105,6 +107,14 @@ def redirection_proc(status, path_proc)
end
end
+ def escape(params)
+ Hash[params.map{ |k,v| [k, Rack::Utils.escape(v)] }]
+ end
+
+ def escape_path(params)
+ Hash[params.map{ |k,v| [k, URI.parser.escape(v)] }]
+ end
+
end
end
end
@@ -2517,3 +2517,38 @@ def simple_app(response)
end
end
end
+
+class TestRedirectInterpolation < ActionDispatch::IntegrationTest
+ Routes = ActionDispatch::Routing::RouteSet.new.tap do |app|
+ app.draw do
+ 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 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 options 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
+ assert_equal url, @response.headers['Location']
+ assert_equal expected_redirect_body(url), @response.body
+ end
+
+ def expected_redirect_body(url)
+ %(<html><body>You are being <a href="#{ERB::Util.h(url)}">redirected</a>.</body></html>)
+ end
+end

0 comments on commit 78c181b

Please sign in to comment.