fix: Return specific and consistent errors from vault_info (#8015)

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Timur Yalymov
2026-08-20 16:05:18 +00:00
committed by GitHub
parent 422e5245e4
commit cc76708563
3 changed files with 135 additions and 38 deletions

View File

@@ -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)

View File

@@ -9,11 +9,13 @@
#include <xrpl/basics/chrono.h>
#include <xrpl/basics/strHex.h>
#include <xrpl/beast/unit_test/suite.h>
#include <xrpl/beast/utility/Zero.h>
#include <xrpl/json/json_forwards.h>
#include <xrpl/json/json_value.h>
#include <xrpl/json/to_string.h>
#include <xrpl/protocol/AccountID.h>
#include <xrpl/protocol/Asset.h>
#include <xrpl/protocol/ErrorCodes.h>
#include <xrpl/protocol/Issue.h>
#include <xrpl/protocol/Keylet.h>
#include <xrpl/protocol/LedgerFormats.h>
@@ -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.");
}
{

View File

@@ -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<AccountID>(params[jss::owner].asString());
auto const& owner = params[jss::owner];
auto const id = owner.isString() ? parseBase58<AccountID>(owner.asString())
: std::optional<AccountID>{};
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;
}