summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2016-06-09 15:01:10 +0000
committerDouwe Maan <douwe@gitlab.com>2016-06-09 15:01:10 +0000
commit3803b380b43af2bdb20dced27fce79511dfb2897 (patch)
tree03d357c03a6b6827fd18f63d1011d3994a919d04
parent30ee4ea6659c91ac6a249d700a9fcdd266676942 (diff)
parentbf63964b4d1f42f7f091e25c81b87a1ddb110cba (diff)
downloadgitlab-ce-3803b380b43af2bdb20dced27fce79511dfb2897.tar.gz
Merge branch 'fix-git-http-routing' into 'master'
Ensure only IDs ending in .git perform git actions ## What does this MR do? Rails's routing is pretty strange. Previously, `GET /namespace/project/info/refs` would go to the Git HTTP controller (if the redirect for that case was taken out). ## Are there points in the code the reviewer needs to double check? The specs fail if the redirect is moved to above the Git HTTP routes, removed altogether, or the Git HTTP constraints are changed. But there might still be missing cases. ## Why was this MR needed? The master build and HTTP cloning were both broken. ## What are the relevant issue numbers? Closes #18376. ## Screenshots (if relevant) Nope. ## Does this MR meet the acceptance criteria? - [x] [not needed] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added - [x] [not needed] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md) - [ ] (not needed) API support added - [ ] Tests - [x] Added for this feature/bug - [ ] All builds are passing - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [ ] Branch has no merge conflicts with `master` (if you do - rebase it please) - [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) See merge request !4558
-rw-r--r--config/routes.rb34
-rw-r--r--spec/requests/git_http_spec.rb83
2 files changed, 99 insertions, 18 deletions
diff --git a/config/routes.rb b/config/routes.rb
index 4191ec3598c..95fbe7dd9df 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..c44a4a7a1fc 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,87 @@ 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
+
+ context "retrieving an info/refs file" do
+ before { project.update_attribute(:visibility_level, Project::PUBLIC) }
+
+ context "when the file exists" do
+ before do
+ # Provide a dummy file in its place
+ allow_any_instance_of(Repository).to receive(:blob_at).and_call_original
+ allow_any_instance_of(Repository).to receive(:blob_at).with('5937ac0a7beb003549fc5fd26fc247adbce4a52e', 'info/refs') do
+ Gitlab::Git::Blob.find(project.repository, 'master', '.gitignore')
+ end
+
+ get "/#{project.path_with_namespace}/blob/master/info/refs"
+ end
+
+ it "returns the file" do
+ expect(response.status).to eq(200)
+ end
+ end
+
+ context "when the file exists" do
+ before { get "/#{project.path_with_namespace}/blob/master/info/refs" }
+
+ it "returns not found" do
+ expect(response.status).to eq(404)
+ end
+ end
+ end
+
def clone_get(project, options={})
get "/#{project}/info/refs", { service: 'git-upload-pack' }, auth_env(*options.values_at(:user, :password))
end