Merge branch 'dm-fix-routes' into 'master'

Fix ambiguous routing issues by teaching router about reserved words

See merge request !11570

4 files changed, 25 insertions, 6 deletions

diff --git a/config/routes/admin.rb b/config/routes/admin.rb
index 48993420ed9..b1b6ef33a47 100644
--- a/config/routes/admin.rb
+++ b/config/routes/admin.rb
@@ -68,7 +68,9 @@ namespace :admin do
 
   resources :projects, only: [:index]
 
-  scope(path: 'projects/*namespace_id', as: :namespace) do
+  scope(path: 'projects/*namespace_id',
+        as: :namespace,
+        constraints: { namespace_id: Gitlab::Regex.namespace_route_regex }) do
     resources(:projects,
               path: '/',
               constraints: { id: Gitlab::Regex.project_route_regex },
diff --git a/config/routes/git_http.rb b/config/routes/git_http.rb
index 42d874eeebc..cdf658c3e4a 100644
--- a/config/routes/git_http.rb
+++ b/config/routes/git_http.rb
@@ -1,4 +1,6 @@
-scope(path: '*namespace_id/:project_id', constraints: { format: nil }) do
+scope(path: '*namespace_id/:project_id',
+      format: nil,
+      constraints: { namespace_id: Gitlab::Regex.namespace_route_regex }) do
   scope(constraints: { project_id: Gitlab::Regex.project_git_route_regex }, module: :projects) do
     # Git HTTP clients ('git clone' etc.)
     scope(controller: :git_http) do
diff --git a/config/routes/project.rb b/config/routes/project.rb
index 01b94f9f2b8..9fe8372edf9 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -5,7 +5,22 @@ resources :projects, only: [:index, :new, :create]
 draw :git_http
 
 constraints(ProjectUrlConstrainer.new) do
-  scope(path: '*namespace_id', as: :namespace) do
+  # If the route has a wildcard segment, the segment has a regex constraint,
+  # the segment is potentially followed by _another_ wildcard segment, and
+  # the `format` option is not set to false, we need to specify that
+  # regex constraint _outside_ of `constraints: {}`.
+  #
+  # Otherwise, Rails will overwrite the constraint with `/.+?/`,
+  # which breaks some of our wildcard routes like `/blob/*id`
+  # and `/tree/*id` that depend on the negative lookahead inside
+  # `Gitlab::Regex.namespace_route_regex`, which helps the router
+  # determine whether a certain path segment is part of `*namespace_id`,
+  # `:project_id`, or `*id`.
+  #
+  # See https://github.com/rails/rails/blob/v4.2.8/actionpack/lib/action_dispatch/routing/mapper.rb#L155
+  scope(path: '*namespace_id',
+        as: :namespace,
+        namespace_id: Gitlab::Regex.namespace_route_regex) do
     scope(path: ':project_id',
           constraints: { project_id: Gitlab::Regex.project_route_regex },
           module: :projects,
diff --git a/config/routes/user.rb b/config/routes/user.rb
index b064a15e802..0f3bec9cf58 100644
--- a/config/routes/user.rb
+++ b/config/routes/user.rb
@@ -13,17 +13,17 @@ end
 
 constraints(UserUrlConstrainer.new) do
   # Get all keys of user
-  get ':username.keys' => 'profiles/keys#get_keys', constraints: { username: Gitlab::Regex.namespace_route_regex }
+  get ':username.keys' => 'profiles/keys#get_keys', constraints: { username: Gitlab::Regex.root_namespace_route_regex }
 
   scope(path: ':username',
         as: :user,
-        constraints: { username: Gitlab::Regex.namespace_route_regex },
+        constraints: { username: Gitlab::Regex.root_namespace_route_regex },
         controller: :users) do
     get '/', action: :show
   end
 end
 
-scope(constraints: { username: Gitlab::Regex.namespace_route_regex }) do
+scope(constraints: { username: Gitlab::Regex.root_namespace_route_regex }) do
   scope(path: 'users/:username',
         as: :user,
         controller: :users) do