Test both GET and POST for git-upload-pack

config/routes.rb

@@ -429,8 +429,8 @@ Rails.application.routes.draw do
         # Git HTTP clients ('git clone' etc.)
         scope constraints: { format: /(git|wiki\.git)/ } do
           get '/info/refs', to: 'git_http#info_refs', only: :get
-          get '/git-upload-pack', to: 'git_http#git_upload_pack', only: :post
+          post '/git-upload-pack', to: 'git_http#git_upload_pack', only: :post
-          get '/git-receive-pack', to: 'git_http#git_receive_pack', only: :post
+          post '/git-receive-pack', to: 'git_http#git_receive_pack', only: :post
         end
 
         # Blob routes:

spec/requests/git_http_spec.rb

@@ -8,26 +8,26 @@ describe 'Git HTTP requests', lib: true do
     context "when the project doesn't exist" do
       context "when no authentication is provided" do
         it "responds with status 401" do
-          clone_get '/doesnt/exist.git/info/refs'
+          download('doesnt/exist.git') do |response|
-
+            expect(response.status).to eq(401)
-          expect(response.status).to eq(401)
+          end
         end
       end
 
       context "when username and password are provided" do
         context "when authentication fails" do
           it "responds with status 401" do
-            clone_get '/doesnt/exist.git/info/refs', user: user.username, password: "nope"
+            download('doesnt/exist.git', user: user.username, password: "nope") do |response|
-
+              expect(response.status).to eq(401)
-            expect(response.status).to eq(401)
+            end
           end
         end
 
         context "when authentication succeeds" do
           it "responds with status 404" do
-            clone_get '/doesnt/exist.git/info/refs', user: user.username, password: user.password
+            download('/doesnt/exist.git', user: user.username, password: user.password) do |response|
-
+              expect(response.status).to eq(404)
-            expect(response.status).to eq(404)
+            end
           end
         end
       end
@@ -38,23 +38,25 @@ describe 'Git HTTP requests', lib: true do
         wiki = ProjectWiki.new(project)
         project.update_attribute(:visibility_level, Project::PUBLIC)
 
-        clone_get "/#{wiki.repository.path_with_namespace}.git/info/refs"
+        download("/#{wiki.repository.path_with_namespace}.git") do |response|
-        json_body = ActiveSupport::JSON.decode(response.body)
+          json_body = ActiveSupport::JSON.decode(response.body)
-
+  
-        expect(response.status).to eq(200)
+          expect(response.status).to eq(200)
-        expect(json_body['RepoPath']).to include(wiki.repository.path_with_namespace)
+          expect(json_body['RepoPath']).to include(wiki.repository.path_with_namespace)
+        end
       end
     end
 
     context "when the project exists" do
-      let(:path) { clone_path(project) }
+      let(:path) { "#{project.path_with_namespace}.git" }
+      let(:env) { {} }
 
       context "when the project is public" do
         it "responds with status 200" do
           project.update_attribute(:visibility_level, Project::PUBLIC)
-          clone_get path
+          download(path, env) do |response|
-
+            expect(response.status).to eq(200)
-          expect(response.status).to eq(200)
+          end
         end
       end
 
@@ -65,33 +67,37 @@ describe 'Git HTTP requests', lib: true do
 
         context "when no authentication is provided" do
           it "responds with status 401" do
-            clone_get path
+            download(path, env) do |response|
-
+              expect(response.status).to eq(401)
-            expect(response.status).to eq(401)
+            end
           end
         end
 
         context "when username and password are provided" do
+          let(:env) { { user: user.username, password: 'nope' } }
+          
           context "when authentication fails" do
             it "responds with status 401" do
-              clone_get path, user: user.username, password: 'nope'
+              download(path, env) do |response|
-
+                expect(response.status).to eq(401)
-              expect(response.status).to eq(401)
+              end
             end
 
             context "when the user is IP banned" do
               it "responds with status 401" do
                 expect(Rack::Attack::Allow2Ban).to receive(:filter).and_return(true)
                 allow_any_instance_of(Rack::Request).to receive(:ip).and_return('1.2.3.4')
-
+                
-                clone_get path, user: user.username, password: 'nope'
+                clone_get(path, env)
-
+              
                 expect(response.status).to eq(401)
               end
             end
           end
 
           context "when authentication succeeds" do
+            let(:env) { { user: user.username, password: user.password } }
+            
             context "when the user has access to the project" do
               before do
                 project.team << [user, :master]
@@ -102,18 +108,18 @@ describe 'Git HTTP requests', lib: true do
                   user.block
                   project.team << [user, :master]
 
-                  clone_get path, user: user.username, password: user.password
+                  download(path, env) do |response|
-
+                    expect(response.status).to eq(404)
-                  expect(response.status).to eq(404)
+                  end
                 end
               end
 
               context "when the user isn't blocked" do
                 it "responds with status 200" do
                   expect(Rack::Attack::Allow2Ban).to receive(:reset)
-
+                  
-                  clone_get path, user: user.username, password: user.password
+                  clone_get(path, env)
-
+                  
                   expect(response.status).to eq(200)
                 end
               end
@@ -151,9 +157,9 @@ describe 'Git HTTP requests', lib: true do
 
             context "when the user doesn't have access to the project" do
               it "responds with status 404" do
-                clone_get path, user: user.username, password: user.password
+                download(path, user: user.username, password: user.password) do |response|
-
+                  expect(response.status).to eq(404)
-                expect(response.status).to eq(404)
+                end
               end
             end
           end
@@ -165,7 +171,7 @@ describe 'Git HTTP requests', lib: true do
             project = FactoryGirl.create :empty_project
             project.update_attributes(runners_token: token, builds_enabled: true)
 
-            clone_get clone_path(project), user: 'gitlab-ci-token', password: token
+            clone_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: token
 
             expect(response.status).to eq(200)
           end
@@ -174,17 +180,33 @@ describe 'Git HTTP requests', lib: true do
     end
   end
 
-  def clone_get(url, user: nil, password: nil)
+  def clone_get(project, options={})
-    if user && password
+    get "/#{project}/info/refs", { service: 'git-upload-pack' }, auth_env(*options.values_at(:user, :password))
-      env = { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials(user, password) }
+  end
-    else
+  
-      env = {}
+  def clone_post(project, options={})
-    end
+    post "/#{project}/git-upload-pack", {}, auth_env(*options.values_at(:user, :password))
-
-    get url, { 'service' => 'git-upload-pack' }, env
   end
 
   def clone_path(project)
     "/#{project.path_with_namespace}.git/info/refs"
   end
+
+  def download(project, user: nil, password: nil)
+    args = [project, {user: user, password: password}]
+
+    clone_get *args
+    yield response
+    
+    clone_post *args
+    yield response
+  end
+  
+  def auth_env(user, password)
+    if user && password
+      { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials(user, password) }
+    else
+      {}
+    end
+  end
 end