From 7c7fb3a862651d87c4071e40a1799b973f626b11 Mon Sep 17 00:00:00 2001 From: Andrew White Date: Wed, 2 May 2012 23:42:43 +0100 Subject: Reset the request parameters after a constraints check A callable object passed as a constraint for a route may access the request parameters as part of its check. This causes the combined parameters hash to be cached in the environment hash. If the constraint fails then any subsequent access of the request parameters will be against that stale hash. To fix this we delete the cache after every call to `matches?`. This may have a negative performance impact if the contraint wraps a large number of routes as the parameters hash is built by merging GET, POST and path parameters. Fixes #2510. (cherry picked from commit 56030506563352944fed12a6bb4793bb2462094b) --- actionpack/lib/action_dispatch/http/parameters.rb | 4 ++++ actionpack/lib/action_dispatch/routing/mapper.rb | 2 ++ actionpack/test/dispatch/routing_test.rb | 19 +++++++++++++++++++ 3 files changed, 25 insertions(+) diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index ef5d207b26..cba98544bb 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -35,6 +35,10 @@ module ActionDispatch @env["action_dispatch.request.path_parameters"] ||= {} end + def reset_parameters #:nodoc: + @env.delete("action_dispatch.request.parameters") + end + private # TODO: Validate that the characters are UTF-8. If they aren't, diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 4c018968bf..82062c0cd5 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -34,6 +34,8 @@ module ActionDispatch } return true + ensure + req.reset_parameters end def call(env) diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 82bfa084f1..5a6be058ec 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -2702,3 +2702,22 @@ private %(You are being redirected.) end end + +class TestConstraintsAccessingParameters < ActionDispatch::IntegrationTest + Routes = ActionDispatch::Routing::RouteSet.new.tap do |app| + app.draw do + ok = lambda { |env| [200, { 'Content-Type' => 'text/plain' }, []] } + + get "/:foo" => ok, :constraints => lambda { |r| r.params[:foo] == 'foo' } + get "/:bar" => ok + end + end + + def app; Routes end + + test "parameters are reset between constraint checks" do + get "/bar" + assert_equal nil, @request.params[:foo] + assert_equal "bar", @request.params[:bar] + end +end -- cgit v1.2.3