Remove invalid conditions from route [#4989 state:resolved]

Signed-off-by: José Valim <jose.valim@gmail.com>

3 files changed, 26 insertions, 4 deletions

diff --git a/actionpack/lib/action_dispatch/routing/route.rb b/actionpack/lib/action_dispatch/routing/route.rb
index 6f37eadd6b..aefebf8f80 100644
--- a/actionpack/lib/action_dispatch/routing/route.rb
+++ b/actionpack/lib/action_dispatch/routing/route.rb
@@ -2,9 +2,10 @@ module ActionDispatch
   module Routing
     class Route #:nodoc:
       attr_reader :app, :conditions, :defaults, :name
-      attr_reader :path, :requirements
+      attr_reader :path, :requirements, :set
 
-      def initialize(app, conditions, requirements, defaults, name, anchor)
+      def initialize(set, app, conditions, requirements, defaults, name, anchor)
+        @set = set
         @app = app
         @defaults = defaults
         @name = name
@@ -24,6 +25,9 @@ module ActionDispatch
           h[k] = Rack::Mount::RegexpWithNamedGroups.new(v)
           h
         }
+
+        @conditions.delete_if{ |k,v| k != :path_info && !valid_condition?(k) }
+        @requirements.delete_if{ |k,v| !valid_condition?(k) }
       end
 
       def verb
@@ -50,6 +54,11 @@ module ActionDispatch
           "%-6s %-40s %s" % [(verb || :any).to_s.upcase, path, requirements.inspect]
         end
       end
+
+      private
+        def valid_condition?(method)
+          segment_keys.include?(method) || set.valid_conditions.include?(method)
+        end
     end
   end
 end
diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb
index 05200f0338..10de4853c5 100644
--- a/actionpack/lib/action_dispatch/routing/route_set.rb
+++ b/actionpack/lib/action_dispatch/routing/route_set.rb
@@ -189,7 +189,7 @@ module ActionDispatch
 
       attr_accessor :set, :routes, :named_routes
       attr_accessor :disable_clear_and_finalize, :resources_path_names
-      attr_accessor :default_url_options, :request_class
+      attr_accessor :default_url_options, :request_class, :valid_conditions
 
       def self.default_resources_path_names
         { :new => 'new', :edit => 'edit' }
@@ -202,6 +202,7 @@ module ActionDispatch
         self.controller_namespaces = Set.new
         self.default_url_options = {}
         self.request_class = request_class
+        self.valid_conditions = request_class.public_instance_methods.select{ |m| m != "id" }.map{ |m| m.to_sym }
 
         @disable_clear_and_finalize = false
         clear!
@@ -279,7 +280,7 @@ module ActionDispatch
       end
 
       def add_route(app, conditions = {}, requirements = {}, defaults = {}, name = nil, anchor = true)
-        route = Route.new(app, conditions, requirements, defaults, name, anchor)
+        route = Route.new(self, app, conditions, requirements, defaults, name, anchor)
         @set.add_route(*route)
         named_routes[name] = route if name
         routes << route
diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb
index cf92b039e3..0548456b63 100644
--- a/actionpack/test/dispatch/routing_test.rb
+++ b/actionpack/test/dispatch/routing_test.rb
@@ -330,6 +330,10 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
 
       resources :content
 
+      scope :constraints => { :id => /\d+/ } do
+        get '/tickets', :to => 'tickets#index', :as => :tickets
+      end
+
       match '/:locale/*file.:format', :to => 'files#show', :file => /path\/to\/existing\/file/
     end
   end
@@ -1546,6 +1550,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest
     end
   end
 
+  def test_router_removes_invalid_conditions
+    with_test_routes do
+      get '/tickets'
+      assert_equal 'tickets#index', @response.body
+      assert_equal '/tickets', tickets_path
+    end
+  end
+
   private
     def with_test_routes
       yield