From fa83b39ff3ae3fbad93df002915c0e5f99c104a9 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 18 May 2020 09:06:55 -0400 Subject: [PATCH 1/3] init: Remove confusing and redundant InitError The "A fatal internal error occurred, see debug.log for details" is redundant because init.cpp will already show an InitError with a better error message as well as the hint to check the debug.log --- src/httprpc.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 3c3e6e5bb..f17101ef5 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -9,7 +9,6 @@ #include #include #include -#include #include #include #include @@ -251,9 +250,6 @@ static bool InitRPCAuthentication() { LogPrintf("No rpcpassword set - using random cookie authentication.\n"); if (!GenerateAuthCookie(&strRPCUserColonPass)) { - uiInterface.ThreadSafeMessageBox( - _("Error: A fatal internal error occurred, see debug.log for details"), // Same message as AbortNode - "", CClientUIInterface::MSG_ERROR); return false; } } else { From fa12a37b27f0570a551b8c103ea6537ee4a8e399 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 19 May 2020 10:43:12 -0400 Subject: [PATCH 2/3] test: Replace inline-comments with logs, pep8 formatting --- test/functional/rpc_users.py | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/test/functional/rpc_users.py b/test/functional/rpc_users.py index b75ce15f2..976065f3f 100755 --- a/test/functional/rpc_users.py +++ b/test/functional/rpc_users.py @@ -20,6 +20,7 @@ import string import configparser import sys + def call_with_auth(node, user, password): url = urllib.parse.urlparse(node.url) headers = {"Authorization": "Basic " + str_to_b64str('{}:{}'.format(user, password))} @@ -64,9 +65,9 @@ class HTTPBasicsTest(BitcoinTestFramework): self.password = lines[3] with open(os.path.join(get_datadir_path(self.options.tmpdir, 0), "bitcoin.conf"), 'a', encoding='utf8') as f: - f.write(rpcauth+"\n") - f.write(rpcauth2+"\n") - f.write(rpcauth3+"\n") + f.write(rpcauth + "\n") + f.write(rpcauth2 + "\n") + f.write(rpcauth3 + "\n") with open(os.path.join(get_datadir_path(self.options.tmpdir, 1), "bitcoin.conf"), 'a', encoding='utf8') as f: f.write("rpcuser={}\n".format(self.rpcuser)) f.write("rpcpassword={}\n".format(self.rpcpassword)) @@ -76,19 +77,16 @@ class HTTPBasicsTest(BitcoinTestFramework): assert_equal(200, call_with_auth(node, user, password).status) self.log.info('Wrong...') - assert_equal(401, call_with_auth(node, user, password+'wrong').status) + assert_equal(401, call_with_auth(node, user, password + 'wrong').status) self.log.info('Wrong...') - assert_equal(401, call_with_auth(node, user+'wrong', password).status) + assert_equal(401, call_with_auth(node, user + 'wrong', password).status) self.log.info('Wrong...') - assert_equal(401, call_with_auth(node, user+'wrong', password+'wrong').status) + assert_equal(401, call_with_auth(node, user + 'wrong', password + 'wrong').status) def run_test(self): - - ################################################## - # Check correctness of the rpcauth config option # - ################################################## + self.log.info('Check correctness of the rpcauth config option') url = urllib.parse.urlparse(self.nodes[0].url) self.test_auth(self.nodes[0], url.username, url.password) @@ -96,12 +94,10 @@ class HTTPBasicsTest(BitcoinTestFramework): self.test_auth(self.nodes[0], 'rt2', self.rt2password) self.test_auth(self.nodes[0], self.user, self.password) - ############################################################### - # Check correctness of the rpcuser/rpcpassword config options # - ############################################################### + self.log.info('Check correctness of the rpcuser/rpcpassword config options') url = urllib.parse.urlparse(self.nodes[1].url) self.test_auth(self.nodes[1], self.rpcuser, self.rpcpassword) if __name__ == '__main__': - HTTPBasicsTest ().main () + HTTPBasicsTest().main() From faf45d1f1f997c316fc4c611a23c4456533eefe9 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 18 May 2020 09:17:28 -0400 Subject: [PATCH 3/3] http: Avoid crash when g_thread_http was never started g_thread_http can not be joined when it is not joinable. Avoid crashing the node by adding the required check and add a test. --- src/httpserver.cpp | 6 +++--- test/functional/rpc_users.py | 8 ++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index ffe246b24..5e78fd1d7 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -421,7 +421,7 @@ bool UpdateHTTPServerLogging(bool enable) { #endif } -static std::thread threadHTTP; +static std::thread g_thread_http; static std::vector g_thread_http_workers; void StartHTTPServer() @@ -429,7 +429,7 @@ void StartHTTPServer() LogPrint(BCLog::HTTP, "Starting HTTP server\n"); int rpcThreads = std::max((long)gArgs.GetArg("-rpcthreads", DEFAULT_HTTP_THREADS), 1L); LogPrintf("HTTP: starting %d worker threads\n", rpcThreads); - threadHTTP = std::thread(ThreadHTTP, eventBase); + g_thread_http = std::thread(ThreadHTTP, eventBase); for (int i = 0; i < rpcThreads; i++) { g_thread_http_workers.emplace_back(HTTPWorkQueueRun, workQueue, i); @@ -467,7 +467,7 @@ void StopHTTPServer() boundSockets.clear(); if (eventBase) { LogPrint(BCLog::HTTP, "Waiting for HTTP event thread to exit\n"); - threadHTTP.join(); + if (g_thread_http.joinable()) g_thread_http.join(); } if (eventHTTP) { evhttp_free(eventHTTP); diff --git a/test/functional/rpc_users.py b/test/functional/rpc_users.py index 976065f3f..daf02fc4f 100755 --- a/test/functional/rpc_users.py +++ b/test/functional/rpc_users.py @@ -99,5 +99,13 @@ class HTTPBasicsTest(BitcoinTestFramework): self.test_auth(self.nodes[1], self.rpcuser, self.rpcpassword) + self.log.info('Check that failure to write cookie file will abort the node gracefully') + self.stop_node(0) + cookie_file = os.path.join(get_datadir_path(self.options.tmpdir, 0), self.chain, '.cookie.tmp') + os.mkdir(cookie_file) + init_error = 'Error: Unable to start HTTP server. See debug log for details.' + self.nodes[0].assert_start_raises_init_error(expected_msg=init_error) + + if __name__ == '__main__': HTTPBasicsTest().main()