From 77c70d8a64714c7f6a6f9afa57572268558ee73b Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Fri, 25 Apr 2025 15:23:41 +0100 Subject: [PATCH] Update RPC codes, improve seq parsing --- src/test/app/AMM_test.cpp | 18 ++++--- src/test/app/MPToken_test.cpp | 14 ++--- src/test/app/Vault_test.cpp | 71 +++++++++++++++++++++++++- src/xrpld/rpc/handlers/LedgerEntry.cpp | 3 +- src/xrpld/rpc/handlers/VaultInfo.cpp | 13 +++-- 5 files changed, 99 insertions(+), 20 deletions(-) diff --git a/src/test/app/AMM_test.cpp b/src/test/app/AMM_test.cpp index dca30b53ca..9e3ef943ab 100644 --- a/src/test/app/AMM_test.cpp +++ b/src/test/app/AMM_test.cpp @@ -56,11 +56,16 @@ private: using namespace jtx; - // XRP to IOU - testAMM([&](AMM& ammAlice, Env&) { - BEAST_EXPECT(ammAlice.expectBalances( - XRP(10'000), USD(10'000), IOUAmount{10'000'000, 0})); - }); + // XRP to IOU, with featureSingleAssetVault + testAMM( + [&](AMM& ammAlice, Env&) { + BEAST_EXPECT(ammAlice.expectBalances( + XRP(10'000), USD(10'000), IOUAmount{10'000'000, 0})); + }, + {}, + 0, + {}, + {supported_amendments() | featureSingleAssetVault}); // XRP to IOU, without featureSingleAssetVault testAMM( @@ -7187,7 +7192,8 @@ private: : ter{tecDUPLICATE}); }; - testCase("tecDUPLICATE", supported_amendments()); + testCase( + "tecDUPLICATE", supported_amendments() - featureSingleAssetVault); testCase( "terADDRESS_COLLISION", supported_amendments() | featureSingleAssetVault); diff --git a/src/test/app/MPToken_test.cpp b/src/test/app/MPToken_test.cpp index 52b2d7dad8..c26f669a7e 100644 --- a/src/test/app/MPToken_test.cpp +++ b/src/test/app/MPToken_test.cpp @@ -2305,26 +2305,26 @@ public: FeatureBitset const all{supported_amendments()}; // MPTokenIssuanceCreate - testCreateValidation(all); + testCreateValidation(all - featureSingleAssetVault); testCreateValidation(all | featureSingleAssetVault); - testCreateEnabled(all); + testCreateEnabled(all - featureSingleAssetVault); testCreateEnabled(all | featureSingleAssetVault); // MPTokenIssuanceDestroy - testDestroyValidation(all); + testDestroyValidation(all - featureSingleAssetVault); testDestroyValidation(all | featureSingleAssetVault); - testDestroyEnabled(all); + testDestroyEnabled(all - featureSingleAssetVault); testDestroyEnabled(all | featureSingleAssetVault); // MPTokenAuthorize - testAuthorizeValidation(all); + testAuthorizeValidation(all - featureSingleAssetVault); testAuthorizeValidation(all | featureSingleAssetVault); - testAuthorizeEnabled(all); + testAuthorizeEnabled(all - featureSingleAssetVault); testAuthorizeEnabled(all | featureSingleAssetVault); // MPTokenIssuanceSet testSetValidation(all); - testSetEnabled(all); + testSetEnabled(all - featureSingleAssetVault); testSetEnabled(all | featureSingleAssetVault); // MPT clawback diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 3ea111eda8..d0dfca4eaf 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -1806,7 +1806,43 @@ class Vault_test : public beast::unit_test::suite Json::Value jvParams; jvParams[jss::ledger_index] = jss::validated; jvParams[jss::vault][jss::owner] = issuer.human(); - jvParams[jss::vault][jss::seq] = "42"; + jvParams[jss::vault][jss::seq] = "foo"; + auto jvVault = env.rpc("json", "ledger_entry", to_string(jvParams)); + BEAST_EXPECT( + jvVault[jss::result][jss::error].asString() == + "malformedRequest"); + } + + { + testcase("RPC ledger_entry zero seq"); + Json::Value jvParams; + jvParams[jss::ledger_index] = jss::validated; + jvParams[jss::vault][jss::owner] = issuer.human(); + jvParams[jss::vault][jss::seq] = 0; + auto jvVault = env.rpc("json", "ledger_entry", to_string(jvParams)); + BEAST_EXPECT( + jvVault[jss::result][jss::error].asString() == + "malformedRequest"); + } + + { + testcase("RPC ledger_entry negative seq"); + Json::Value jvParams; + jvParams[jss::ledger_index] = jss::validated; + jvParams[jss::vault][jss::owner] = issuer.human(); + jvParams[jss::vault][jss::seq] = -1; + auto jvVault = env.rpc("json", "ledger_entry", to_string(jvParams)); + BEAST_EXPECT( + jvVault[jss::result][jss::error].asString() == + "malformedRequest"); + } + + { + testcase("RPC ledger_entry oversized seq"); + Json::Value jvParams; + jvParams[jss::ledger_index] = jss::validated; + jvParams[jss::vault][jss::owner] = issuer.human(); + jvParams[jss::vault][jss::seq] = 1e20; auto jvVault = env.rpc("json", "ledger_entry", to_string(jvParams)); BEAST_EXPECT( jvVault[jss::result][jss::error].asString() == @@ -1901,6 +1937,39 @@ class Vault_test : public beast::unit_test::suite jv[jss::result][jss::error].asString() == "malformedRequest"); } + { + testcase("RPC vault_info json invalid sequence"); + Json::Value jvParams; + jvParams[jss::ledger_index] = jss::validated; + jvParams[jss::owner] = owner.human(); + jvParams[jss::seq] = 0; + auto jv = env.rpc("json", "vault_info", to_string(jvParams)); + BEAST_EXPECT( + jv[jss::result][jss::error].asString() == "malformedRequest"); + } + + { + testcase("RPC vault_info json negative sequence"); + Json::Value jvParams; + jvParams[jss::ledger_index] = jss::validated; + jvParams[jss::owner] = owner.human(); + jvParams[jss::seq] = -1; + auto jv = env.rpc("json", "vault_info", to_string(jvParams)); + BEAST_EXPECT( + jv[jss::result][jss::error].asString() == "malformedRequest"); + } + + { + testcase("RPC vault_info json oversized sequence"); + Json::Value jvParams; + jvParams[jss::ledger_index] = jss::validated; + jvParams[jss::owner] = owner.human(); + jvParams[jss::seq] = 1e20; + auto jv = env.rpc("json", "vault_info", to_string(jvParams)); + BEAST_EXPECT( + jv[jss::result][jss::error].asString() == "malformedRequest"); + } + { testcase("RPC vault_info json malformed owner"); Json::Value jvParams; diff --git a/src/xrpld/rpc/handlers/LedgerEntry.cpp b/src/xrpld/rpc/handlers/LedgerEntry.cpp index aa2d75a440..f1fb6f81fc 100644 --- a/src/xrpld/rpc/handlers/LedgerEntry.cpp +++ b/src/xrpld/rpc/handlers/LedgerEntry.cpp @@ -720,7 +720,8 @@ parseVault(Json::Value const& params, Json::Value& jvResult) } if (!params.isMember(jss::owner) || !params.isMember(jss::seq) || - !params[jss::seq].isIntegral()) + !params[jss::seq].isIntegral() || params[jss::seq].asDouble() <= 0.0 || + params[jss::seq].asDouble() > double(Json::Value::maxUInt)) { jvResult[jss::error] = "malformedRequest"; return std::nullopt; diff --git a/src/xrpld/rpc/handlers/VaultInfo.cpp b/src/xrpld/rpc/handlers/VaultInfo.cpp index aee29b8993..012ef6f980 100644 --- a/src/xrpld/rpc/handlers/VaultInfo.cpp +++ b/src/xrpld/rpc/handlers/VaultInfo.cpp @@ -47,7 +47,7 @@ parseVault(Json::Value const& params, Json::Value& jvResult) { if (!uNodeIndex.parseHex(params[jss::vault_id].asString())) { - jvResult[jss::error] = "malformedRequest"; + RPC::inject_error(rpcINVALID_PARAMS, jvResult); return std::nullopt; } // else uNodeIndex holds the value we need @@ -57,12 +57,15 @@ parseVault(Json::Value const& params, Json::Value& jvResult) auto const id = parseBase58(params[jss::owner].asString()); if (!id) { - jvResult[jss::error] = "malformedOwner"; + RPC::inject_error(rpcACT_MALFORMED, jvResult); return std::nullopt; } - else if (!params[jss::seq].isIntegral()) + else if ( + !params[jss::seq].isIntegral() || + params[jss::seq].asDouble() <= 0.0 || + params[jss::seq].asDouble() > double(Json::Value::maxUInt)) { - jvResult[jss::error] = "malformedRequest"; + RPC::inject_error(rpcINVALID_PARAMS, jvResult); return std::nullopt; } @@ -71,7 +74,7 @@ parseVault(Json::Value const& params, Json::Value& jvResult) else { // Invalid combination of fields vault_id/owner/seq - jvResult[jss::error] = "malformedRequest"; + RPC::inject_error(rpcINVALID_PARAMS, jvResult); return std::nullopt; }