1 files changed, 43 insertions, 12 deletions

diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb
index b5abc46e80c..2ef2e61babc 100644
--- a/spec/services/web_hook_service_spec.rb
+++ b/spec/services/web_hook_service_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
-describe WebHookService, services: true do
-  let(:project) { create(:empty_project) }
+describe WebHookService do
+  let(:project) { create(:project) }
   let(:project_hook) { create(:project_hook) }
   let(:headers) do
     {
@@ -12,10 +12,24 @@ describe WebHookService, services: true do
   let(:data) do
     { before: 'oldrev', after: 'newrev', ref: 'ref' }
   end
-  let(:service_instance) { WebHookService.new(project_hook, data, 'push_hooks') }
+  let(:service_instance) { described_class.new(project_hook, data, :push_hooks) }
+
+  describe '#initialize' do
+    it 'allow_local_requests is true if hook is a SystemHook' do
+      instance = described_class.new(build(:system_hook), data, :system_hook)
+      expect(instance.request_options[:allow_local_requests]).to be_truthy
+    end
+
+    it 'allow_local_requests is false if hook is not a SystemHook' do
+      %i(project_hook service_hook web_hook_log).each do |hook|
+        instance = described_class.new(build(hook), data, hook)
+        expect(instance.request_options[:allow_local_requests]).to be_falsey
+      end
+    end
+  end
 
   describe '#execute' do
-    before(:each) do
+    before do
       project.hooks << [project_hook]
 
       WebMock.stub_request(:post, project_hook.url)
@@ -53,12 +67,12 @@ describe WebHookService, services: true do
     end
 
     it 'handles exceptions' do
-      exceptions = [SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Net::OpenTimeout]
+      exceptions = [SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Net::OpenTimeout, Net::ReadTimeout]
       exceptions.each do |exception_class|
         exception = exception_class.new('Exception message')
 
         WebMock.stub_request(:post, project_hook.url).to_raise(exception)
-        expect(service_instance.execute).to eq([nil, exception.message])
+        expect(service_instance.execute).to eq({ status: :error, message: exception.message })
         expect { service_instance.execute }.not_to raise_error
       end
     end
@@ -66,13 +80,13 @@ describe WebHookService, services: true do
     it 'handles 200 status code' do
       WebMock.stub_request(:post, project_hook.url).to_return(status: 200, body: 'Success')
 
-      expect(service_instance.execute).to eq([200, 'Success'])
+      expect(service_instance.execute).to include({ status: :success, http_status: 200, message: 'Success' })
     end
 
     it 'handles 2xx status codes' do
       WebMock.stub_request(:post, project_hook.url).to_return(status: 201, body: 'Success')
 
-      expect(service_instance.execute).to eq([201, 'Success'])
+      expect(service_instance.execute).to include({ status: :success, http_status: 201, message: 'Success' })
     end
 
     context 'execution logging' do
@@ -112,9 +126,26 @@ describe WebHookService, services: true 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) { WebHookService.new(service_hook, data, 'service_hook') }
+        let(:service_instance) { described_class.new(service_hook, data, 'service_hook') }
 
         before do
           WebMock.stub_request(:post, service_hook.url).to_return(status: 200, body: 'Success')
@@ -127,11 +158,11 @@ describe WebHookService, services: true do
 
   describe '#async_execute' do
     let(:system_hook) { create(:system_hook) }
-    
+
     it 'enqueue WebHookWorker' do
-      expect(Sidekiq::Client).to receive(:enqueue).with(WebHookWorker, project_hook.id, data, 'push_hooks')
+      expect(WebHookWorker).to receive(:perform_async).with(project_hook.id, data, 'push_hooks')
 
-      WebHookService.new(project_hook, data, 'push_hooks').async_execute
+      described_class.new(project_hook, data, 'push_hooks').async_execute
     end
   end
 end