fix: Add more restrictions to admin fields (#1643)

XRPLF · Sep 10, 2024 · bb0d912 · bb0d912
1 parent d02d6af
commit bb0d912
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 13 deletions.
diff --git a/src/rpc/RPCHelpers.cpp b/src/rpc/RPCHelpers.cpp
@@ -36,6 +36,7 @@
 #include <boost/json/array.hpp>
 #include <boost/json/object.hpp>
 #include <boost/json/parse.hpp>
+#include <boost/json/serialize.hpp>
 #include <boost/json/string.hpp>
 #include <boost/json/value.hpp>
 #include <boost/json/value_to.hpp>
@@ -49,6 +50,7 @@
 #include <ripple/basics/chrono.h>
 #include <ripple/basics/strHex.h>
 #include <ripple/beast/utility/Zero.h>
+#include <ripple/json/json_reader.h>
 #include <ripple/json/json_value.h>
 #include <ripple/protocol/AccountID.h>
 #include <ripple/protocol/Book.h>
@@ -1276,13 +1278,15 @@ specifiesCurrentOrClosedLedger(boost::json::object const& request)
 bool
 isAdminCmd(std::string const& method, boost::json::object const& request)
 {
-    // rippled considers the string as true: https://github.com/XRPLF/rippled/issues/5119
-    auto const isFieldSet = [&request](auto const field) {
-        return request.contains(field) and
-            ((request.at(field).is_bool() and request.at(field).as_bool()) or request.at(field).is_string());
-    };
-
     if (method == JS(ledger)) {
+        auto const requestStr = boost::json::serialize(request);
+        Json::Value jv;
+        Json::Reader{}.parse(requestStr, jv);
+        // rippled considers string/non-zero int/non-empty array/ non-empty json as true.
+        // Use rippled's API asBool to get the same result.
+        // https://github.com/XRPLF/rippled/issues/5119
+        auto const isFieldSet = [&jv](auto const field) { return jv.isMember(field) and jv[field].asBool(); };
+
         if (isFieldSet(JS(full)) or isFieldSet(JS(accounts)) or isFieldSet(JS(type)))
             return true;
     }

diff --git a/tests/unit/rpc/RPCHelpersTests.cpp b/tests/unit/rpc/RPCHelpersTests.cpp
@@ -553,19 +553,54 @@ static auto
 generateTestValuesForParametersTest()
 {
     return std::vector<IsAdminCmdParamTestCaseBundle>{
+        {"ledgerEntry", "ledger_entry", R"({"type": false})", false},
+
         {"featureVetoedTrue", "feature", R"({"vetoed": true, "feature": "foo"})", true},
         {"featureVetoedFalse", "feature", R"({"vetoed": false, "feature": "foo"})", true},
+        {"featureVetoedIsStr", "feature", R"({"vetoed": "String"})", true},
+
+        {"ledger", "ledger", R"({})", false},
         {"ledgerFullTrue", "ledger", R"({"full": true})", true},
-        {"ledgerAccountsTrue", "ledger", R"({"accounts": true})", true},
-        {"ledgerTypeTrue", "ledger", R"({"type": true})", true},
         {"ledgerFullFalse", "ledger", R"({"full": false})", false},
-        {"ledgerAccountsFalse", "ledger", R"({"accounts": false})", false},
-        {"ledgerTypeFalse", "ledger", R"({"type": false})", false},
-        {"ledgerEntry", "ledger_entry", R"({"type": false})", false},
         {"ledgerFullIsStr", "ledger", R"({"full": "String"})", true},
-        {"ledgerAccoutsIsStr", "ledger", R"({"accounts": "String"})", true},
+        {"ledgerFullIsEmptyStr", "ledger", R"({"full": ""})", false},
+        {"ledgerFullIsNumber1", "ledger", R"({"full": 1})", true},
+        {"ledgerFullIsNumber0", "ledger", R"({"full": 0})", false},
+        {"ledgerFullIsNull", "ledger", R"({"full": null})", false},
+        {"ledgerFullIsFloat0", "ledger", R"({"full": 0.0})", false},
+        {"ledgerFullIsFloat1", "ledger", R"({"full": 0.1})", true},
+        {"ledgerFullIsArray", "ledger", R"({"full": [1]})", true},
+        {"ledgerFullIsEmptyArray", "ledger", R"({"full": []})", false},
+        {"ledgerFullIsObject", "ledger", R"({"full": {"key": 1}})", true},
+        {"ledgerFullIsEmptyObject", "ledger", R"({"full": {}})", false},
+
+        {"ledgerTypeTrue", "ledger", R"({"type": true})", true},
+        {"ledgerTypeFalse", "ledger", R"({"type": false})", false},
         {"ledgerTypeIsStr", "ledger", R"({"type": "String"})", true},
-        {"featureVetoedIsStr", "feature", R"({"vetoed": "String"})", true},
+        {"ledgerTypeIsEmptyStr", "ledger", R"({"type": ""})", false},
+        {"ledgerTypeIsNumber1", "ledger", R"({"type": 1})", true},
+        {"ledgerTypeIsNumber0", "ledger", R"({"type": 0})", false},
+        {"ledgerTypeIsNull", "ledger", R"({"type": null})", false},
+        {"ledgerTypeIsFloat0", "ledger", R"({"type": 0.0})", false},
+        {"ledgerTypeIsFloat1", "ledger", R"({"type": 0.1})", true},
+        {"ledgerTypeIsArray", "ledger", R"({"type": [1]})", true},
+        {"ledgerTypeIsEmptyArray", "ledger", R"({"type": []})", false},
+        {"ledgerTypeIsObject", "ledger", R"({"type": {"key": 1}})", true},
+        {"ledgerTypeIsEmptyObject", "ledger", R"({"type": {}})", false},
+
+        {"ledgerAccountsTrue", "ledger", R"({"accounts": true})", true},
+        {"ledgerAccountsFalse", "ledger", R"({"accounts": false})", false},
+        {"ledgerAccountsIsStr", "ledger", R"({"accounts": "String"})", true},
+        {"ledgerAccountsIsEmptyStr", "ledger", R"({"accounts": ""})", false},
+        {"ledgerAccountsIsNumber1", "ledger", R"({"type": 1})", true},
+        {"ledgerAccountsIsNumber0", "ledger", R"({"accounts": 0})", false},
+        {"ledgerAccountsIsNull", "ledger", R"({"accounts": null})", false},
+        {"ledgerAccountsIsFloat0", "ledger", R"({"accounts": 0.0})", false},
+        {"ledgerAccountsIsFloat1", "ledger", R"({"accounts": 0.1})", true},
+        {"ledgerAccountsIsArray", "ledger", R"({"accounts": [1]})", true},
+        {"ledgerAccountsIsEmptyArray", "ledger", R"({"accounts": []})", false},
+        {"ledgerAccountsIsObject", "ledger", R"({"accounts": {"key": 1}})", true},
+        {"ledgerAccountsIsEmptyObject", "ledger", R"({"accounts": {}})", false},
     };
 }