diff options
| author | Andrew White <andyw@pixeltrix.co.uk> | 2012-05-02 23:42:43 +0100 |
|---|---|---|
| committer | Andrew White <andyw@pixeltrix.co.uk> | 2012-05-02 23:58:40 +0100 |
| commit | 56030506563352944fed12a6bb4793bb2462094b (patch) | |
| tree | a682370c8a21922b0136dd2b4bfd5d83f01c0834 | |
| parent | b656134450d5d49d79ae288a537df64795e9d34d (diff) | |
| download | rails-56030506563352944fed12a6bb4793bb2462094b.tar.gz rails-56030506563352944fed12a6bb4793bb2462094b.tar.bz2 rails-56030506563352944fed12a6bb4793bb2462094b.zip | |
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.
| -rw-r--r-- | actionpack/lib/action_dispatch/http/parameters.rb | 4 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/routing/mapper.rb | 2 | ||||
| -rw-r--r-- | actionpack/test/dispatch/routing_test.rb | 19 |
3 files changed, 25 insertions, 0 deletions
diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index d9b63faf5e..bcfd0b0d00 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 1a1a054985..4ea3937057 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -35,6 +35,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 4b8d308043..a96d2edcf9 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -2512,3 +2512,22 @@ private %(<html><body>You are being <a href="#{ERB::Util.h(url)}">redirected</a>.</body></html>) 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 |