specify [min, default, max] limits in handler table (#135)

* specify rpc limits in the handler table
* special case in ledger_data if !binary

src/rpc/RPC.cpp

@@ -1,6 +1,7 @@
 #include <boost/asio/spawn.hpp>
 #include <etl/ETLSource.h>
 #include <rpc/Handlers.h>
+#include <rpc/RPCHelpers.h>
 #include <unordered_map>
 
 namespace RPC {
@@ -126,33 +127,81 @@ make_error(Status const& status)
     json["type"] = "response";
     return json;
 }
-static std::unordered_map<std::string, std::function<Result(Context const&)>>
+
-    handlerTable{
+using LimitRange = std::tuple<std::uint32_t, std::uint32_t, std::uint32_t>;
-        {"account_channels", &doAccountChannels},
+using HandlerFunction = std::function<Result(Context const&)>;
-        {"account_currencies", &doAccountCurrencies},
+
-        {"account_info", &doAccountInfo},
+struct Handler
-        {"account_lines", &doAccountLines},
+{
-        {"account_nfts", &doAccountNFTs},
+    std::string method;
-        {"account_objects", &doAccountObjects},
+    std::function<Result(Context const&)> handler;
-        {"account_offers", &doAccountOffers},
+    std::optional<LimitRange> limit;
-        {"account_tx", &doAccountTx},
+};
-        {"gateway_balances", &doGatewayBalances},
+
-        {"noripple_check", &doNoRippleCheck},
+class HandlerTable
-        {"book_offers", &doBookOffers},
+{
-        {"channel_authorize", &doChannelAuthorize},
+    std::unordered_map<std::string, Handler> handlerMap_;
-        {"channel_verify", &doChannelVerify},
+
-        {"ledger", &doLedger},
+public:
-        {"ledger_data", &doLedgerData},
+    HandlerTable(std::initializer_list<Handler> handlers)
-        {"ledger_entry", &doLedgerEntry},
+    {
-        {"ledger_range", &doLedgerRange},
+        for (auto const& handler : handlers)
-        {"nft_buy_offers", &doNFTBuyOffers},
+        {
-        {"nft_sell_offers", &doNFTSellOffers},
+            handlerMap_[handler.method] = std::move(handler);
-        {"subscribe", &doSubscribe},
+        }
-        {"server_info", &doServerInfo},
+    }
-        {"unsubscribe", &doUnsubscribe},
+
-        {"tx", &doTx},
+    bool
-        {"transaction_entry", &doTransactionEntry},
+    contains(std::string const& method)
-        {"random", &doRandom}};
+    {
+        return handlerMap_.contains(method);
+    }
+
+    std::optional<LimitRange>
+    getLimitRange(std::string const& command)
+    {
+        if (!handlerMap_.contains(command))
+            return {};
+
+        return handlerMap_[command].limit;
+    }
+
+    std::optional<HandlerFunction>
+    getHandler(std::string const& command)
+    {
+        if (!handlerMap_.contains(command))
+            return {};
+
+        return handlerMap_[command].handler;
+    }
+};
+
+static HandlerTable handlerTable{
+    {"account_channels", &doAccountChannels, LimitRange{10, 50, 256}},
+    {"account_currencies", &doAccountCurrencies, {}},
+    {"account_info", &doAccountInfo, {}},
+    {"account_lines", &doAccountLines, LimitRange{10, 50, 256}},
+    {"account_nfts", &doAccountNFTs, LimitRange{1, 5, 10}},
+    {"account_objects", &doAccountObjects, LimitRange{10, 50, 256}},
+    {"account_offers", &doAccountOffers, LimitRange{10, 50, 256}},
+    {"account_tx", &doAccountTx, LimitRange{1, 50, 100}},
+    {"gateway_balances", &doGatewayBalances, {}},
+    {"noripple_check", &doNoRippleCheck, {}},
+    {"book_offers", &doBookOffers, LimitRange{1, 50, 100}},
+    {"channel_authorize", &doChannelAuthorize, {}},
+    {"channel_verify", &doChannelVerify, {}},
+    {"ledger", &doLedger, {}},
+    {"ledger_data", &doLedgerData, LimitRange{1, 100, 2048}},
+    {"nft_buy_offers", &doNFTBuyOffers, LimitRange{1, 50, 100}},
+    {"nft_sell_offers", &doNFTSellOffers, LimitRange{1, 50, 100}},
+    {"ledger_entry", &doLedgerEntry, {}},
+    {"ledger_range", &doLedgerRange, {}},
+    {"subscribe", &doSubscribe, {}},
+    {"server_info", &doServerInfo, {}},
+    {"unsubscribe", &doUnsubscribe, {}},
+    {"tx", &doTx, {}},
+    {"transaction_entry", &doTransactionEntry, {}},
+    {"random", &doRandom, {}}};
 
 static std::unordered_set<std::string> forwardCommands{
     "submit",
@@ -169,6 +218,36 @@ validHandler(std::string const& method)
     return handlerTable.contains(method) || forwardCommands.contains(method);
 }
 
+Status
+getLimit(RPC::Context const& context, std::uint32_t& limit)
+{
+    if (!handlerTable.getHandler(context.method))
+        return Status{Error::rpcUNKNOWN_COMMAND};
+
+    if (!handlerTable.getLimitRange(context.method))
+        return Status{Error::rpcINVALID_PARAMS, "rpcDoesNotRequireLimit"};
+
+    auto [lo, def, hi] = *handlerTable.getLimitRange(context.method);
+
+    if (context.params.contains(JS(limit)))
+    {
+        if (!context.params.at(JS(limit)).is_int64())
+            return Status{Error::rpcINVALID_PARAMS, "limitNotInt"};
+
+        limit = context.params.at(JS(limit)).as_int64();
+        if (limit <= 0)
+            return Status{Error::rpcINVALID_PARAMS, "limitNotPositive"};
+
+        limit = std::clamp(limit, lo, hi);
+    }
+    else
+    {
+        limit = def;
+    }
+
+    return {};
+}
+
 bool
 shouldForwardToRippled(Context const& ctx)
 {
@@ -219,14 +298,14 @@ buildResponse(Context const& ctx)
     if (ctx.method == "ping")
         return boost::json::object{};
 
-    if (handlerTable.find(ctx.method) == handlerTable.end())
+    auto method = handlerTable.getHandler(ctx.method);
-        return Status{Error::rpcUNKNOWN_COMMAND};
 
-    auto method = handlerTable[ctx.method];
+    if (!method)
+        return Status{Error::rpcUNKNOWN_COMMAND};
 
     try
     {
-        auto v = method(ctx);
+        auto v = (*method)(ctx);
 
         if (auto object = std::get_if<boost::json::object>(&v))
             (*object)["validated"] = true;

src/rpc/RPC.h

@@ -199,6 +199,9 @@ buildResponse(Context const& ctx);
 bool
 validHandler(std::string const& method);
 
+Status
+getLimit(RPC::Context const& context, std::uint32_t& limit);
+
 template <class T>
 void
 logDuration(Context const& ctx, T const& dur)

src/rpc/RPCHelpers.cpp

@@ -191,22 +191,6 @@ getHexMarker(boost::json::object const& request, ripple::uint256& marker)
     return {};
 }
 
-Status
-getLimit(boost::json::object const& request, std::uint32_t& limit)
-{
-    if (request.contains(JS(limit)))
-    {
-        if (!request.at(JS(limit)).is_int64())
-            return Status{Error::rpcINVALID_PARAMS, "limitNotInt"};
-
-        limit = request.at(JS(limit)).as_int64();
-        if (limit <= 0)
-            return Status{Error::rpcINVALID_PARAMS, "limitNotPositive"};
-    }
-
-    return {};
-}
-
 Status
 getAccount(
     boost::json::object const& request,

src/rpc/RPCHelpers.h

@@ -229,9 +229,6 @@ getString(
 Status
 getHexMarker(boost::json::object const& request, ripple::uint256& marker);
 
-Status
-getLimit(boost::json::object const& request, std::uint32_t& limit);
-
 Status
 getAccount(boost::json::object const& request, ripple::AccountID& accountId);
 

src/rpc/handlers/AccountChannels.cpp

@@ -70,8 +70,8 @@ doAccountChannels(Context const& context)
         status)
         return status;
 
-    std::uint32_t limit = 200;
+    std::uint32_t limit;
-    if (auto const status = getLimit(request, limit); status)
+    if (auto const status = getLimit(context, limit); status)
         return status;
 
     std::optional<std::string> marker = {};
@@ -93,11 +93,6 @@ doAccountChannels(Context const& context)
             (!destAccount ||
              destAccount == sle.getAccountID(ripple::sfDestination)))
         {
-            if (limit-- == 0)
-            {
-                return false;
-            }
-
             addChannel(jsonChannels, sle);
         }
 

src/rpc/handlers/AccountLines.cpp

@@ -113,8 +113,8 @@ doAccountLines(Context const& context)
     if (auto const status = getAccount(request, peerAccount, JS(peer)); status)
         return status;
 
-    std::uint32_t limit = 200;
+    std::uint32_t limit;
-    if (auto const status = getLimit(request, limit); status)
+    if (auto const status = getLimit(context, limit); status)
         return status;
 
     std::optional<std::string> marker = {};

src/rpc/handlers/AccountObjects.cpp

@@ -53,10 +53,8 @@ doAccountNFTs(Context const& context)
     if (!rawAcct)
         return Status{Error::rpcACT_NOT_FOUND, "accountNotFound"};
 
-    // limit controls the number of pages being returned. each page could have
+    std::uint32_t limit;
-    // 32 nfts
+    if (auto const status = getLimit(context, limit); status)
-    std::uint32_t limit = 10;
-    if (auto const status = getLimit(request, limit); status)
         return status;
 
     ripple::uint256 marker;
@@ -150,8 +148,8 @@ doAccountObjects(Context const& context)
     if (auto const status = getAccount(request, accountID); status)
         return status;
 
-    std::uint32_t limit = 200;
+    std::uint32_t limit;
-    if (auto const status = getLimit(request, limit); status)
+    if (auto const status = getLimit(context, limit); status)
         return status;
 
     std::optional<std::string> marker = {};

src/rpc/handlers/AccountOffers.cpp

@@ -86,8 +86,8 @@ doAccountOffers(Context const& context)
     if (!rawAcct)
         return Status{Error::rpcACT_NOT_FOUND, "accountNotFound"};
 
-    std::uint32_t limit = 200;
+    std::uint32_t limit;
-    if (auto const status = getLimit(request, limit); status)
+    if (auto const status = getLimit(context, limit); status)
         return status;
 
     std::optional<std::string> marker = {};
@@ -108,11 +108,6 @@ doAccountOffers(Context const& context)
     auto const addToResponse = [&](ripple::SLE const& sle) {
         if (sle.getType() == ripple::ltOFFER)
         {
-            if (limit-- == 0)
-            {
-                return false;
-            }
-
             addOffer(jsonLines, sle);
         }
 

src/rpc/handlers/AccountTx.cpp

@@ -121,8 +121,8 @@ doAccountTx(Context const& context)
             cursor = {maxIndex, INT32_MAX};
     }
 
-    std::uint32_t limit = 200;
+    std::uint32_t limit;
-    if (auto const status = getLimit(request, limit); status)
+    if (auto const status = getLimit(context, limit); status)
         return status;
 
     if (request.contains(JS(limit)))

src/rpc/handlers/BookOffers.cpp

@@ -48,8 +48,8 @@ doBookOffers(Context const& context)
         }
     }
 
