Skip to content
This repository
Browse code

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.
  • Loading branch information...
commit 56030506563352944fed12a6bb4793bb2462094b 1 parent b656134
Andrew White authored May 02, 2012
4  actionpack/lib/action_dispatch/http/parameters.rb
@@ -35,6 +35,10 @@ def path_parameters
35 35
         @env["action_dispatch.request.path_parameters"] ||= {}
36 36
       end
37 37
 
  38
+      def reset_parameters #:nodoc:
  39
+        @env.delete("action_dispatch.request.parameters")
  40
+      end
  41
+
38 42
     private
39 43
 
40 44
       # TODO: Validate that the characters are UTF-8. If they aren't,
2  actionpack/lib/action_dispatch/routing/mapper.rb
@@ -35,6 +35,8 @@ def matches?(env)
35 35
           }
36 36
 
37 37
           return true
  38
+        ensure
  39
+          req.reset_parameters
38 40
         end
39 41
 
40 42
         def call(env)
19  actionpack/test/dispatch/routing_test.rb
@@ -2512,3 +2512,22 @@ def expected_redirect_body(url)
2512 2512
     %(<html><body>You are being <a href="#{ERB::Util.h(url)}">redirected</a>.</body></html>)
2513 2513
   end
2514 2514
 end
  2515
+
  2516
+class TestConstraintsAccessingParameters < ActionDispatch::IntegrationTest
  2517
+  Routes = ActionDispatch::Routing::RouteSet.new.tap do |app|
  2518
+    app.draw do
  2519
+      ok = lambda { |env| [200, { 'Content-Type' => 'text/plain' }, []] }
  2520
+
  2521
+      get "/:foo" => ok, :constraints => lambda { |r| r.params[:foo] == 'foo' }
  2522
+      get "/:bar" => ok
  2523
+    end
  2524
+  end
  2525
+
  2526
+  def app; Routes end
  2527
+
  2528
+  test "parameters are reset between constraint checks" do
  2529
+    get "/bar"
  2530
+    assert_equal nil, @request.params[:foo]
  2531
+    assert_equal "bar", @request.params[:bar]
  2532
+  end
  2533
+end

0 notes on commit 5603050

Please sign in to comment.
Something went wrong with that request. Please try again.