Fix http params handling discrepancy (#913)

Fixes #909
This commit is contained in:
Alex Kremer
2023-10-10 12:23:40 +01:00
committed by GitHub
parent a541e6d00e
commit fca29694a0
4 changed files with 29 additions and 9 deletions

View File

@@ -145,9 +145,8 @@ public:
LOG(perfLog_.debug()) << ctx.tag() << " finish executing rpc `" << ctx.method << '`';
if (v)
{
return v->as_object();
}
notifyErrored(ctx.method);
return Status{v.error()};
}

View File

@@ -89,8 +89,8 @@ public:
auto req = boost::json::parse(request).as_object();
LOG(perfLog_.debug()) << connection->tag() << "Adding to work queue";
if (not connection->upgraded and not req.contains("params"))
req["params"] = boost::json::array({boost::json::object{}});
if (not connection->upgraded and shouldReplaceParams(req))
req[JS(params)] = boost::json::array({boost::json::object{}});
if (!rpcEngine_->post(
[this, request = std::move(req), connection](boost::asio::yield_context yield) mutable {
@@ -269,6 +269,27 @@ private:
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

View File

@@ -187,10 +187,10 @@ public:
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();
upgraded = true;
return derived().upgrade();
}

View File

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