diff --git a/src/test/rpc/AccountOffers_test.cpp b/src/test/rpc/AccountOffers_test.cpp index 2aa85ac9b9..59a6942ed2 100644 --- a/src/test/rpc/AccountOffers_test.cpp +++ b/src/test/rpc/AccountOffers_test.cpp @@ -190,12 +190,6 @@ public: } { - // now make a limit (= 0) query for the same data - // since we operate on the admin port, the limit - // value of 0 is not adjusted into tuned ranges for admin requests - // so we literally get 0 elements in that case. For non-admin - // requests, we get limit defaults applied thus all our results - // come back (we are below the min results limit) Json::Value jvParams; jvParams[jss::account] = bob.human(); jvParams[jss::limit] = 0u; @@ -203,18 +197,7 @@ public: "json", "account_offers", jvParams.toStyledString())[jss::result]; - auto const& jro = jrr[jss::offers]; - if (asAdmin) - { - // limit == 0 is invalid - BEAST_EXPECT(jrr.isMember(jss::error_message)); - } - else - { - // Call should enforce min limit of 10 - BEAST_EXPECT(checkArraySize(jro, 3u)); - BEAST_EXPECT(!jrr.isMember(jss::marker)); - } + BEAST_EXPECT(jrr.isMember(jss::error_message)); } } diff --git a/src/test/rpc/AccountTx_test.cpp b/src/test/rpc/AccountTx_test.cpp index 82809b5c5b..cbe7cc8f4a 100644 --- a/src/test/rpc/AccountTx_test.cpp +++ b/src/test/rpc/AccountTx_test.cpp @@ -193,26 +193,26 @@ class AccountTx_test : public beast::unit_test::suite j[jss::result][jss::error] == RPC::get_error_info(code).token; }; - Json::Value jParms; - jParms[jss::api_version] = apiVersion; + Json::Value jParams; + jParams[jss::api_version] = apiVersion; BEAST_EXPECT(isErr( - env.rpc("json", "account_tx", to_string(jParms)), + env.rpc("json", "account_tx", to_string(jParams)), rpcINVALID_PARAMS)); - jParms[jss::account] = "0xDEADBEEF"; + jParams[jss::account] = "0xDEADBEEF"; BEAST_EXPECT(isErr( - env.rpc("json", "account_tx", to_string(jParms)), + env.rpc("json", "account_tx", to_string(jParams)), rpcACT_MALFORMED)); - jParms[jss::account] = A1.human(); + jParams[jss::account] = A1.human(); BEAST_EXPECT(hasTxs( - env.rpc(apiVersion, "json", "account_tx", to_string(jParms)))); + env.rpc(apiVersion, "json", "account_tx", to_string(jParams)))); // Ledger min/max index { - Json::Value p{jParms}; + Json::Value p{jParams}; p[jss::ledger_index_min] = -1; p[jss::ledger_index_max] = -1; BEAST_EXPECT(hasTxs( @@ -247,7 +247,7 @@ class AccountTx_test : public beast::unit_test::suite } // Ledger index min only { - Json::Value p{jParms}; + Json::Value p{jParams}; p[jss::ledger_index_min] = -1; BEAST_EXPECT(hasTxs( env.rpc(apiVersion, "json", "account_tx", to_string(p)))); @@ -270,7 +270,7 @@ class AccountTx_test : public beast::unit_test::suite // Ledger index max only { - Json::Value p{jParms}; + Json::Value p{jParams}; p[jss::ledger_index_max] = -1; BEAST_EXPECT(hasTxs( env.rpc(apiVersion, "json", "account_tx", to_string(p)))); @@ -298,7 +298,7 @@ class AccountTx_test : public beast::unit_test::suite // Ledger Sequence { - Json::Value p{jParms}; + Json::Value p{jParams}; p[jss::ledger_index] = env.closed()->info().seq; BEAST_EXPECT(hasTxs( @@ -319,7 +319,7 @@ class AccountTx_test : public beast::unit_test::suite // Ledger Hash { - Json::Value p{jParms}; + Json::Value p{jParams}; p[jss::ledger_hash] = to_string(env.closed()->info().hash); BEAST_EXPECT(hasTxs( @@ -332,9 +332,9 @@ class AccountTx_test : public beast::unit_test::suite // 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}; + jParams[jss::account] = "0xDEADBEEF"; + jParams[jss::account] = A1.human(); + Json::Value p{jParams}; p[jss::ledger_index_max] = -1; p[jss::ledger_index_min] = -1; @@ -351,7 +351,7 @@ class AccountTx_test : public beast::unit_test::suite // Ledger index max only { - Json::Value p{jParms}; + Json::Value p{jParams}; p[jss::ledger_index_max] = env.current()->info().seq; if (apiVersion < 2u) BEAST_EXPECT(hasTxs( @@ -382,7 +382,7 @@ class AccountTx_test : public beast::unit_test::suite } // test binary and forward for bool/non bool values { - Json::Value p{jParms}; + Json::Value p{jParams}; p[jss::binary] = "asdf"; if (apiVersion < 2u) { @@ -410,6 +410,117 @@ class AccountTx_test : public beast::unit_test::suite result = env.rpc("json", "account_tx", to_string(p)); BEAST_EXPECT(result[jss::result][jss::status] == "success"); } + // test limit with malformed values + { + Json::Value p{jParams}; + + // Test case: limit = 0 should fail (below minimum) + p[jss::limit] = 0; + BEAST_EXPECT(isErr( + env.rpc("json", "account_tx", to_string(p)), + rpcINVALID_PARAMS)); + + // Test case: limit = 1.2 should fail (not an integer) + p[jss::limit] = 1.2; + BEAST_EXPECT( + env.rpc( + "json", + "account_tx", + to_string(p))[jss::result][jss::error_message] == + RPC::expected_field_message(jss::limit, "unsigned integer")); + + // Test case: limit = "10" should fail (string instead of integer) + p[jss::limit] = "10"; + BEAST_EXPECT( + env.rpc( + "json", + "account_tx", + to_string(p))[jss::result][jss::error_message] == + RPC::expected_field_message(jss::limit, "unsigned integer")); + + // Test case: limit = true should fail (boolean instead of integer) + p[jss::limit] = true; + BEAST_EXPECT( + env.rpc( + "json", + "account_tx", + to_string(p))[jss::result][jss::error_message] == + RPC::expected_field_message(jss::limit, "unsigned integer")); + + // Test case: limit = false should fail (boolean instead of integer) + p[jss::limit] = false; + BEAST_EXPECT( + env.rpc( + "json", + "account_tx", + to_string(p))[jss::result][jss::error_message] == + RPC::expected_field_message(jss::limit, "unsigned integer")); + + // Test case: limit = -1 should fail (negative number) + p[jss::limit] = -1; + BEAST_EXPECT( + env.rpc( + "json", + "account_tx", + to_string(p))[jss::result][jss::error_message] == + RPC::expected_field_message(jss::limit, "unsigned integer")); + + // Test case: limit = [] should fail (array instead of integer) + p[jss::limit] = Json::Value(Json::arrayValue); + BEAST_EXPECT( + env.rpc( + "json", + "account_tx", + to_string(p))[jss::result][jss::error_message] == + RPC::expected_field_message(jss::limit, "unsigned integer")); + + // Test case: limit = {} should fail (object instead of integer) + p[jss::limit] = Json::Value(Json::objectValue); + BEAST_EXPECT( + env.rpc( + "json", + "account_tx", + to_string(p))[jss::result][jss::error_message] == + RPC::expected_field_message(jss::limit, "unsigned integer")); + + // Test case: limit = "malformed" should fail (malformed string) + p[jss::limit] = "malformed"; + BEAST_EXPECT( + env.rpc( + "json", + "account_tx", + to_string(p))[jss::result][jss::error_message] == + RPC::expected_field_message(jss::limit, "unsigned integer")); + + // Test case: limit = ["limit"] should fail (array with string) + p[jss::limit] = Json::Value(Json::arrayValue); + p[jss::limit].append("limit"); + BEAST_EXPECT( + env.rpc( + "json", + "account_tx", + to_string(p))[jss::result][jss::error_message] == + RPC::expected_field_message(jss::limit, "unsigned integer")); + + // Test case: limit = {"limit": 10} should fail (object with + // property) + p[jss::limit] = Json::Value(Json::objectValue); + p[jss::limit][jss::limit] = 10; + BEAST_EXPECT( + env.rpc( + "json", + "account_tx", + to_string(p))[jss::result][jss::error_message] == + RPC::expected_field_message(jss::limit, "unsigned integer")); + + // Test case: limit = 10 should succeed (valid integer) + p[jss::limit] = 10; + BEAST_EXPECT( + env.rpc( + "json", + "account_tx", + to_string(p))[jss::result][jss::status] == "success"); + } } void diff --git a/src/test/rpc/Book_test.cpp b/src/test/rpc/Book_test.cpp index 7177ab847c..0d5366d3ef 100644 --- a/src/test/rpc/Book_test.cpp +++ b/src/test/rpc/Book_test.cpp @@ -1633,6 +1633,20 @@ public: "Invalid field 'limit', not unsigned integer."); } + { + Json::Value jvParams; + jvParams[jss::ledger_index] = "validated"; + jvParams[jss::taker] = env.master.human(); + jvParams[jss::limit] = 0; // must be > 0 + jvParams[jss::taker_pays][jss::currency] = "XRP"; + jvParams[jss::taker_gets][jss::currency] = "USD"; + jvParams[jss::taker_gets][jss::issuer] = gw.human(); + auto const jrr = env.rpc( + "json", "book_offers", to_string(jvParams))[jss::result]; + BEAST_EXPECT(jrr[jss::error] == "invalidParams"); + BEAST_EXPECT(jrr[jss::error_message] == "Invalid field 'limit'."); + } + { Json::Value jvParams; jvParams[jss::ledger_index] = "validated"; @@ -1710,11 +1724,6 @@ public: BEAST_EXPECT(jrr[jss::offers].size() == (asAdmin ? 1u : 0u)); // NOTE - a marker field is not returned for this method - jvParams[jss::limit] = 0u; - jrr = env.rpc("json", "book_offers", to_string(jvParams))[jss::result]; - BEAST_EXPECT(jrr[jss::offers].isArray()); - BEAST_EXPECT(jrr[jss::offers].size() == 0u); - jvParams[jss::limit] = RPC::Tuning::bookOffers.rmax + 1; jrr = env.rpc("json", "book_offers", to_string(jvParams))[jss::result]; BEAST_EXPECT(jrr[jss::offers].isArray()); diff --git a/src/xrpld/rpc/detail/RPCHelpers.cpp b/src/xrpld/rpc/detail/RPCHelpers.cpp index edb91611be..9c49751bcd 100644 --- a/src/xrpld/rpc/detail/RPCHelpers.cpp +++ b/src/xrpld/rpc/detail/RPCHelpers.cpp @@ -704,15 +704,21 @@ readLimitField( JsonContext const& context) { limit = range.rdefault; - if (auto const& jvLimit = context.params[jss::limit]) - { - if (!(jvLimit.isUInt() || (jvLimit.isInt() && jvLimit.asInt() >= 0))) - return RPC::expected_field_error(jss::limit, "unsigned integer"); + if (!context.params.isMember(jss::limit) || + context.params[jss::limit].isNull()) + return std::nullopt; + + auto const& jvLimit = context.params[jss::limit]; + if (!(jvLimit.isUInt() || (jvLimit.isInt() && jvLimit.asInt() >= 0))) + return RPC::expected_field_error(jss::limit, "unsigned integer"); + + limit = jvLimit.asUInt(); + if (limit == 0) + return RPC::invalid_field_error(jss::limit); + + if (!isUnlimited(context.role)) + limit = std::max(range.rmin, std::min(range.rmax, limit)); - limit = jvLimit.asUInt(); - if (!isUnlimited(context.role)) - limit = std::max(range.rmin, std::min(range.rmax, limit)); - } return std::nullopt; } diff --git a/src/xrpld/rpc/detail/Tuning.h b/src/xrpld/rpc/detail/Tuning.h index 4f4a8be1bf..f3218a3fb1 100644 --- a/src/xrpld/rpc/detail/Tuning.h +++ b/src/xrpld/rpc/detail/Tuning.h @@ -45,6 +45,9 @@ static LimitRange constexpr accountObjects = {10, 200, 400}; /** Limits for the account_offers command. */ static LimitRange constexpr accountOffers = {10, 200, 400}; +/** Limits for the account_tx command. */ +static LimitRange constexpr accountTx = {10, 200, 400}; + /** Limits for the book_offers command. */ static LimitRange constexpr bookOffers = {0, 60, 100}; diff --git a/src/xrpld/rpc/handlers/AccountChannels.cpp b/src/xrpld/rpc/handlers/AccountChannels.cpp index 17e46f052f..1a21f67860 100644 --- a/src/xrpld/rpc/handlers/AccountChannels.cpp +++ b/src/xrpld/rpc/handlers/AccountChannels.cpp @@ -103,9 +103,6 @@ doAccountChannels(RPC::JsonContext& context) if (auto err = readLimitField(limit, RPC::Tuning::accountChannels, context)) return *err; - if (limit == 0u) - return rpcError(rpcINVALID_PARAMS); - Json::Value jsonChannels{Json::arrayValue}; struct VisitData { diff --git a/src/xrpld/rpc/handlers/AccountLines.cpp b/src/xrpld/rpc/handlers/AccountLines.cpp index 146a9527a9..9fb076445b 100644 --- a/src/xrpld/rpc/handlers/AccountLines.cpp +++ b/src/xrpld/rpc/handlers/AccountLines.cpp @@ -120,9 +120,6 @@ doAccountLines(RPC::JsonContext& context) if (auto err = readLimitField(limit, RPC::Tuning::accountLines, context)) return *err; - if (limit == 0) - return rpcError(rpcINVALID_PARAMS); - // this flag allows the requester to ask incoming trustlines in default // state be omitted bool ignoreDefault = params.isMember(jss::ignore_default) && diff --git a/src/xrpld/rpc/handlers/AccountOffers.cpp b/src/xrpld/rpc/handlers/AccountOffers.cpp index 1f2b76efe4..737f39cc13 100644 --- a/src/xrpld/rpc/handlers/AccountOffers.cpp +++ b/src/xrpld/rpc/handlers/AccountOffers.cpp @@ -86,9 +86,6 @@ doAccountOffers(RPC::JsonContext& context) if (auto err = readLimitField(limit, RPC::Tuning::accountOffers, context)) return *err; - if (limit == 0) - return RPC::invalid_field_error(jss::limit); - Json::Value& jsonOffers(result[jss::offers] = Json::arrayValue); std::vector> offers; uint256 startAfter = beast::zero; diff --git a/src/xrpld/rpc/handlers/AccountTx.cpp b/src/xrpld/rpc/handlers/AccountTx.cpp index e053c2adc0..64247c732e 100644 --- a/src/xrpld/rpc/handlers/AccountTx.cpp +++ b/src/xrpld/rpc/handlers/AccountTx.cpp @@ -26,6 +26,8 @@ #include #include #include +#include +#include #include #include @@ -429,7 +431,10 @@ doAccountTxJson(RPC::JsonContext& context) return RPC::invalid_field_error(jss::forward); } - args.limit = params.isMember(jss::limit) ? params[jss::limit].asUInt() : 0; + if (auto const err = + RPC::readLimitField(args.limit, RPC::Tuning::accountTx, context)) + return *err; + args.binary = params.isMember(jss::binary) && params[jss::binary].asBool(); args.forward = params.isMember(jss::forward) && params[jss::forward].asBool();