APIv2(account_tx, noripple_check): return error on invalid input (XRPLF#4620)

For the `account_tx` and `noripple_check` methods, perform input
validation for optional parameters such as "binary", "forward",
"strict", "transactions". Previously, when these parameters had invalid
values (e.g. not a bool), no error would be returned. Now, it returns an
`invalidParams` error.

* This updates the behavior to match Clio
  (https://github.com/XRPLF/clio).
* Since this is potentially a breaking change, it only applies to
  requests specifying api_version: 2.
* Fix XRPLF#4543.

Author: PeterChen13579

src/ripple/rpc/handlers/AccountInfo.cpp

@@ -134,10 +134,12 @@ doAccountInfo(RPC::JsonContext& context)
 
         result[jss::account_flags] = std::move(acctFlags);
 
-        // The document states that signer_lists is a bool, however
-        // assigning any string value works. Do not allow this.
-        // This check is for api Version 2 onwards only
-        if (!params[jss::signer_lists].isBool() && context.apiVersion > 1)
+        // The document[https://xrpl.org/account_info.html#account_info] states
+        // that signer_lists is a bool, however assigning any string value
+        // works. Do not allow this. This check is for api Version 2 onwards
+        // only
+        if (context.apiVersion > 1u && params.isMember(jss::signer_lists) &&
+            !params[jss::signer_lists].isBool())
         {
             RPC::inject_error(rpcINVALID_PARAMS, result);
             return result;

src/ripple/rpc/handlers/AccountTx.cpp

@@ -58,7 +58,7 @@ parseLedgerArgs(RPC::Context& context, Json::Value const& params)
     Json::Value response;
     // if ledger_index_min or max is specified, then ledger_hash or ledger_index
     // should not be specified. Error out if it is
-    if (context.apiVersion > 1)
+    if (context.apiVersion > 1u)
     {
         if ((params.isMember(jss::ledger_index_min) ||
              params.isMember(jss::ledger_index_max)) &&
@@ -162,7 +162,7 @@ getLedgerRange(
                     // if ledger_index_min or ledger_index_max is out of
                     // valid ledger range, error out. exclude -1 as
                     // it is a valid input
-                    if (context.apiVersion > 1)
+                    if (context.apiVersion > 1u)
                     {
                         if ((ls.max > uValidatedMax && ls.max != -1) ||
                             (ls.min < uValidatedMin && ls.min != 0))
@@ -389,6 +389,21 @@ doAccountTxJson(RPC::JsonContext& context)
     AccountTxArgs args;
     Json::Value response;
 
+    // The document[https://xrpl.org/account_tx.html#account_tx] states that
+    // binary and forward params are both boolean values, however, assigning any
+    // string value works. Do not allow this. This check is for api Version 2
+    // onwards only
+    if (context.apiVersion > 1u && params.isMember(jss::binary) &&
+        !params[jss::binary].isBool())
+    {
+        return rpcError(rpcINVALID_PARAMS);
+    }
+    if (context.apiVersion > 1u && params.isMember(jss::forward) &&
+        !params[jss::forward].isBool())
+    {
+        return rpcError(rpcINVALID_PARAMS);
+    }
+
     args.limit = params.isMember(jss::limit) ? params[jss::limit].asUInt() : 0;
     args.binary = params.isMember(jss::binary) && params[jss::binary].asBool();
     args.forward =

src/ripple/rpc/handlers/NoRippleCheck.cpp

@@ -83,6 +83,16 @@ doNoRippleCheck(RPC::JsonContext& context)
     if (params.isMember(jss::transactions))
         transactions = params["transactions"].asBool();
 
+    // The document[https://xrpl.org/noripple_check.html#noripple_check] states
+    // that transactions params is a boolean value, however, assigning any
+    // string value works. Do not allow this. This check is for api Version 2
+    // onwards only
+    if (context.apiVersion > 1u && params.isMember(jss::transactions) &&
+        !params[jss::transactions].isBool())
+    {
+        return rpcError(rpcINVALID_PARAMS);
+    }
+
     std::shared_ptr<ReadView const> ledger;
     auto result = RPC::lookupLedger(ledger, context);
     if (!ledger)

src/test/rpc/AccountTx_test.cpp

@@ -144,176 +144,192 @@ class AccountTx_test : public beast::unit_test::suite
         Json::Value jParms;
         jParms[jss::api_version] = apiVersion;
 
-        if (apiVersion < 2)
-        {
-            BEAST_EXPECT(isErr(
-                env.rpc("json", "account_tx", to_string(jParms)),
-                rpcINVALID_PARAMS));
+        BEAST_EXPECT(isErr(
+            env.rpc("json", "account_tx", to_string(jParms)),
+            rpcINVALID_PARAMS));
 
-            jParms[jss::account] = "0xDEADBEEF";
+        jParms[jss::account] = "0xDEADBEEF";
 
-            BEAST_EXPECT(isErr(
-                env.rpc("json", "account_tx", to_string(jParms)),
-                rpcACT_MALFORMED));
+        BEAST_EXPECT(isErr(
+            env.rpc("json", "account_tx", to_string(jParms)),
+            rpcACT_MALFORMED));
 
-            jParms[jss::account] = A1.human();
-            BEAST_EXPECT(
-                hasTxs(env.rpc("json", "account_tx", to_string(jParms))));
+        jParms[jss::account] = A1.human();
+        BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(jParms))));
 
-            // Ledger min/max index
-            {
-                Json::Value p{jParms};
-                p[jss::ledger_index_min] = -1;
-                p[jss::ledger_index_max] = -1;
-                BEAST_EXPECT(
-                    hasTxs(env.rpc("json", "account_tx", to_string(p))));
-
-                p[jss::ledger_index_min] = 0;
-                p[jss::ledger_index_max] = 100;
+        // Ledger min/max index
+        {
+            Json::Value p{jParms};
+            p[jss::ledger_index_min] = -1;
+            p[jss::ledger_index_max] = -1;
+            BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));
+
+            p[jss::ledger_index_min] = 0;
+            p[jss::ledger_index_max] = 100;
+            if (apiVersion < 2u)
                 BEAST_EXPECT(
                     hasTxs(env.rpc("json", "account_tx", to_string(p))));
+            else
+                BEAST_EXPECT(isErr(
+                    env.rpc("json", "account_tx", to_string(p)),
+                    rpcLGR_IDX_MALFORMED));
 
-                p[jss::ledger_index_min] = 1;
-                p[jss::ledger_index_max] = 2;
+            p[jss::ledger_index_min] = 1;
+            p[jss::ledger_index_max] = 2;
+            if (apiVersion < 2u)
                 BEAST_EXPECT(
                     noTxs(env.rpc("json", "account_tx", to_string(p))));
-
-                p[jss::ledger_index_min] = 2;
-                p[jss::ledger_index_max] = 1;
+            else
                 BEAST_EXPECT(isErr(
                     env.rpc("json", "account_tx", to_string(p)),
-                    (RPC::apiMaximumSupportedVersion == 1
-                         ? rpcLGR_IDXS_INVALID
-                         : rpcINVALID_LGR_RANGE)));
-            }
+                    rpcLGR_IDX_MALFORMED));
 
-            // Ledger index min only
-            {
-                Json::Value p{jParms};
-                p[jss::ledger_index_min] = -1;
-                BEAST_EXPECT(
-                    hasTxs(env.rpc("json", "account_tx", to_string(p))));
+            p[jss::ledger_index_min] = 2;
+            p[jss::ledger_index_max] = 1;
+            BEAST_EXPECT(isErr(
+                env.rpc("json", "account_tx", to_string(p)),
+                (apiVersion == 1 ? rpcLGR_IDXS_INVALID
+                                 : rpcINVALID_LGR_RANGE)));
+        }
+        // Ledger index min only
+        {
+            Json::Value p{jParms};
+            p[jss::ledger_index_min] = -1;
+            BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));
 
-                p[jss::ledger_index_min] = 1;
+            p[jss::ledger_index_min] = 1;
+            if (apiVersion < 2u)
                 BEAST_EXPECT(
                     hasTxs(env.rpc("json", "account_tx", to_string(p))));
-
-                p[jss::ledger_index_min] = env.current()->info().seq;
+            else
                 BEAST_EXPECT(isErr(
                     env.rpc("json", "account_tx", to_string(p)),
-                    (RPC::apiMaximumSupportedVersion == 1
-                         ? rpcLGR_IDXS_INVALID
-                         : rpcINVALID_LGR_RANGE)));
-            }
+                    rpcLGR_IDX_MALFORMED));
 
