diff --git a/src/ripple/net/impl/RPCCall.cpp b/src/ripple/net/impl/RPCCall.cpp index 904f5ccacac..c9c09351471 100644 --- a/src/ripple/net/impl/RPCCall.cpp +++ b/src/ripple/net/impl/RPCCall.cpp @@ -879,6 +879,31 @@ class RPCParser return rpcError (rpcINVALID_PARAMS); } + // transaction_entry + Json::Value parseTransactionEntry (Json::Value const& jvParams) + { + // Parameter count should have already been verified. + assert (jvParams.size() == 2); + + std::string const txHash = jvParams[0u].asString(); + if (txHash.length() != 64) + return rpcError (rpcINVALID_PARAMS); + + Json::Value jvRequest; + jvRequest[jss::tx_hash] = txHash; + + jvParseLedger (jvRequest, jvParams[1u].asString()); + + // jvParseLedger inserts a "ledger_index" of 0 if it doesn't + // find a match. + if (jvRequest.isMember(jss::ledger_index) && + jvRequest[jss::ledger_index] == 0) + return rpcError (rpcINVALID_PARAMS); + + return jvRequest; + } + + // tx Json::Value parseTx (Json::Value const& jvParams) { @@ -1073,7 +1098,7 @@ class RPCParser { "server_info", &RPCParser::parseAsIs, 0, 0 }, { "server_state", &RPCParser::parseAsIs, 0, 0 }, { "stop", &RPCParser::parseAsIs, 0, 0 }, - // { "transaction_entry", &RPCParser::parseTransactionEntry, -1, -1 }, + { "transaction_entry", &RPCParser::parseTransactionEntry, 2, 2 }, { "tx", &RPCParser::parseTx, 1, 2 }, { "tx_account", &RPCParser::parseTxAccount, 1, 7 }, { "tx_history", &RPCParser::parseTxHistory, 1, 1 }, @@ -1323,12 +1348,6 @@ rpcClient(std::vector const& args, jvParams.append(jvRequest[i]); } - if (jvRequest.isMember(jss::params)) - { - auto const& params = jvRequest[jss::params]; - assert(params.size() == 1); - jvParams.append(params[0u]); - } { boost::asio::io_service isService; RPCCall::fromNetwork ( diff --git a/src/test/rpc/JSONRPC_test.cpp b/src/test/rpc/JSONRPC_test.cpp index f34954eade4..90b5e66ec46 100644 --- a/src/test/rpc/JSONRPC_test.cpp +++ b/src/test/rpc/JSONRPC_test.cpp @@ -1840,6 +1840,17 @@ R"({ class JSONRPC_test : public beast::unit_test::suite { public: + void testBadRpcCommand () + { + test::jtx::Env env(*this); + Json::Value const result { + env.rpc ("bad_command", R"({"MakingThisUp": 0})")}; + + BEAST_EXPECT (result[jss::result][jss::error] == "unknownCmd"); + BEAST_EXPECT ( + result[jss::result][jss::request][jss::command] == "bad_command"); + } + void testAutoFillFees () { test::jtx::Env env(*this); @@ -2345,6 +2356,7 @@ class JSONRPC_test : public beast::unit_test::suite void run () { + testBadRpcCommand (); testAutoFillFees (); testAutoFillEscalatedFees (); testTransactionRPC (); diff --git a/src/test/rpc/TransactionEntry_test.cpp b/src/test/rpc/TransactionEntry_test.cpp index 0f3d491762d..142d23fad65 100644 --- a/src/test/rpc/TransactionEntry_test.cpp +++ b/src/test/rpc/TransactionEntry_test.cpp @@ -69,6 +69,76 @@ class TransactionEntry_test : public beast::unit_test::suite BEAST_EXPECT(result[jss::error] == "transactionNotFound"); BEAST_EXPECT(result[jss::status] == "error"); } + + std::string const txHash { + "E2FE8D4AF3FCC3944DDF6CD8CDDC5E3F0AD50863EF8919AFEF10CB6408CD4D05"}; + + // Command line format + { + // No arguments + Json::Value const result {env.rpc ("transaction_entry")}; + BEAST_EXPECT(result[jss::ledger_hash].asString().empty()); + BEAST_EXPECT(result[jss::error] == "badSyntax"); + BEAST_EXPECT(result[jss::status] == "error"); + } + + { + // One argument + Json::Value const result {env.rpc ("transaction_entry", txHash)}; + BEAST_EXPECT(result[jss::error] == "badSyntax"); + BEAST_EXPECT(result[jss::status] == "error"); + } + + { + // First argument with too few characters + Json::Value const result {env.rpc ( + "transaction_entry", txHash.substr (1), "closed")}; + BEAST_EXPECT(result[jss::error] == "invalidParams"); + BEAST_EXPECT(result[jss::status] == "error"); + } + + { + // First argument with too many characters + Json::Value const result {env.rpc ( + "transaction_entry", txHash + "A", "closed")}; + BEAST_EXPECT(result[jss::error] == "invalidParams"); + BEAST_EXPECT(result[jss::status] == "error"); + } + + { + // Second argument not valid + Json::Value const result {env.rpc ( + "transaction_entry", txHash, "closer")}; + BEAST_EXPECT(result[jss::error] == "invalidParams"); + BEAST_EXPECT(result[jss::status] == "error"); + } + + { + // Ledger index of 0 is not valid + Json::Value const result {env.rpc ( + "transaction_entry", txHash, "0")}; + BEAST_EXPECT(result[jss::error] == "invalidParams"); + BEAST_EXPECT(result[jss::status] == "error"); + } + + { + // Three arguments + Json::Value const result {env.rpc ( + "transaction_entry", txHash, "closed", "extra")}; + BEAST_EXPECT(result[jss::error] == "badSyntax"); + BEAST_EXPECT(result[jss::status] == "error"); + } + + { + // Valid structure, but transaction not found. + Json::Value const result {env.rpc ( + "transaction_entry", txHash, "closed")}; + BEAST_EXPECT( + ! result[jss::result][jss::ledger_hash].asString().empty()); + BEAST_EXPECT( + result[jss::result][jss::error] == "transactionNotFound"); + BEAST_EXPECT(result[jss::result][jss::status] == "error"); + } } void testRequest() @@ -78,24 +148,28 @@ class TransactionEntry_test : public beast::unit_test::suite Env env {*this}; auto check_tx = [this, &env] - (int index, std::string txhash, std::string type = "") + (int index, std::string const txhash, std::string const type = "") { - Json::Value resIndex, resHash; // first request using ledger_index to lookup + Json::Value const resIndex {[&env, index, &txhash] () { Json::Value params {Json::objectValue}; params[jss::ledger_index] = index; params[jss::tx_hash] = txhash; - resIndex = env.client() + return env.client() .invoke("transaction_entry", params)[jss::result]; - if(! BEAST_EXPECTS(resIndex.isMember(jss::tx_json), txhash)) - return; - BEAST_EXPECT(resIndex[jss::tx_json][jss::hash] == txhash); - if(! type.empty()) - BEAST_EXPECTS( - resIndex[jss::tx_json][jss::TransactionType] == type, - txhash + " is " + - resIndex[jss::tx_json][jss::TransactionType].asString()); + }()}; + + if(! BEAST_EXPECTS(resIndex.isMember(jss::tx_json), txhash)) + return; + + BEAST_EXPECT(resIndex[jss::tx_json][jss::hash] == txhash); + if(! type.empty()) + { + BEAST_EXPECTS( + resIndex[jss::tx_json][jss::TransactionType] == type, + txhash + " is " + + resIndex[jss::tx_json][jss::TransactionType].asString()); } // second request using ledger_hash to lookup and verify @@ -104,10 +178,25 @@ class TransactionEntry_test : public beast::unit_test::suite Json::Value params {Json::objectValue}; params[jss::ledger_hash] = resIndex[jss::ledger_hash]; params[jss::tx_hash] = txhash; - resHash = env.client() + Json::Value const resHash = env.client() .invoke("transaction_entry", params)[jss::result]; BEAST_EXPECT(resHash == resIndex); } + + // Use the command line form with the index. + { + Json::Value const clIndex {env.rpc ( + "transaction_entry", txhash, std::to_string (index))}; + BEAST_EXPECT (clIndex["result"] == resIndex); + } + + // Use the command line form with the ledger_hash. + { + Json::Value const clHash {env.rpc ( + "transaction_entry", txhash, + resIndex[jss::ledger_hash].asString())}; + BEAST_EXPECT (clHash["result"] == resIndex); + } }; Account A1 {"A1"};