From cc767085633a6617d7013a5a6dd9d1fafd760d15 Mon Sep 17 00:00:00 2001 From: Timur Yalymov <36795566+tyalymov@users.noreply.github.com> Date: Thu, 20 Aug 2026 16:05:18 +0000 Subject: [PATCH] fix: Return specific and consistent errors from vault_info (#8015) Co-authored-by: Cursor --- API-CHANGELOG.md | 3 + src/test/app/vault/VaultRPC_test.cpp | 115 ++++++++++++++++++++++----- src/xrpld/rpc/handlers/VaultInfo.cpp | 55 ++++++++----- 3 files changed, 135 insertions(+), 38 deletions(-) diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index c853cfb07c..d521f9c024 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -54,6 +54,9 @@ 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. [#8015](https://github.com/XRPLF/rippled/pull/8015) +- `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. [#8015](https://github.com/XRPLF/rippled/pull/8015) +- `vault_info`: `vault_id` and `owner` must now be strings, matching how `ledger_entry` reads the same fields. An object or an array in either field previously produced an internal error, and a number was silently converted to its decimal text; `vault_id` now returns `invalidParams` and `owner` returns `actMalformed`. [#8015](https://github.com/XRPLF/rippled/pull/8015) - `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/VaultRPC_test.cpp b/src/test/app/vault/VaultRPC_test.cpp index 2ac092b5a7..dbceb1cb9c 100644 --- a/src/test/app/vault/VaultRPC_test.cpp +++ b/src/test/app/vault/VaultRPC_test.cpp @@ -9,11 +9,13 @@ #include #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -122,6 +124,22 @@ private: } }; + // 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; @@ -276,16 +294,57 @@ private: 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 numeric 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], + "invalidParams", + RpcInvalidParams, + "Invalid field 'vault_id', not hex string."); + } + + { + testcase("RPC vault_info json object vault_id"); + json::Value jvParams; + jvParams[jss::ledger_index] = jss::validated; + jvParams[jss::vault_id] = json::Value(json::ValueType::Object); + auto jv = env.rpc("json", "vault_info", to_string(jvParams)); + checkError( + jv[jss::result], + "invalidParams", + RpcInvalidParams, + "Invalid field 'vault_id', not hex string."); + } + + { + // An all-zero key is a well-formed request for a vault that cannot exist, not a + // malformed one. parseHex accepts both the padded form and the short "0". + 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."); + } + + { + testcase("RPC vault_info json short 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)); + checkError(jv[jss::result], "entryNotFound", RpcEntryNotFound, "Entry not found."); } { @@ -308,7 +367,7 @@ private: 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); } { @@ -318,7 +377,7 @@ private: 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); } { @@ -328,7 +387,7 @@ private: 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); } { @@ -338,7 +397,7 @@ private: 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); } { @@ -348,7 +407,7 @@ private: 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); } { @@ -358,7 +417,25 @@ private: 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."); + } + + { + testcase("RPC vault_info json array owner"); + json::Value jvParams; + jvParams[jss::ledger_index] = jss::validated; + jvParams[jss::owner] = json::Value(json::ValueType::Array); + jvParams[jss::seq] = sequence; + auto jv = env.rpc("json", "vault_info", to_string(jvParams)); + checkError( + jv[jss::result], + "actMalformed", + RpcActMalformed, + "Invalid field 'owner', not AccountID."); } { @@ -367,7 +444,7 @@ private: 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); } { @@ -376,7 +453,7 @@ private: 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); } { @@ -386,7 +463,7 @@ private: 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); } { @@ -396,7 +473,7 @@ private: 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); } { @@ -409,7 +486,7 @@ private: 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); } { @@ -417,7 +494,7 @@ private: 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); } { @@ -427,15 +504,15 @@ private: } { - 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..0aa5334bd2 100644 --- a/src/xrpld/rpc/handlers/VaultInfo.cpp +++ b/src/xrpld/rpc/handlers/VaultInfo.cpp @@ -26,36 +26,48 @@ parseVault(json::Value const& params, json::Value& jvResult) uint256 uNodeIndex = beast::kZero; if (hasVaultId && !hasOwner && !hasSeq) { - if (!uNodeIndex.parseHex(params[jss::vault_id].asString())) + // asString() throws on an object or an array, so the type comes first. + auto const& vaultId = params[jss::vault_id]; + if (!vaultId.isString() || !uNodeIndex.parseHex(vaultId.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 } else if (!hasVaultId && hasOwner && hasSeq) { - auto const id = parseBase58(params[jss::owner].asString()); + auto const& owner = params[jss::owner]; + auto const id = owner.isString() ? parseBase58(owner.asString()) + : std::optional{}; if (!id) { - rpc::injectError(RpcActMalformed, 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( + RpcActMalformed, rpc::expectedFieldMessage(jss::owner, "AccountID"), jvResult); return std::nullopt; } - auto const seq = SeqProxy::rawSequence(params[jss::seq].asUInt()); + // Int and UInt are both 32 bits wide, so the type check is the only upper bound needed. + auto const& seqField = params[jss::seq]; + if (!(seqField.isInt() || seqField.isUInt()) || seqField.asDouble() <= 0.0) + { + rpc::injectError( + RpcInvalidParams, + rpc::expectedFieldMessage(jss::seq, "a positive 32-bit integer"), + jvResult); + return std::nullopt; + } + + auto const seq = SeqProxy::rawSequence(seqField.asUInt()); uNodeIndex = keylet::vault(*id, seq).key; } 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 +83,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; }