-    std::uint32_t limit = 200;
+    std::uint32_t limit;
-    if (auto const status = getLimit(request, limit); status)
+    if (auto const status = getLimit(context, limit); status)
         return status;
 
     ripple::AccountID takerID = beast::zero;

src/rpc/handlers/LedgerData.cpp

@@ -30,10 +30,13 @@ doLedgerData(Context const& context)
 
     bool const binary = getBool(request, "binary", false);
 
-    std::uint32_t limit = binary ? 2048 : 256;
+    std::uint32_t limit;
-    if (auto const status = getLimit(request, limit); status)
+    if (auto const status = getLimit(context, limit); status)
         return status;
 
+    if (!binary)
+        limit = std::clamp(limit, {1}, {256});
+
     bool outOfOrder = false;
     if (request.contains("out_of_order"))
     {

src/rpc/handlers/NFTOffers.cpp

@@ -50,8 +50,8 @@ enumerateNFTOffers(
             directory.key, lgrInfo.seq, context.yield))
         return Status{Error::rpcOBJECT_NOT_FOUND, "notFound"};
 
-    std::uint32_t limit = 200;
+    std::uint32_t limit;
-    if (auto const status = getLimit(request, limit); status)
+    if (auto const status = getLimit(context, limit); status)
         return status;
 
     boost::json::object response = {};

src/rpc/handlers/NoRippleCheck.cpp

@@ -35,7 +35,7 @@ doNoRippleCheck(Context const& context)
     }
 
     std::uint32_t limit = 300;
-    if (auto const status = getLimit(request, limit); status)
+    if (auto const status = getLimit(context, limit); status)
         return status;
 
     bool includeTxs = getBool(request, "transactions", false);