ARCHIVE/rippled
1
0 Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-10-21 19:18:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Select the reply envelope from a ripplerpc the server supports

Browse Source 
The `ripplerpc` version is read in three places: twice inside the dispatch
loop to pick the error shape, and once after it to derive the HTTP status
by re-reading the version off the finished reply. Replace all three with
one `shapeReply` keyed by an `RpcVersion` enum naming the three envelopes.
The version is read once per request and passed in, and the status is
returned rather than read back, since only the function that wrote the
shape knows where the code went.

The version is now matched exactly against the three valid spellings, where
the `ripplerpc` string was compared with `>=` against "2.0" and "3.0", so
"abc" read as version 3 and "10.0" as version 1. Junk from an
unauthenticated client therefore chose an envelope, and at version 3 chose
real HTTP error codes. Anything but the three exact values is rejected with
a 400, the malformed-RPC fee and -32602, which is a break for API versions
1 and 2 on the JSON-RPC transport, listed under a breaking-changes heading
of its own in the changelog.

Hoisting the log statement above the shape branch fixes a second defect. It
read `error_message` after the version 2 and 3 branch renamed that member
to `message`, and reading a missing member non-const puts it back as an
explicit null, so an error reply carried `"error_message": null` exactly
when the `Server` log partition wrote at debug.
This commit is contained in:
Bart committed 2026-10-10 20:02:26 +09:00
1 parent cf19602641
commit e2ce3de0fc
3 files changed
+291 -74
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

