diff --git a/src/ripple/rpc/handlers/AccountObjects.cpp b/src/ripple/rpc/handlers/AccountObjects.cpp index 2531cd03115..264fa2251d0 100644 --- a/src/ripple/rpc/handlers/AccountObjects.cpp +++ b/src/ripple/rpc/handlers/AccountObjects.cpp @@ -259,18 +259,32 @@ doAccountObjects(RPC::JsonContext& context) return RPC::invalid_field_error(jss::marker); } - if (!RPC::getAccountObjects( - *ledger, - accountID, - typeFilter, - dirIndex, - entryIndex, - limit, - result)) + // check if dirIndex is valid + if (!dirIndex.isZero()) { - result[jss::account_objects] = Json::arrayValue; + auto sle = ledger->read({ltANY, dirIndex}); + + if (!sle || sle->getType() != ltDIR_NODE) + return RPC::invalid_field_error(jss::marker); + + if (sle->isFieldPresent(sfOwner) && sle->getAccountID(sfOwner) != accountID) + return RPC::invalid_field_error(jss::marker); } + auto accountObjectStatus = RPC::getAccountObjects( + *ledger, + accountID, + typeFilter, + dirIndex, + entryIndex, + limit, + result); + + if (accountObjectStatus == RPC::AccountObjectStatus::InvalidMarker) + return RPC::invalid_field_error(jss::marker); + else if (accountObjectStatus == RPC::AccountObjectStatus::Other) + result[jss::account_objects] = Json::arrayValue; + result[jss::account] = toBase58(accountID); context.loadType = Resource::feeMediumBurdenRPC; return result; diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp index a6179b04c88..90da25f4879 100644 --- a/src/ripple/rpc/impl/RPCHelpers.cpp +++ b/src/ripple/rpc/impl/RPCHelpers.cpp @@ -148,7 +148,7 @@ isRelatedToAccount( return false; } -bool +AccountObjectStatus getAccountObjects( ReadView const& ledger, AccountID const& account, @@ -219,7 +219,7 @@ getAccountObjects( { jvResult[jss::limit] = limit; jvResult[jss::marker] = std::string("0,") + to_string(ck); - return true; + return AccountObjectStatus::Success; } } @@ -250,7 +250,10 @@ getAccountObjects( { // it's possible the user had nftoken pages but no // directory entries - return mlimit < limit; + if (mlimit < limit) + return AccountObjectStatus::Success; + else + return AccountObjectStatus::Other; } std::uint32_t i = 0; @@ -262,8 +265,9 @@ getAccountObjects( if (!found) { iter = std::find(iter, entries.end(), entryIndex); + // can't find entryIndex provided invalid entryIndex if (iter == entries.end()) - return false; + return AccountObjectStatus::InvalidMarker; found = true; } @@ -275,7 +279,7 @@ getAccountObjects( jvResult[jss::limit] = limit; jvResult[jss::marker] = to_string(dirIndex) + ',' + to_string(*iter); - return true; + return AccountObjectStatus::Success; } for (; iter != entries.end(); ++iter) @@ -295,7 +299,7 @@ getAccountObjects( jvResult[jss::limit] = limit; jvResult[jss::marker] = to_string(dirIndex) + ',' + to_string(*iter); - return true; + return AccountObjectStatus::Success; } break; @@ -304,12 +308,12 @@ getAccountObjects( auto const nodeIndex = dir->getFieldU64(sfIndexNext); if (nodeIndex == 0) - return true; + return AccountObjectStatus::Success; dirIndex = keylet::page(root, nodeIndex).key; dir = ledger.read({ltDIR_NODE, dirIndex}); if (!dir) - return true; + return AccountObjectStatus::Success; if (i == mlimit) { @@ -321,7 +325,7 @@ getAccountObjects( to_string(dirIndex) + ',' + to_string(*e.begin()); } - return true; + return AccountObjectStatus::Success; } } } diff --git a/src/ripple/rpc/impl/RPCHelpers.h b/src/ripple/rpc/impl/RPCHelpers.h index e003773e50c..03b7de55739 100644 --- a/src/ripple/rpc/impl/RPCHelpers.h +++ b/src/ripple/rpc/impl/RPCHelpers.h @@ -48,6 +48,8 @@ namespace RPC { struct JsonContext; +enum AccountObjectStatus { Success, InvalidMarker, Other }; + /** Get an AccountID from an account ID or public key. */ std::optional accountFromStringStrict(std::string const&); @@ -102,7 +104,7 @@ isRelatedToAccount( @param limit Maximum number of objects to find. @param jvResult A JSON result that holds the request objects. */ -bool +AccountObjectStatus getAccountObjects( ReadView const& ledger, AccountID const& account, diff --git a/src/test/rpc/AccountObjects_test.cpp b/src/test/rpc/AccountObjects_test.cpp index 17217f2c880..e3917f7f8fe 100644 --- a/src/test/rpc/AccountObjects_test.cpp +++ b/src/test/rpc/AccountObjects_test.cpp @@ -1032,6 +1032,100 @@ class AccountObjects_test : public beast::unit_test::suite BEAST_EXPECT(acct_objs_is_size(acct_objs(gw, jss::hashes), 0)); } + void + testAccountObjectMarker() + { + testcase("AccountObjectMarker"); + + using namespace jtx; + Env env(*this); + + Account const alice{"alice"}; + Account const bob{"bob"}; + + env.fund(XRP(10000), alice); + env.fund(XRP(10000), bob); + + unsigned accountObjectSize = 30; + for (unsigned i = 0; i < accountObjectSize; i++) { + env(check::create(alice, bob, XRP(10))); + } + + env.close(); + + unsigned limit = 11; + Json::Value marker; + // get account_objects without marker with limit and update marker + { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::limit] = limit; + params[jss::ledger_index] = "validated"; + auto resp = env.rpc("json", "account_objects", to_string(params)); + auto& accountObjects = resp[jss::result][jss::account_objects]; + marker = resp[jss::result][jss::marker]; + BEAST_EXPECT(!resp[jss::result].isMember(jss::error)); + BEAST_EXPECT(accountObjects.size() == limit); + } + + // get account_objects with valid marker and update marker + { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::limit] = limit; + params[jss::marker] = marker; + params[jss::ledger_index] = "validated"; + auto resp = env.rpc("json", "account_objects", to_string(params)); + auto& accountObjects = resp[jss::result][jss::account_objects]; + marker = resp[jss::result][jss::marker]; + BEAST_EXPECT(!resp[jss::result].isMember(jss::error)); + BEAST_EXPECT(accountObjects.size() == limit); + } + + // continue getting account_objects with valid marker + { + Json::Value params; + params[jss::account] = bob.human(); + params[jss::limit] = limit; + params[jss::marker] = marker; + params[jss::ledger_index] = "validated"; + auto resp = env.rpc("json", "account_objects", to_string(params)); + auto& accountObjects = resp[jss::result][jss::account_objects]; + BEAST_EXPECT(!resp[jss::result].isMember(jss::error)); + BEAST_EXPECT(accountObjects.size() == accountObjectSize - limit * 2); + } + + // get account_objects with an invalid marker containing invalid dirIndex + { + std::string s = marker.asString(); + s.replace(0, 7, "FFFFFFF"); + Json::Value params; + params[jss::account] = bob.human(); + params[jss::limit] = limit; + params[jss::marker] = s; + params[jss::ledger_index] = "validated"; + auto resp = env.rpc("json", "account_objects", to_string(params)); + BEAST_EXPECT(resp[jss::result][jss::error_message] == "Invalid field \'marker\'."); + } + + // get account_objects with an invalid marker containing invalid entryIndex + { + // construct an invalid entry + std::stringstream ss(marker.asString()); + std::string s; + std::getline(ss, s, ','); + s = s + ",0"; + + Json::Value params; + params[jss::account] = bob.human(); + params[jss::limit] = limit; + params[jss::marker] = s; + params[jss::ledger_index] = "validated"; + auto resp = env.rpc("json", "account_objects", to_string(params)); + BEAST_EXPECT(resp[jss::result][jss::error_message] == "Invalid field \'marker\'."); + } + } + void run() override { @@ -1039,6 +1133,7 @@ class AccountObjects_test : public beast::unit_test::suite testUnsteppedThenStepped(); testUnsteppedThenSteppedWithNFTs(); testObjectTypes(); + testAccountObjectMarker(); } };