Skip to content

Commit

Permalink
Merge branch '52692-catch-redirect-loops' into 'master'
Browse files Browse the repository at this point in the history
Catch `RedirectionTooDeep` Exception in webhooks

Closes #52692

See merge request gitlab-org/gitlab-ce!22447
  • Loading branch information
stanhu committed Oct 19, 2018
2 parents b9cb0e1 + 880792a commit 64fabd5
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 2 deletions.
2 changes: 1 addition & 1 deletion app/services/web_hook_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def execute
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,
Expand Down
5 changes: 5 additions & 0 deletions changelogs/unreleased/52692-catch-redirect-loops.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
title: Fix 500 error when testing webhooks with redirect loops
merge_request: 22447
author: Heinrich Lee Yu
type: fixed
7 changes: 7 additions & 0 deletions lib/gitlab/http.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
26 changes: 26 additions & 0 deletions spec/lib/gitlab/http_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,30 @@
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
2 changes: 1 addition & 1 deletion spec/services/web_hook_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@
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')

Expand Down

0 comments on commit 64fabd5

Please sign in to comment.