Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #9573 (reopened defect)

Opened 10 months ago

Last modified 8 months ago

[PATCH] Bug in actionpack: Routing.possible_controllers not sorted

Reported by: pkcahier Assigned to: nseckar@gmail.com
Priority: normal Milestone: 1.x
Component: ActionPack Version: edge
Severity: normal Keywords: tiny patch
Cc:

Description

Bug in routing.rb, line 290: Dir#{load_path}/**/*_controller.rb?.collect does not return the proper order for the rb files, which need to be sorted before. Otherwise different behavior is observed in different file systems. That line is the basic generator for the order in which routes will be matched to their rule(since it is the basis for the regular expression that does the matching), and the change of that order can alter behavior.

Example: With the route mapping: map.connect ':controller/:action/:id' The URL '/admin/statistics' will be sometimes mapped as: {"action"=>"index", "controller"=>"admin/statistics"} and sometimes as: {"action"=>"statistics", "controller"=>"admin"} when in both case the two following files existed: - app/controllers/admin/statistics_controller.rb - app/controllers/admin_controller.rb

The unpredictability of that behavior makes it slightly problematic. Preferred behavior would be load sub-controllers first as in attached patch(their existence is at least guaranteed)

Attachments

routing.rb (50.0 kB) - added by pkcahier on 09/17/07 01:15:15.
actionpack.routing.possible_controllers.collect.diff (0.6 kB) - added by pkcahier on 09/17/07 03:35:26.
action_pack_routing_sorts_possible_controllers.diff (1.3 kB) - added by danger on 11/10/07 17:32:32.
test case + bugfix

Change History

09/17/07 01:15:15 changed by pkcahier

  • attachment routing.rb added.

09/17/07 03:35:26 changed by pkcahier

  • attachment actionpack.routing.possible_controllers.collect.diff added.

09/23/07 22:42:46 changed by david

  • owner changed from core to nseckar@gmail.com.

09/23/07 23:10:52 changed by ulysses

  • status changed from new to closed.
  • resolution set to untested.

Needs test.

09/23/07 23:16:34 changed by ulysses

Could also use a comment to explain why a sort has been thrown in. Good work tracking that down.

11/10/07 17:32:32 changed by danger

  • attachment action_pack_routing_sorts_possible_controllers.diff added.

test case + bugfix

11/10/07 17:34:18 changed by danger

  • keywords changed from actionpack, routing to tiny patch.
  • status changed from closed to reopened.
  • resolution deleted.
  • summary changed from Bug in actionpack: Routing.possible_controllers.collect to [PATCH] Bug in actionpack: Routing.possible_controllers not sorted.

The test was actually quite easy: The current tests ran possible_controllers() and then sorted the results before asserting. The sort has now been moved into possible_controllers itself so the test isn't giving false negatives.