Fix http params handling discrepancy (#913)

Fixes #909
This commit is contained in:
Alex Kremer
2023-10-10 12:23:40 +01:00
parent 7e621b2518
commit 7a03999382
4 changed files with 31 additions and 12 deletions

View File

@@ -150,11 +150,9 @@ public:
if (v) if (v)
return v->as_object(); return v->as_object();
else
{ notifyErrored(ctx.method);
notifyErrored(ctx.method); return Status{v.error()};
return Status{v.error()};
}
} }
catch (data::DatabaseTimeout const& t) catch (data::DatabaseTimeout const& t)
{ {

View File

@@ -89,8 +89,8 @@ public:
auto req = boost::json::parse(request).as_object(); auto req = boost::json::parse(request).as_object();
LOG(perfLog_.debug()) << connection->tag() << "Adding to work queue"; LOG(perfLog_.debug()) << connection->tag() << "Adding to work queue";
if (not connection->upgraded and not req.contains("params")) if (not connection->upgraded and shouldReplaceParams(req))
req["params"] = boost::json::array({boost::json::object{}}); req[JS(params)] = boost::json::array({boost::json::object{}});
if (!rpcEngine_->post( if (!rpcEngine_->post(
[this, request = std::move(req), connection](boost::asio::yield_context yield) mutable { [this, request = std::move(req), connection](boost::asio::yield_context yield) mutable {
@@ -267,6 +267,27 @@ private:
return web::detail::ErrorHelper(connection, request).sendInternalError(); return web::detail::ErrorHelper(connection, request).sendInternalError();
} }
} }
bool
shouldReplaceParams(boost::json::object const& req) const
{
auto const hasParams = req.contains(JS(params));
auto const paramsIsArray = hasParams and req.at(JS(params)).is_array();
auto const paramsIsEmptyString =
hasParams and req.at(JS(params)).is_string() and req.at(JS(params)).as_string().empty();
auto const paramsIsEmptyObject =
hasParams and req.at(JS(params)).is_object() and req.at(JS(params)).as_object().empty();
auto const paramsIsNull = hasParams and req.at(JS(params)).is_null();
auto const arrayIsEmpty = paramsIsArray and req.at(JS(params)).as_array().empty();
auto const arrayIsNotEmpty = paramsIsArray and not req.at(JS(params)).as_array().empty();
auto const firstArgIsNull = arrayIsNotEmpty and req.at(JS(params)).as_array().at(0).is_null();
auto const firstArgIsEmptyString = arrayIsNotEmpty and req.at(JS(params)).as_array().at(0).is_string() and
req.at(JS(params)).as_array().at(0).as_string().empty();
// Note: all this compatibility dance is to match `rippled` as close as possible
return not hasParams or paramsIsEmptyString or paramsIsNull or paramsIsEmptyObject or arrayIsEmpty or
firstArgIsEmptyString or firstArgIsNull;
}
}; };
} // namespace web } // namespace web

View File

@@ -180,10 +180,10 @@ public:
if (boost::beast::websocket::is_upgrade(req_)) if (boost::beast::websocket::is_upgrade(req_))
{ {
upgraded = true; // Disable the timeout. The websocket::stream uses its own timeout settings.
// Disable the timeout.
// The websocket::stream uses its own timeout settings.
boost::beast::get_lowest_layer(derived().stream()).expires_never(); boost::beast::get_lowest_layer(derived().stream()).expires_never();
upgraded = true;
return derived().upgrade(); return derived().upgrade();
} }

View File

@@ -549,7 +549,7 @@ TEST_F(WebRPCServerHandlerTest, HTTPParamsUnparseableNotArray)
EXPECT_EQ(session->lastStatus, boost::beast::http::status::bad_request); EXPECT_EQ(session->lastStatus, boost::beast::http::status::bad_request);
} }
TEST_F(WebRPCServerHandlerTest, HTTPParamsUnparseableEmptyArray) TEST_F(WebRPCServerHandlerTest, HTTPParamsUnparseableArrayWithDigit)
{ {
static auto constexpr response = "params unparseable"; static auto constexpr response = "params unparseable";
@@ -558,7 +558,7 @@ TEST_F(WebRPCServerHandlerTest, HTTPParamsUnparseableEmptyArray)
static auto constexpr requestJSON = R"({ static auto constexpr requestJSON = R"({
"method": "ledger", "method": "ledger",
"params": [] "params": [1]
})"; })";
EXPECT_CALL(*rpcEngine, notifyBadSyntax).Times(1); EXPECT_CALL(*rpcEngine, notifyBadSyntax).Times(1);