Fix ensure_canonical_path for top level routes

Don’t replace a substring of the path if it is part of the top level route.

E.g. When redirecting from `/groups/ups` to `/groups/foo`, be careful not to do `/grofoo/ups`.

Projects are unaffected by this issue, but I am grouping the `#ensure_canonical_path` tests similar to the group and user tests.

1 files changed, 121 insertions, 132 deletions

diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index 1d61719f1d0..d33e2ba1e53 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -53,40 +53,6 @@ describe UsersController do
       end
     end
 
-    context 'when requesting the canonical path' do
-      let(:user) { create(:user, username: 'CamelCaseUser') }
-
-      before { sign_in(user) }
-
-      context 'with exactly matching casing' do
-        it 'responds with success' do
-          get :show, username: user.username
-
-          expect(response).to be_success
-        end
-      end
-
-      context 'with different casing' do
-        it 'redirects to the correct casing' do
-          get :show, username: user.username.downcase
-
-          expect(response).to redirect_to(user)
-          expect(controller).not_to set_flash[:notice]
-        end
-      end
-    end
-
-    context 'when requesting a redirected path' do
-      let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') }
-
-      it 'redirects to the canonical path' do
-        get :show, username: redirect_route.path
-
-        expect(response).to redirect_to(user)
-        expect(controller).to set_flash[:notice].to(user_moved_message(redirect_route, user))
-      end
-    end
-
     context 'when a user by that username does not exist' do
       context 'when logged out' do
         it 'redirects to login page' do
@@ -131,40 +97,6 @@ describe UsersController do
         expect(assigns(:contributions_calendar).projects.count).to eq(2)
       end
     end
-
-    context 'when requesting the canonical path' do
-      let(:user) { create(:user, username: 'CamelCaseUser') }
-
-      before { sign_in(user) }
-
-      context 'with exactly matching casing' do
-        it 'responds with success' do
-          get :calendar, username: user.username
-
-          expect(response).to be_success
-        end
-      end
-
-      context 'with different casing' do
-        it 'redirects to the correct casing' do
-          get :calendar, username: user.username.downcase
-
-          expect(response).to redirect_to(user_calendar_path(user))
-          expect(controller).not_to set_flash[:notice]
-        end
-      end
-    end
-
-    context 'when requesting a redirected path' do
-      let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') }
-
-      it 'redirects to the canonical path' do
-        get :calendar, username: redirect_route.path
-
-        expect(response).to redirect_to(user_calendar_path(user))
-        expect(controller).to set_flash[:notice].to(user_moved_message(redirect_route, user))
-      end
-    end
   end
 
   describe 'GET #calendar_activities' do
@@ -187,38 +119,6 @@ describe UsersController do
       get :calendar_activities, username: user.username
       expect(response).to render_template('calendar_activities')
     end
-
-    context 'when requesting the canonical path' do
-      let(:user) { create(:user, username: 'CamelCaseUser') }
-
-      context 'with exactly matching casing' do
-        it 'responds with success' do
-          get :calendar_activities, username: user.username
-
-          expect(response).to be_success
-        end
-      end
-
-      context 'with different casing' do
-        it 'redirects to the correct casing' do
-          get :calendar_activities, username: user.username.downcase
-
-          expect(response).to redirect_to(user_calendar_activities_path(user))
-          expect(controller).not_to set_flash[:notice]
-        end
-      end
-    end
-
-    context 'when requesting a redirected path' do
-      let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') }
-
-      it 'redirects to the canonical path' do
-        get :calendar_activities, username: redirect_route.path
-
-        expect(response).to redirect_to(user_calendar_activities_path(user))
-        expect(controller).to set_flash[:notice].to(user_moved_message(redirect_route, user))
-      end
-    end
   end
 
   describe 'GET #snippets' do
@@ -241,38 +141,6 @@ describe UsersController do
         expect(JSON.parse(response.body)).to have_key('html')
       end
     end
-
-    context 'when requesting the canonical path' do
-      let(:user) { create(:user, username: 'CamelCaseUser') }
-
-      context 'with exactly matching casing' do
-        it 'responds with success' do
-          get :snippets, username: user.username
-
-          expect(response).to be_success
-        end
-      end
-
-      context 'with different casing' do
-        it 'redirects to the correct casing' do
-          get :snippets, username: user.username.downcase
-
-          expect(response).to redirect_to(user_snippets_path(user))
-          expect(controller).not_to set_flash[:notice]
-        end
-      end
-    end
-
-    context 'when requesting a redirected path' do
-      let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-username') }
-
-      it 'redirects to the canonical path' do
-        get :snippets, username: redirect_route.path
-
-        expect(response).to redirect_to(user_snippets_path(user))
-        expect(controller).to set_flash[:notice].to(user_moved_message(redirect_route, user))
-      end
-    end
   end
 
   describe 'GET #exists' do
