From 70442b3da995bd3dd344083aec6bde4103f55c37 Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Tue, 4 Jul 2023 13:30:47 -0400 Subject: [PATCH 1/4] Catch exceptions when calling `http_fwd();` --- .../include/eosio/http_plugin/macros.hpp | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/plugins/http_plugin/include/eosio/http_plugin/macros.hpp b/plugins/http_plugin/include/eosio/http_plugin/macros.hpp index b7848e870c..406be10f53 100644 --- a/plugins/http_plugin/include/eosio/http_plugin/macros.hpp +++ b/plugins/http_plugin/include/eosio/http_plugin/macros.hpp @@ -61,15 +61,19 @@ _http_plugin.post_http_thread_pool([resp_code=http_resp_code, cb=std::move(cb), \ body=std::move(body), \ http_fwd = std::move(http_fwd)]() { \ - chain::t_or_exception result = http_fwd(); \ - if (std::holds_alternative(result)) { \ - try { \ - std::get(result)->dynamic_rethrow_exception(); \ - } catch (...) { \ - http_plugin::handle_exception(#api_name, #call_name, body, cb); \ + try { \ + chain::t_or_exception result = http_fwd(); \ + if (std::holds_alternative(result)) { \ + try { \ + std::get(result)->dynamic_rethrow_exception(); \ + } catch (...) { \ + http_plugin::handle_exception(#api_name, #call_name, body, cb); \ + } \ + } else { \ + cb(resp_code, fc::variant(std::get(std::move(result)))); \ } \ - } else { \ - cb(resp_code, fc::variant(std::get(std::move(result)))); \ + } catch (...) { \ + http_plugin::handle_exception(#api_name, #call_name, body, cb); \ } \ }); \ } catch (...) { \ From 3196fd7d9f7d4614fb29b81eadf75b8f75c25c0d Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Wed, 26 Jul 2023 14:58:37 -0400 Subject: [PATCH 2/4] Remove duplicate test. --- tests/CMakeLists.txt | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index c9cf6bbc12..700abde685 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -182,9 +182,6 @@ add_test(NAME full-version-label-test COMMAND tests/full-version-label.sh "v${VE add_test(NAME nested_container_multi_index_test COMMAND tests/nested_container_multi_index_test.py -n 2 WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST nested_container_multi_index_test PROPERTY LABELS nonparallelizable_tests) -add_test(NAME nodeos_run_check_test COMMAND tests/nodeos_run_test.py -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) -set_property(TEST nodeos_run_check_test PROPERTY LABELS nonparallelizable_tests) - add_test(NAME p2p_multiple_listen_test COMMAND tests/p2p_multiple_listen_test.py -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) set_property(TEST p2p_multiple_listen_test PROPERTY LABELS nonparallelizable_tests) add_test(NAME p2p_no_listen_test COMMAND tests/p2p_no_listen_test.py -v ${UNSHARE} WORKING_DIRECTORY ${CMAKE_BINARY_DIR}) From 9792b2fa0b3aeb556f12fe238a172336e4ec959c Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Thu, 27 Jul 2023 10:32:33 -0400 Subject: [PATCH 3/4] Add section in `tests/nodeos_run_test.py` to verify that exceptions in http_plugin are caught. --- tests/nodeos_run_test.py | 29 +++ .../contracts/eosio.token/CMakeLists.txt | 2 + .../contracts/eosio.token/eosio.token.bad.abi | 193 ++++++++++++++++++ 3 files changed, 224 insertions(+) create mode 100644 unittests/contracts/eosio.token/eosio.token.bad.abi diff --git a/tests/nodeos_run_test.py b/tests/nodeos_run_test.py index 48e72a202b..3b31996a9b 100755 --- a/tests/nodeos_run_test.py +++ b/tests/nodeos_run_test.py @@ -746,6 +746,35 @@ accounts=[testeraAccount, currencyAccount, exchangeAccount] cluster.validateAccounts(accounts) + # run a get_table_rows API call for a table which has an incorrect abi (missing type), to make sure that + # the resulting exception in http-plugin is caught and doesn't cause nodeos to crash (leap issue #1372). + contractDir="unittests/contracts/eosio.token" + wasmFile="eosio.token.wasm" + abiFile="eosio.token.bad.abi" + Print("Publish contract") + trans=node.publishContract(currencyAccount, contractDir, wasmFile, abiFile, waitForTransBlock=True) + if trans is None: + cmdError("%s set contract currency1111" % (ClientName)) + errorExit("Failed to publish contract.") + + contract="currency1111" + table="accounts" + row0=node.getTableRow(contract, currencyAccount.name, table, 0) + + # because we set a bad abi (missing type, see "eosio.token.bad.abi") on the contract, the + # getTableRow() is expected to fail and return None + try: + assert(not row0) + except (AssertionError, KeyError) as _: + Print("ERROR: Failed get table row assertion. %s" % (row0)) + raise + + # However check that the node is still running and didn't crash when processing getTableRow on a contract + # with a bad abi. If node does crash and we get an exception during "get info", it means that we did not + # catch the exception that occured while calling `cb()` in `http_plugin/macros.hpp`. + currentBlockNum=node.getHeadBlockNum() + Print("CurrentBlockNum: %d" % (currentBlockNum)) + testSuccessful=True finally: TestHelper.shutdown(cluster, walletMgr, testSuccessful, dumpErrorDetails) diff --git a/unittests/contracts/eosio.token/CMakeLists.txt b/unittests/contracts/eosio.token/CMakeLists.txt index aeb142c3ff..74a864355a 100644 --- a/unittests/contracts/eosio.token/CMakeLists.txt +++ b/unittests/contracts/eosio.token/CMakeLists.txt @@ -4,3 +4,5 @@ else() configure_file( ${CMAKE_CURRENT_SOURCE_DIR}/eosio.token.wasm ${CMAKE_CURRENT_BINARY_DIR}/eosio.token.wasm COPYONLY ) configure_file( ${CMAKE_CURRENT_SOURCE_DIR}/eosio.token.abi ${CMAKE_CURRENT_BINARY_DIR}/eosio.token.abi COPYONLY ) endif() +configure_file( ${CMAKE_CURRENT_SOURCE_DIR}/eosio.token.bad.abi ${CMAKE_CURRENT_BINARY_DIR}/eosio.token.bad.abi COPYONLY ) + diff --git a/unittests/contracts/eosio.token/eosio.token.bad.abi b/unittests/contracts/eosio.token/eosio.token.bad.abi new file mode 100644 index 0000000000..87ce6748e9 --- /dev/null +++ b/unittests/contracts/eosio.token/eosio.token.bad.abi @@ -0,0 +1,193 @@ +{ + "____comment": "This file was generated with eosio-abigen. DO NOT EDIT ", + "version": "eosio::abi/1.2", + "types": [], + "structs": [ + { + "name": "account", + "base": "", + "fields": [ + { + "name": "balance", + "type": "asset" + } + ] + }, + { + "name": "close", + "base": "", + "fields": [ + { + "name": "owner", + "type": "name" + }, + { + "name": "symbol", + "type": "symbol" + } + ] + }, + { + "name": "create", + "base": "", + "fields": [ + { + "name": "issuer", + "type": "name" + }, + { + "name": "maximum_supply", + "type": "asset" + } + ] + }, + { + "name": "currency_stats", + "base": "", + "fields": [ + { + "name": "supply", + "type": "asset" + }, + { + "name": "max_supply", + "type": "asset" + }, + { + "name": "issuer", + "type": "name" + } + ] + }, + { + "name": "issue", + "base": "", + "fields": [ + { + "name": "to", + "type": "name" + }, + { + "name": "quantity", + "type": "asset" + }, + { + "name": "memo", + "type": "string" + } + ] + }, + { + "name": "open", + "base": "", + "fields": [ + { + "name": "owner", + "type": "name" + }, + { + "name": "symbol", + "type": "symbol" + }, + { + "name": "ram_payer", + "type": "name" + } + ] + }, + { + "name": "retire", + "base": "", + "fields": [ + { + "name": "quantity", + "type": "asset" + }, + { + "name": "memo", + "type": "string" + } + ] + }, + { + "name": "transfer", + "base": "", + "fields": [ + { + "name": "from", + "type": "name" + }, + { + "name": "to", + "type": "name" + }, + { + "name": "quantity", + "type": "asset" + }, + { + "name": "memo", + "type": "string" + } + ] + } + ], + "actions": [ + { + "name": "close", + "type": "close", + "ricardian_contract": "" + }, + { + "name": "create", + "type": "create", + "ricardian_contract": "" + }, + { + "name": "issue", + "type": "issue", + "ricardian_contract": "" + }, + { + "name": "open", + "type": "open", + "ricardian_contract": "" + }, + { + "name": "retire", + "type": "retire", + "ricardian_contract": "" + }, + { + "name": "transfer", + "type": "transfer", + "ricardian_contract": "" + } + ], + "tables": [ + { + "name": "accounts", + "type": "account", + "index_type": "i64", + "key_names": [], + "key_types": [] + }, + { + "name": "stat", + "type": "currency_stats", + "index_type": "i64", + "key_names": [], + "key_types": [] + }, + { + "name": "global", + "type": "global", + "index_type": "i64", + "key_names": [], + "key_types": [] + } + ], + "ricardian_clauses": [], + "variants": [], + "action_results": [] +} \ No newline at end of file From 5d1bb30c6dfccc572a3394586c3a8f503196247a Mon Sep 17 00:00:00 2001 From: greg7mdp Date: Thu, 27 Jul 2023 10:50:06 -0400 Subject: [PATCH 4/4] Update comment in new abi file --- unittests/contracts/eosio.token/eosio.token.bad.abi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittests/contracts/eosio.token/eosio.token.bad.abi b/unittests/contracts/eosio.token/eosio.token.bad.abi index 87ce6748e9..6aba102765 100644 --- a/unittests/contracts/eosio.token/eosio.token.bad.abi +++ b/unittests/contracts/eosio.token/eosio.token.bad.abi @@ -1,5 +1,5 @@ { - "____comment": "This file was generated with eosio-abigen. DO NOT EDIT ", + "____comment": "This file was generated with eosio-abigen. Manually edited to add a table named global with an undefined type ", "version": "eosio::abi/1.2", "types": [], "structs": [