From 7f8ccd354cedec1b34f6d1df4adc82386aca3710 Mon Sep 17 00:00:00 2001 From: Timur Ialymov Date: Wed, 12 Aug 2026 14:39:07 +0100 Subject: [PATCH] fix: Stop discarding error diagnostics in vault_info parseVault injects a specific error for each way a request can fail, but doVaultInfo replaced the token with the unregistered string "malformedRequest" and kept the code and message from that error, so every failure looked alike and contradicted its own code. Return what parseVault reported, give each branch its own message, and report a missing vault through injectError so that response carries a code and message as well. A well-formed all-zero vault_id now answers entryNotFound, the way ledger_entry does, rather than being called malformed. The zero key still never reaches the ledger, which treats reading one as unreachable. Co-authored-by: Cursor --- API-CHANGELOG.md | 2 + src/test/app/Vault_test.cpp | 74 +++++++++++++++++++++------- src/xrpld/rpc/handlers/VaultInfo.cpp | 32 ++++++++---- 3 files changed, 79 insertions(+), 29 deletions(-) diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index c853cfb07c..52025b1b83 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -54,6 +54,8 @@ This section contains changes targeting a future version. - `submit`: The `fail_hard` field now returns an error if the value is not a boolean. [#6529](https://github.com/XRPLF/rippled/pull/6529) - `subscribe`: The `taker` field in the `books` array now returns `actMalformed` instead of `badIssuer` if the value is not a valid account. [#6529](https://github.com/XRPLF/rippled/pull/6529) - Fixed a bug in `Forwarded` HTTP header parsing where the extracted IP address could be incorrect when no comma or semicolon delimiter follows the address. This could cause the server to misidentify a client's IP address when operating behind a reverse proxy. [#6529](https://github.com/XRPLF/rippled/pull/6529) +- `vault_info`: Errors now identify what the request got wrong instead of reporting every failure as the unregistered token `malformedRequest`, and the `error`, `error_code` and `error_message` fields now agree with each other. An invalid `vault_id` or `seq` returns `invalidParams`, an invalid `owner` returns `actMalformed`, and a request that mixes `vault_id` with `owner`/`seq` or supplies neither returns `invalidParams` with a message naming the accepted combinations. +- `vault_info`: A well-formed all-zero `vault_id` now returns `entryNotFound` instead of being rejected as malformed, and `entryNotFound` responses now include `error_code` and `error_message`. Clients that request `ripplerpc` 3.0 or above therefore receive HTTP 400 with that error rather than HTTP 200. - `gateway_balances`: The `account` and `ident` fields now return an `invalidParams` error if the value is not a string, instead of an `internal` error. [#7655](https://github.com/XRPLF/rippled/pull/7655) - `account_lines`: The `peer` field now returns an error if the value is not a string. [#7728](https://github.com/XRPLF/rippled/pull/7728) diff --git a/src/test/app/Vault_test.cpp b/src/test/app/Vault_test.cpp index 70527f570d..b6d3e0923c 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -43,6 +43,7 @@ #include #include #include +#include #include #include #include @@ -4082,6 +4083,22 @@ class Vault_test : public beast::unit_test::Suite } }; + // An error response must carry a registered token together with the matching code and + // message, so that clients dispatching on either of them reach the same conclusion. + auto const checkError = [this]( + json::Value const& result, + std::string const& token, + ErrorCodeI const code, + std::string const& message) { + BEAST_EXPECT(result[jss::error].asString() == token); + BEAST_EXPECT(result[jss::error_code].asInt() == code); + BEAST_EXPECT(result[jss::error_message].asString() == message); + }; + + std::string const badSeqMessage = "Invalid field 'seq', not a positive 32-bit integer."; + std::string const badFieldsMessage = + "Must specify either 'vault_id' or both 'owner' and 'seq'."; + { testcase("RPC ledger_entry selected by key"); json::Value jvParams; @@ -4236,16 +4253,31 @@ class Vault_test : public beast::unit_test::Suite jvParams[jss::ledger_index] = jss::validated; jvParams[jss::vault_id] = "foobar"; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError( + jv[jss::result], + "invalidParams", + RpcInvalidParams, + "Invalid field 'vault_id', not hex string."); } { - testcase("RPC vault_info json invalid index"); + testcase("RPC vault_info json zero vault_id"); json::Value jvParams; jvParams[jss::ledger_index] = jss::validated; jvParams[jss::vault_id] = 0; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "entryNotFound", RpcEntryNotFound, "Entry not found."); + } + + { + // An all-zero key is a well-formed request for a vault that cannot exist, not a + // malformed one. + testcase("RPC vault_info json all zero vault_id"); + json::Value jvParams; + jvParams[jss::ledger_index] = jss::validated; + jvParams[jss::vault_id] = strHex(uint256(beast::kZero)); + auto jv = env.rpc("json", "vault_info", to_string(jvParams)); + checkError(jv[jss::result], "entryNotFound", RpcEntryNotFound, "Entry not found."); } { @@ -4268,7 +4300,7 @@ class Vault_test : public beast::unit_test::Suite jvParams[jss::owner] = owner.human(); jvParams[jss::seq] = "foobar"; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badSeqMessage); } { @@ -4278,7 +4310,7 @@ class Vault_test : public beast::unit_test::Suite 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"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badSeqMessage); } { @@ -4288,7 +4320,7 @@ class Vault_test : public beast::unit_test::Suite 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"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badSeqMessage); } { @@ -4298,7 +4330,7 @@ class Vault_test : public beast::unit_test::Suite 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"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badSeqMessage); } { @@ -4308,7 +4340,7 @@ class Vault_test : public beast::unit_test::Suite jvParams[jss::owner] = owner.human(); jvParams[jss::seq] = true; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badSeqMessage); } { @@ -4318,7 +4350,11 @@ class Vault_test : public beast::unit_test::Suite jvParams[jss::owner] = "foobar"; jvParams[jss::seq] = sequence; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError( + jv[jss::result], + "actMalformed", + RpcActMalformed, + "Invalid field 'owner', not AccountID."); } { @@ -4327,7 +4363,7 @@ class Vault_test : public beast::unit_test::Suite jvParams[jss::ledger_index] = jss::validated; jvParams[jss::owner] = owner.human(); auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badFieldsMessage); } { @@ -4336,7 +4372,7 @@ class Vault_test : public beast::unit_test::Suite jvParams[jss::ledger_index] = jss::validated; jvParams[jss::seq] = sequence; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badFieldsMessage); } { @@ -4346,7 +4382,7 @@ class Vault_test : public beast::unit_test::Suite jvParams[jss::vault_id] = strHex(keylet.key); jvParams[jss::seq] = sequence; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badFieldsMessage); } { @@ -4356,7 +4392,7 @@ class Vault_test : public beast::unit_test::Suite jvParams[jss::vault_id] = strHex(keylet.key); jvParams[jss::owner] = owner.human(); auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badFieldsMessage); } { @@ -4369,7 +4405,7 @@ class Vault_test : public beast::unit_test::Suite jvParams[jss::seq] = sequence; jvParams[jss::owner] = owner.human(); auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badFieldsMessage); } { @@ -4377,7 +4413,7 @@ class Vault_test : public beast::unit_test::Suite json::Value jvParams; jvParams[jss::ledger_index] = jss::validated; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "invalidParams", RpcInvalidParams, badFieldsMessage); } { @@ -4387,15 +4423,15 @@ class Vault_test : public beast::unit_test::Suite } { - testcase("RPC vault_info command line invalid index"); + testcase("RPC vault_info command line zero index"); json::Value jv = env.rpc("vault_info", "0", "validated"); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "malformedRequest"); + checkError(jv[jss::result], "entryNotFound", RpcEntryNotFound, "Entry not found."); } { - testcase("RPC vault_info command line invalid index"); + testcase("RPC vault_info command line unknown index"); json::Value jv = env.rpc("vault_info", strHex(uint256(42)), "validated"); - BEAST_EXPECT(jv[jss::result][jss::error].asString() == "entryNotFound"); + checkError(jv[jss::result], "entryNotFound", RpcEntryNotFound, "Entry not found."); } { diff --git a/src/xrpld/rpc/handlers/VaultInfo.cpp b/src/xrpld/rpc/handlers/VaultInfo.cpp index c216192ab3..5ed238e25b 100644 --- a/src/xrpld/rpc/handlers/VaultInfo.cpp +++ b/src/xrpld/rpc/handlers/VaultInfo.cpp @@ -28,7 +28,8 @@ parseVault(json::Value const& params, json::Value& jvResult) { if (!uNodeIndex.parseHex(params[jss::vault_id].asString())) { - rpc::injectError(RpcInvalidParams, jvResult); + rpc::injectError( + RpcInvalidParams, rpc::expectedFieldMessage(jss::vault_id, "hex string"), jvResult); return std::nullopt; } // else uNodeIndex holds the value we need @@ -38,14 +39,18 @@ parseVault(json::Value const& params, json::Value& jvResult) auto const id = parseBase58(params[jss::owner].asString()); if (!id) { - rpc::injectError(RpcActMalformed, jvResult); + rpc::injectError( + RpcActMalformed, rpc::expectedFieldMessage(jss::owner, "AccountID"), jvResult); return std::nullopt; } if (!(params[jss::seq].isInt() || params[jss::seq].isUInt()) || params[jss::seq].asDouble() <= 0.0 || params[jss::seq].asDouble() > double(json::Value::kMaxUInt)) { - rpc::injectError(RpcInvalidParams, jvResult); + rpc::injectError( + RpcInvalidParams, + rpc::expectedFieldMessage(jss::seq, "a positive 32-bit integer"), + jvResult); return std::nullopt; } @@ -54,8 +59,10 @@ parseVault(json::Value const& params, json::Value& jvResult) } else { - // Invalid combination of fields vault_id/owner/seq - rpc::injectError(RpcInvalidParams, jvResult); + rpc::injectError( + RpcInvalidParams, + "Must specify either 'vault_id' or both 'owner' and 'seq'.", + jvResult); return std::nullopt; } @@ -71,20 +78,25 @@ doVaultInfo(rpc::JsonContext& context) if (!lpLedger) return jvResult; - auto const uNodeIndex = parseVault(context.params, jvResult).value_or(beast::kZero); - if (uNodeIndex == beast::kZero) + // No key means the request could not be turned into one, and parseVault has already said why. + auto const uNodeIndex = parseVault(context.params, jvResult); + if (!uNodeIndex) + return jvResult; + + // A zero key names an entry that cannot exist, and the ledger refuses to be asked for one. + if (*uNodeIndex == beast::kZero) { - jvResult[jss::error] = "malformedRequest"; + rpc::injectError(RpcEntryNotFound, jvResult); return jvResult; } - auto const sleVault = lpLedger->read(keylet::vault(uNodeIndex)); + auto const sleVault = lpLedger->read(keylet::vault(*uNodeIndex)); auto const sleIssuance = sleVault == nullptr // ? nullptr : lpLedger->read(keylet::mptokenIssuance(sleVault->at(sfShareMPTID))); if (!sleVault || !sleIssuance) { - jvResult[jss::error] = "entryNotFound"; + rpc::injectError(RpcEntryNotFound, jvResult); return jvResult; }