From 801b1580f5c5129dee1932968743cedda445ed0f Mon Sep 17 00:00:00 2001 From: Gregory Tsipenyuk Date: Wed, 5 Aug 2020 18:28:20 -0400 Subject: [PATCH] Improve handling of RPC ledger_index argument: Some RPC commands return `ledger_index` as a quoted numeric string. This change allows the returned value to be directly copied and used for follow-on RPC commands. This commit fixes #3533 --- src/ripple/json/json_value.h | 2 +- src/ripple/rpc/impl/RPCHelpers.cpp | 40 ++++++++++--------------- src/test/rpc/DepositAuthorized_test.cpp | 9 +++++- src/test/rpc/LedgerRPC_test.cpp | 36 +++++++++++++++++++--- 4 files changed, 57 insertions(+), 30 deletions(-) diff --git a/src/ripple/json/json_value.h b/src/ripple/json/json_value.h index 503a053a9..c8312e514 100644 --- a/src/ripple/json/json_value.h +++ b/src/ripple/json/json_value.h @@ -136,7 +136,7 @@ operator!=(StaticString x, std::string const& y) * The sequence of an #arrayValue will be automatically resize and initialized * with #nullValue. resize() can be used to enlarge or truncate an #arrayValue. * - * The get() methods can be used to obtanis default value in the case the + * The get() methods can be used to obtain a default value in the case the * required element does not exist. * * It is possible to iterate over the list of a #objectValue values using diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp index 2a8e18eed..61862e913 100644 --- a/src/ripple/rpc/impl/RPCHelpers.cpp +++ b/src/ripple/rpc/impl/RPCHelpers.cpp @@ -228,31 +228,23 @@ ledgerFromRequest(T& ledger, JsonContext& context) return {rpcINVALID_PARAMS, "ledgerHashMalformed"}; return getLedger(ledger, ledgerHash, context); } - else if (indexValue.isNumeric()) - { - return getLedger(ledger, indexValue.asInt(), context); - } - else - { - auto const index = indexValue.asString(); - if (index == "validated") - { - return getLedger(ledger, LedgerShortcut::VALIDATED, context); - } - else - { - if (index.empty() || index == "current") - return getLedger(ledger, LedgerShortcut::CURRENT, context); - else if (index == "closed") - return getLedger(ledger, LedgerShortcut::CLOSED, context); - else - { - return {rpcINVALID_PARAMS, "ledgerIndexMalformed"}; - } - } - } - return Status::OK; + auto const index = indexValue.asString(); + + if (index.empty() || index == "current") + return getLedger(ledger, LedgerShortcut::CURRENT, context); + + if (index == "validated") + return getLedger(ledger, LedgerShortcut::VALIDATED, context); + + if (index == "closed") + return getLedger(ledger, LedgerShortcut::CLOSED, context); + + std::uint32_t iVal; + if (beast::lexicalCastChecked(iVal, index)) + return getLedger(ledger, iVal, context); + + return {rpcINVALID_PARAMS, "ledgerIndexMalformed"}; } } // namespace diff --git a/src/test/rpc/DepositAuthorized_test.cpp b/src/test/rpc/DepositAuthorized_test.cpp index 9a4129f6e..42b871e31 100644 --- a/src/test/rpc/DepositAuthorized_test.cpp +++ b/src/test/rpc/DepositAuthorized_test.cpp @@ -228,11 +228,18 @@ public: } { // Request an invalid ledger. - Json::Value args{depositAuthArgs(alice, becky, "17")}; + Json::Value args{depositAuthArgs(alice, becky, "-1")}; Json::Value const result{ env.rpc("json", "deposit_authorized", args.toStyledString())}; verifyErr(result, "invalidParams", "ledgerIndexMalformed"); } + { + // Request a ledger that doesn't exist yet as a string. + Json::Value args{depositAuthArgs(alice, becky, "17")}; + Json::Value const result{ + env.rpc("json", "deposit_authorized", args.toStyledString())}; + verifyErr(result, "lgrNotFound", "ledgerNotFound"); + } { // Request a ledger that doesn't exist yet. Json::Value args{depositAuthArgs(alice, becky)}; diff --git a/src/test/rpc/LedgerRPC_test.cpp b/src/test/rpc/LedgerRPC_test.cpp index 2992e0f15..b58d21359 100644 --- a/src/test/rpc/LedgerRPC_test.cpp +++ b/src/test/rpc/LedgerRPC_test.cpp @@ -72,8 +72,24 @@ class LedgerRPC_test : public beast::unit_test::suite BEAST_EXPECT(env.current()->info().seq == 4); { - // in this case, numeric string converted to number - auto const jrr = env.rpc("ledger", "1")[jss::result]; + Json::Value jvParams; + // can be either numeric or quoted numeric + jvParams[jss::ledger_index] = 1; + auto const jrr = + env.rpc("json", "ledger", to_string(jvParams))[jss::result]; + BEAST_EXPECT(jrr[jss::ledger][jss::closed] == true); + BEAST_EXPECT(jrr[jss::ledger][jss::ledger_index] == "1"); + BEAST_EXPECT(jrr[jss::ledger][jss::accepted] == true); + BEAST_EXPECT( + jrr[jss::ledger][jss::totalCoins] == + env.balance(env.master).value().getText()); + } + + { + Json::Value jvParams; + jvParams[jss::ledger_index] = "1"; + auto const jrr = + env.rpc("json", "ledger", to_string(jvParams))[jss::result]; BEAST_EXPECT(jrr[jss::ledger][jss::closed] == true); BEAST_EXPECT(jrr[jss::ledger][jss::ledger_index] == "1"); BEAST_EXPECT(jrr[jss::ledger][jss::accepted] == true); @@ -110,8 +126,18 @@ class LedgerRPC_test : public beast::unit_test::suite env.close(); { + // ask for an arbitrary string - index Json::Value jvParams; - jvParams[jss::ledger_index] = "0"; // NOT an integer + jvParams[jss::ledger_index] = "potato"; + auto const jrr = + env.rpc("json", "ledger", to_string(jvParams))[jss::result]; + checkErrorValue(jrr, "invalidParams", "ledgerIndexMalformed"); + } + + { + // ask for a negative index + Json::Value jvParams; + jvParams[jss::ledger_index] = -1; auto const jrr = env.rpc("json", "ledger", to_string(jvParams))[jss::result]; checkErrorValue(jrr, "invalidParams", "ledgerIndexMalformed"); @@ -393,8 +419,10 @@ class LedgerRPC_test : public beast::unit_test::suite void testLedgerEntryDepositPreauth() { - testcase("ledger_entry Request DepositPreauth"); + testcase("ledger_entry Deposit Preauth"); + using namespace test::jtx; + Env env{*this}; Account const alice{"alice"}; Account const becky{"becky"};