From 880792a04eaed2409019d54961ecddd402d8471c Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Fri, 19 Oct 2018 05:55:06 +0000 Subject: [PATCH] Catch `RedirectionTooDeep` Exception in webhooks --- app/services/web_hook_service.rb | 2 +- .../unreleased/52692-catch-redirect-loops.yml | 5 ++++ lib/gitlab/http.rb | 7 +++++ spec/lib/gitlab/http_spec.rb | 26 +++++++++++++++++++ spec/services/web_hook_service_spec.rb | 2 +- 5 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/52692-catch-redirect-loops.yml diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 144d738dfd2..1fee8bfcd31 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -43,7 +43,7 @@ class WebHookService http_status: response.code, message: response.to_s } - rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Net::OpenTimeout, Net::ReadTimeout, Gitlab::HTTP::BlockedUrlError => e + rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Net::OpenTimeout, Net::ReadTimeout, Gitlab::HTTP::BlockedUrlError, Gitlab::HTTP::RedirectionTooDeep => e log_execution( trigger: hook_name, url: hook.url, diff --git a/changelogs/unreleased/52692-catch-redirect-loops.yml b/changelogs/unreleased/52692-catch-redirect-loops.yml new file mode 100644 index 00000000000..655124b8fb4 --- /dev/null +++ b/changelogs/unreleased/52692-catch-redirect-loops.yml @@ -0,0 +1,5 @@ +--- +title: Fix 500 error when testing webhooks with redirect loops +merge_request: 22447 +author: Heinrich Lee Yu +type: fixed diff --git a/lib/gitlab/http.rb b/lib/gitlab/http.rb index 9aca3b0fb26..538b51c0008 100644 --- a/lib/gitlab/http.rb +++ b/lib/gitlab/http.rb @@ -5,9 +5,16 @@ module Gitlab class HTTP BlockedUrlError = Class.new(StandardError) + RedirectionTooDeep = Class.new(StandardError) include HTTParty # rubocop:disable Gitlab/HTTParty connection_adapter ProxyHTTPConnectionAdapter + + def self.perform_request(http_method, path, options, &block) + super + rescue HTTParty::RedirectionTooDeep + raise RedirectionTooDeep + end end end diff --git a/spec/lib/gitlab/http_spec.rb b/spec/lib/gitlab/http_spec.rb index d0dadfa78da..6c37c157f5d 100644 --- a/spec/lib/gitlab/http_spec.rb +++ b/spec/lib/gitlab/http_spec.rb @@ -46,4 +46,30 @@ describe Gitlab::HTTP do end end end + + describe 'handle redirect loops' do + before do + WebMock.stub_request(:any, "http://example.org").to_raise(HTTParty::RedirectionTooDeep.new("Redirection Too Deep")) + end + + it 'handles GET requests' do + expect { described_class.get('http://example.org') }.to raise_error(Gitlab::HTTP::RedirectionTooDeep) + end + + it 'handles POST requests' do + expect { described_class.post('http://example.org') }.to raise_error(Gitlab::HTTP::RedirectionTooDeep) + end + + it 'handles PUT requests' do + expect { described_class.put('http://example.org') }.to raise_error(Gitlab::HTTP::RedirectionTooDeep) + end + + it 'handles DELETE requests' do + expect { described_class.delete('http://example.org') }.to raise_error(Gitlab::HTTP::RedirectionTooDeep) + end + + it 'handles HEAD requests' do + expect { described_class.head('http://example.org') }.to raise_error(Gitlab::HTTP::RedirectionTooDeep) + end + end end diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 622e56e1da5..b1cc6d2eb83 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -97,7 +97,7 @@ describe WebHookService do end it 'handles exceptions' do - exceptions = [SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Net::OpenTimeout, Net::ReadTimeout, Gitlab::HTTP::BlockedUrlError] + exceptions = [SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Net::OpenTimeout, Net::ReadTimeout, Gitlab::HTTP::BlockedUrlError, Gitlab::HTTP::RedirectionTooDeep] exceptions.each do |exception_class| exception = exception_class.new('Exception message')