-            // Ledger index max only
-            {
-                Json::Value p{jParms};
-                p[jss::ledger_index_max] = -1;
-                BEAST_EXPECT(
-                    hasTxs(env.rpc("json", "account_tx", to_string(p))));
+            p[jss::ledger_index_min] = env.current()->info().seq;
+            BEAST_EXPECT(isErr(
+                env.rpc("json", "account_tx", to_string(p)),
+                (apiVersion == 1 ? rpcLGR_IDXS_INVALID
+                                 : rpcINVALID_LGR_RANGE)));
+        }
 
-                p[jss::ledger_index_max] = env.current()->info().seq;
-                BEAST_EXPECT(
-                    hasTxs(env.rpc("json", "account_tx", to_string(p))));
+        // Ledger index max only
+        {
+            Json::Value p{jParms};
+            p[jss::ledger_index_max] = -1;
+            BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));
 
-                p[jss::ledger_index_max] = 3;
+            p[jss::ledger_index_max] = env.current()->info().seq;
+            if (apiVersion < 2u)
                 BEAST_EXPECT(
                     hasTxs(env.rpc("json", "account_tx", to_string(p))));
+            else
+                BEAST_EXPECT(isErr(
+                    env.rpc("json", "account_tx", to_string(p)),
+                    rpcLGR_IDX_MALFORMED));
 
