From 0135d57b017e4b044511249d534bd26ab77c063d Mon Sep 17 00:00:00 2001
From: Alexander Randa <randa.alex@gmail.com>
Date: Tue, 1 Aug 2017 17:30:29 +0300
Subject: [PATCH] Fix encoding error for WebHook logging

---
 app/services/web_hook_service.rb                |  8 +++++++-
 .../35815-webhook-log-encoding-error.yml        |  4 ++++
 spec/services/web_hook_service_spec.rb          | 17 +++++++++++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/unreleased/35815-webhook-log-encoding-error.yml

diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb
index 27c3ba197ac..2825478926a 100644
--- a/app/services/web_hook_service.rb
+++ b/app/services/web_hook_service.rb
@@ -101,7 +101,7 @@ class WebHookService
       request_headers: build_headers(hook_name),
       request_data: request_data,
       response_headers: format_response_headers(response),
-      response_body: response.body,
+      response_body: safe_response_body(response),
       response_status: response.code,
       internal_error_message: error_message
     )
@@ -124,4 +124,10 @@ class WebHookService
   def format_response_headers(response)
     response.headers.each_capitalized.to_h
   end
+
+  def safe_response_body(response)
+    return '' unless response.body
+
+    response.body.encode('UTF-8', invalid: :replace, undef: :replace, replace: '')
+  end
 end
diff --git a/changelogs/unreleased/35815-webhook-log-encoding-error.yml b/changelogs/unreleased/35815-webhook-log-encoding-error.yml
new file mode 100644
index 00000000000..76ec235086c
--- /dev/null
+++ b/changelogs/unreleased/35815-webhook-log-encoding-error.yml
@@ -0,0 +1,4 @@
+---
+title: Fix encoding error for WebHook logging
+merge_request: 13230
+author: Alexander Randa (@randaalex)
diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb
index e79c12daa1c..0d710db812c 100644
--- a/spec/services/web_hook_service_spec.rb
+++ b/spec/services/web_hook_service_spec.rb
@@ -112,6 +112,23 @@ describe WebHookService do
         end
       end
 
+      context 'with unsafe response body' do
+        before do
+          WebMock.stub_request(:post, project_hook.url).to_return(status: 200, body: "\xBB")
+          service_instance.execute
+        end
+
+        it 'log successful execution' do
+          expect(hook_log.trigger).to eq('push_hooks')
+          expect(hook_log.url).to eq(project_hook.url)
+          expect(hook_log.request_headers).to eq(headers)
+          expect(hook_log.response_body).to eq('')
+          expect(hook_log.response_status).to eq('200')
+          expect(hook_log.execution_duration).to be > 0
+          expect(hook_log.internal_error_message).to be_nil
+        end
+      end
+
       context 'should not log ServiceHooks' do
         let(:service_hook) { create(:service_hook) }
         let(:service_instance) { described_class.new(service_hook, data, 'service_hook') }