Fix ambiguous routing issues by teaching router about reserved words
This fixes the 422 on https://gitlab.com/gitlab-org/gitlab-ce/blob/master/app/assets/javascripts/blob/viewer/index.js.
The Rails router interpreted that URL as { namespace_id: 'gitlab-org/gitlab-ce/blob/master/app/assets', project_id: 'javascripts', id: 'viewer/index.js' }
instead of the actual { namespace_id: 'gitlab-org', project_id: 'gitlab-ce', id: 'master/app/assets/javascripts/blob/viewer/index.js' }
.
Since namespace gitlab-org/gitlab-ce/blob/master/app/assets
obviously doesn't exist, this request gets handled by the *unmatched_route
route, which renders 404. We see 422 instead because some Rails middleware sees the .js
extension and raises an error:
Security warning: an embedded <script> tag on another site requested protected JavaScript. If you know what you're doing, go ahead and disable forgery protection on this action to permit cross-origin JavaScript embedding.
This will happen for any blob or tree URL that includes blob
or tree
in the path itself, because of Rails's greedy matching.
This MR "teaches" the router about reserved words using a constraint regex for namespace_id
and project_id
that includes a negative lookahead for these reserved words, so that the router will not even try to parse the namespace_id
or project_id
using those words.
This MR is so big because we need to construct appropriate regexes for the router to use, which the original design of DynamicPathValidator
did not allow for. I've refactored DynamicPathValidator
and extracted some of its previous responsibility into Gitlab::PathRegex
to give us the regexes we need, and hopefully make the whole thing easier to understand.
I think this issue was introduced by https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10413, because as far as I can see it exists on 9-2-stable but not 9-1-stable and because the issue seems related, but I cannot actually pinpoint the line in that MR that causes the issue, since it doesn't touch the routing files at all...
@reprazent Curious to hear your thoughts about the issue and my solution!