Commit ed903367 authored by GitLab Bot's avatar GitLab Bot

Add latest changes from gitlab-org/security/gitlab@13-12-stable-ee

parent bca6156a
......@@ -173,6 +173,7 @@ module Integrations
query_params[:os_authType] = 'basic'
params[:basic_auth] = basic_auth
params[:use_read_total_timeout] = true
params
end
......
......@@ -50,9 +50,11 @@ class DroneCiService < CiService
end
def calculate_reactive_cache(sha, ref)
response = Gitlab::HTTP.try_get(commit_status_path(sha, ref),
response = Gitlab::HTTP.try_get(
commit_status_path(sha, ref),
verify: enable_ssl_verification,
extra_log_info: { project_id: project_id })
extra_log_info: { project_id: project_id },
use_read_total_timeout: true)
status =
if response && response.code == 200 && response['status']
......
......@@ -38,7 +38,7 @@ class ExternalWikiService < Integration
end
def execute(_data)
response = Gitlab::HTTP.get(properties['external_wiki_url'], verify: true)
response = Gitlab::HTTP.get(properties['external_wiki_url'], verify: true, use_read_total_timeout: true)
response.body if response.code == 200
rescue StandardError
nil
......
......@@ -106,7 +106,7 @@ class IssueTrackerService < Integration
result = false
begin
response = Gitlab::HTTP.head(self.project_url, verify: true)
response = Gitlab::HTTP.head(self.project_url, verify: true, use_read_total_timeout: true)
if response
message = "#{self.type} received response #{response.code} when attempting to connect to #{self.project_url}"
......
......@@ -56,7 +56,7 @@ class MockCiService < CiService
#
#
def commit_status(sha, ref)
response = Gitlab::HTTP.get(commit_status_path(sha), verify: false)
response = Gitlab::HTTP.get(commit_status_path(sha), verify: false, use_read_total_timeout: true)
read_commit_status(response)
rescue Errno::ECONNREFUSED
:error
......
......@@ -17,7 +17,7 @@ module SlackMattermost
class HTTPClient
def self.post(uri, params = {})
params.delete(:http_options) # these are internal to the client and we do not want them
Gitlab::HTTP.post(uri, body: params)
Gitlab::HTTP.post(uri, body: params, use_read_total_timeout: true)
end
end
end
......
......@@ -169,7 +169,7 @@ class TeamcityService < CiService
end
def get_path(path)
Gitlab::HTTP.try_get(build_url(path), verify: false, basic_auth: basic_auth, extra_log_info: { project_id: project_id })
Gitlab::HTTP.try_get(build_url(path), verify: false, basic_auth: basic_auth, extra_log_info: { project_id: project_id }, use_read_total_timeout: true)
end
def post_to_build_queue(data, branch)
......@@ -179,7 +179,8 @@ class TeamcityService < CiService
"<buildType id=#{build_type.encode(xml: :attr)}/>"\
'</build>',
headers: { 'Content-type' => 'application/xml' },
basic_auth: basic_auth
basic_auth: basic_auth,
use_read_total_timeout: true
)
end
......
......@@ -48,7 +48,8 @@ class UnifyCircuitService < ChatNotificationService
response = Gitlab::HTTP.post(webhook, body: {
subject: message.project_name,
text: message.summary,
markdown: true
markdown: true,
use_read_total_timeout: true
}.to_json)
response if response.success?
......
......@@ -43,7 +43,7 @@ class WebexTeamsService < ChatNotificationService
def notify(message, opts)
header = { 'Content-Type' => 'application/json' }
response = Gitlab::HTTP.post(webhook, headers: header, body: { markdown: message.summary }.to_json)
response = Gitlab::HTTP.post(webhook, headers: header, body: { markdown: message.summary }.to_json, use_read_total_timeout: true)
response if response.success?
end
......
......@@ -41,6 +41,7 @@ class WebHookService
@hook_name = hook_name.to_s
@request_options = {
timeout: Gitlab.config.gitlab.webhook_timeout,
use_read_total_timeout: true,
allow_local_requests: hook.allow_local_requests?
}
end
......@@ -67,7 +68,7 @@ class WebHookService
{
status: :success,
http_status: response.code,
message: response.to_s
message: response.body
}
rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Errno::EHOSTUNREACH,
Net::OpenTimeout, Net::ReadTimeout, Gitlab::HTTP::BlockedUrlError, Gitlab::HTTP::RedirectionTooDeep,
......
......@@ -8,9 +8,10 @@ module Gitlab
class HTTP
BlockedUrlError = Class.new(StandardError)
RedirectionTooDeep = Class.new(StandardError)
ReadTotalTimeout = Class.new(Net::ReadTimeout)
HTTP_TIMEOUT_ERRORS = [
Net::OpenTimeout, Net::ReadTimeout, Net::WriteTimeout
Net::OpenTimeout, Net::ReadTimeout, Net::WriteTimeout, Gitlab::HTTP::ReadTotalTimeout
].freeze
HTTP_ERRORS = HTTP_TIMEOUT_ERRORS + [
SocketError, OpenSSL::SSL::SSLError, OpenSSL::OpenSSLError,
......@@ -23,6 +24,7 @@ module Gitlab
read_timeout: 20,
write_timeout: 30
}.freeze
DEFAULT_READ_TOTAL_TIMEOUT = 20.seconds
include HTTParty # rubocop:disable Gitlab/HTTParty
......@@ -41,7 +43,19 @@ module Gitlab
options
end
httparty_perform_request(http_method, path, options_with_timeouts, &block)
unless options.has_key?(:use_read_total_timeout)
return httparty_perform_request(http_method, path, options_with_timeouts, &block)
end
start_time = Gitlab::Metrics::System.monotonic_time
read_total_timeout = options.fetch(:timeout, DEFAULT_READ_TOTAL_TIMEOUT)
httparty_perform_request(http_method, path, options_with_timeouts) do |fragment|
elapsed = Gitlab::Metrics::System.monotonic_time - start_time
raise ReadTotalTimeout, "Request timed out after #{elapsed} seconds" if elapsed > read_total_timeout
block.call fragment if block
end
rescue HTTParty::RedirectionTooDeep
raise RedirectionTooDeep
rescue *HTTP_ERRORS => e
......
......@@ -27,6 +27,47 @@ RSpec.describe Gitlab::HTTP do
end
end
context 'when reading the response is too slow' do
before do
stub_const("#{described_class}::DEFAULT_READ_TOTAL_TIMEOUT", 0.001.seconds)
WebMock.stub_request(:post, /.*/).to_return do |request|
sleep 0.002.seconds
{ body: 'I\m slow', status: 200 }
end
end
let(:options) { {} }
subject(:request_slow_responder) { described_class.post('http://example.org', **options) }
specify do
expect { request_slow_responder }.not_to raise_error
end
context 'with use_read_total_timeout option' do
let(:options) { { use_read_total_timeout: true } }
it 'raises a timeout error' do
expect { request_slow_responder }.to raise_error(Gitlab::HTTP::ReadTotalTimeout, /Request timed out after ?([0-9]*[.])?[0-9]+ seconds/)
end
context 'and timeout option' do
let(:options) { { use_read_total_timeout: true, timeout: 10.seconds } }
it 'overrides the default timeout when timeout option is present' do
expect { request_slow_responder }.not_to raise_error
end
end
end
end
it 'calls a block' do
WebMock.stub_request(:post, /.*/)
expect { |b| described_class.post('http://example.org', &b) }.to yield_with_args
end
describe 'allow_local_requests_from_web_hooks_and_services is' do
before do
WebMock.stub_request(:get, /.*/).to_return(status: 200, body: 'Success')
......
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