Ensure only IDs ending in .git perform git actionsfix-git-http-routing

It doesn't seem possible to set constraints based on format for project
IDs ending in .git, so set the constraint on the ID and ensure the
format is nil to avoid the case where the project ID is something like
project.git.foo.

2 files changed, 71 insertions, 18 deletions

diff --git a/config/routes.rb b/config/routes.rb
index 417289829db..4d122549633 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -442,22 +442,6 @@ Rails.application.routes.draw do
     resources(:projects, constraints: { id: /[a-zA-Z.0-9_\-]+(?<!\.atom)/ }, except:
               [:new, :create, :index], path: "/") do
 
-      # Allow /info/refs, /info/refs?service=git-upload-pack, and
-      # /info/refs?service=git-receive-pack, but nothing else.
-      #
-      git_http_handshake = lambda do |request|
-        request.query_string.blank? ||
-          request.query_string.match(/\Aservice=git-(upload|receive)-pack\z/)
-      end
-
-      ref_redirect = redirect do |params, request|
-        path = "#{params[:namespace_id]}/#{params[:project_id]}.git/info/refs"
-        path << "?#{request.query_string}" unless request.query_string.blank?
-        path
-      end
-
-      get '/info/refs', constraints: git_http_handshake, to: ref_redirect
-
       member do
         put :transfer
         delete :remove_fork
@@ -472,12 +456,28 @@ Rails.application.routes.draw do
 
       scope module: :projects do
         # Git HTTP clients ('git clone' etc.)
-        scope constraints: { format: /(git|wiki\.git)/ } do
+        scope constraints: { id: /.+\.git/, format: nil } do
           get '/info/refs', to: 'git_http#info_refs'
           post '/git-upload-pack', to: 'git_http#git_upload_pack'
           post '/git-receive-pack', to: 'git_http#git_receive_pack'
         end
 
+        # Allow /info/refs, /info/refs?service=git-upload-pack, and
+        # /info/refs?service=git-receive-pack, but nothing else.
+        #
+        git_http_handshake = lambda do |request|
+          request.query_string.blank? ||
+            request.query_string.match(/\Aservice=git-(upload|receive)-pack\z/)
+        end
+
+        ref_redirect = redirect do |params, request|
+          path = "#{params[:namespace_id]}/#{params[:project_id]}.git/info/refs"
+          path << "?#{request.query_string}" unless request.query_string.blank?
+          path
+        end
+
+        get '/info/refs', constraints: git_http_handshake, to: ref_redirect
+
         # Blob routes:
         get '/new/*id', to: 'blob#new', constraints: { id: /.+/ }, as: 'new_blob'
         post '/create/*id', to: 'blob#create', constraints: { id: /.+/ }, as: 'create_blob'
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index 594a60a4340..df6a6d5da7c 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -2,7 +2,7 @@ require "spec_helper"
 
 describe 'Git HTTP requests', lib: true do
   let(:user)    { create(:user) }
-  let(:project) { create(:project) }
+  let(:project) { create(:project, path: 'project.git-project') }
 
   it "gives WWW-Authenticate hints" do
     clone_get('doesnt/exist.git')
@@ -268,6 +268,59 @@ describe 'Git HTTP requests', lib: true do
     end
   end
 
+  context "when the project path doesn't end in .git" do
+    context "GET info/refs" do
+      let(:path) { "/#{project.path_with_namespace}/info/refs" }
+
+      context "when no params are added" do
+        before { get path }
+
+        it "redirects to the .git suffix version" do
+          expect(response).to redirect_to("/#{project.path_with_namespace}.git/info/refs")
+        end
+      end
+
+      context "when the upload-pack service is requested" do
+        let(:params) { { service: 'git-upload-pack' } }
+        before { get path, params }
+
+        it "redirects to the .git suffix version" do
+          expect(response).to redirect_to("/#{project.path_with_namespace}.git/info/refs?service=#{params[:service]}")
+        end
+      end
+
+      context "when the receive-pack service is requested" do
+        let(:params) { { service: 'git-receive-pack' } }
+        before { get path, params }
+
+        it "redirects to the .git suffix version" do
+          expect(response).to redirect_to("/#{project.path_with_namespace}.git/info/refs?service=#{params[:service]}")
+        end
+      end
+
+      context "when the params are anything else" do
+        let(:params) { { service: 'git-implode-pack' } }
+        before { get path, params }
+
+        it "redirects to the sign-in page" do
+          expect(response).to redirect_to(new_user_session_path)
+        end
+      end
+    end
+
+    context "POST git-upload-pack" do
+      it "fails to find a route" do
+        expect { clone_post(project.path_with_namespace) }.to raise_error(ActionController::RoutingError)
+      end
+    end
+
+    context "POST git-receive-pack" do
+      it "failes to find a route" do
+        expect { push_post(project.path_with_namespace) }.to raise_error(ActionController::RoutingError)
+      end
+    end
+  end
+
   def clone_get(project, options={})
     get "/#{project}/info/refs", { service: 'git-upload-pack' }, auth_env(*options.values_at(:user, :password))
   end