fix account_objects with invalid marker

XRPLF · Oct 4, 2024 · e66a096 · e66a096
1 parent a753099
commit e66a096
Show file tree

Hide file tree

Showing 3 changed files with 108 additions and 4 deletions.
diff --git a/src/test/rpc/AccountObjects_test.cpp b/src/test/rpc/AccountObjects_test.cpp
@@ -1220,6 +1220,104 @@ class AccountObjects_test : public beast::unit_test::suite
         }
     }
 
+    void
+    testAccountObjectMarker()
+    {
+        testcase("AccountObjectMarker");
+
+        using namespace jtx;
+        Env env(*this);
+
+        Account const alice{"alice"};
+        Account const bob{"bob"};
+        env.fund(XRP(10000), alice, bob);
+
+        unsigned const accountObjectSize = 30;
+        for (unsigned i = 0; i < accountObjectSize; i++)
+            env(check::create(alice, bob, XRP(10)));
+
+        env.close();
+
+        unsigned const 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
     {
@@ -1229,6 +1327,7 @@ class AccountObjects_test : public beast::unit_test::suite
         testObjectTypes();
         testNFTsMarker();
         testAccountNFTs();
+        testAccountObjectMarker();
     }
 };
 

diff --git a/src/xrpld/rpc/detail/RPCHelpers.cpp b/src/xrpld/rpc/detail/RPCHelpers.cpp
@@ -250,7 +250,10 @@ getAccountObjects(
     {
         // it's possible the user had nftoken pages but no
         // directory entries
-        return mlimit < limit;
+        if (mlimit >= limit)
+            jvResult[jss::account_objects] = Json::arrayValue;
+
+        return true;
     }
 
     std::uint32_t i = 0;

diff --git a/src/xrpld/rpc/handlers/AccountObjects.cpp b/src/xrpld/rpc/handlers/AccountObjects.cpp
@@ -283,6 +283,10 @@ doAccountObjects(RPC::JsonContext& context)
             return RPC::invalid_field_error(jss::marker);
     }
 
+    // check if dirIndex is valid
+    if (!dirIndex.isZero() && !ledger->read({ltDIR_NODE, dirIndex}))
+        return RPC::invalid_field_error(jss::marker);
+
     if (!RPC::getAccountObjects(
             *ledger,
             accountID,
@@ -291,9 +295,7 @@ doAccountObjects(RPC::JsonContext& context)
             entryIndex,
             limit,
             result))
-    {
-        result[jss::account_objects] = Json::arrayValue;
-    }
+        return RPC::invalid_field_error(jss::marker);
 
     result[jss::account] = toBase58(accountID);
     context.loadType = Resource::feeMediumBurdenRPC;