Merge branch '52692-catch-redirect-loops' into 'master'

Catch `RedirectionTooDeep` Exception in webhooks

Closes #52692

See merge request gitlab-org/gitlab-ce!22447

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,

changelogs/unreleased/52692-catch-redirect-loops.yml

+---
+title: Fix 500 error when testing webhooks with redirect loops
+merge_request: 22447
+author: Heinrich Lee Yu
+type: fixed

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

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

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')