API-CHANGELOG.md
+5
View File
@@ -24,6 +24,10 @@ This version is supported by all `xrpld` versions. For WebSocket and HTTP JSON-R
## Unreleased
### Breaking changes
- The `ripplerpc` request field, which selects the shape of the JSON-RPC reply envelope, is now validated, and a value that is not exactly `"1.0"`, `"2.0"` or `"3.0"` is rejected. This reaches the JSON-RPC transport at every API version. It does not reach a WebSocket session, which reads the field only to echo it back. A request sending `"2"`, `" 2.0"`, `"2.00"`, `"02.0"`, `"2.0.0"`, `"10.0"` or any other text, such as `"abc"` or `"x2"`, gets a working reply today. After this ships, such a request sent alone gets HTTP 400 with `ripplerpc is not a supported version`, charged as a malformed request, and such an entry of a `"method": "batch"` body gets that error in its own reply while the batch answers 200. A client that spells the version loosely must therefore be corrected to one of the three exact values, or omit the field. Previously the value was compared as a string, which both accepted values that name no version and ordered multi-digit versions incorrectly: `"abc"` and `"x2"` sorted above `"3.0"` and so selected version 3, and `"10.0"` sorted below `"2.0"` and so selected version 1. Requests that send one of the three supported values, or omit the field, are unaffected.
### Bugfixes
- A request echoed back in an error reply now has every credential-bearing field masked: `admin_password`, `admin_user`, `passphrase`, `password`, `secret`, `seed`, `seed_hex`, `url_password`, `url_username` and `username`. Nesting no longer matters, so a credential inside `params` is masked too. The same masking is applied to every request and reply written to the log, and it covers six further names that only a reply carries: `master_key`, `master_seed`, `master_seed_hex`, `validation_key`, `validation_private_key` and `validation_seed`, which is how `wallet_propose` and `validation_create` used to write a live private key to the log. A request or reply written to the log is truncated at 10,000 characters.
@@ -32,6 +36,7 @@ This version is supported by all `xrpld` versions. For WebSocket and HTTP JSON-R
- Four error codes that named no HTTP status of their own, and so answered 200 on a reply reporting an error, now name one: `actMalformed`, `alreadyMultisig` and `alreadySingleSig` answer 400, and `actNotFound` answers 404. **No shipped envelope reports these four.** A request sending `ripplerpc: "3.0"` still receives 200 for all four, as it always has, so `account_info` on a malformed account or one the ledger does not hold answers 200 exactly as before.
- `submit`, `simulate`, `transaction_entry`, `ledger_entry` and `ledger_accept`: Errors from these methods now include `error_code` and `error_message` alongside the `error` token, as every other method already did. Each error now answers the status its code names: 400 for a malformed request, 404 for `transactionNotFound`, 500 for an internal failure, and 501 for `notYetImplemented` and `notStandAlone`. That status change reaches only a request sending `ripplerpc: "3.0"`, which is the envelope that derives the status from the error. With `ripplerpc` `"1.0"` the status stays 200 and the two new members appear beside `error`; with `"2.0"` the status stays 200, `error_code` appears, and the `code` and `message` members carry the code and the message rather than null, since that envelope copies them from `error_code` and `error_message` and drops `error_message`.
- A reply reporting HTTP 402 or 502 now carries a status line. Those two statuses named no case in the switch that writes one, so such a reply began with a header instead and did not parse as an HTTP response at all. Both are reachable at any API version with `ripplerpc: "3.0"`, which derives the status from the error code: 402 through `highFee` from `sign`, `sign_for` or `submit` with a low `fee_mult_max`, and 502 through `dbDeserialization` from `tx`. The eleven statuses that already named a case report the same phrase they always have.
- An error reply to a request sending `ripplerpc: "2.0"` or `"3.0"` no longer carries a stray `"error_message": null` beside the error it reports. The member appeared only when the `Server` log partition was set to debug or lower, because the log statement read `error_message` after the reply had renamed it to `message`, and reading it put it back as null. So the reply a client received depended on the server's log level, and the log line itself printed an empty message. Both are fixed.
## XRP Ledger server version 3.5.0
src/test/server/ServerStatus_test.cpp
+152
View File
@@ -1342,6 +1342,41 @@ class ServerStatus_test : public beast::unit_test::Suite, public beast::test::En
boost::system::error_code ec;
// A value naming no supported version is rejected rather than coerced.
for (auto const* version : {
"0.0", // unsupported major
"4.0", //
"10.0", // would sort below "3.0" lexicographically
"02.0", // leading zero
"2.00", // non-canonical minor
"2", // missing minor
"2.", //
".0", // missing major
"2.x", // non-numeric minor
"2x", // trailing junk
"x2", //
"abc", //
"-1.0", // signs are not part of a version
"+2.0", //
" 2.0", // surrounding whitespace
"2.0 ", //
"2.0.0", // patch component is not accepted
"2e0", //
"2.7", // no version defines a nonzero minor
"2.99", //
"3.99", //
"2.01", // leading zero in the minor
"1.4294967295", //
"999999999999999999999999999.0", // too long to name a version
"1.999999999999999999999999999", // too long to name a version
})
{
Response resp;
doHTTPRequest(env, yield, false, resp, ec, makeRippleRpcRequest("ping", version));
BEAST_EXPECTS(resp.result() == kBadRequest, version);
BEAST_EXPECTS(resp.body() == "ripplerpc is not a supported version\r\n", version);
}
// A non-string is reported separately, before any version parsing.
{
Response resp;
@@ -1367,6 +1402,91 @@ class ServerStatus_test : public beast::unit_test::Suite, public beast::test::En
BEAST_EXPECTS(reply[jss::result][jss::status] == jss::success, version);
}
// Every version reports success the same way, so only the error envelope
// distinguishes them. `account_info` without an account fails in the handler,
// carrying an `error_code` that version 3 maps onto an HTTP status.
{
Response resp;
doHTTPRequest(
env,
yield,
false,
resp,
ec,
makeRippleRpcRequest("account_info", rpc::kRippleRpcVersion1));
BEAST_EXPECT(resp.result() == kOk);
json::Value reply;
BEAST_EXPECT(json::Reader{}.parse(resp.body(), reply));
BEAST_EXPECT(reply[jss::result][jss::status] == jss::error);
BEAST_EXPECT(reply[jss::result].isMember(jss::error_message));
BEAST_EXPECT(reply[jss::result].isMember(jss::request));
BEAST_EXPECT(!reply.isMember(jss::error));
}
// Versions 2 and 3 share an error envelope, so one loop covers both.
for (auto const version : {rpc::kRippleRpcVersion2, rpc::kRippleRpcVersion3})
{
Response resp;
auto const reply = postAndParse(
env, yield, resp, ec, makeRippleRpcRequest("account_info", version), version);
BEAST_EXPECTS(reply[jss::error][jss::status] == jss::error, version);
BEAST_EXPECTS(reply[jss::error].isMember(jss::message), version);
BEAST_EXPECTS(!reply[jss::error].isMember(jss::error_message), version);
BEAST_EXPECTS(!reply[jss::error].isMember(jss::request), version);
BEAST_EXPECTS(!reply.isMember(jss::request), version);
auto const expected = version == rpc::kRippleRpcVersion3 ? kBadRequest : kOk;
BEAST_EXPECTS(resp.result() == expected, version);
}
// A batch entry rejected before dispatch is echoed back, so it is masked too.
{
Response resp;
json::Value entry(json::ValueType::Object);
entry[jss::method] = "ping";
entry[jss::ripplerpc] = "10.0";
for (auto const field : rpc::kCredentialFields)
entry[std::string{field}] = "sensitive";
json::Value jv;
jv[jss::method] = "batch";
jv[jss::params] = json::ValueType::Array;
jv[jss::params][0u] = entry;
auto const reply = postAndParse(env, yield, resp, ec, to_string(jv));
BEAST_EXPECT(reply.isArray() && reply.size() == 1);
for (auto const field : rpc::kCredentialFields)
BEAST_EXPECTS(reply[0u][std::string{field}] == "<masked>", std::string{field});
}
// A rejected entry does not decide the version for the rest, and the batch returns 200.
{
Response resp;
json::Value jv;
jv[jss::method] = "batch";
jv[jss::params] = json::ValueType::Array;
for (auto const& [index, version] :
{std::pair{0u, "2.0"}, std::pair{1u, "abc"}, std::pair{2u, "1.0"}})
{
json::Value entry(json::ValueType::Object);
entry[jss::method] = "ping";
entry[jss::ripplerpc] = version;
entry[jss::id] = index;
jv[jss::params][index] = entry;
}
auto const reply = postAndParse(env, yield, resp, ec, to_string(jv));
BEAST_EXPECT(resp.result() == kOk);
BEAST_EXPECT(reply.isArray() && reply.size() == 3);
BEAST_EXPECT(reply[0u][jss::result][jss::status] == jss::success);
BEAST_EXPECT(reply[2u][jss::result][jss::status] == jss::success);
// A version the server cannot honor is a bad parameter, not a missing method.
BEAST_EXPECT(
reply[1u][jss::error][jss::error][jss::message] ==
"ripplerpc is not a supported version");
BEAST_EXPECT(reply[1u][jss::error][jss::error][jss::code] == -32602);
BEAST_EXPECT(reply[1u][jss::ripplerpc] == "abc");
}
// An entry naming an `api_version` the server cannot serve is returned under `request`,
// rather than spliced beside the error the way other rejections are.
{
@@ -1455,6 +1575,37 @@ class ServerStatus_test : public beast::unit_test::Suite, public beast::test::En
}
}
/**
* What a client receives cannot depend on how the server logs.
*
* Reading an absent member off a non-const value inserts it as an explicit
* null, so reading `error_message` after the rename to `message` puts
* `"error_message": null` back into the reply. The Env below logs at debug,
* which is what arms these assertions.
*
* @param yield The coroutine the requests run on.
*/
void
testAnErrorReplyDoesNotFollowTheLogLevel(boost::asio::yield_context& yield)
{
testcase("An error reply is the same at every log level");
using namespace test::jtx;
Env env{*this, envconfig(), nullptr, beast::Severity::Debug};
boost::system::error_code ec;
// `account_info` naming no account fails in the handler, where both members come from.
for (auto const version : {rpc::kRippleRpcVersion2, rpc::kRippleRpcVersion3})
{
boost::beast::http::response<boost::beast::http::string_body> resp;
auto const reply = postAndParse(
env, yield, resp, ec, makeRippleRpcRequest("account_info", version), version);
BEAST_EXPECTS(reply[jss::error].isMember(jss::message), version);
BEAST_EXPECTS(!reply[jss::error].isMember(jss::error_message), version);
}
}
/**
* The five handlers that report a bare token carry a code and message with
* it.
@@ -2241,6 +2392,7 @@ public:
testRipplerpcVersions(yield);
testPrivilegedRequestIsNotShed(yield);
testLegacyBatchEntryRejections(yield);
testAnErrorReplyDoesNotFollowTheLogLevel(yield);
testHandlerErrorsCarryCodes(yield);
testGainedStatusesStayOffLegacyEnvelope(yield);
testUncommonHttpStatus(yield);
src/xrpld/rpc/detail/ServerHandler.cpp
+134 -74
View File
@@ -68,6 +68,7 @@
#include <map>
#include <memory>
#include <mutex>
#include <optional>
#include <ostream>
#include <string>
#include <string_view>
@@ -594,6 +595,41 @@ makeJsonError(json::Int code, json::Value&& message)
return r;
}
// Version of the JSON-RPC reply envelope, selected by the `ripplerpc` request parameter. It shapes
// a completed result into a reply; no handler observes it.
enum class RpcVersion {
// Errors are returned under `result`, keyed by `error_message`, and echo the (secret-masked)
// request. HTTP status is always 200.
V1,
// Errors are returned under `error`, keyed by `message`, and the request is not echoed. HTTP
// status is always 200.
V2,
// As V2, but the error code selects a 4xx/5xx HTTP status.
V3,
};
constexpr RpcVersion kRpcVersionIfUnspecified = RpcVersion::V1;
/**
* Maps a `ripplerpc` value onto the envelope version it selects.
*
* The value is unauthenticated, so anything but an exact match is refused.
*
* @param value The `ripplerpc` member, as sent.
* @return The envelope version, or nullopt for the caller to reject.
*/
static std::optional<RpcVersion>
rpcVersion(std::string_view value)
{
if (value == rpc::kRippleRpcVersion1)
return RpcVersion::V1;
if (value == rpc::kRippleRpcVersion2)
return RpcVersion::V2;
if (value == rpc::kRippleRpcVersion3)
return RpcVersion::V3;
return std::nullopt;
}
/**
* The HTTP status the `ripplerpc: "3.0"` envelope reports for @p code.
*
@@ -619,6 +655,84 @@ legacyHttpStatus(ErrorCodeI code)
}
}
/**
* Shapes a handler @p result into the reply envelope for @p version, appending
* it to @p reply.
*
* @param version The envelope the reply takes.
* @param result A handler result, consumed.
* @param request The request's parameters, echoed on a version 1 error only,
* and where the `jsonrpc`, `ripplerpc` and `id` members are read.
* @param batch Whether to append to @p reply rather than assign it.
* @param reply The reply this writes.
* @param journal Where an error is logged.
* @return The HTTP status the reply is sent with. Only version 3 derives it
* from the error code; the others report 200. A batch discards it.
*/
static int
shapeReply(
RpcVersion version,
json::Value result,
json::Value const& request,
bool batch,
json::Value& reply,
beast::Journal journal)
{
json::Value r(json::ValueType::Object);
int status = 200;
if (!result.isMember(jss::error))
{
result[jss::status] = jss::success;
r[jss::result] = std::move(result);
}
else
{
// Read through a const reference: the non-const `operator[]` inserts a null member for an
// absent name, and the reply below is built out of `result`.
json::Value const& reported = result;
JLOG(journal.debug()) << "rpcError: " << reported[jss::error] << ": "
<< reported[jss::error_message];
if (version == RpcVersion::V3 && reported[jss::error_code].isInt())
status = legacyHttpStatus(static_cast<ErrorCodeI>(reported[jss::error_code].asInt()));
result[jss::status] = jss::error;
if (version == RpcVersion::V1)
{
result[jss::request] = rpc::maskSecrets(request);
r[jss::result] = std::move(result);
}
else
{
result[jss::code] = result[jss::error_code];
result[jss::message] = result[jss::error_message];
result.removeMember(jss::error_message);
r[jss::error] = std::move(result);
}
}
if (request.isMember(jss::jsonrpc))
r[jss::jsonrpc] = request[jss::jsonrpc];
if (request.isMember(jss::ripplerpc))
r[jss::ripplerpc] = request[jss::ripplerpc];
if (request.isMember(jss::id))
r[jss::id] = request[jss::id];
if (batch)
{
reply.append(std::move(r));
}
else
{
reply = std::move(r);
}
return status;
}
void
ServerHandler::processRequest(
Port const& port,
@@ -662,6 +776,9 @@ ServerHandler::processRequest(
}
json::Value reply(batch ? json::ValueType::Array : json::ValueType::Object);
// Only a lone request selects the HTTP status: a batch may mix versions and reports each
// entry's outcome in its own reply, so the batch itself always succeeds.
int httpStatus = 200;
auto const start(std::chrono::high_resolution_clock::now());
for (unsigned i = 0; i < size; ++i)
{
@@ -837,10 +954,11 @@ ServerHandler::processRequest(
params = jsonRPC;
}
std::string ripplerpc = "1.0";
RpcVersion envelope = kRpcVersionIfUnspecified;
if (params.isMember(jss::ripplerpc))
{
// Not a method-not-found condition, but it is the code shipped versions report.
// A `ripplerpc` the server cannot honor is a bad parameter, so the second check reports
// that code. The first keeps the method-not-found code shipped versions report.
if (!params[jss::ripplerpc].isString())
{
usage.charge(resource::kFeeMalformedRpc);
@@ -848,7 +966,17 @@ ServerHandler::processRequest(
return;
continue;
}
ripplerpc = params[jss::ripplerpc].asString();
auto const parsed = rpcVersion(params[jss::ripplerpc].asString());
if (!parsed)
{
usage.charge(resource::kFeeMalformedRpc);
if (!reject(
400, rpc::kJsonRpcInvalidParams, "ripplerpc is not a supported version"))
return;
continue;
}
envelope = *parsed;
}
/**
@@ -905,58 +1033,9 @@ ServerHandler::processRequest(
if (usage.warn())
result[jss::warning] = jss::load;
json::Value r(json::ValueType::Object);
if (ripplerpc >= "2.0")
{
if (result.isMember(jss::error))
{
result[jss::status] = jss::error;
result["code"] = result[jss::error_code];
result["message"] = result[jss::error_message];
result.removeMember(jss::error_message);
JLOG(journal_.debug())
<< "rpcError: " << result[jss::error] << ": " << result[jss::error_message];
r[jss::error] = std::move(result);
}
else
{
result[jss::status] = jss::success;
r[jss::result] = std::move(result);
}
}
else
{
// Always report "status". On an error report the request as
// received.
if (result.isMember(jss::error))
{
result[jss::status] = jss::error;
result[jss::request] = rpc::maskSecrets(params);
JLOG(journal_.debug())
<< "rpcError: " << result[jss::error] << ": " << result[jss::error_message];
}
else
{
result[jss::status] = jss::success;
}
r[jss::result] = std::move(result);
}
if (params.isMember(jss::jsonrpc))
r[jss::jsonrpc] = params[jss::jsonrpc];
if (params.isMember(jss::ripplerpc))
r[jss::ripplerpc] = params[jss::ripplerpc];
if (params.isMember(jss::id))
r[jss::id] = params[jss::id];
if (batch)
{
reply.append(std::move(r));
}
else
{
reply = std::move(r);
}
int const status = shapeReply(envelope, std::move(result), params, batch, reply, journal_);
if (!batch)
httpStatus = status;
if (reply.isMember(jss::result) && reply[jss::result].isMember(jss::result))
{
@@ -969,25 +1048,6 @@ ServerHandler::processRequest(
}
}
// If we're returning an error_code, use that to determine the HTTP status.
int const httpStatus = [&reply] {
// This feature is enabled with ripplerpc version 3.0 and above.
// Before ripplerpc version 3.0 always return 200.
if (reply.isMember(jss::ripplerpc) && reply[jss::ripplerpc].isString() &&
reply[jss::ripplerpc].asString() >= "3.0")
{
// If there's an error_code, use that to determine the HTTP Status.
if (reply.isMember(jss::error) && reply[jss::error].isMember(jss::error_code) &&
reply[jss::error][jss::error_code].isInt())
{
int const errCode = reply[jss::error][jss::error_code].asInt();
return legacyHttpStatus(static_cast<ErrorCodeI>(errCode));
}
}
// Return OK.
return 200;
}();
auto response = to_string(reply);
rpcTime_.notify(