-                p[jss::ledger_index_max] = env.closed()->info().seq;
-                BEAST_EXPECT(
-                    hasTxs(env.rpc("json", "account_tx", to_string(p))));
+            p[jss::ledger_index_max] = 3;
+            BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));
 
-                p[jss::ledger_index_max] = env.closed()->info().seq - 1;
-                BEAST_EXPECT(
-                    noTxs(env.rpc("json", "account_tx", to_string(p))));
-            }
+            p[jss::ledger_index_max] = env.closed()->info().seq;
+            BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));
 
-            // Ledger Sequence
-            {
-                Json::Value p{jParms};
+            p[jss::ledger_index_max] = env.closed()->info().seq - 1;
+            BEAST_EXPECT(noTxs(env.rpc("json", "account_tx", to_string(p))));
+        }
 
-                p[jss::ledger_index] = env.closed()->info().seq;
-                BEAST_EXPECT(
-                    hasTxs(env.rpc("json", "account_tx", to_string(p))));
+        // Ledger Sequence
+        {
+            Json::Value p{jParms};
 
-                p[jss::ledger_index] = env.closed()->info().seq - 1;
-                BEAST_EXPECT(
-                    noTxs(env.rpc("json", "account_tx", to_string(p))));
+            p[jss::ledger_index] = env.closed()->info().seq;
+            BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));
 
-                p[jss::ledger_index] = env.current()->info().seq;
-                BEAST_EXPECT(isErr(
-                    env.rpc("json", "account_tx", to_string(p)),
-                    rpcLGR_NOT_VALIDATED));
+            p[jss::ledger_index] = env.closed()->info().seq - 1;
+            BEAST_EXPECT(noTxs(env.rpc("json", "account_tx", to_string(p))));
 
-                p[jss::ledger_index] = env.current()->info().seq + 1;
-                BEAST_EXPECT(isErr(
-                    env.rpc("json", "account_tx", to_string(p)),
-                    rpcLGR_NOT_FOUND));
-            }
+            p[jss::ledger_index] = env.current()->info().seq;
+            BEAST_EXPECT(isErr(
+                env.rpc("json", "account_tx", to_string(p)),
+                rpcLGR_NOT_VALIDATED));
 
-            // Ledger Hash
-            {
-                Json::Value p{jParms};
+            p[jss::ledger_index] = env.current()->info().seq + 1;
+            BEAST_EXPECT(isErr(
+                env.rpc("json", "account_tx", to_string(p)), rpcLGR_NOT_FOUND));
+        }
 
