diff options
author | Andrew White <andrew.white@unboxed.co> | 2017-03-15 16:47:21 +0000 |
---|---|---|
committer | Andrew White <andrew.white@unboxed.co> | 2017-03-15 16:51:51 +0000 |
commit | 886085d6431e4f42191028d05519789f67d32bf8 (patch) | |
tree | aef84a7418b87555d5d9aa4ab266b87177175e97 | |
parent | 85c2b7565f37351c3d6091eab2a4b966a2f1c8e5 (diff) | |
download | rails-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.rb | 3 | ||||
-rw-r--r-- | actionpack/test/dispatch/routing_test.rb | 61 |
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 |