This specifically affects ColdBox 3.1.0, but probably affects lesser versions too.
The problem in a nutshell :
The route, /admin/staffnotices, mapped to a module stopped working and would always end up as an invalid event. Changing the module route from /admin/staffnotices to /adminstaffnotices (without the / in the middle) would cause the module to become accessible again via the route.
Nothing had really changed other than the addition of a couple modules and a new route.
The /admin/staffnotices route actually maps to a module called staffnoticesadmin. It’s one of about 16 modules in the application so far.
I’ve been working on some security changes and needed to be able to make sure I could consistently match the URL patterns, so while /adminstaffnotices is not that far out, I didn’t know why /admin/staffnotices didn’t work. This means that at any point during future developments the routing could fail and so could the security.
After a short time digging and some good old fashioned dumping to files I found myself looking around in the SES interceptor and the getRoutes() and findRoute() functions.
What I discovered is that findRoute() is not that great at finding routes. Don’t get me wrong there isn’t anything wrong with the findRoute() code. Its the array of routes that is the issue.
The array of routes is populated automatically with routes that match the modules in the application as well as those specified in Routes.cfm to provide SES URLs to those modules, as well as the catch all regex for :handler/:event/:id/, :handler/:event/ and :handler/.
When I looked at which of the routes was actually being picked up by findRoute() and the location of the /admin/staffnotice route in the array it all became clear.
We have a module called “admin” that provides a “dashboard” to the other admin applications. The route for this is automatically added. Looking in the route array it looked a little like this
- noticeboard/
- admin/articles/
- articles/
- admin/
- admin/staffnotices/
- :handler/:action/:id/
- :handler/:action/
- :handler/
This is obviously a simplification of the contents of the route array for the purposes of explanation.
The code in findRoute takes the current path and loops through the array of routes using the regex pattern in a reFindNoCase against the current path. So in my case my current path is “admin/staffnotices/” and it goes through and tries to use “noticeboard/” as regex against “admin/staffnotices/” and so on. The thing is the route “admin/” comes before “admin/staffnotices/” and will create a route match for the first part, “admin/”. This will send any requests for admin/staffnotice/ through the “admin” module. Obviously this is not correct.
I couldn’t fathom if there was actually any rhyme or reason to the order of the routes in the array, other than the obvious for the “:handler” prefixed patterns. As there doesn’t appear to be a logical reason for the order of the routes I worried that at any time a change to the application could suddenly prevent a previously working route from working, but shoving it below the automatic “admin” module pattern. First match is not a particularly good way to handle the routes, so I decided to make it so that I would get best match instead.
I actually tried a number of things, but in the end the simple solution was just to so sort the array of route in reverse order. Using the pattern attribute in the route information as the sort key means that the catch all “:handler” prefixed patterns are sorted in the correct order. Sorting by the Regex Pattern causes these routes to be reversed, so that “:handler/” would be first in line in the route finder instead of “:handler/:action/:id”. This gave me a set of routes that looked like this :
- noticeboard/
- articles/
- admin/staffnotices/
- admin/articles/
- admin/
- :handler/:action/:id/
- :handler/:action/
- :handler/
This way all the routes are in alphabetic order in reverse order, meaning that best matches are made before reaching the lesser matches.
Ideally the best solution would be to make sure that the routes are actually inserted into the array in the correct order, rather than having to sort the array each time that the findRoute function is called. I did contemplate making this change, but the code in the addRoute function made my brain bleed a bit trying to work out was relevant and what was not.
Something else I noticed when I sorted the array of routes was that some routes are in the array multiple times. While this doesn’t really cause a massive issue, I guess the addRoute function probably should make sure it isn’t duplicating route entries.
I hope that makes better sense than my previous thread.
Please do give me feed back on why this might be good/back/indifferent.
Regards
Stephen
SES.cfc (46 KB)
