fix: AccountNFT with invalid marker (#1589)

Fixes [#1497](#1497) Mimics the behavior of the [fix on Rippled side](XRPLF/rippled#5045)
XRPLF · Aug 27, 2024 · 01ecedc · 01ecedc
1 parent 9a9de50
commit 01ecedc
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 1 deletion.
diff --git a/src/rpc/handlers/AccountNFTs.cpp b/src/rpc/handlers/AccountNFTs.cpp
@@ -78,10 +78,17 @@ AccountNFTsHandler::process(AccountNFTsHandler::Input input, Context const& ctx)
         input.marker ? ripple::uint256{input.marker->c_str()} : ripple::keylet::nftpage_max(*accountID).key;
     auto const blob = sharedPtrBackend_->fetchLedgerObject(pageKey, lgrInfo.seq, ctx.yield);
 
-    if (!blob)
+    if (!blob) {
+        if (input.marker.has_value())
+            return Error{Status{RippledError::rpcINVALID_PARAMS, "Marker field does not match any valid Page ID"}};
         return response;
+    }
 
     std::optional<ripple::SLE const> page{ripple::SLE{ripple::SerialIter{blob->data(), blob->size()}, pageKey}};
+
+    if (page->getType() != ripple::ltNFTOKEN_PAGE)
+        return Error{Status{RippledError::rpcINVALID_PARAMS, "Marker matches Page ID from another Account"}};
+
     auto numPages = 0u;
 
     while (page) {

diff --git a/tests/unit/rpc/handlers/AccountNFTsTests.cpp b/tests/unit/rpc/handlers/AccountNFTsTests.cpp
@@ -49,6 +49,7 @@ constexpr static auto TAXON = 0;
 constexpr static auto FLAG = 8;
 constexpr static auto TXNID = "E6DBAFC99223B42257915A63DFC6B0C032D4070F9A574B255AD97466726FC321";
 constexpr static auto PAGE = "E6DBAFC99223B42257915A63DFC6B0C032D4070F9A574B255AD97466726FC322";
+constexpr static auto INVALIDPAGE = "E6DBAFC99223B42257915A63DFC6B0C032D4070F9A574B255AD97466726FCAAA";
 constexpr static auto MAXSEQ = 30;
 constexpr static auto MINSEQ = 10;
 
@@ -402,6 +403,98 @@ TEST_F(RPCAccountNFTsHandlerTest, Marker)
     });
 }
 
+TEST_F(RPCAccountNFTsHandlerTest, InvalidMarker)
+{
+    backend->setRange(MINSEQ, MAXSEQ);
+    auto const ledgerHeader = CreateLedgerHeader(LEDGERHASH, MAXSEQ);
+    EXPECT_CALL(*backend, fetchLedgerBySequence).Times(1);
+    ON_CALL(*backend, fetchLedgerBySequence).WillByDefault(Return(ledgerHeader));
+
+    auto const accountObject = CreateAccountRootObject(ACCOUNT, 0, 1, 10, 2, TXNID, 3);
+    auto const accountID = GetAccountIDWithString(ACCOUNT);
+    ON_CALL(*backend, doFetchLedgerObject(ripple::keylet::account(accountID).key, 30, _))
+        .WillByDefault(Return(accountObject.getSerializer().peekData()));
+
+    auto static const input = json::parse(fmt::format(
+        R"({{
+            "account":"{}",
+            "marker":"{}"
+        }})",
+        ACCOUNT,
+        INVALIDPAGE
+    ));
+    auto const handler = AnyHandler{AccountNFTsHandler{backend}};
+    runSpawn([&](auto yield) {
+        auto const output = handler.process(input, Context{yield});
+        ASSERT_FALSE(output);
+        auto const err = rpc::makeError(output.result.error());
+        EXPECT_EQ(err.at("error").as_string(), "invalidParams");
+        EXPECT_EQ(err.at("error_message").as_string(), "Marker field does not match any valid Page ID");
+    });
+}
+
+TEST_F(RPCAccountNFTsHandlerTest, AccountWithNoNFT)
+{
+    backend->setRange(MINSEQ, MAXSEQ);
+    auto const ledgerHeader = CreateLedgerHeader(LEDGERHASH, MAXSEQ);
+    EXPECT_CALL(*backend, fetchLedgerBySequence).Times(1);
+    ON_CALL(*backend, fetchLedgerBySequence).WillByDefault(Return(ledgerHeader));
+
+    auto const accountObject = CreateAccountRootObject(ACCOUNT, 0, 1, 10, 2, TXNID, 3);
+    auto const accountID = GetAccountIDWithString(ACCOUNT);
+    ON_CALL(*backend, doFetchLedgerObject(ripple::keylet::account(accountID).key, 30, _))
+        .WillByDefault(Return(accountObject.getSerializer().peekData()));
+
+    auto static const input = json::parse(fmt::format(
+        R"({{
+            "account":"{}"
+        }})",
+        ACCOUNT
+    ));
+    auto const handler = AnyHandler{AccountNFTsHandler{backend}};
+    runSpawn([&](auto yield) {
+        auto const output = handler.process(input, Context{yield});
+        ASSERT_TRUE(output);
+        EXPECT_EQ(output.result->as_object().at("account_nfts").as_array().size(), 0);
+    });
+}
+
+TEST_F(RPCAccountNFTsHandlerTest, invalidPage)
+{
+    backend->setRange(MINSEQ, MAXSEQ);
+    auto const ledgerHeader = CreateLedgerHeader(LEDGERHASH, MAXSEQ);
+    EXPECT_CALL(*backend, fetchLedgerBySequence).Times(1);
+    ON_CALL(*backend, fetchLedgerBySequence).WillByDefault(Return(ledgerHeader));
+
+    auto const accountObject = CreateAccountRootObject(ACCOUNT, 0, 1, 10, 2, TXNID, 3);
+    auto const accountID = GetAccountIDWithString(ACCOUNT);
+    ON_CALL(*backend, doFetchLedgerObject(ripple::keylet::account(accountID).key, 30, _))
+        .WillByDefault(Return(accountObject.getSerializer().peekData()));
+
+    auto const pageObject =
+        CreateNFTTokenPage(std::vector{std::make_pair<std::string, std::string>(TOKENID, "www.ok.com")}, std::nullopt);
+    ON_CALL(*backend, doFetchLedgerObject(ripple::uint256{PAGE}, 30, _))
+        .WillByDefault(Return(accountObject.getSerializer().peekData()));
+    EXPECT_CALL(*backend, doFetchLedgerObject).Times(2);
+
+    auto static const input = json::parse(fmt::format(
+        R"({{
+            "account":"{}",
+            "marker":"{}"
+        }})",
+        ACCOUNT,
+        PAGE
+    ));
+    auto const handler = AnyHandler{AccountNFTsHandler{backend}};
+    runSpawn([&](auto yield) {
+        auto const output = handler.process(input, Context{yield});
+        ASSERT_FALSE(output);
+        auto const err = rpc::makeError(output.result.error());
+        EXPECT_EQ(err.at("error").as_string(), "invalidParams");
+        EXPECT_EQ(err.at("error_message").as_string(), "Marker matches Page ID from another Account");
+    });
+}
+
 TEST_F(RPCAccountNFTsHandlerTest, LimitLessThanMin)
 {
     static auto const expectedOutput = fmt::format(