diff --git a/src/rpc/RPCEngine.h b/src/rpc/RPCEngine.h index f4f340ce..c5d371a8 100644 --- a/src/rpc/RPCEngine.h +++ b/src/rpc/RPCEngine.h @@ -150,11 +150,9 @@ public: if (v) return v->as_object(); - else - { - notifyErrored(ctx.method); - return Status{v.error()}; - } + + notifyErrored(ctx.method); + return Status{v.error()}; } catch (data::DatabaseTimeout const& t) { diff --git a/src/web/RPCServerHandler.h b/src/web/RPCServerHandler.h index 764c201f..84e718f4 100644 --- a/src/web/RPCServerHandler.h +++ b/src/web/RPCServerHandler.h @@ -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 { @@ -267,6 +267,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 diff --git a/src/web/impl/HttpBase.h b/src/web/impl/HttpBase.h index 3809058a..86ab8e0c 100644 --- a/src/web/impl/HttpBase.h +++ b/src/web/impl/HttpBase.h @@ -180,10 +180,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(); } diff --git a/unittests/web/RPCServerHandlerTests.cpp b/unittests/web/RPCServerHandlerTests.cpp index 7c577a20..db9a8210 100644 --- a/unittests/web/RPCServerHandlerTests.cpp +++ b/unittests/web/RPCServerHandlerTests.cpp @@ -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);