@@ -321,6 +189,127 @@ describe UsersController do
     end
   end
 
+  describe '#ensure_canonical_path' do
+    before do
+      sign_in(user)
+    end
+
+    context 'for a GET request' do
+      context 'when requesting users at the root path' do
+        context 'when requesting the canonical path' do
+          let(:user) { create(:user, username: 'CamelCaseUser') }
+
+          context 'with exactly matching casing' do
+            it 'responds with success' do
+              get :show, username: user.username
+
+              expect(response).to be_success
+            end
+          end
+
+          context 'with different casing' do
+            it 'redirects to the correct casing' do
+              get :show, username: user.username.downcase
+
+              expect(response).to redirect_to(user)
+              expect(controller).not_to set_flash[:notice]
+            end
+          end
+        end
+
+        context 'when requesting a redirected path' do
+          let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-path') }
+
+          it 'redirects to the canonical path' do
+            get :show, username: redirect_route.path
+
+            expect(response).to redirect_to(user)
+            expect(controller).to set_flash[:notice].to(user_moved_message(redirect_route, user))
+          end
+
+          context 'when the old path is a substring of the scheme or host' do
+            let(:redirect_route) { user.namespace.redirect_routes.create(path: 'http') }
+
+            it 'does not modify the requested host' do
+              get :show, username: redirect_route.path
+
+              expect(response).to redirect_to(user)
+              expect(controller).to set_flash[:notice].to(user_moved_message(redirect_route, user))
+            end
+          end
+
+          context 'when the old path is substring of users' do
+            let(:redirect_route) { user.namespace.redirect_routes.create(path: 'ser') }
+
+            it 'redirects to the canonical path' do
+              get :show, username: redirect_route.path
+
+              expect(response).to redirect_to(user)
+              expect(controller).to set_flash[:notice].to(user_moved_message(redirect_route, user))
+            end
+          end
+        end
+      end
+
+      context 'when requesting users under the /users path' do
+        context 'when requesting the canonical path' do
+          let(:user) { create(:user, username: 'CamelCaseUser') }
+
+          context 'with exactly matching casing' do
+            it 'responds with success' do
+              get :projects, username: user.username
+
+              expect(response).to be_success
+            end
+          end
+
+          context 'with different casing' do
+            it 'redirects to the correct casing' do
+              get :projects, username: user.username.downcase
+
+              expect(response).to redirect_to(user_projects_path(user))
+              expect(controller).not_to set_flash[:notice]
+            end
+          end
+        end
+
+        context 'when requesting a redirected path' do
+          let(:redirect_route) { user.namespace.redirect_routes.create(path: 'old-path') }
+
+          it 'redirects to the canonical path' do
+            get :projects, username: redirect_route.path
+
+            expect(response).to redirect_to(user_projects_path(user))
+            expect(controller).to set_flash[:notice].to(user_moved_message(redirect_route, user))
+          end
+
+          context 'when the old path is a substring of the scheme or host' do
+            let(:redirect_route) { user.namespace.redirect_routes.create(path: 'http') }
+
+            it 'does not modify the requested host' do
+              get :projects, username: redirect_route.path
+
+              expect(response).to redirect_to(user_projects_path(user))
+              expect(controller).to set_flash[:notice].to(user_moved_message(redirect_route, user))
+            end
+          end
+
+          context 'when the old path is substring of users' do
+            let(:redirect_route) { user.namespace.redirect_routes.create(path: 'ser') }
+
+            # I.e. /users/ser should not become /ufoos/ser
+            it 'does not modify the /users part of the path' do
+              get :projects, username: redirect_route.path
+
+              expect(response).to redirect_to(user_projects_path(user))
+              expect(controller).to set_flash[:notice].to(user_moved_message(redirect_route, user))
+            end
+          end
+        end
+      end
+    end
+  end
+
   def user_moved_message(redirect_route, user)
     "User '#{redirect_route.path}' was moved to '#{user.full_path}'. Please update any links and bookmarks that may still have the old path."
   end