Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

unwrap the constraints object on initialization, eliminate loops

Unwrap Constraints objects.  I don't actually think it's possible
to pass a Constraints object to this constructor, but there were
multiple places that kept testing children of this object.  I
*think* they were just being defensive, but I have no idea.
  • Loading branch information...
commit 98c7fe87690ca4de6c46e8f69806e82e3f8af42d 1 parent 107c27b
@tenderlove tenderlove authored
View
2  actionpack/lib/action_dispatch/journey/route.rb
@@ -19,7 +19,7 @@ def initialize(name, app, path, constraints, defaults = {})
# Unwrap any constraints so we can see what's inside for route generation.
# This allows the formatter to skip over any mounted applications or redirects
# that shouldn't be matched when using a url_for without a route name.
- while app.is_a?(Routing::Mapper::Constraints) do
+ if app.is_a?(Routing::Mapper::Constraints)
app = app.app
end
@dispatcher = app.is_a?(Routing::RouteSet::Dispatcher)
View
2  actionpack/lib/action_dispatch/routing/inspector.rb
@@ -16,7 +16,7 @@ def rack_app(app = self.app)
@rack_app ||= begin
class_name = app.class.name.to_s
if class_name == "ActionDispatch::Routing::Mapper::Constraints"
- rack_app(app.app)
+ app.app
elsif ActionDispatch::Routing::Redirect === app || class_name !~ /^ActionDispatch::Routing/
app
end
View
9 actionpack/lib/action_dispatch/routing/mapper.rb
@@ -19,6 +19,15 @@ class Constraints #:nodoc:
attr_reader :app, :constraints
def initialize(app, constraints, request)
+ # Unwrap Constraints objects. I don't actually think it's possible
+ # to pass a Constraints object to this constructor, but there were
+ # multiple places that kept testing children of this object. I
+ # *think* they were just being defensive, but I have no idea.
+ while app.is_a?(self.class)
+ constraints += app.constraints
+ app = app.app
+ end
+
@app, @constraints, @request = app, constraints, request
end
View
2  actionpack/lib/action_dispatch/routing/route_set.rb
@@ -704,7 +704,7 @@ def recognize_path(path, environment = {})
old_params = req.path_parameters
req.path_parameters = old_params.merge params
dispatcher = route.app
- while dispatcher.is_a?(Mapper::Constraints) && dispatcher.matches?(env) do
+ if dispatcher.is_a?(Mapper::Constraints) && dispatcher.matches?(env)
dispatcher = dispatcher.app
end

1 comment on commit 98c7fe8

@pixeltrix
Owner

The original source for the while pattern was in 8079484 - I've just assumed that it was possible when making some other fixes. Looking at it further it maybe technically possible but unlikely in real-world usage - you'd have to pass a Constraints instance as the endpoint for the route (i.e. via the :to option). Where'd you'd get such an instance without manually constructing one I don't know.

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