Skip to content

Commit 7360c4f

Browse files
fix: AccountNFT with invalid marker (#1589)
Fixes [#1497](#1497) Mimics the behavior of the [fix on Rippled side](XRPLF/rippled#5045)
1 parent 9a9de50 commit 7360c4f

File tree

2 files changed

+101
-1
lines changed

2 files changed

+101
-1
lines changed

src/rpc/handlers/AccountNFTs.cpp

+8-1
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,17 @@ AccountNFTsHandler::process(AccountNFTsHandler::Input input, Context const& ctx)
7878
input.marker ? ripple::uint256{input.marker->c_str()} : ripple::keylet::nftpage_max(*accountID).key;
7979
auto const blob = sharedPtrBackend_->fetchLedgerObject(pageKey, lgrInfo.seq, ctx.yield);
8080

81-
if (!blob)
81+
if (!blob) {
82+
if (input.marker.has_value())
83+
return Error{Status{RippledError::rpcINVALID_PARAMS, "Marker field does not match any valid Page ID"}};
8284
return response;
85+
}
8386

8487
std::optional<ripple::SLE const> page{ripple::SLE{ripple::SerialIter{blob->data(), blob->size()}, pageKey}};
88+
89+
if (page->getType() != ripple::ltNFTOKEN_PAGE)
90+
return Error{Status{RippledError::rpcINVALID_PARAMS, "Marker matches Page ID from another Account"}};
91+
8592
auto numPages = 0u;
8693

8794
while (page) {

tests/unit/rpc/handlers/AccountNFTsTests.cpp

+93
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ constexpr static auto TAXON = 0;
4949
constexpr static auto FLAG = 8;
5050
constexpr static auto TXNID = "E6DBAFC99223B42257915A63DFC6B0C032D4070F9A574B255AD97466726FC321";
5151
constexpr static auto PAGE = "E6DBAFC99223B42257915A63DFC6B0C032D4070F9A574B255AD97466726FC322";
52+
constexpr static auto INVALIDPAGE = "E6DBAFC99223B42257915A63DFC6B0C032D4070F9A574B255AD97466726FCAAA";
5253
constexpr static auto MAXSEQ = 30;
5354
constexpr static auto MINSEQ = 10;
5455

@@ -402,6 +403,98 @@ TEST_F(RPCAccountNFTsHandlerTest, Marker)
402403
});
403404
}
404405

406+
TEST_F(RPCAccountNFTsHandlerTest, InvalidMarker)
407+
{
408+
backend->setRange(MINSEQ, MAXSEQ);
409+
auto const ledgerHeader = CreateLedgerHeader(LEDGERHASH, MAXSEQ);
410+
EXPECT_CALL(*backend, fetchLedgerBySequence).Times(1);
411+
ON_CALL(*backend, fetchLedgerBySequence).WillByDefault(Return(ledgerHeader));
412+
413+
auto const accountObject = CreateAccountRootObject(ACCOUNT, 0, 1, 10, 2, TXNID, 3);
414+
auto const accountID = GetAccountIDWithString(ACCOUNT);
415+
ON_CALL(*backend, doFetchLedgerObject(ripple::keylet::account(accountID).key, 30, _))
416+
.WillByDefault(Return(accountObject.getSerializer().peekData()));
417+
418+
auto static const input = json::parse(fmt::format(
419+
R"({{
420+
"account":"{}",
421+
"marker":"{}"
422+
}})",
423+
ACCOUNT,
424+
INVALIDPAGE
425+
));
426+
auto const handler = AnyHandler{AccountNFTsHandler{backend}};
427+
runSpawn([&](auto yield) {
428+
auto const output = handler.process(input, Context{yield});
429+
ASSERT_FALSE(output);
430+
auto const err = rpc::makeError(output.result.error());
431+
EXPECT_EQ(err.at("error").as_string(), "invalidParams");
432+
EXPECT_EQ(err.at("error_message").as_string(), "Marker field does not match any valid Page ID");
433+
});
434+
}
435+
436+
TEST_F(RPCAccountNFTsHandlerTest, AccountWithNoNFT)
437+
{
438+
backend->setRange(MINSEQ, MAXSEQ);
439+
auto const ledgerHeader = CreateLedgerHeader(LEDGERHASH, MAXSEQ);
440+
EXPECT_CALL(*backend, fetchLedgerBySequence).Times(1);
441+
ON_CALL(*backend, fetchLedgerBySequence).WillByDefault(Return(ledgerHeader));
442+
443+
auto const accountObject = CreateAccountRootObject(ACCOUNT, 0, 1, 10, 2, TXNID, 3);
444+
auto const accountID = GetAccountIDWithString(ACCOUNT);
445+
ON_CALL(*backend, doFetchLedgerObject(ripple::keylet::account(accountID).key, 30, _))
446+
.WillByDefault(Return(accountObject.getSerializer().peekData()));
447+
448+
auto static const input = json::parse(fmt::format(
449+
R"({{
450+
"account":"{}"
451+
}})",
452+
ACCOUNT
453+
));
454+
auto const handler = AnyHandler{AccountNFTsHandler{backend}};
455+
runSpawn([&](auto yield) {
456+
auto const output = handler.process(input, Context{yield});
457+
ASSERT_TRUE(output);
458+
EXPECT_EQ(output.result->as_object().at("account_nfts").as_array().size(), 0);
459+
});
460+
}
461+
462+
TEST_F(RPCAccountNFTsHandlerTest, invalidPage)
463+
{
464+
backend->setRange(MINSEQ, MAXSEQ);
465+
auto const ledgerHeader = CreateLedgerHeader(LEDGERHASH, MAXSEQ);
466+
EXPECT_CALL(*backend, fetchLedgerBySequence).Times(1);
467+
ON_CALL(*backend, fetchLedgerBySequence).WillByDefault(Return(ledgerHeader));
468+
469+
auto const accountObject = CreateAccountRootObject(ACCOUNT, 0, 1, 10, 2, TXNID, 3);
470+
auto const accountID = GetAccountIDWithString(ACCOUNT);
471+
ON_CALL(*backend, doFetchLedgerObject(ripple::keylet::account(accountID).key, 30, _))
472+
.WillByDefault(Return(accountObject.getSerializer().peekData()));
473+
474+
auto const pageObject =
475+
CreateNFTTokenPage(std::vector{std::make_pair<std::string, std::string>(TOKENID, "www.ok.com")}, std::nullopt);
476+
ON_CALL(*backend, doFetchLedgerObject(ripple::uint256{PAGE}, 30, _))
477+
.WillByDefault(Return(accountObject.getSerializer().peekData()));
478+
EXPECT_CALL(*backend, doFetchLedgerObject).Times(2);
479+
480+
auto static const input = json::parse(fmt::format(
481+
R"({{
482+
"account":"{}",
483+
"marker":"{}"
484+
}})",
485+
ACCOUNT,
486+
PAGE
487+
));
488+
auto const handler = AnyHandler{AccountNFTsHandler{backend}};
489+
runSpawn([&](auto yield) {
490+
auto const output = handler.process(input, Context{yield});
491+
ASSERT_FALSE(output);
492+
auto const err = rpc::makeError(output.result.error());
493+
EXPECT_EQ(err.at("error").as_string(), "invalidParams");
494+
EXPECT_EQ(err.at("error_message").as_string(), "Marker matches Page ID from another Account");
495+
});
496+
}
497+
405498
TEST_F(RPCAccountNFTsHandlerTest, LimitLessThanMin)
406499
{
407500
static auto const expectedOutput = fmt::format(

0 commit comments

Comments
 (0)