aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndrew White <andrew.white@unboxed.co>2017-03-15 16:47:21 +0000
committerAndrew White <andrew.white@unboxed.co>2017-03-15 16:51:51 +0000
commit886085d6431e4f42191028d05519789f67d32bf8 (patch)
treeaef84a7418b87555d5d9aa4ab266b87177175e97
parent85c2b7565f37351c3d6091eab2a4b966a2f1c8e5 (diff)
downloadrails-886085d6431e4f42191028d05519789f67d32bf8.tar.gz
rails-886085d6431e4f42191028d05519789f67d32bf8.tar.bz2
rails-886085d6431e4f42191028d05519789f67d32bf8.zip
Remove unnecessary params munging
In 9b654d4 some params munging was added to ensure that they were set whenever `recognize_path` would call either a proc or callable constraint. Since we no longer mutate the environment hash within the method it's now unnecessary and actually causes params to leak between route matches before checking constraints. Fixes #28398.
-rw-r--r--actionpack/lib/action_dispatch/routing/route_set.rb3
-rw-r--r--actionpack/test/dispatch/routing_test.rb61
2 files changed, 62 insertions, 2 deletions
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index c4719f8a71..118dec2ad4 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -860,8 +860,7 @@ module ActionDispatch
params[key] = URI.parser.unescape(value)
end
end
- old_params = req.path_parameters
- req.path_parameters = old_params.merge params
+ req.path_parameters = params
app = route.app
if app.matches?(req) && app.dispatcher?
begin
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index d563df91df..64818e6ca1 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -4962,3 +4962,64 @@ class FlashRedirectTest < ActionDispatch::IntegrationTest
assert_equal "bar", response.body
end
end
+
+class TestRecognizePath < ActionDispatch::IntegrationTest
+ class PageConstraint
+ attr_reader :key, :pattern
+
+ def initialize(key, pattern)
+ @key = key
+ @pattern = pattern
+ end
+
+ def matches?(request)
+ request.path_parameters[key] =~ pattern
+ end
+ end
+
+ stub_controllers do |routes|
+ Routes = routes
+ routes.draw do
+ get "/hash/:foo", to: "pages#show", constraints: { foo: /foo/ }
+ get "/hash/:bar", to: "pages#show", constraints: { bar: /bar/ }
+
+ get "/proc/:foo", to: "pages#show", constraints: proc { |r| r.path_parameters[:foo] =~ /foo/ }
+ get "/proc/:bar", to: "pages#show", constraints: proc { |r| r.path_parameters[:bar] =~ /bar/ }
+
+ get "/class/:foo", to: "pages#show", constraints: PageConstraint.new(:foo, /foo/)
+ get "/class/:bar", to: "pages#show", constraints: PageConstraint.new(:bar, /bar/)
+ end
+ end
+
+ APP = build_app Routes
+ def app
+ APP
+ end
+
+ def test_hash_constraints_dont_leak_between_routes
+ expected_params = { controller: "pages", action: "show", bar: "bar" }
+ actual_params = recognize_path("/hash/bar")
+
+ assert_equal expected_params, actual_params
+ end
+
+ def test_proc_constraints_dont_leak_between_routes
+ expected_params = { controller: "pages", action: "show", bar: "bar" }
+ actual_params = recognize_path("/proc/bar")
+
+ assert_equal expected_params, actual_params
+ end
+
+ def test_class_constraints_dont_leak_between_routes
+ expected_params = { controller: "pages", action: "show", bar: "bar" }
+ actual_params = recognize_path("/class/bar")
+
+ assert_equal expected_params, actual_params
+ end
+
+ private
+
+ def recognize_path(*args)
+ Routes.recognize_path(*args)
+ end
+end