Commit a120b789 authored by Stan Hu's avatar Stan Hu

Handle and report SSL errors in Web hook test. Check for status 200 for success.

If a Web hook test fails due to an SSL error or some other error, report
the result back to the user instead of an Error 500.

Closes #3656

Handle response
parent 238ca3e4
Please view this file on the master branch, on stable branches it's out of date. Please view this file on the master branch, on stable branches it's out of date.
v 8.3.0 (unreleased) v 8.3.0 (unreleased)
- Handle and report SSL errors in Web hook test (Stan Hu)
- Fix: Assignee selector is empty when 'Unassigned' is selected (Jose Corcuera) - Fix: Assignee selector is empty when 'Unassigned' is selected (Jose Corcuera)
- Fix 500 error when update group member permission - Fix 500 error when update group member permission
- Trim leading and trailing whitespace of milestone and issueable titles (Jose Corcuera) - Trim leading and trailing whitespace of milestone and issueable titles (Jose Corcuera)
......
...@@ -25,13 +25,12 @@ class Projects::HooksController < Projects::ApplicationController ...@@ -25,13 +25,12 @@ class Projects::HooksController < Projects::ApplicationController
def test def test
if !@project.empty_repo? if !@project.empty_repo?
status = TestHookService.new.execute(hook, current_user) status, message = TestHookService.new.execute(hook, current_user)
if status if status
flash[:notice] = 'Hook successfully executed.' flash[:notice] = 'Hook successfully executed.'
else else
flash[:alert] = 'Hook execution failed. '\ flash[:alert] = "Hook execution failed: #{message}"
'Ensure hook URL is correct and service is up.'
end end
else else
flash[:alert] = 'Hook execution failed. Ensure the project has commits.' flash[:alert] = 'Hook execution failed. Ensure the project has commits.'
......
...@@ -37,7 +37,7 @@ class WebHook < ActiveRecord::Base ...@@ -37,7 +37,7 @@ class WebHook < ActiveRecord::Base
def execute(data, hook_name) def execute(data, hook_name)
parsed_url = URI.parse(url) parsed_url = URI.parse(url)
if parsed_url.userinfo.blank? if parsed_url.userinfo.blank?
WebHook.post(url, response = WebHook.post(url,
body: data.to_json, body: data.to_json,
headers: { headers: {
"Content-Type" => "application/json", "Content-Type" => "application/json",
...@@ -50,7 +50,7 @@ class WebHook < ActiveRecord::Base ...@@ -50,7 +50,7 @@ class WebHook < ActiveRecord::Base
username: URI.decode(parsed_url.user), username: URI.decode(parsed_url.user),
password: URI.decode(parsed_url.password), password: URI.decode(parsed_url.password),
} }
WebHook.post(post_url, response = WebHook.post(post_url,
body: data.to_json, body: data.to_json,
headers: { headers: {
"Content-Type" => "application/json", "Content-Type" => "application/json",
...@@ -59,9 +59,11 @@ class WebHook < ActiveRecord::Base ...@@ -59,9 +59,11 @@ class WebHook < ActiveRecord::Base
verify: enable_ssl_verification, verify: enable_ssl_verification,
basic_auth: auth) basic_auth: auth)
end end
rescue SocketError, Errno::ECONNRESET, Errno::ECONNREFUSED, Net::OpenTimeout => e
[response.code == 200, ActionView::Base.full_sanitizer.sanitize(response.to_s)]
rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Net::OpenTimeout => e
logger.error("WebHook Error => #{e}") logger.error("WebHook Error => #{e}")
false [false, e.to_s]
end end
def async_execute(data, hook_name) def async_execute(data, hook_name)
......
...@@ -71,5 +71,11 @@ describe ProjectHook do ...@@ -71,5 +71,11 @@ describe ProjectHook do
expect { @project_hook.execute(@data, 'push_hooks') }.to raise_error(RuntimeError) expect { @project_hook.execute(@data, 'push_hooks') }.to raise_error(RuntimeError)
end end
it "handles SSL exceptions" do
expect(WebHook).to receive(:post).and_raise(OpenSSL::SSL::SSLError.new('SSL error'))
expect(@project_hook.execute(@data, 'push_hooks')).to eq([false, 'SSL error'])
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment