APIv2(ledger_entry): return invalidParams for bad parameters (#4630)

- Verify "check", used to retrieve a Check object, is a string.
- Verify "nft_page", used to retrieve an NFT Page, is a string.
- Verify "index", used to retrieve any type of ledger object by its
  unique ID, is a string.
- Verify "directory", used to retrieve a DirectoryNode, is a string or
  an object.

This change only impacts api_version 2 since it is a breaking change.

https://xrpl.org/ledger_entry.html

Fix #4550

src/ripple/protocol/impl/Issue.cpp

@@ -19,6 +19,7 @@
 
 #include <ripple/protocol/Issue.h>
 
+#include <ripple/json/json_errors.h>
 #include <ripple/protocol/AccountID.h>
 #include <ripple/protocol/UintTypes.h>
 #include <ripple/protocol/jss.h>
@@ -78,7 +79,7 @@ issueFromJson(Json::Value const& v)
 {
     if (!v.isObject())
     {
-        Throw<std::runtime_error>(
+        Throw<Json::error>(
             "issueFromJson can only be specified with a 'object' Json value");
     }
 
@@ -87,37 +88,34 @@ issueFromJson(Json::Value const& v)
 
     if (!curStr.isString())
     {
-        Throw<std::runtime_error>(
+        Throw<Json::error>(
             "issueFromJson currency must be a string Json value");
     }
 
     auto const currency = to_currency(curStr.asString());
     if (currency == badCurrency() || currency == noCurrency())
     {
-        Throw<std::runtime_error>(
-            "issueFromJson currency must be a valid currency");
+        Throw<Json::error>("issueFromJson currency must be a valid currency");
     }
 
     if (isXRP(currency))
     {
         if (!issStr.isNull())
         {
-            Throw<std::runtime_error>("Issue, XRP should not have issuer");
+            Throw<Json::error>("Issue, XRP should not have issuer");
         }
         return xrpIssue();
     }
 
     if (!issStr.isString())
     {
-        Throw<std::runtime_error>(
-            "issueFromJson issuer must be a string Json value");
+        Throw<Json::error>("issueFromJson issuer must be a string Json value");
     }
     auto const issuer = parseBase58<AccountID>(issStr.asString());
 
     if (!issuer)
     {
-        Throw<std::runtime_error>(
-            "issueFromJson issuer must be a valid account");
+        Throw<Json::error>("issueFromJson issuer must be a valid account");
     }
 
     return Issue{currency, *issuer};

src/ripple/rpc/handlers/LedgerEntry.cpp

@@ -20,6 +20,7 @@
 #include <ripple/app/main/Application.h>
 #include <ripple/basics/StringUtilities.h>
 #include <ripple/basics/strHex.h>
+#include <ripple/json/json_errors.h>
 #include <ripple/ledger/ReadView.h>
 #include <ripple/net/RPCErr.h>
 #include <ripple/protocol/ErrorCodes.h>
@@ -49,135 +50,306 @@ doLedgerEntry(RPC::JsonContext& context)
     bool bNodeBinary = false;
     LedgerEntryType expectedType = ltANY;
 
-    if (context.params.isMember(jss::index))
+    try
     {
-        if (!uNodeIndex.parseHex(context.params[jss::index].asString()))
+        if (context.params.isMember(jss::index))
         {
-            uNodeIndex = beast::zero;
-            jvResult[jss::error] = "malformedRequest";
-        }
-    }
-    else if (context.params.isMember(jss::account_root))
-    {
-        expectedType = ltACCOUNT_ROOT;
-        auto const account = parseBase58<AccountID>(
-            context.params[jss::account_root].asString());
-        if (!account || account->isZero())
-            jvResult[jss::error] = "malformedAddress";
-        else
-            uNodeIndex = keylet::account(*account).key;
-    }
-    else if (context.params.isMember(jss::check))
-    {
-        expectedType = ltCHECK;
-
-        if (!uNodeIndex.parseHex(context.params[jss::check].asString()))
-        {
-            uNodeIndex = beast::zero;
-            jvResult[jss::error] = "malformedRequest";
-        }
-    }
-    else if (context.params.isMember(jss::deposit_preauth))
-    {
-        expectedType = ltDEPOSIT_PREAUTH;
-
-        if (!context.params[jss::deposit_preauth].isObject())
-        {
-            if (!context.params[jss::deposit_preauth].isString() ||
-                !uNodeIndex.parseHex(
-                    context.params[jss::deposit_preauth].asString()))
+            if (!uNodeIndex.parseHex(context.params[jss::index].asString()))
             {
                 uNodeIndex = beast::zero;
                 jvResult[jss::error] = "malformedRequest";
             }
         }
-        else if (
-            !context.params[jss::deposit_preauth].isMember(jss::owner) ||
-            !context.params[jss::deposit_preauth][jss::owner].isString() ||
-            !context.params[jss::deposit_preauth].isMember(jss::authorized) ||
-            !context.params[jss::deposit_preauth][jss::authorized].isString())
+        else if (context.params.isMember(jss::account_root))
         {
-            jvResult[jss::error] = "malformedRequest";
-        }
-        else
-        {
-            auto const owner = parseBase58<AccountID>(
-                context.params[jss::deposit_preauth][jss::owner].asString());
-
-            auto const authorized = parseBase58<AccountID>(
-                context.params[jss::deposit_preauth][jss::authorized]
-                    .asString());
-
-            if (!owner)
-                jvResult[jss::error] = "malformedOwner";
-            else if (!authorized)
-                jvResult[jss::error] = "malformedAuthorized";
+            expectedType = ltACCOUNT_ROOT;
+            auto const account = parseBase58<AccountID>(
+                context.params[jss::account_root].asString());
+            if (!account || account->isZero())
+                jvResult[jss::error] = "malformedAddress";
             else
-                uNodeIndex = keylet::depositPreauth(*owner, *authorized).key;
+                uNodeIndex = keylet::account(*account).key;
         }
-    }
-    else if (context.params.isMember(jss::directory))
-    {
-        expectedType = ltDIR_NODE;
-        if (context.params[jss::directory].isNull())
+        else if (context.params.isMember(jss::check))
         {
-            jvResult[jss::error] = "malformedRequest";
-        }
-        else if (!context.params[jss::directory].isObject())
-        {
-            if (!uNodeIndex.parseHex(context.params[jss::directory].asString()))
+            expectedType = ltCHECK;
+            if (!uNodeIndex.parseHex(context.params[jss::check].asString()))
             {
                 uNodeIndex = beast::zero;
                 jvResult[jss::error] = "malformedRequest";
             }
         }
-        else if (
-            context.params[jss::directory].isMember(jss::sub_index) &&
-            !context.params[jss::directory][jss::sub_index].isIntegral())
+        else if (context.params.isMember(jss::deposit_preauth))
         {
-            jvResult[jss::error] = "malformedRequest";
-        }
-        else
-        {
-            std::uint64_t uSubIndex =
-                context.params[jss::directory].isMember(jss::sub_index)
-                ? context.params[jss::directory][jss::sub_index].asUInt()
-                : 0;
+            expectedType = ltDEPOSIT_PREAUTH;
 
-            if (context.params[jss::directory].isMember(jss::dir_root))
+            if (!context.params[jss::deposit_preauth].isObject())
             {
-                uint256 uDirRoot;
-
-                if (context.params[jss::directory].isMember(jss::owner))
-                {
-                    // May not specify both dir_root and owner.
-                    jvResult[jss::error] = "malformedRequest";
-                }
-                else if (!uDirRoot.parseHex(
-                             context.params[jss::directory][jss::dir_root]
-                                 .asString()))
+                if (!context.params[jss::deposit_preauth].isString() ||
+                    !uNodeIndex.parseHex(
+                        context.params[jss::deposit_preauth].asString()))
                 {
                     uNodeIndex = beast::zero;
                     jvResult[jss::error] = "malformedRequest";
                 }
+            }
+            else if (
+                !context.params[jss::deposit_preauth].isMember(jss::owner) ||
+                !context.params[jss::deposit_preauth][jss::owner].isString() ||
+                !context.params[jss::deposit_preauth].isMember(
+                    jss::authorized) ||
+                !context.params[jss::deposit_preauth][jss::authorized]
+                     .isString())
+            {
+                jvResult[jss::error] = "malformedRequest";
+            }
+            else
+            {
+                auto const owner = parseBase58<AccountID>(
+                    context.params[jss::deposit_preauth][jss::owner]
+                        .asString());
+
+                auto const authorized = parseBase58<AccountID>(
+                    context.params[jss::deposit_preauth][jss::authorized]
+                        .asString());
+
+                if (!owner)
+                    jvResult[jss::error] = "malformedOwner";
+                else if (!authorized)
+                    jvResult[jss::error] = "malformedAuthorized";
                 else
+                    uNodeIndex =
+                        keylet::depositPreauth(*owner, *authorized).key;
+            }
+        }
+        else if (context.params.isMember(jss::directory))
+        {
+            expectedType = ltDIR_NODE;
+            if (context.params[jss::directory].isNull())
+            {
+                jvResult[jss::error] = "malformedRequest";
+            }
+            else if (!context.params[jss::directory].isObject())
+            {
+                if (!uNodeIndex.parseHex(
+                        context.params[jss::directory].asString()))
                 {
-                    uNodeIndex = keylet::page(uDirRoot, uSubIndex).key;
+                    uNodeIndex = beast::zero;
+                    jvResult[jss::error] = "malformedRequest";
                 }
             }
-            else if (context.params[jss::directory].isMember(jss::owner))
+            else if (
+                context.params[jss::directory].isMember(jss::sub_index) &&
+                !context.params[jss::directory][jss::sub_index].isIntegral())
             {
-                auto const ownerID = parseBase58<AccountID>(
-                    context.params[jss::directory][jss::owner].asString());
+                jvResult[jss::error] = "malformedRequest";
+            }
+            else
+            {
+                std::uint64_t uSubIndex =
+                    context.params[jss::directory].isMember(jss::sub_index)
+                    ? context.params[jss::directory][jss::sub_index].asUInt()
+                    : 0;
 
-                if (!ownerID)
+                if (context.params[jss::directory].isMember(jss::dir_root))
+                {
+                    uint256 uDirRoot;
+
+                    if (context.params[jss::directory].isMember(jss::owner))
+                    {
+                        // May not specify both dir_root and owner.
+                        jvResult[jss::error] = "malformedRequest";
+                    }
+                    else if (!uDirRoot.parseHex(
+                                 context.params[jss::directory][jss::dir_root]
+                                     .asString()))
+                    {
+                        uNodeIndex = beast::zero;
+                        jvResult[jss::error] = "malformedRequest";
+                    }
+                    else
+                    {
+                        uNodeIndex = keylet::page(uDirRoot, uSubIndex).key;
+                    }
+                }
+                else if (context.params[jss::directory].isMember(jss::owner))
+                {
+                    auto const ownerID = parseBase58<AccountID>(
+                        context.params[jss::directory][jss::owner].asString());
+
+                    if (!ownerID)
+                    {
+                        jvResult[jss::error] = "malformedAddress";
+                    }
+                    else
+                    {
+                        uNodeIndex =
+                            keylet::page(keylet::ownerDir(*ownerID), uSubIndex)
+                                .key;
+                    }
+                }
+                else
+                {
+                    jvResult[jss::error] = "malformedRequest";
+                }
+            }
+        }
+        else if (context.params.isMember(jss::escrow))
+        {
+            expectedType = ltESCROW;
+            if (!context.params[jss::escrow].isObject())
+            {
+                if (!uNodeIndex.parseHex(
+                        context.params[jss::escrow].asString()))
+                {
+                    uNodeIndex = beast::zero;
+                    jvResult[jss::error] = "malformedRequest";
+                }
+            }
+            else if (
+                !context.params[jss::escrow].isMember(jss::owner) ||
+                !context.params[jss::escrow].isMember(jss::seq) ||
+                !context.params[jss::escrow][jss::seq].isIntegral())
+            {
+                jvResult[jss::error] = "malformedRequest";
+            }
+            else
+            {
+                auto const id = parseBase58<AccountID>(
+                    context.params[jss::escrow][jss::owner].asString());
+                if (!id)
+                    jvResult[jss::error] = "malformedOwner";
+                else
+                    uNodeIndex =
+                        keylet::escrow(
+                            *id, context.params[jss::escrow][jss::seq].asUInt())
+                            .key;
+            }
+        }
+        else if (context.params.isMember(jss::offer))
+        {
+            expectedType = ltOFFER;
+            if (!context.params[jss::offer].isObject())
+            {
+                if (!uNodeIndex.parseHex(context.params[jss::offer].asString()))
+                {
+                    uNodeIndex = beast::zero;
+                    jvResult[jss::error] = "malformedRequest";
+                }
+            }
+            else if (
+                !context.params[jss::offer].isMember(jss::account) ||
+                !context.params[jss::offer].isMember(jss::seq) ||
+                !context.params[jss::offer][jss::seq].isIntegral())
+            {
+                jvResult[jss::error] = "malformedRequest";
+            }
+            else
+            {
+                auto const id = parseBase58<AccountID>(
+                    context.params[jss::offer][jss::account].asString());
+                if (!id)
+                    jvResult[jss::error] = "malformedAddress";
+                else
+                    uNodeIndex =
+                        keylet::offer(
+                            *id, context.params[jss::offer][jss::seq].asUInt())
+                            .key;
+            }
+        }
+        else if (context.params.isMember(jss::payment_channel))
+        {
+            expectedType = ltPAYCHAN;
+
+            if (!uNodeIndex.parseHex(
+                    context.params[jss::payment_channel].asString()))
+            {
+                uNodeIndex = beast::zero;
+                jvResult[jss::error] = "malformedRequest";
+            }
+        }
+        else if (context.params.isMember(jss::ripple_state))
+        {
+            expectedType = ltRIPPLE_STATE;
+            Currency uCurrency;
+            Json::Value jvRippleState = context.params[jss::ripple_state];
+
+            if (!jvRippleState.isObject() ||
+                !jvRippleState.isMember(jss::currency) ||
+                !jvRippleState.isMember(jss::accounts) ||
+                !jvRippleState[jss::accounts].isArray() ||
+                2 != jvRippleState[jss::accounts].size() ||
+                !jvRippleState[jss::accounts][0u].isString() ||
+                !jvRippleState[jss::accounts][1u].isString() ||
+                (jvRippleState[jss::accounts][0u].asString() ==
+                 jvRippleState[jss::accounts][1u].asString()))
+            {
+                jvResult[jss::error] = "malformedRequest";
+            }
+            else
+            {
+                auto const id1 = parseBase58<AccountID>(
+                    jvRippleState[jss::accounts][0u].asString());
+                auto const id2 = parseBase58<AccountID>(
+                    jvRippleState[jss::accounts][1u].asString());
+                if (!id1 || !id2)
                 {
                     jvResult[jss::error] = "malformedAddress";
                 }
+                else if (!to_currency(
+                             uCurrency,
+                             jvRippleState[jss::currency].asString()))
+                {
+                    jvResult[jss::error] = "malformedCurrency";
+                }
                 else
                 {
-                    uNodeIndex =
-                        keylet::page(keylet::ownerDir(*ownerID), uSubIndex).key;
+                    uNodeIndex = keylet::line(*id1, *id2, uCurrency).key;
+                }
+            }
+        }
+        else if (context.params.isMember(jss::ticket))
+        {
+            expectedType = ltTICKET;
+            if (!context.params[jss::ticket].isObject())
+            {
+                if (!uNodeIndex.parseHex(
+                        context.params[jss::ticket].asString()))
+                {
+                    uNodeIndex = beast::zero;
+                    jvResult[jss::error] = "malformedRequest";
+                }
+            }
+            else if (
+                !context.params[jss::ticket].isMember(jss::account) ||
+                !context.params[jss::ticket].isMember(jss::ticket_seq) ||
+                !context.params[jss::ticket][jss::ticket_seq].isIntegral())
+            {
+                jvResult[jss::error] = "malformedRequest";
+            }
+            else
+            {
+                auto const id = parseBase58<AccountID>(
+                    context.params[jss::ticket][jss::account].asString());
+                if (!id)
+                    jvResult[jss::error] = "malformedAddress";
+                else
+                    uNodeIndex = getTicketIndex(
+                        *id,
+                        context.params[jss::ticket][jss::ticket_seq].asUInt());
+            }
+        }
+        else if (context.params.isMember(jss::nft_page))
+        {
+            expectedType = ltNFTOKEN_PAGE;
+
+            if (context.params[jss::nft_page].isString())
+            {
+                if (!uNodeIndex.parseHex(
+                        context.params[jss::nft_page].asString()))
+                {
+                    uNodeIndex = beast::zero;
+                    jvResult[jss::error] = "malformedRequest";
                 }
             }
             else
@@ -185,218 +357,74 @@ doLedgerEntry(RPC::JsonContext& context)
                 jvResult[jss::error] = "malformedRequest";
             }
         }
-    }
-    else if (context.params.isMember(jss::escrow))
-    {
-        expectedType = ltESCROW;
-        if (!context.params[jss::escrow].isObject())
+        else if (context.params.isMember(jss::amm))
         {
-            if (!uNodeIndex.parseHex(context.params[jss::escrow].asString()))
+            expectedType = ltAMM;
+            if (!context.params[jss::amm].isObject())
+            {
+                if (!uNodeIndex.parseHex(context.params[jss::amm].asString()))
+                {
+                    uNodeIndex = beast::zero;
+                    jvResult[jss::error] = "malformedRequest";
+                }
+            }
+            else if (
+                !context.params[jss::amm].isMember(jss::asset) ||
+                !context.params[jss::amm].isMember(jss::asset2))
             {
-                uNodeIndex = beast::zero;
                 jvResult[jss::error] = "malformedRequest";
             }
-        }
-        else if (
-            !context.params[jss::escrow].isMember(jss::owner) ||
-            !context.params[jss::escrow].isMember(jss::seq) ||
-            !context.params[jss::escrow][jss::seq].isIntegral())
-        {
-            jvResult[jss::error] = "malformedRequest";
+            else
+            {
+                try
+                {
+                    auto const issue =
+                        issueFromJson(context.params[jss::amm][jss::asset]);
+                    auto const issue2 =
+                        issueFromJson(context.params[jss::amm][jss::asset2]);
+                    uNodeIndex = keylet::amm(issue, issue2).key;
+                }
+                catch (std::runtime_error const&)
+                {
+                    jvResult[jss::error] = "malformedRequest";
+                }
+            }
         }
         else
         {
-            auto const id = parseBase58<AccountID>(
-                context.params[jss::escrow][jss::owner].asString());
-            if (!id)
-                jvResult[jss::error] = "malformedOwner";
-            else
-                uNodeIndex =
-                    keylet::escrow(
-                        *id, context.params[jss::escrow][jss::seq].asUInt())
-                        .key;
-        }
-    }
-    else if (context.params.isMember(jss::offer))
-    {
-        expectedType = ltOFFER;
-        if (!context.params[jss::offer].isObject())
-        {
-            if (!uNodeIndex.parseHex(context.params[jss::offer].asString()))
+            if (context.params.isMember("params") &&
+                context.params["params"].isArray() &&
+                context.params["params"].size() == 1 &&
+                context.params["params"][0u].isString())
             {
-                uNodeIndex = beast::zero;
-                jvResult[jss::error] = "malformedRequest";
+                if (!uNodeIndex.parseHex(
+                        context.params["params"][0u].asString()))
+                {
+                    uNodeIndex = beast::zero;
+                    jvResult[jss::error] = "malformedRequest";
+                }
+            }
+            else
+            {
+                if (context.apiVersion < 2u)
+                    jvResult[jss::error] = "unknownOption";
+                else
+                    jvResult[jss::error] = "invalidParams";
             }
         }
-        else if (
-            !context.params[jss::offer].isMember(jss::account) ||
-            !context.params[jss::offer].isMember(jss::seq) ||
-            !context.params[jss::offer][jss::seq].isIntegral())
-        {
-            jvResult[jss::error] = "malformedRequest";
-        }
-        else
-        {
-            auto const id = parseBase58<AccountID>(
-                context.params[jss::offer][jss::account].asString());
-            if (!id)
-                jvResult[jss::error] = "malformedAddress";
-            else
-                uNodeIndex =
-                    keylet::offer(
-                        *id, context.params[jss::offer][jss::seq].asUInt())
-                        .key;
-        }
     }
-    else if (context.params.isMember(jss::payment_channel))
+    catch (Json::error& e)
     {
-        expectedType = ltPAYCHAN;
-
-        if (!uNodeIndex.parseHex(
-                context.params[jss::payment_channel].asString()))
+        if (context.apiVersion > 1u)
         {
+            // For apiVersion 2 onwards, any parsing failures that throw
+            // this
+            // exception return an invalidParam error.
             uNodeIndex = beast::zero;
-            jvResult[jss::error] = "malformedRequest";
-        }
-    }
-    else if (context.params.isMember(jss::ripple_state))
-    {
-        expectedType = ltRIPPLE_STATE;
-        Currency uCurrency;
-        Json::Value jvRippleState = context.params[jss::ripple_state];
-
-        if (!jvRippleState.isObject() ||
-            !jvRippleState.isMember(jss::currency) ||
-            !jvRippleState.isMember(jss::accounts) ||
-            !jvRippleState[jss::accounts].isArray() ||
-            2 != jvRippleState[jss::accounts].size() ||
-            !jvRippleState[jss::accounts][0u].isString() ||
-            !jvRippleState[jss::accounts][1u].isString() ||
-            (jvRippleState[jss::accounts][0u].asString() ==
-             jvRippleState[jss::accounts][1u].asString()))
-        {
-            jvResult[jss::error] = "malformedRequest";
+            jvResult[jss::error] = "invalidParams";
         }
         else
-        {
-            auto const id1 = parseBase58<AccountID>(
-                jvRippleState[jss::accounts][0u].asString());
-            auto const id2 = parseBase58<AccountID>(
-                jvRippleState[jss::accounts][1u].asString());
-            if (!id1 || !id2)
-            {
-                jvResult[jss::error] = "malformedAddress";
-            }
-            else if (!to_currency(
-                         uCurrency, jvRippleState[jss::currency].asString()))
-            {
-                jvResult[jss::error] = "malformedCurrency";
-            }
-            else
-            {
-                uNodeIndex = keylet::line(*id1, *id2, uCurrency).key;
-            }
-        }
-    }
-    else if (context.params.isMember(jss::ticket))
-    {
-        expectedType = ltTICKET;
-        if (!context.params[jss::ticket].isObject())
-        {
-            if (!uNodeIndex.parseHex(context.params[jss::ticket].asString()))
-            {
-                uNodeIndex = beast::zero;
-                jvResult[jss::error] = "malformedRequest";
-            }
-        }
-        else if (
-            !context.params[jss::ticket].isMember(jss::account) ||
-            !context.params[jss::ticket].isMember(jss::ticket_seq) ||
-            !context.params[jss::ticket][jss::ticket_seq].isIntegral())
-        {
-            jvResult[jss::error] = "malformedRequest";
-        }
-        else
-        {
-            auto const id = parseBase58<AccountID>(
-                context.params[jss::ticket][jss::account].asString());
-            if (!id)
-                jvResult[jss::error] = "malformedAddress";
-            else
-                uNodeIndex = getTicketIndex(
-                    *id, context.params[jss::ticket][jss::ticket_seq].asUInt());
-        }
-    }
-    else if (context.params.isMember(jss::nft_page))
-    {
-        expectedType = ltNFTOKEN_PAGE;
-
-        if (context.params[jss::nft_page].isString())
-        {
-            if (!uNodeIndex.parseHex(context.params[jss::nft_page].asString()))
-            {
-                uNodeIndex = beast::zero;
-                jvResult[jss::error] = "malformedRequest";
-            }
-        }
-        else
-        {
-            jvResult[jss::error] = "malformedRequest";
-        }
-    }
-    else if (context.params.isMember(jss::amm))
-    {
-        expectedType = ltAMM;
-        if (!context.params[jss::amm].isObject())
-        {
-            if (!uNodeIndex.parseHex(context.params[jss::amm].asString()))
-            {
-                uNodeIndex = beast::zero;
-                jvResult[jss::error] = "malformedRequest";
-            }
-        }
-        else if (
-            !context.params[jss::amm].isMember(jss::asset) ||
-            !context.params[jss::amm].isMember(jss::asset2))
-        {
-            jvResult[jss::error] = "malformedRequest";
-        }
-        else
-        {
-            try
-            {
-                auto const issue =
-                    issueFromJson(context.params[jss::amm][jss::asset]);
-                auto const issue2 =
-                    issueFromJson(context.params[jss::amm][jss::asset2]);
-                uNodeIndex = keylet::amm(issue, issue2).key;
-            }
-            catch (std::runtime_error const&)
-            {
-                jvResult[jss::error] = "malformedRequest";
-            }
-        }
-    }
-    else
-    {
-        if (context.params.isMember("params") &&
-            context.params["params"].isArray() &&
-            context.params["params"].size() == 1 &&
-            context.params["params"][0u].isString())
-        {
-            if (!uNodeIndex.parseHex(context.params["params"][0u].asString()))
-            {
-                uNodeIndex = beast::zero;
-                jvResult[jss::error] = "malformedRequest";
-            }
-        }
-        else
-        {
-            if (context.apiVersion < 2u)
-                jvResult[jss::error] = "unknownOption";
-            else
-                jvResult[jss::error] = "invalidParams";
-        }
+            throw;
     }
 
     if (uNodeIndex.isNonZero())

src/test/rpc/LedgerRPC_test.cpp

@@ -1222,18 +1222,216 @@ class LedgerRPC_test : public beast::unit_test::suite
 
         std::string const ledgerHash{to_string(env.closed()->info().hash)};
 
+        auto makeParams = [&apiVersion](std::function<void(Json::Value&)> f) {
+            Json::Value params;
+            params[jss::api_version] = apiVersion;
+            f(params);
+            return params;
+        };
         // "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];
+        {
+            auto const jvParams =
+                makeParams([&ledgerHash](Json::Value& jvParams) {
+                    jvParams[jss::features] = ledgerHash;
+                    jvParams[jss::ledger_hash] = ledgerHash;
+                });
+            Json::Value const jrr = env.rpc(
+                "json", "ledger_entry", to_string(jvParams))[jss::result];
 
-        if (apiVersion < 2u)
-            checkErrorValue(jrr, "unknownOption", "");
-        else
-            checkErrorValue(jrr, "invalidParams", "");
+            if (apiVersion < 2u)
+                checkErrorValue(jrr, "unknownOption", "");
+            else
+                checkErrorValue(jrr, "invalidParams", "");
+        }
+        Json::Value const injectObject = []() {
+            Json::Value obj(Json::objectValue);
+            obj[jss::account] = "rhigTLJJyXXSRUyRCQtqi1NoAZZzZnS4KU";
+            obj[jss::ledger_index] = "validated";
+            return obj;
+        }();
+        Json::Value const injectArray = []() {
+            Json::Value arr(Json::arrayValue);
+            arr[0u] = "rhigTLJJyXXSRUyRCQtqi1NoAZZzZnS4KU";
+            arr[1u] = "validated";
+            return arr;
+        }();
+
+        // invalid input for fields that can handle an object, but can't handle
+        // an array
+        for (auto const& field :
+             {jss::directory, jss::escrow, jss::offer, jss::ticket, jss::amm})
+        {
+            auto const jvParams =
+                makeParams([&field, &injectArray](Json::Value& jvParams) {
+                    jvParams[field] = injectArray;
+                });
+
+            Json::Value const jrr = env.rpc(
+                "json", "ledger_entry", to_string(jvParams))[jss::result];
+
+            if (apiVersion < 2u)
+                checkErrorValue(jrr, "internal", "Internal error.");
+            else
+                checkErrorValue(jrr, "invalidParams", "");
+        }
+        // Fields that can handle objects just fine
+        for (auto const& field :
+             {jss::directory, jss::escrow, jss::offer, jss::ticket, jss::amm})
+        {
+            auto const jvParams =
+                makeParams([&field, &injectObject](Json::Value& jvParams) {
+                    jvParams[field] = injectObject;
+                });
+
+            Json::Value const jrr = env.rpc(
+                "json", "ledger_entry", to_string(jvParams))[jss::result];
+
+            checkErrorValue(jrr, "malformedRequest", "");
+        }
+
+        for (auto const& inject : {injectObject, injectArray})
+        {
+            // invalid input for fields that can't handle an object or an array
+            for (auto const& field :
+                 {jss::index,
+                  jss::account_root,
+                  jss::check,
+                  jss::payment_channel})
+            {
+                auto const jvParams =
+                    makeParams([&field, &inject](Json::Value& jvParams) {
+                        jvParams[field] = inject;
+                    });
+
+                Json::Value const jrr = env.rpc(
+                    "json", "ledger_entry", to_string(jvParams))[jss::result];
+
+                if (apiVersion < 2u)
+                    checkErrorValue(jrr, "internal", "Internal error.");
+                else
+                    checkErrorValue(jrr, "invalidParams", "");
+            }
+            // directory sub-fields
+            for (auto const& field : {jss::dir_root, jss::owner})
+            {
+                auto const jvParams =
+                    makeParams([&field, &inject](Json::Value& jvParams) {
+                        jvParams[jss::directory][field] = inject;
+                    });
+
+                Json::Value const jrr = env.rpc(
+                    "json", "ledger_entry", to_string(jvParams))[jss::result];
+
+                if (apiVersion < 2u)
+                    checkErrorValue(jrr, "internal", "Internal error.");
+                else
+                    checkErrorValue(jrr, "invalidParams", "");
+            }
+            // escrow sub-fields
+            {
+                auto const jvParams =
+                    makeParams([&inject](Json::Value& jvParams) {
+                        jvParams[jss::escrow][jss::owner] = inject;
+                        jvParams[jss::escrow][jss::seq] = 99;
+                    });
+
+                Json::Value const jrr = env.rpc(
+                    "json", "ledger_entry", to_string(jvParams))[jss::result];
+
+                if (apiVersion < 2u)
+                    checkErrorValue(jrr, "internal", "Internal error.");
+                else
+                    checkErrorValue(jrr, "invalidParams", "");
+            }
+            // offer sub-fields
+            {
+                auto const jvParams =
+                    makeParams([&inject](Json::Value& jvParams) {
+                        jvParams[jss::offer][jss::account] = inject;
+                        jvParams[jss::offer][jss::seq] = 99;
+                    });
+
+                Json::Value const jrr = env.rpc(
+                    "json", "ledger_entry", to_string(jvParams))[jss::result];
+
+                if (apiVersion < 2u)
+                    checkErrorValue(jrr, "internal", "Internal error.");
+                else
+                    checkErrorValue(jrr, "invalidParams", "");
+            }
+            // ripple_state sub-fields
+            {
+                auto const jvParams =
+                    makeParams([&inject](Json::Value& jvParams) {
+                        Json::Value rs(Json::objectValue);
+                        rs[jss::currency] = "FOO";
+                        rs[jss::accounts] = Json::Value(Json::arrayValue);
+                        rs[jss::accounts][0u] =
+                            "rhigTLJJyXXSRUyRCQtqi1NoAZZzZnS4KU";
+                        rs[jss::accounts][1u] =
+                            "rKssEq6pg1KbqEqAFnua5mFAL6Ggpsh2wv";
+                        rs[jss::currency] = inject;
+                        jvParams[jss::ripple_state] = std::move(rs);
+                    });
+
+                Json::Value const jrr = env.rpc(
+                    "json", "ledger_entry", to_string(jvParams))[jss::result];
+
+                if (apiVersion < 2u)
+                    checkErrorValue(jrr, "internal", "Internal error.");
+                else
+                    checkErrorValue(jrr, "invalidParams", "");
+            }
+            // ticket sub-fields
+            {
+                auto const jvParams =
+                    makeParams([&inject](Json::Value& jvParams) {
+                        jvParams[jss::ticket][jss::account] = inject;
+                        jvParams[jss::ticket][jss::ticket_seq] = 99;
+                    });
+
+                Json::Value const jrr = env.rpc(
+                    "json", "ledger_entry", to_string(jvParams))[jss::result];
+
+                if (apiVersion < 2u)
+                    checkErrorValue(jrr, "internal", "Internal error.");
+                else
+                    checkErrorValue(jrr, "invalidParams", "");
+            }
+
+            // Fields that can handle malformed inputs just fine
+            for (auto const& field : {jss::nft_page, jss::deposit_preauth})
+            {
+                auto const jvParams =
+                    makeParams([&field, &inject](Json::Value& jvParams) {
+                        jvParams[field] = inject;
+                    });
+
+                Json::Value const jrr = env.rpc(
+                    "json", "ledger_entry", to_string(jvParams))[jss::result];
+
+                checkErrorValue(jrr, "malformedRequest", "");
+            }
+            // Subfields of deposit_preauth that can handle malformed inputs
+            // fine
+            for (auto const& field : {jss::owner, jss::authorized})
+            {
+                auto const jvParams =
+                    makeParams([&field, &inject](Json::Value& jvParams) {
+                        auto pa = Json::Value(Json::objectValue);
+                        pa[jss::owner] = "rhigTLJJyXXSRUyRCQtqi1NoAZZzZnS4KU";
+                        pa[jss::authorized] =
+                            "rKssEq6pg1KbqEqAFnua5mFAL6Ggpsh2wv";
+                        pa[field] = inject;
+                        jvParams[jss::deposit_preauth] = std::move(pa);
+                    });
+
+                Json::Value const jrr = env.rpc(
+                    "json", "ledger_entry", to_string(jvParams))[jss::result];
+
+                checkErrorValue(jrr, "malformedRequest", "");
+            }
+        }
     }
 
     /// @brief ledger RPC requests as a way to drive