diff --git a/API-CHANGELOG.md b/API-CHANGELOG.md index 6f75b3a880..d521f9c024 100644 --- a/API-CHANGELOG.md +++ b/API-CHANGELOG.md @@ -56,6 +56,7 @@ This section contains changes targeting a future version. - 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_test.cpp b/src/test/app/Vault_test.cpp index cb87442850..bad2d62323 100644 --- a/src/test/app/Vault_test.cpp +++ b/src/test/app/Vault_test.cpp @@ -5464,21 +5464,47 @@ class Vault_test : public beast::unit_test::Suite } { - testcase("RPC vault_info json zero vault_id"); + 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)); + 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."); } { - // 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"); + testcase("RPC vault_info json short zero vault_id"); json::Value jvParams; jvParams[jss::ledger_index] = jss::validated; - jvParams[jss::vault_id] = strHex(uint256(beast::kZero)); + jvParams[jss::vault_id] = "0"; auto jv = env.rpc("json", "vault_info", to_string(jvParams)); checkError(jv[jss::result], "entryNotFound", RpcEntryNotFound, "Entry not found."); } @@ -5560,6 +5586,20 @@ class Vault_test : public beast::unit_test::Suite "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."); + } + { testcase("RPC vault_info json invalid combination only owner"); json::Value jvParams; diff --git a/src/xrpld/rpc/handlers/VaultInfo.cpp b/src/xrpld/rpc/handlers/VaultInfo.cpp index 5ed238e25b..0aa5334bd2 100644 --- a/src/xrpld/rpc/handlers/VaultInfo.cpp +++ b/src/xrpld/rpc/handlers/VaultInfo.cpp @@ -26,7 +26,9 @@ 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, rpc::expectedFieldMessage(jss::vault_id, "hex string"), jvResult); @@ -36,16 +38,19 @@ parseVault(json::Value const& params, json::Value& jvResult) } 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, 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)) + + // 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, @@ -54,7 +59,7 @@ parseVault(json::Value const& params, json::Value& jvResult) return std::nullopt; } - auto const seq = SeqProxy::rawSequence(params[jss::seq].asUInt()); + auto const seq = SeqProxy::rawSequence(seqField.asUInt()); uNodeIndex = keylet::vault(*id, seq).key; } else