fix account_objects with invalid marker

XRPLF · Oct 3, 2024 · 735dc6a · 735dc6a
1 parent 3f51432
commit 735dc6a
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 39 deletions.
diff --git a/src/test/rpc/AccountObjects_test.cpp b/src/test/rpc/AccountObjects_test.cpp
@@ -1230,19 +1230,15 @@ class AccountObjects_test : public beast::unit_test::suite
 
         Account const alice{"alice"};
         Account const bob{"bob"};
+        env.fund(XRP(10000), alice, bob);
 
-        env.fund(XRP(10000), alice);
-        env.fund(XRP(10000), bob);
-
-        unsigned accountObjectSize = 30;
+        unsigned const accountObjectSize = 30;
         for (unsigned i = 0; i < accountObjectSize; i++)
-        {
             env(check::create(alice, bob, XRP(10)));
-        }
 
         env.close();
 
-        unsigned limit = 11;
+        unsigned const limit = 11;
         Json::Value marker;
         // get account_objects without marker with limit and update marker
         {

diff --git a/src/xrpld/rpc/detail/RPCHelpers.cpp b/src/xrpld/rpc/detail/RPCHelpers.cpp
@@ -148,7 +148,7 @@ isRelatedToAccount(
     return false;
 }
 
-AccountObjectStatus
+bool
 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 AccountObjectStatus::Success;
+                    return true;
                 }
             }
 
@@ -250,10 +250,10 @@ getAccountObjects(
     {
         // it's possible the user had nftoken pages but no
         // directory entries
-        if (mlimit < limit)
-            return AccountObjectStatus::Success;
-        else
-            return AccountObjectStatus::Other;
+        if (mlimit >= limit)
+            jvResult[jss::account_objects] = Json::arrayValue;
+
+        return true;
     }
 
     std::uint32_t i = 0;
@@ -267,7 +267,7 @@ getAccountObjects(
             iter = std::find(iter, entries.end(), entryIndex);
             // can't find entryIndex provided invalid entryIndex
             if (iter == entries.end())
-                return AccountObjectStatus::InvalidMarker;
+                return false;
 
             found = true;
         }
@@ -279,7 +279,7 @@ getAccountObjects(
             jvResult[jss::limit] = limit;
             jvResult[jss::marker] =
                 to_string(dirIndex) + ',' + to_string(*iter);
-            return AccountObjectStatus::Success;
+            return true;
         }
 
         for (; iter != entries.end(); ++iter)
@@ -299,7 +299,7 @@ getAccountObjects(
                     jvResult[jss::limit] = limit;
                     jvResult[jss::marker] =
                         to_string(dirIndex) + ',' + to_string(*iter);
-                    return AccountObjectStatus::Success;
+                    return true;
                 }
 
                 break;
@@ -308,12 +308,12 @@ getAccountObjects(
 
         auto const nodeIndex = dir->getFieldU64(sfIndexNext);
         if (nodeIndex == 0)
-            return AccountObjectStatus::Success;
+            return true;
 
         dirIndex = keylet::page(root, nodeIndex).key;
         dir = ledger.read({ltDIR_NODE, dirIndex});
         if (!dir)
-            return AccountObjectStatus::Success;
+            return true;
 
         if (i == mlimit)
         {
@@ -325,7 +325,7 @@ getAccountObjects(
                     to_string(dirIndex) + ',' + to_string(*e.begin());
             }
 
-            return AccountObjectStatus::Success;
+            return true;
         }
     }
 }

diff --git a/src/xrpld/rpc/detail/RPCHelpers.h b/src/xrpld/rpc/detail/RPCHelpers.h
@@ -48,8 +48,6 @@ 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&);
@@ -104,7 +102,7 @@ isRelatedToAccount(
     @param limit Maximum number of objects to find.
     @param jvResult A JSON result that holds the request objects.
 */
-AccountObjectStatus
+bool
 getAccountObjects(
     ReadView const& ledger,
     AccountID const& account,

diff --git a/src/xrpld/rpc/handlers/AccountObjects.cpp b/src/xrpld/rpc/handlers/AccountObjects.cpp
@@ -284,25 +284,18 @@ doAccountObjects(RPC::JsonContext& context)
     }
 
     // check if dirIndex is valid
-    if (!dirIndex.isZero())
-    {
-        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 (!dirIndex.isZero() && !ledger->read({ltDIR_NODE, dirIndex}))
+        return RPC::invalid_field_error(jss::marker);
 
-    if (accountObjectStatus == RPC::AccountObjectStatus::InvalidMarker)
+    if (!RPC::getAccountObjects(
+            *ledger,
+            accountID,
+            typeFilter,
+            dirIndex,
+            entryIndex,
+            limit,
+            result))
         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;