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; }