From b1738a78ccf5768f92068a27255f9f69be1c3147 Mon Sep 17 00:00:00 2001 From: Azul Date: Mon, 24 Jul 2017 09:55:28 +0200 Subject: fix: #3 handle domains without A-record If a domain only has an mx record but no A record it will trigger a ConnectionError when attempting the nicknym key lookup. We need to detect and handle this in the http adapter already because once the exception is handled by Celluloid our actor will be terminated. So now we allow for handing a rescue option to the adapter with a string that is checked for inclusion in the error message. If the string is found the exception will be caught and the adapter returns nil. We only make use of this when checking the availability of nicknym so far. That should be the only http request going out. --- test/functional/sample_test.rb | 5 +++++ test/remote/Readme.md | 15 +++++++-------- test/remote/nicknym_source_test.rb | 15 +++++++++++++-- test/support/http_stub_helper.rb | 27 +++++++++++++-------------- test/unit/nicknym/source_test.rb | 13 ++++++++++--- 5 files changed, 48 insertions(+), 27 deletions(-) (limited to 'test') diff --git a/test/functional/sample_test.rb b/test/functional/sample_test.rb index 68127e1..4be5c26 100644 --- a/test/functional/sample_test.rb +++ b/test/functional/sample_test.rb @@ -24,6 +24,11 @@ class SampleTest < FunctionalTest # Regression Tests + # #3 handle missing A records + def test_nicknym + assert_lookup_status 404, 'postmaster@cs.ucl.ac.uk' + end + def test_no_file_descriptors_leak lookup 'test@mail.bitmask.net' before = open_files_count diff --git a/test/remote/Readme.md b/test/remote/Readme.md index 957ea12..53f5e65 100644 --- a/test/remote/Readme.md +++ b/test/remote/Readme.md @@ -1,15 +1,14 @@ Integration tests for clients of remote services ================================================ -The tests in this directory are integration test with remote services. However -we aims at testing the client side of the equation as that is what we control -here. +The tests in this directory are integration test with remote services. +However we aims at testing the client side of the equation as that is +what we control here. -So unexpected server behavious should *crash* the test if we are not dealing -with it properly yet and have no unit test for it. +So unexpected server behavious should *crash* the test if we are not +dealing with it properly yet and have no unit test for it. -Server responses that we do not expect but handle in the code and test in unit -tests make the test *skip*. +Server responses that we do not expect but handle in the code and test +in unit tests make the test *skip*. The Behaviour we would normally expect should make the test *pass* - diff --git a/test/remote/nicknym_source_test.rb b/test/remote/nicknym_source_test.rb index 4ca3033..b97f2b2 100644 --- a/test/remote/nicknym_source_test.rb +++ b/test/remote/nicknym_source_test.rb @@ -10,13 +10,24 @@ require 'nickserver/email_address' class RemoteNicknymSourceTest < CelluloidTest include HttpAdapterHelper - def test_availablility_check + def test_available_for_mail source.available_for? 'mail.bitmask.net' - refute source.available_for? 'dl.bitmask.net' # not a provider rescue HTTP::ConnectionError => e skip e.to_s end + # not a provider + def test_not_available + refute source.available_for? 'dl.bitmask.net' + rescue HTTP::ConnectionError => e + skip e.to_s + end + + # cs.ucl.ac.uk only has an MX not an A-record + def test_not_available_without_a_record + refute source.available_for? 'cs.ucl.ac.uk' + end + def test_successful_query response = source.query(email_with_key) skip if response.status == 404 diff --git a/test/support/http_stub_helper.rb b/test/support/http_stub_helper.rb index c9f2bfa..4e3d89b 100644 --- a/test/support/http_stub_helper.rb +++ b/test/support/http_stub_helper.rb @@ -10,32 +10,31 @@ module HttpStubHelper end def stub_nicknym_available_response(domain, response = {}) - stub_http_request :get, "https://#{domain}/provider.json", - response: response + stub_http_get "https://#{domain}/provider.json", + response, + Hash end def stub_sks_vindex_reponse(uid, response = {}) - stub_http_request :get, config.hkp_url, - query: {op: 'vindex', search: uid, exact: 'on', options: 'mr', fingerprint: 'on'}, - response: response + stub_http_get config.hkp_url, response, + query: {op: 'vindex', search: uid, exact: 'on', options: 'mr', fingerprint: 'on'} end def stub_sks_get_reponse(key_id, response = {}) - stub_http_request :get, config.hkp_url, - query: {op: 'get', search: "0x"+key_id, exact: 'on', options: 'mr'}, - response: response + stub_http_get config.hkp_url, response, + query: {op: 'get', search: "0x"+key_id, exact: 'on', options: 'mr'} end def stub_couch_response(uid, response = {}) query = "\?key=#{"%22#{uid}%22"}&reduce=false" - stub_http_request :get, - /#{Regexp.escape(config.couch_url)}.*#{query}/, - response: response + stub_http_get /#{Regexp.escape(config.couch_url)}.*#{query}/, + response end - def stub_http_request(verb, url, options = {}) - response = {status: 200, body: ""}.merge(options.delete(:response) || {}) - options = nil if options == {} + private + + def stub_http_get(url, response, options = nil) + response = {status: 200, body: ""}.merge(response || {}) adapter.expect :get, [response[:status], response[:body]], [url, options].compact end diff --git a/test/unit/nicknym/source_test.rb b/test/unit/nicknym/source_test.rb index f8c9b60..b17f22b 100644 --- a/test/unit/nicknym/source_test.rb +++ b/test/unit/nicknym/source_test.rb @@ -1,4 +1,6 @@ require 'test_helper' +require 'http' +require 'json' require 'nickserver/nicknym/source' require 'nickserver/email_address' @@ -20,6 +22,11 @@ class NicknymSourceTest < Minitest::Test refute available_on?(200, 'blablabla') end + # adapter rescues name resolution errors and returns nothing + def test_not_available_without_response + refute available_on? + end + def test_proxy_successful_query assert proxies_query_response?(200, 'dummy body') end @@ -39,9 +46,9 @@ class NicknymSourceTest < Minitest::Test adapter.verify end - def available_on?(status = 0, body = nil) - adapter.expect :get, [status, body], - ['https://remote.tld/provider.json'] + def available_on?(*args) + adapter.expect :get, args, + ['https://remote.tld/provider.json', Hash] available = source.available_for?('remote.tld') adapter.verify return available -- cgit v1.2.3