diff --git a/src/ripple/rpc/handlers/AccountInfo.cpp b/src/ripple/rpc/handlers/AccountInfo.cpp index 8c85670af6a..bd2184f49a3 100644 --- a/src/ripple/rpc/handlers/AccountInfo.cpp +++ b/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; diff --git a/src/ripple/rpc/handlers/AccountTx.cpp b/src/ripple/rpc/handlers/AccountTx.cpp index 84d087939e7..cee8a629c75 100644 --- a/src/ripple/rpc/handlers/AccountTx.cpp +++ b/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 = diff --git a/src/ripple/rpc/handlers/NoRippleCheck.cpp b/src/ripple/rpc/handlers/NoRippleCheck.cpp index 20137c985c9..1942372f3e3 100644 --- a/src/ripple/rpc/handlers/NoRippleCheck.cpp +++ b/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 ledger; auto result = RPC::lookupLedger(ledger, context); if (!ledger) diff --git a/src/test/rpc/AccountTx_test.cpp b/src/test/rpc/AccountTx_test.cpp index 05baf869eee..24f6737917b 100644 --- a/src/test/rpc/AccountTx_test.cpp +++ b/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"); } } diff --git a/src/test/rpc/NoRipple_test.cpp b/src/test/rpc/NoRipple_test.cpp index 472706f0b73..3077b06f8a3 100644 --- a/src/test/rpc/NoRipple_test.cpp +++ b/src/test/rpc/NoRipple_test.cpp @@ -19,6 +19,7 @@ #include #include +#include #include namespace ripple { @@ -202,9 +203,12 @@ class NoRipple_test : public beast::unit_test::suite } void - testDefaultRipple(FeatureBitset features) + testDefaultRipple(FeatureBitset features, unsigned int apiVersion) { - testcase("Set default ripple on an account and check new trustlines"); + testcase( + "Set default ripple on an account and check new trustlines " + "Version " + + std::to_string(apiVersion)); using namespace jtx; Env env(*this, features); @@ -221,9 +225,10 @@ class NoRipple_test : public beast::unit_test::suite env(trust(gw, USD(100), alice, 0)); env(trust(gw, USD(100), bob, 0)); + Json::Value params; + params[jss::api_version] = apiVersion; { - Json::Value params; params[jss::account] = gw.human(); params[jss::peer] = alice.human(); @@ -232,7 +237,6 @@ class NoRipple_test : public beast::unit_test::suite BEAST_EXPECT(line0[jss::no_ripple_peer].asBool() == true); } { - Json::Value params; params[jss::account] = alice.human(); params[jss::peer] = gw.human(); @@ -241,7 +245,6 @@ class NoRipple_test : public beast::unit_test::suite BEAST_EXPECT(line0[jss::no_ripple].asBool() == true); } { - Json::Value params; params[jss::account] = gw.human(); params[jss::peer] = bob.human(); @@ -250,7 +253,6 @@ class NoRipple_test : public beast::unit_test::suite BEAST_EXPECT(line0[jss::no_ripple].asBool() == false); } { - Json::Value params; params[jss::account] = bob.human(); params[jss::peer] = gw.human(); @@ -258,6 +260,22 @@ class NoRipple_test : public beast::unit_test::suite auto const& line0 = lines[jss::result][jss::lines][0u]; BEAST_EXPECT(line0[jss::no_ripple_peer].asBool() == false); } + { + // test for transactions + { + params[jss::account] = bob.human(); + params[jss::role] = "gateway"; + params[jss::transactions] = "asdf"; + + auto lines = + env.rpc("json", "noripple_check", to_string(params)); + if (apiVersion < 2u) + BEAST_EXPECT(lines[jss::result][jss::status] == "success"); + else + BEAST_EXPECT( + lines[jss::result][jss::error] == "invalidParams"); + } + } } void @@ -266,9 +284,14 @@ class NoRipple_test : public beast::unit_test::suite testSetAndClear(); auto withFeatsTests = [this](FeatureBitset features) { + for (auto testVersion = RPC::apiMinimumSupportedVersion; + testVersion <= RPC::apiBetaVersion; + ++testVersion) + { + testDefaultRipple(features, testVersion); + } testNegativeBalance(features); testPairwise(features); - testDefaultRipple(features); }; using namespace jtx; auto const sa = supported_amendments();