From 49b80c7ad8005abff7afb4b39d4ad3bd1a43e40e Mon Sep 17 00:00:00 2001 From: cyan317 <120398799+cindyyan317@users.noreply.github.com> Date: Wed, 12 Jun 2024 10:31:32 +0100 Subject: [PATCH] Support string type integer for oracle_document_id (#1448) Fix #1420 --- src/rpc/common/MetaProcessors.hpp | 39 +++++++--- src/rpc/common/Modifiers.hpp | 75 +++++++++++++++++++ src/rpc/common/Validators.cpp | 7 +- src/rpc/common/Validators.hpp | 1 + src/rpc/handlers/GetAggregatePrice.hpp | 14 ++-- src/rpc/handlers/LedgerEntry.hpp | 6 +- tests/unit/rpc/BaseTests.cpp | 47 ++++++++++++ .../rpc/handlers/GetAggregatePriceTests.cpp | 49 ++++++++++++ tests/unit/rpc/handlers/LedgerEntryTests.cpp | 29 ++++++- 9 files changed, 246 insertions(+), 21 deletions(-) diff --git a/src/rpc/common/MetaProcessors.hpp b/src/rpc/common/MetaProcessors.hpp index a2bb845f..a411f785 100644 --- a/src/rpc/common/MetaProcessors.hpp +++ b/src/rpc/common/MetaProcessors.hpp @@ -20,10 +20,8 @@ #pragma once #include "rpc/Errors.hpp" -#include "rpc/common/Concepts.hpp" #include "rpc/common/Specs.hpp" #include "rpc/common/Types.hpp" -#include "rpc/common/Validators.hpp" #include #include @@ -146,10 +144,10 @@ public: [[nodiscard]] MaybeError verify(boost::json::value& value, std::string_view key) const { - if (not value.is_object() or not value.as_object().contains(key.data())) + if (not value.is_object() or not value.as_object().contains(key)) return {}; // ignore. field does not exist, let 'required' fail instead - if (not rpc::validation::checkType(value.as_object().at(key.data()))) + if (not rpc::validation::checkType(value.as_object().at(key))) return {}; // ignore if type does not match return processor_(value, key); @@ -162,9 +160,10 @@ private: /** * @brief A meta-processor that wraps a validator and produces a custom error in case the wrapped validator fails. */ -template +template + requires SomeRequirement or SomeModifier class WithCustomError final { - SomeRequirement requirement; + RequirementOrModifierType reqOrModifier; Status error; public: @@ -172,10 +171,11 @@ public: * @brief Constructs a validator that calls the given validator `req` and returns a custom error `err` in case `req` * fails. * - * @param req The requirement to validate against + * @param reqOrModifier The requirement to validate against * @param err The custom error to return in case `req` fails */ - WithCustomError(SomeRequirement req, Status err) : requirement{std::move(req)}, error{std::move(err)} + WithCustomError(RequirementOrModifierType reqOrModifier, Status err) + : reqOrModifier{std::move(reqOrModifier)}, error{std::move(err)} { } @@ -188,8 +188,9 @@ public: */ [[nodiscard]] MaybeError verify(boost::json::value const& value, std::string_view key) const + requires SomeRequirement { - if (auto const res = requirement.verify(value, key); not res) + if (auto const res = reqOrModifier.verify(value, key); not res) return Error{error}; return {}; @@ -205,12 +206,30 @@ public: */ [[nodiscard]] MaybeError verify(boost::json::value& value, std::string_view key) const + requires SomeRequirement { - if (auto const res = requirement.verify(value, key); not res) + if (auto const res = reqOrModifier.verify(value, key); not res) return Error{error}; return {}; } + + /** + * @brief Runs the stored modifier and produces a custom error if the wrapped modifier fails. + * + * @param value The JSON value representing the outer object. This value can be modified by the modifier. + * @param key The key used to retrieve the element from the outer object + * @return Possibly an error + */ + MaybeError + modify(boost::json::value& value, std::string_view key) const + requires SomeModifier + + { + if (auto const res = reqOrModifier.modify(value, key); not res) + return Error{error}; + return {}; + } }; } // namespace rpc::meta diff --git a/src/rpc/common/Modifiers.hpp b/src/rpc/common/Modifiers.hpp index a5577e85..668ab568 100644 --- a/src/rpc/common/Modifiers.hpp +++ b/src/rpc/common/Modifiers.hpp @@ -19,12 +19,16 @@ #pragma once +#include "rpc/Errors.hpp" #include "rpc/common/Types.hpp" #include "util/JsonUtils.hpp" #include #include +#include +#include +#include #include #include @@ -100,4 +104,75 @@ struct ToLower final { } }; +/** + * @brief Convert input string to integer. + * + * Note: the conversion is only performed if the input value is a string. + */ +struct ToNumber final { + /** + * @brief Update the input string to integer if it can be converted to integer by stoi. + * + * @param value The JSON value representing the outer object + * @param key The key used to retrieve the modified value from the outer object + * @return Possibly an error + */ + [[nodiscard]] static MaybeError + modify(boost::json::value& value, std::string_view key) + { + if (not value.is_object() or not value.as_object().contains(key)) + return {}; // ignore. field does not exist, let 'required' fail instead + + if (not value.as_object().at(key).is_string()) + return {}; // ignore for non-string types + + auto const strInt = boost::json::value_to(value.as_object().at(key)); + if (strInt.find('.') != std::string::npos) + return Error{Status{RippledError::rpcINVALID_PARAMS}}; // maybe a float + + try { + value.as_object()[key.data()] = std::stoi(strInt); + } catch (std::exception& e) { + return Error{Status{RippledError::rpcINVALID_PARAMS}}; + } + return {}; + } +}; + +/** + * @brief Customised modifier allowing user define how to modify input in provided callable. + */ +class CustomModifier final { + std::function modifier_; + +public: + /** + * @brief Constructs a custom modifier from any supported callable. + * + * @tparam Fn The type of callable + * @param fn The callable/function object + */ + template + requires std::invocable + explicit CustomModifier(Fn&& fn) : modifier_{std::forward(fn)} + { + } + + /** + * @brief Modify the JSON value according to the custom modifier function stored. + * + * @param value The JSON value representing the outer object + * @param key The key used to retrieve the tested value from the outer object + * @return Any compatible user-provided error if modify/verify failed; otherwise no error is returned + */ + [[nodiscard]] MaybeError + modify(boost::json::value& value, std::string_view key) const + { + if (not value.is_object() or not value.as_object().contains(key)) + return {}; // ignore. field does not exist, let 'required' fail instead + + return modifier_(value.as_object().at(key.data()), key); + }; +}; + } // namespace rpc::modifiers diff --git a/src/rpc/common/Validators.cpp b/src/rpc/common/Validators.cpp index 640be98e..292c2382 100644 --- a/src/rpc/common/Validators.cpp +++ b/src/rpc/common/Validators.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include @@ -45,7 +46,7 @@ namespace rpc::validation { [[nodiscard]] MaybeError Required::verify(boost::json::value const& value, std::string_view key) { - if (not value.is_object() or not value.as_object().contains(key.data())) + if (not value.is_object() or not value.as_object().contains(key)) return Error{Status{RippledError::rpcINVALID_PARAMS, "Required field '" + std::string{key} + "' missing"}}; return {}; @@ -54,10 +55,10 @@ Required::verify(boost::json::value const& value, std::string_view key) [[nodiscard]] MaybeError CustomValidator::verify(boost::json::value const& value, std::string_view key) const { - if (not value.is_object() or not value.as_object().contains(key.data())) + if (not value.is_object() or not value.as_object().contains(key)) return {}; // ignore. field does not exist, let 'required' fail instead - return validator_(value.as_object().at(key.data()), key); + return validator_(value.as_object().at(key), key); } [[nodiscard]] bool diff --git a/src/rpc/common/Validators.hpp b/src/rpc/common/Validators.hpp index 14641215..f78e16a7 100644 --- a/src/rpc/common/Validators.hpp +++ b/src/rpc/common/Validators.hpp @@ -402,6 +402,7 @@ public: * @param fn The callable/function object */ template + requires std::invocable explicit CustomValidator(Fn&& fn) : validator_{std::forward(fn)} { } diff --git a/src/rpc/handlers/GetAggregatePrice.hpp b/src/rpc/handlers/GetAggregatePrice.hpp index ed143738..0fc20dba 100644 --- a/src/rpc/handlers/GetAggregatePrice.hpp +++ b/src/rpc/handlers/GetAggregatePrice.hpp @@ -22,6 +22,7 @@ #include "data/BackendInterface.hpp" #include "rpc/Errors.hpp" #include "rpc/JS.hpp" +#include "rpc/common/Modifiers.hpp" #include "rpc/common/Specs.hpp" #include "rpc/common/Types.hpp" #include "rpc/common/Validators.hpp" @@ -131,23 +132,26 @@ public: static auto constexpr ORACLES_MAX = 200; static auto const oraclesValidator = - validation::CustomValidator{[](boost::json::value const& value, std::string_view) -> MaybeError { + modifiers::CustomModifier{[](boost::json::value& value, std::string_view) -> MaybeError { if (!value.is_array() or value.as_array().empty() or value.as_array().size() > ORACLES_MAX) return Error{Status{RippledError::rpcORACLE_MALFORMED}}; - for (auto oracle : value.as_array()) { + for (auto& oracle : value.as_array()) { if (!oracle.is_object() or !oracle.as_object().contains(JS(oracle_document_id)) or !oracle.as_object().contains(JS(account))) return Error{Status{RippledError::rpcORACLE_MALFORMED}}; - auto maybeError = - validation::Type{}.verify(oracle.as_object(), JS(oracle_document_id)); + auto maybeError = validation::Type{}.verify( + oracle.as_object(), JS(oracle_document_id) + ); + if (!maybeError) + return maybeError; + maybeError = modifiers::ToNumber::modify(oracle, JS(oracle_document_id)); if (!maybeError) return maybeError; maybeError = validation::AccountBase58Validator.verify(oracle.as_object(), JS(account)); - if (!maybeError) return Error{Status{RippledError::rpcINVALID_PARAMS}}; }; diff --git a/src/rpc/handlers/LedgerEntry.hpp b/src/rpc/handlers/LedgerEntry.hpp index 10e63d70..19bf517a 100644 --- a/src/rpc/handlers/LedgerEntry.hpp +++ b/src/rpc/handlers/LedgerEntry.hpp @@ -24,6 +24,7 @@ #include "rpc/JS.hpp" #include "rpc/common/Checkers.hpp" #include "rpc/common/MetaProcessors.hpp" +#include "rpc/common/Modifiers.hpp" #include "rpc/common/Specs.hpp" #include "rpc/common/Types.hpp" #include "rpc/common/Validators.hpp" @@ -296,8 +297,9 @@ public: {JS(oracle_document_id), meta::WithCustomError{validation::Required{}, Status(ClioError::rpcMALFORMED_REQUEST)}, meta::WithCustomError{ - validation::Type{}, Status(ClioError::rpcMALFORMED_ORACLE_DOCUMENT_ID) - }}, + validation::Type{}, Status(ClioError::rpcMALFORMED_ORACLE_DOCUMENT_ID) + }, + meta::WithCustomError{modifiers::ToNumber{}, Status(ClioError::rpcMALFORMED_ORACLE_DOCUMENT_ID)}}, }}}, {JS(ledger), check::Deprecated{}}, }; diff --git a/tests/unit/rpc/BaseTests.cpp b/tests/unit/rpc/BaseTests.cpp index 873c8bde..d195b6e5 100644 --- a/tests/unit/rpc/BaseTests.cpp +++ b/tests/unit/rpc/BaseTests.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -604,3 +605,49 @@ TEST_F(RPCBaseTest, ToLowerModifier) ASSERT_TRUE(spec.process(passingInput4)); // empty str no problem ASSERT_EQ(passingInput4.at("str").as_string(), ""); } + +TEST_F(RPCBaseTest, ToNumberModifier) +{ + auto const spec = RpcSpec{ + {"str", ToNumber{}}, + }; + + auto passingInput = json::parse(R"({ "str": [] })"); + ASSERT_TRUE(spec.process(passingInput)); + + passingInput = json::parse(R"({ "str2": "TesT" })"); + ASSERT_TRUE(spec.process(passingInput)); + + passingInput = json::parse(R"([])"); + ASSERT_TRUE(spec.process(passingInput)); + + passingInput = json::parse(R"({ "str": "123" })"); + ASSERT_TRUE(spec.process(passingInput)); + ASSERT_EQ(passingInput.at("str").as_int64(), 123); + + auto failingInput = json::parse(R"({ "str": "ok" })"); + ASSERT_FALSE(spec.process(failingInput)); + + failingInput = json::parse(R"({ "str": "123.123" })"); + ASSERT_FALSE(spec.process(failingInput)); +} + +TEST_F(RPCBaseTest, CustomModifier) +{ + testing::StrictMock> mockModifier; + auto const customModifier = CustomModifier{mockModifier.AsStdFunction()}; + auto const spec = RpcSpec{ + {"str", customModifier}, + }; + + EXPECT_CALL(mockModifier, Call).WillOnce(testing::Return(MaybeError{})); + auto passingInput = json::parse(R"({ "str": "sss" })"); + ASSERT_TRUE(spec.process(passingInput)); + + passingInput = json::parse(R"({ "strNotExist": 123 })"); + ASSERT_TRUE(spec.process(passingInput)); + + // not a json object + passingInput = json::parse(R"([])"); + ASSERT_TRUE(spec.process(passingInput)); +} diff --git a/tests/unit/rpc/handlers/GetAggregatePriceTests.cpp b/tests/unit/rpc/handlers/GetAggregatePriceTests.cpp index 410cd4b0..1e8baafc 100644 --- a/tests/unit/rpc/handlers/GetAggregatePriceTests.cpp +++ b/tests/unit/rpc/handlers/GetAggregatePriceTests.cpp @@ -427,6 +427,55 @@ TEST_F(RPCGetAggregatePriceHandlerTest, OracleLedgerEntrySinglePriceData) }); } +TEST_F(RPCGetAggregatePriceHandlerTest, OracleLedgerEntryStrOracleDocumentId) +{ + EXPECT_CALL(*backend, fetchLedgerBySequence(RANGEMAX, _)) + .WillOnce(Return(CreateLedgerHeader(LEDGERHASH, RANGEMAX))); + + auto constexpr documentId = 1; + mockLedgerObject(*backend, ACCOUNT, documentId, TX1, 1e3, 2); // 10 + + auto const handler = AnyHandler{GetAggregatePriceHandler{backend}}; + auto const req = json::parse(fmt::format( + R"({{ + "base_asset": "USD", + "quote_asset": "XRP", + "oracles": + [ + {{ + "account": "{}", + "oracle_document_id": "{}" + }} + ] + }})", + ACCOUNT, + documentId + )); + + auto const expected = json::parse(fmt::format( + R"({{ + "entire_set": + {{ + "mean": "10", + "size": 1, + "standard_deviation": "0" + }}, + "median": "10", + "time": 4321, + "ledger_index": {}, + "ledger_hash": "{}", + "validated": true + }})", + RANGEMAX, + LEDGERHASH + )); + runSpawn([&](auto yield) { + auto const output = handler.process(req, Context{yield}); + ASSERT_TRUE(output); + EXPECT_EQ(output.result.value(), expected); + }); +} + TEST_F(RPCGetAggregatePriceHandlerTest, PreviousTxNotFound) { EXPECT_CALL(*backend, fetchLedgerBySequence(RANGEMAX, _)) diff --git a/tests/unit/rpc/handlers/LedgerEntryTests.cpp b/tests/unit/rpc/handlers/LedgerEntryTests.cpp index 2c52941f..361d5a8e 100644 --- a/tests/unit/rpc/handlers/LedgerEntryTests.cpp +++ b/tests/unit/rpc/handlers/LedgerEntryTests.cpp @@ -2315,7 +2315,7 @@ generateTestValuesForNormalPathTest() CreateChainOwnedClaimIDObject(ACCOUNT, ACCOUNT, ACCOUNT2, "JPY", ACCOUNT3, ACCOUNT) }, NormalPathTestBundle{ - "OracleEntryFoundViaObject", + "OracleEntryFoundViaIntOracleDocumentId", fmt::format( R"({{ "binary": true, @@ -2341,6 +2341,33 @@ generateTestValuesForNormalPathTest() ) ) }, + NormalPathTestBundle{ + "OracleEntryFoundViaStrOracleDocumentId", + fmt::format( + R"({{ + "binary": true, + "oracle": {{ + "account": "{}", + "oracle_document_id": "1" + }} + }})", + ACCOUNT + ), + ripple::keylet::oracle(GetAccountIDWithString(ACCOUNT), 1).key, + CreateOracleObject( + ACCOUNT, + "70726F7669646572", + 32u, + 1234u, + ripple::Blob(8, 's'), + ripple::Blob(8, 's'), + RANGEMAX - 2, + ripple::uint256{"E6DBAFC99223B42257915A63DFC6B0C032D4070F9A574B255AD97466726FC321"}, + CreatePriceDataSeries( + {CreateOraclePriceData(2e4, ripple::to_currency("XRP"), ripple::to_currency("USD"), 3)} + ) + ) + }, NormalPathTestBundle{ "OracleEntryFoundViaString", fmt::format(