-                p[jss::ledger_hash] = to_string(env.closed()->info().hash);
-                BEAST_EXPECT(
-                    hasTxs(env.rpc("json", "account_tx", to_string(p))));
+        // Ledger Hash
+        {
+            Json::Value p{jParms};
 
-                p[jss::ledger_hash] =
-                    to_string(env.closed()->info().parentHash);
-                BEAST_EXPECT(
-                    noTxs(env.rpc("json", "account_tx", to_string(p))));
-            }
+            p[jss::ledger_hash] = to_string(env.closed()->info().hash);
+            BEAST_EXPECT(hasTxs(env.rpc("json", "account_tx", to_string(p))));
+
+            p[jss::ledger_hash] = to_string(env.closed()->info().parentHash);
+            BEAST_EXPECT(noTxs(env.rpc("json", "account_tx", to_string(p))));
         }
-        else
+
+        // Ledger index max/min/index all specified
+        // ERRORS out with invalid Parenthesis
         {
-            // Ledger index max/min/index all specified
-            // ERRORS out with invalid Parenthesis
-            {
-                jParms[jss::account] = "0xDEADBEEF";
-                jParms[jss::account] = A1.human();
-                Json::Value p{jParms};
+            jParms[jss::account] = "0xDEADBEEF";
+            jParms[jss::account] = A1.human();
+            Json::Value p{jParms};
 
-                p[jss::ledger_index_max] = -1;
-                p[jss::ledger_index_min] = -1;
-                p[jss::ledger_index] = -1;
+            p[jss::ledger_index_max] = -1;
+            p[jss::ledger_index_min] = -1;
+            p[jss::ledger_index] = -1;
 
+            if (apiVersion < 2u)
+                BEAST_EXPECT(
+                    hasTxs(env.rpc("json", "account_tx", to_string(p))));
+            else
                 BEAST_EXPECT(isErr(
                     env.rpc("json", "account_tx", to_string(p)),
                     rpcINVALID_PARAMS));
-            }
-
-            // Ledger index min/max only
-            {
-                Json::Value p{jParms};
-                p[jss::ledger_index_max] = 100;
-                p[jss::ledger_index_min] = 0;
-                BEAST_EXPECT(isErr(
-                    env.rpc("json", "account_tx", to_string(p)),
-                    rpcLGR_IDX_MALFORMED));
+        }
 
-                p[jss::ledger_index_max] = -1;
-                p[jss::ledger_index_min] = -1;
+        // Ledger index max only
+        {
+            Json::Value p{jParms};
+            p[jss::ledger_index_max] = env.current()->info().seq;
+            if (apiVersion < 2u)
                 BEAST_EXPECT(
                     hasTxs(env.rpc("json", "account_tx", to_string(p))));
-
-                p[jss::ledger_index_min] = 2;
-                p[jss::ledger_index_max] = 1;
+            else
                 BEAST_EXPECT(isErr(
                     env.rpc("json", "account_tx", to_string(p)),
-                    rpcINVALID_LGR_RANGE));
+                    rpcLGR_IDX_MALFORMED));
+        }
+        // test binary and forward for bool/non bool values
+        {
+            Json::Value p{jParms};
+            p[jss::binary] = "asdf";
+            if (apiVersion < 2u)
+            {
+                Json::Value result{env.rpc("json", "account_tx", to_string(p))};
+                BEAST_EXPECT(result[jss::result][jss::status] == "success");
             }
+            else
+                BEAST_EXPECT(isErr(
+                    env.rpc("json", "account_tx", to_string(p)),
+                    rpcINVALID_PARAMS));
 
-            // Ledger index max only
-            {
-                Json::Value p{jParms};
-                p[jss::ledger_index_max] = env.current()->info().seq;
+            p[jss::binary] = true;
+            Json::Value result{env.rpc("json", "account_tx", to_string(p))};
+            BEAST_EXPECT(result[jss::result][jss::status] == "success");
+
+            p[jss::forward] = "true";
+            if (apiVersion < 2u)
+                BEAST_EXPECT(result[jss::result][jss::status] == "success");
+            else
                 BEAST_EXPECT(isErr(
                     env.rpc("json", "account_tx", to_string(p)),
-                    rpcLGR_IDX_MALFORMED));
-            }
+                    rpcINVALID_PARAMS));
+
+            p[jss::forward] = false;
+            result = env.rpc("json", "account_tx", to_string(p));
+            BEAST_EXPECT(result[jss::result][jss::status] == "success");
         }
     }