Skip to content

Commit

Permalink
fix account_objects with invalid marker do not return an error
Browse files Browse the repository at this point in the history 
fix account_objects with invalid marker do not return an error

fix account_objects with invalid marker do not return an error
  • Loading branch information
@yinyiqian1
yinyiqian1 committed Jun 18, 2024
1 parent 58f3abe commit 6117405
Show file tree
Hide file tree
Showing 4 changed files with 134 additions and 19 deletions.
32 changes: 23 additions & 9 deletions src/ripple/rpc/handlers/AccountObjects.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
22 changes: 13 additions & 9 deletions src/ripple/rpc/impl/RPCHelpers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ isRelatedToAccount(
return false;
}

bool
AccountObjectStatus
getAccountObjects(
ReadView const& ledger,
AccountID const& account,
Expand Down Expand Up @@ -219,7 +219,7 @@ getAccountObjects(
{
jvResult[jss::limit] = limit;
jvResult[jss::marker] = std::string("0,") + to_string(ck);
return true;
return AccountObjectStatus::Success;
}
}

Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
Expand All @@ -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)
Expand All @@ -295,7 +299,7 @@ getAccountObjects(
jvResult[jss::limit] = limit;
jvResult[jss::marker] =
to_string(dirIndex) + ',' + to_string(*iter);
return true;
return AccountObjectStatus::Success;
}

break;
Expand All @@ -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;

Check warning on line 316 in src/ripple/rpc/impl/RPCHelpers.cpp

View check run for this annotation

Codecov / codecov/patch

src/ripple/rpc/impl/RPCHelpers.cpp#L316 

Added line #L316 was not covered by tests

if (i == mlimit)
{
Expand All @@ -321,7 +325,7 @@ getAccountObjects(
to_string(dirIndex) + ',' + to_string(*e.begin());
}

return true;
return AccountObjectStatus::Success;

Check warning on line 328 in src/ripple/rpc/impl/RPCHelpers.cpp

View check run for this annotation

Codecov / codecov/patch

src/ripple/rpc/impl/RPCHelpers.cpp#L328 

Added line #L328 was not covered by tests
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/ripple/rpc/impl/RPCHelpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<AccountID>
accountFromStringStrict(std::string const&);
Expand Down Expand Up @@ -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,
Expand Down
95 changes: 95 additions & 0 deletions src/test/rpc/AccountObjects_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1032,13 +1032,108 @@ 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
{
testErrors();
testUnsteppedThenStepped();
testUnsteppedThenSteppedWithNFTs();
testObjectTypes();
testAccountObjectMarker();
}
};

Expand Down

0 comments on commit 6117405

Please sign in to comment.