From a45a95e5ea9f79458366ef21018778427a344b0e Mon Sep 17 00:00:00 2001 From: Arihant Kothari Date: Thu, 6 Jul 2023 14:58:53 -0400 Subject: [PATCH] APIv2(ledger_entry): return "invalidParams" when fields missing (#4552) Improve error handling for ledger_entry by returning an "invalidParams" error when one or more request fields are specified incorrectly, or one or more required fields are missing. For example, if none of of the following fields is provided, then the API should return an invalidParams error: * index, account_root, directory, offer, ripple_state, check, escrow, payment_channel, deposit_preauth, ticket Prior to this commit, the API returned an "unknownOption" error instead. Since the error was actually due to invalid parameters, rather than unknown options, this error was misleading. Since this is an API breaking change, the "invalidParams" error is only returned for requests using api_version: 2 and above. To maintain backward compatibility, the "unknownOption" error is still returned for api_version: 1. Related: #4573 Fix #4303 --- src/ripple/rpc/handlers/LedgerEntry.cpp | 7 ++++++- src/test/rpc/LedgerRPC_test.cpp | 23 +++++++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/ripple/rpc/handlers/LedgerEntry.cpp b/src/ripple/rpc/handlers/LedgerEntry.cpp index fff2ceac01..19eedd395c 100644 --- a/src/ripple/rpc/handlers/LedgerEntry.cpp +++ b/src/ripple/rpc/handlers/LedgerEntry.cpp @@ -358,7 +358,12 @@ doLedgerEntry(RPC::JsonContext& context) } } else - jvResult[jss::error] = "unknownOption"; + { + if (context.apiVersion < 2u) + jvResult[jss::error] = "unknownOption"; + else + jvResult[jss::error] = "invalidParams"; + } } if (uNodeIndex.isNonZero()) diff --git a/src/test/rpc/LedgerRPC_test.cpp b/src/test/rpc/LedgerRPC_test.cpp index 8a09f2a23b..9c9a63005a 100644 --- a/src/test/rpc/LedgerRPC_test.cpp +++ b/src/test/rpc/LedgerRPC_test.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include namespace ripple { @@ -1212,9 +1213,11 @@ class LedgerRPC_test : public beast::unit_test::suite } void - testLedgerEntryUnknownOption() + testLedgerEntryInvalidParams(unsigned int apiVersion) { - testcase("ledger_entry Request Unknown Option"); + testcase( + "ledger_entry Request With Invalid Parameters v" + + std::to_string(apiVersion)); using namespace test::jtx; Env env{*this}; @@ -1222,11 +1225,16 @@ class LedgerRPC_test : public beast::unit_test::suite // "features" is not an option supported by ledger_entry. Json::Value jvParams; + jvParams[jss::api_version] = apiVersion; jvParams[jss::features] = ledgerHash; jvParams[jss::ledger_hash] = ledgerHash; Json::Value const jrr = env.rpc("json", "ledger_entry", to_string(jvParams))[jss::result]; - checkErrorValue(jrr, "unknownOption", ""); + + if (apiVersion < 2u) + checkErrorValue(jrr, "unknownOption", ""); + else + checkErrorValue(jrr, "invalidParams", ""); } /// @brief ledger RPC requests as a way to drive @@ -1724,11 +1732,18 @@ public: testLedgerEntryPayChan(); testLedgerEntryRippleState(); testLedgerEntryTicket(); - testLedgerEntryUnknownOption(); testLookupLedger(); testNoQueue(); testQueue(); testLedgerAccountsOption(); + + // version specific tests + for (auto testVersion = RPC::apiMinimumSupportedVersion; + testVersion <= RPC::apiBetaVersion; + ++testVersion) + { + testLedgerEntryInvalidParams(testVersion); + } } };