diff options
author | Douwe Maan <douwe@gitlab.com> | 2017-11-21 16:10:47 +0000 |
---|---|---|
committer | Winnie Hellmann <winnie@gitlab.com> | 2017-11-21 19:59:41 +0000 |
commit | 86a2b38479b9942314dc698fb3b798a06bd31ed1 (patch) | |
tree | e2090398849eac799bff7127c94282f7710a1428 | |
parent | a96a0d707e1077f45f56b7adeb9b69eb01230a56 (diff) | |
download | gitlab-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.rb | 8 | ||||
-rw-r--r-- | spec/lib/gitlab/middleware/read_only_spec.rb | 11 |
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 |