summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2017-11-21 16:10:47 +0000
committerWinnie Hellmann <winnie@gitlab.com>2017-11-21 19:59:41 +0000
commit86a2b38479b9942314dc698fb3b798a06bd31ed1 (patch)
treee2090398849eac799bff7127c94282f7710a1428
parenta96a0d707e1077f45f56b7adeb9b69eb01230a56 (diff)
downloadgitlab-ce-86a2b38479b9942314dc698fb3b798a06bd31ed1.tar.gz
Merge branch 'sh-optimize-read-only-check' into 'master'
Optimize read-only middleware so that it does not consume as much CPU Closes #40185 and gitlab-com/infrastructure#3240 See merge request gitlab-org/gitlab-ce!15504 (cherry picked from commit 6c3398165469de7890cd7881b175858f7c6be8e5) 3c52e2f0 Optimize read-only middleware so that it does not consume as much CPU cba68d33 use `Gitlab::Routing.url_helpers` instead of `Rails.application.routes.url_helpers` 91075c82 check for `read_only?` first before seeing if request is disallowed aeb2f49f Revert "check for `read_only?` first before seeing if request is disallowed"
-rw-r--r--lib/gitlab/middleware/read_only.rb8
-rw-r--r--spec/lib/gitlab/middleware/read_only_spec.rb11
2 files changed, 18 insertions, 1 deletions
diff --git a/lib/gitlab/middleware/read_only.rb b/lib/gitlab/middleware/read_only.rb
index 5e4932e4e57..c26656704d7 100644
--- a/lib/gitlab/middleware/read_only.rb
+++ b/lib/gitlab/middleware/read_only.rb
@@ -58,7 +58,7 @@ module Gitlab
end
def last_visited_url
- @env['HTTP_REFERER'] || rack_session['user_return_to'] || Rails.application.routes.url_helpers.root_url
+ @env['HTTP_REFERER'] || rack_session['user_return_to'] || Gitlab::Routing.url_helpers.root_url
end
def route_hash
@@ -74,10 +74,16 @@ module Gitlab
end
def grack_route
+ # Calling route_hash may be expensive. Only do it if we think there's a possible match
+ return false unless request.path.end_with?('.git/git-upload-pack')
+
route_hash[:controller] == 'projects/git_http' && route_hash[:action] == 'git_upload_pack'
end
def lfs_route
+ # Calling route_hash may be expensive. Only do it if we think there's a possible match
+ return false unless request.path.end_with?('/info/lfs/objects/batch')
+
route_hash[:controller] == 'projects/lfs_api' && route_hash[:action] == 'batch'
end
end
diff --git a/spec/lib/gitlab/middleware/read_only_spec.rb b/spec/lib/gitlab/middleware/read_only_spec.rb
index b14735943a5..07ba11b93a3 100644
--- a/spec/lib/gitlab/middleware/read_only_spec.rb
+++ b/spec/lib/gitlab/middleware/read_only_spec.rb
@@ -84,14 +84,23 @@ describe Gitlab::Middleware::ReadOnly do
end
it 'expects POST of new file that looks like an LFS batch url to be disallowed' do
+ expect(Rails.application.routes).to receive(:recognize_path).and_call_original
response = request.post('/root/gitlab-ce/new/master/app/info/lfs/objects/batch')
expect(response).to be_a_redirect
expect(subject).to disallow_request
end
+ it 'returns last_vistited_url for disallowed request' do
+ response = request.post('/test_request')
+
+ expect(response.location).to eq 'http://localhost/'
+ end
+
context 'whitelisted requests' do
it 'expects a POST internal request to be allowed' do
+ expect(Rails.application.routes).not_to receive(:recognize_path)
+
response = request.post("/api/#{API::API.version}/internal")
expect(response).not_to be_a_redirect
@@ -99,6 +108,7 @@ describe Gitlab::Middleware::ReadOnly do
end
it 'expects a POST LFS request to batch URL to be allowed' do
+ expect(Rails.application.routes).to receive(:recognize_path).and_call_original
response = request.post('/root/rouge.git/info/lfs/objects/batch')
expect(response).not_to be_a_redirect
@@ -106,6 +116,7 @@ describe Gitlab::Middleware::ReadOnly do
end
it 'expects a POST request to git-upload-pack URL to be allowed' do
+ expect(Rails.application.routes).to receive(:recognize_path).and_call_original
response = request.post('/root/rouge.git/git-upload-pack')
expect(response).not_to be_a_redirect