1 files changed, 54 insertions, 31 deletions

diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb
index c0727e52cc3..f2f94563e56 100644
--- a/app/services/web_hook_service.rb
+++ b/app/services/web_hook_service.rb
@@ -26,6 +26,12 @@ class WebHookService
   end
 
   REQUEST_BODY_SIZE_LIMIT = 25.megabytes
+  # Response body is for UI display only. It does not make much sense to save
+  # whatever the receivers throw back at us
+  RESPONSE_BODY_SIZE_LIMIT = 8.kilobytes
+  # The headers are for debugging purpose. They are displayed on the UI only.
+  RESPONSE_HEADERS_COUNT_LIMIT = 50
+  RESPONSE_HEADERS_SIZE_LIMIT = 1.kilobytes
 
   attr_accessor :hook, :data, :hook_name, :request_options
   attr_reader :uniqueness_token
@@ -98,7 +104,7 @@ class WebHookService
 
   def async_execute
     Gitlab::ApplicationContext.with_context(hook.application_context) do
-      break log_rate_limited if rate_limited?
+      break log_rate_limited if rate_limit!
       break log_recursion_blocked if recursion_blocked?
 
       params = {
@@ -141,7 +147,7 @@ class WebHookService
       execution_duration: execution_duration,
       request_headers: build_headers,
       request_data: data,
-      response_headers: format_response_headers(response),
+      response_headers: safe_response_headers(response),
       response_body: safe_response_body(response),
       response_status: response.code,
       internal_error_message: error_message
@@ -150,8 +156,21 @@ class WebHookService
     if @force # executed as part of test - run log-execution inline.
       ::WebHooks::LogExecutionService.new(hook: hook, log_data: log_data, response_category: category).execute
     else
-      ::WebHooks::LogExecutionWorker
-        .perform_async(hook.id, log_data, category, uniqueness_token)
+      queue_log_execution_with_retry(log_data, category)
+    end
+  end
+
+  def queue_log_execution_with_retry(log_data, category)
+    retried = false
+    begin
+      ::WebHooks::LogExecutionWorker.perform_async(hook.id, log_data, category, uniqueness_token)
+    rescue Gitlab::SidekiqMiddleware::SizeLimiter::ExceedLimitError
+      raise if retried
+
+      # Strip request data
+      log_data[:request_data] = ::WebHookLog::OVERSIZE_REQUEST_DATA
+      retried = true
+      retry
     end
   end
 
@@ -181,52 +200,56 @@ class WebHookService
   # Make response headers more stylish
   # Net::HTTPHeader has downcased hash with arrays: { 'content-type' => ['text/html; charset=utf-8'] }
   # This method format response to capitalized hash with strings: { 'Content-Type' => 'text/html; charset=utf-8' }
-  def format_response_headers(response)
-    response.headers.each_capitalized.to_h
+  # rubocop:disable Style/HashTransformValues
+  def safe_response_headers(response)
+    response.headers.each_capitalized.first(RESPONSE_HEADERS_COUNT_LIMIT).to_h do |header_key, header_value|
+      [enforce_utf8(header_key), string_size_limit(enforce_utf8(header_value), RESPONSE_HEADERS_SIZE_LIMIT)]
+    end
   end
+  # rubocop:enable Style/HashTransformValues
 
   def safe_response_body(response)
     return '' unless response.body
 
-    response.body.encode('UTF-8', invalid: :replace, undef: :replace, replace: '')
+    response_body = enforce_utf8(response.body)
+    string_size_limit(response_body, RESPONSE_BODY_SIZE_LIMIT)
   end
 
-  def rate_limited?
-    return false if rate_limit.nil?
-
-    Gitlab::ApplicationRateLimiter.throttled?(
-      :web_hook_calls,
-      scope: [hook],
-      threshold: rate_limit
-    )
+  # Increments rate-limit counter.
+  # Returns true if hook should be rate-limited.
+  def rate_limit!
+    Gitlab::WebHooks::RateLimiter.new(hook).rate_limit!
   end
 
   def recursion_blocked?
     Gitlab::WebHooks::RecursionDetection.block?(hook)
   end
 
-  def rate_limit
-    @rate_limit ||= hook.rate_limit
+  def log_rate_limited
+    log_auth_error('Webhook rate limit exceeded')
   end
 
-  def log_rate_limited
-    Gitlab::AuthLogger.error(
-      message: 'Webhook rate limit exceeded',
-      hook_id: hook.id,
-      hook_type: hook.type,
-      hook_name: hook_name,
-      **Gitlab::ApplicationContext.current
+  def log_recursion_blocked
+    log_auth_error(
+      'Recursive webhook blocked from executing',
+      recursion_detection: ::Gitlab::WebHooks::RecursionDetection.to_log(hook)
     )
   end
 
-  def log_recursion_blocked
+  def log_auth_error(message, params = {})
     Gitlab::AuthLogger.error(
-      message: 'Recursive webhook blocked from executing',
-      hook_id: hook.id,
-      hook_type: hook.type,
-      hook_name: hook_name,
-      recursion_detection: ::Gitlab::WebHooks::RecursionDetection.to_log(hook),
-      **Gitlab::ApplicationContext.current
+      params.merge(
+        { message: message, hook_id: hook.id, hook_type: hook.type, hook_name: hook_name },
+        Gitlab::ApplicationContext.current
+      )
     )
   end
+
+  def string_size_limit(str, limit)
+    str.truncate_bytes(limit)
+  end
+
+  def enforce_utf8(str)
+    Gitlab::EncodingHelper.encode_utf8(str)
+  end
 end