Commit 86a2b384 authored by Douwe Maan's avatar Douwe Maan Committed by Winnie Hellmann
Browse files

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"
parent 7fc3fe6abfae
...@@ -58,7 +58,7 @@ def request ...@@ -58,7 +58,7 @@ def request
end end
def last_visited_url 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 end
def route_hash def route_hash
...@@ -74,10 +74,16 @@ def sidekiq_route ...@@ -74,10 +74,16 @@ def sidekiq_route
end end
def grack_route 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' route_hash[:controller] == 'projects/git_http' && route_hash[:action] == 'git_upload_pack'
end end
def lfs_route 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' route_hash[:controller] == 'projects/lfs_api' && route_hash[:action] == 'batch'
end end
end end
......
...@@ -84,14 +84,23 @@ ...@@ -84,14 +84,23 @@
end end
it 'expects POST of new file that looks like an LFS batch url to be disallowed' do 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') response = request.post('/root/gitlab-ce/new/master/app/info/lfs/objects/batch')
expect(response).to be_a_redirect expect(response).to be_a_redirect
expect(subject).to disallow_request expect(subject).to disallow_request
end 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 context 'whitelisted requests' do
it 'expects a POST internal request to be allowed' 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") response = request.post("/api/#{API::API.version}/internal")
expect(response).not_to be_a_redirect expect(response).not_to be_a_redirect
...@@ -99,6 +108,7 @@ ...@@ -99,6 +108,7 @@
end end
it 'expects a POST LFS request to batch URL to be allowed' do 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') response = request.post('/root/rouge.git/info/lfs/objects/batch')
expect(response).not_to be_a_redirect expect(response).not_to be_a_redirect
...@@ -106,6 +116,7 @@ ...@@ -106,6 +116,7 @@
end end
it 'expects a POST request to git-upload-pack URL to be allowed' do 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') response = request.post('/root/rouge.git/git-upload-pack')
expect(response).not_to be_a_redirect expect(response).not_to be_a_redirect
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment