From d4d46a978069f52d2b23b208f1e76f0d3205ca78 Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Thu, 11 Sep 2025 13:12:23 +0200 Subject: [PATCH] HTTP: stream responses where appropriate --- lib/remote/actionshandler.cpp | 17 +++++++++---- lib/remote/configobjectslock.hpp | 9 +++++++ lib/remote/configpackageshandler.cpp | 16 +++++++------ lib/remote/configpackageshandler.hpp | 2 +- lib/remote/configstageshandler.cpp | 25 +++++++++---------- lib/remote/configstageshandler.hpp | 2 +- lib/remote/deleteobjecthandler.cpp | 18 ++++++++++---- lib/remote/httputility.cpp | 22 +++++++++++++++++ lib/remote/httputility.hpp | 2 ++ lib/remote/modifyobjecthandler.cpp | 17 +++++++++---- lib/remote/objectqueryhandler.cpp | 10 +++----- lib/remote/templatequeryhandler.cpp | 12 ++++++---- lib/remote/typequeryhandler.cpp | 36 ++++++++++++---------------- lib/remote/variablequeryhandler.cpp | 22 +++++++---------- test/base-json.cpp | 8 +++---- 15 files changed, 130 insertions(+), 88 deletions(-) diff --git a/lib/remote/actionshandler.cpp b/lib/remote/actionshandler.cpp index 8dde35987..59d03ef6e 100644 --- a/lib/remote/actionshandler.cpp +++ b/lib/remote/actionshandler.cpp @@ -20,7 +20,7 @@ bool ActionsHandler::HandleRequest( const WaitGroup::Ptr& waitGroup, const HttpApiRequest& request, HttpApiResponse& response, - boost::asio::yield_context& + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; @@ -126,6 +126,11 @@ bool ActionsHandler::HandleRequest( } } + if (wgLock.owns_lock()) { + // Unlock before starting to stream the response, so that we don't block the shutdown process. + wgLock.unlock(); + } + int statusCode = 500; std::set okStatusCodes, nonOkStatusCodes; @@ -156,11 +161,13 @@ bool ActionsHandler::HandleRequest( response.result(statusCode); - Dictionary::Ptr result = new Dictionary({ - { "results", new Array(std::move(results)) } - }); + Array::Ptr resultArray{new Array{std::move(results)}}; + resultArray->Freeze(); // Allows the JSON encoder to yield while encoding the array. - HttpUtility::SendJsonBody(response, params, result); + Dictionary::Ptr result = new Dictionary{{"results", std::move(resultArray)}}; + result->Freeze(); + + HttpUtility::SendJsonBody(response, params, result, yc); return true; } diff --git a/lib/remote/configobjectslock.hpp b/lib/remote/configobjectslock.hpp index 3d6653b87..bdec65ac4 100644 --- a/lib/remote/configobjectslock.hpp +++ b/lib/remote/configobjectslock.hpp @@ -32,6 +32,10 @@ public: { return true; } + + constexpr void Unlock() + { + } }; #else /* _WIN32 */ @@ -69,6 +73,11 @@ public: return m_Lock.owns(); } + void Unlock() + { + m_Lock.unlock(); + } + private: boost::interprocess::sharable_lock m_Lock; }; diff --git a/lib/remote/configpackageshandler.cpp b/lib/remote/configpackageshandler.cpp index 4ce4a1967..7e0dc976d 100644 --- a/lib/remote/configpackageshandler.cpp +++ b/lib/remote/configpackageshandler.cpp @@ -16,7 +16,7 @@ bool ConfigPackagesHandler::HandleRequest( const WaitGroup::Ptr&, const HttpApiRequest& request, HttpApiResponse& response, - boost::asio::yield_context& + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; @@ -29,7 +29,7 @@ bool ConfigPackagesHandler::HandleRequest( return false; if (request.method() == http::verb::get) - HandleGet(request, response); + HandleGet(request, response, yc); else if (request.method() == http::verb::post) HandlePost(request, response); else if (request.method() == http::verb::delete_) @@ -40,7 +40,7 @@ bool ConfigPackagesHandler::HandleRequest( return true; } -void ConfigPackagesHandler::HandleGet(const HttpApiRequest& request, HttpApiResponse& response) +void ConfigPackagesHandler::HandleGet(const HttpApiRequest& request, HttpApiResponse& response, boost::asio::yield_context& yc) { namespace http = boost::beast::http; @@ -80,12 +80,14 @@ void ConfigPackagesHandler::HandleGet(const HttpApiRequest& request, HttpApiResp } } - Dictionary::Ptr result = new Dictionary({ - { "results", new Array(std::move(results)) } - }); + Array::Ptr resultsArr = new Array(std::move(results)); + resultsArr->Freeze(); + + Dictionary::Ptr result = new Dictionary{{"results", resultsArr}}; + result->Freeze(); response.result(http::status::ok); - HttpUtility::SendJsonBody(response, params, result); + HttpUtility::SendJsonBody(response, params, result, yc); } void ConfigPackagesHandler::HandlePost(const HttpApiRequest& request, HttpApiResponse& response) diff --git a/lib/remote/configpackageshandler.hpp b/lib/remote/configpackageshandler.hpp index d581ce709..e8742ebd1 100644 --- a/lib/remote/configpackageshandler.hpp +++ b/lib/remote/configpackageshandler.hpp @@ -22,7 +22,7 @@ public: ) override; private: - void HandleGet(const HttpApiRequest& request, HttpApiResponse& response); + void HandleGet(const HttpApiRequest& request, HttpApiResponse& response, boost::asio::yield_context& yc); void HandlePost(const HttpApiRequest& request, HttpApiResponse& response); void HandleDelete(const HttpApiRequest& request, HttpApiResponse& response); diff --git a/lib/remote/configstageshandler.cpp b/lib/remote/configstageshandler.cpp index 823f954a1..8bcd16bbe 100644 --- a/lib/remote/configstageshandler.cpp +++ b/lib/remote/configstageshandler.cpp @@ -23,7 +23,7 @@ bool ConfigStagesHandler::HandleRequest( const WaitGroup::Ptr&, const HttpApiRequest& request, HttpApiResponse& response, - boost::asio::yield_context& + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; @@ -36,7 +36,7 @@ bool ConfigStagesHandler::HandleRequest( return false; if (request.method() == http::verb::get) - HandleGet(request, response); + HandleGet(request, response, yc); else if (request.method() == http::verb::post) HandlePost(request, response); else if (request.method() == http::verb::delete_) @@ -47,7 +47,7 @@ bool ConfigStagesHandler::HandleRequest( return true; } -void ConfigStagesHandler::HandleGet(const HttpApiRequest& request, HttpApiResponse& response) +void ConfigStagesHandler::HandleGet(const HttpApiRequest& request, HttpApiResponse& response, boost::asio::yield_context& yc) { namespace http = boost::beast::http; @@ -72,25 +72,22 @@ void ConfigStagesHandler::HandleGet(const HttpApiRequest& request, HttpApiRespon if (!ConfigPackageUtility::ValidateStageName(stageName)) return HttpUtility::SendJsonError(response, params, 400, "Invalid stage name '" + stageName + "'."); - ArrayData results; - std::vector > paths = ConfigPackageUtility::GetFiles(packageName, stageName); String prefixPath = ConfigPackageUtility::GetPackageDir() + "/" + packageName + "/" + stageName + "/"; - for (const auto& kv : paths) { - results.push_back(new Dictionary({ + auto generatorFunc = [&prefixPath](const std::pair& kv) -> Value { + return new Dictionary{ { "type", kv.second ? "directory" : "file" }, - { "name", kv.first.SubStr(prefixPath.GetLength()) } - })); - } + { "name", kv.first.SubStr(prefixPath.GetLength()) }, + }; + }; - Dictionary::Ptr result = new Dictionary({ - { "results", new Array(std::move(results)) } - }); + Dictionary::Ptr result = new Dictionary{{"results", new ValueGenerator{paths, generatorFunc}}}; + result->Freeze(); response.result(http::status::ok); - HttpUtility::SendJsonBody(response, params, result); + HttpUtility::SendJsonBody(response, params, result, yc); } void ConfigStagesHandler::HandlePost(const HttpApiRequest& request, HttpApiResponse& response) diff --git a/lib/remote/configstageshandler.hpp b/lib/remote/configstageshandler.hpp index 499cc083c..8dda34122 100644 --- a/lib/remote/configstageshandler.hpp +++ b/lib/remote/configstageshandler.hpp @@ -22,7 +22,7 @@ public: ) override; private: - void HandleGet(const HttpApiRequest& request, HttpApiResponse& response); + void HandleGet(const HttpApiRequest& request, HttpApiResponse& response, boost::asio::yield_context& yc); void HandlePost(const HttpApiRequest& request, HttpApiResponse& response); void HandleDelete(const HttpApiRequest& request, HttpApiResponse& response); }; diff --git a/lib/remote/deleteobjecthandler.cpp b/lib/remote/deleteobjecthandler.cpp index 33468f224..a918d4a30 100644 --- a/lib/remote/deleteobjecthandler.cpp +++ b/lib/remote/deleteobjecthandler.cpp @@ -20,7 +20,7 @@ bool DeleteObjectHandler::HandleRequest( const WaitGroup::Ptr& waitGroup, const HttpApiRequest& request, HttpApiResponse& response, - boost::asio::yield_context& + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; @@ -134,16 +134,24 @@ bool DeleteObjectHandler::HandleRequest( results.push_back(result); } - Dictionary::Ptr result = new Dictionary({ - { "results", new Array(std::move(results)) } - }); + // Unlock the mutexes before starting to stream the response, so that we don't block the shutdown process. + lock.Unlock(); + if (wgLock.owns_lock()) { + wgLock.unlock(); + } + + Array::Ptr resultArray = new Array{std::move(results)}; + resultArray->Freeze(); + + Dictionary::Ptr result = new Dictionary({{ "results", std::move(resultArray)}}); + result->Freeze(); if (!success) response.result(http::status::internal_server_error); else response.result(http::status::ok); - HttpUtility::SendJsonBody(response, params, result); + HttpUtility::SendJsonBody(response, params, result, yc); return true; } diff --git a/lib/remote/httputility.cpp b/lib/remote/httputility.cpp index 6a154842a..5e466bb8a 100644 --- a/lib/remote/httputility.cpp +++ b/lib/remote/httputility.cpp @@ -53,6 +53,28 @@ Value HttpUtility::GetLastParameter(const Dictionary::Ptr& params, const String& return arr->Get(arr->GetLength() - 1); } +/** + * Stream a JSON-encoded body to the client. + * + * This function sets the Content-Type header to "application/json", starts the streaming of the response, + * and encodes the given value as JSON to the client. If pretty-print is requested, the JSON output will be + * formatted accordingly. It is assumed that the response status code and other necessary headers have already + * been set. + * + * @param response The HTTP response to send the body to. + * @param params The request parameters. + * @param val The value to encode as JSON and stream to the client. + * @param yc The yield context to use for asynchronous operations. + */ +void HttpUtility::SendJsonBody(HttpApiResponse& response, const Dictionary::Ptr& params, const Value& val, boost::asio::yield_context& yc) +{ + namespace http = boost::beast::http; + + response.set(http::field::content_type, "application/json"); + response.StartStreaming(false); + response.GetJsonEncoder(params && GetLastParameter(params, "pretty")).Encode(val, &yc); +} + void HttpUtility::SendJsonBody(HttpApiResponse& response, const Dictionary::Ptr& params, const Value& val) { namespace http = boost::beast::http; diff --git a/lib/remote/httputility.hpp b/lib/remote/httputility.hpp index 3df28eb63..c8e5ccf24 100644 --- a/lib/remote/httputility.hpp +++ b/lib/remote/httputility.hpp @@ -7,6 +7,7 @@ #include "remote/url.hpp" #include "base/dictionary.hpp" #include "remote/httpmessage.hpp" +#include #include namespace icinga @@ -24,6 +25,7 @@ public: static Dictionary::Ptr FetchRequestParameters(const Url::Ptr& url, const std::string& body); static Value GetLastParameter(const Dictionary::Ptr& params, const String& key); + static void SendJsonBody(HttpApiResponse& response, const Dictionary::Ptr& params, const Value& val, boost::asio::yield_context& yc); static void SendJsonBody(HttpApiResponse& response, const Dictionary::Ptr& params, const Value& val); static void SendJsonError(HttpApiResponse& response, const Dictionary::Ptr& params, const int code, const String& info = {}, const String& diagnosticInformation = {}); diff --git a/lib/remote/modifyobjecthandler.cpp b/lib/remote/modifyobjecthandler.cpp index 2e46d1537..5d97919dd 100644 --- a/lib/remote/modifyobjecthandler.cpp +++ b/lib/remote/modifyobjecthandler.cpp @@ -18,7 +18,7 @@ bool ModifyObjectHandler::HandleRequest( const WaitGroup::Ptr& waitGroup, const HttpApiRequest& request, HttpApiResponse& response, - boost::asio::yield_context& + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; @@ -179,13 +179,20 @@ bool ModifyObjectHandler::HandleRequest( results.push_back(std::move(result1)); } + // Unlock the mutexes before starting to stream the response, so that we don't block the shutdown process. + lock.Unlock(); + if (wgLock.owns_lock()) { + wgLock.unlock(); + } - Dictionary::Ptr result = new Dictionary({ - { "results", new Array(std::move(results)) } - }); + Array::Ptr resultArray = new Array{std::move(results)}; + resultArray->Freeze(); + + Dictionary::Ptr result = new Dictionary{{"results", std::move(resultArray)}}; + result->Freeze(); response.result(http::status::ok); - HttpUtility::SendJsonBody(response, params, result); + HttpUtility::SendJsonBody(response, params, result, yc); return true; } diff --git a/lib/remote/objectqueryhandler.cpp b/lib/remote/objectqueryhandler.cpp index b55554b3c..2c81c6a73 100644 --- a/lib/remote/objectqueryhandler.cpp +++ b/lib/remote/objectqueryhandler.cpp @@ -210,7 +210,7 @@ bool ObjectQueryHandler::HandleRequest( std::unordered_map>> typePermissions; std::unordered_map objectAccessAllowed; - auto generatorFunc = [&](const ConfigObject::Ptr& obj) -> std::optional { + auto generatorFunc = [&](const ConfigObject::Ptr& obj) -> Value { DictionaryData result1{ { "name", obj->GetName() }, { "type", obj->GetReflectionType()->GetName() } @@ -319,15 +319,11 @@ bool ObjectQueryHandler::HandleRequest( return new Dictionary{std::move(result1)}; }; - response.result(http::status::ok); - response.set(http::field::content_type, "application/json"); - response.StartStreaming(false); - Dictionary::Ptr results = new Dictionary{{"results", new ValueGenerator{objs, generatorFunc}}}; results->Freeze(); - bool pretty = HttpUtility::GetLastParameter(params, "pretty"); - response.GetJsonEncoder(pretty).Encode(results, &yc); + response.result(http::status::ok); + HttpUtility::SendJsonBody(response, params, results, yc); return true; } diff --git a/lib/remote/templatequeryhandler.cpp b/lib/remote/templatequeryhandler.cpp index e3adca282..1aaba7906 100644 --- a/lib/remote/templatequeryhandler.cpp +++ b/lib/remote/templatequeryhandler.cpp @@ -80,7 +80,7 @@ bool TemplateQueryHandler::HandleRequest( const WaitGroup::Ptr&, const HttpApiRequest& request, HttpApiResponse& response, - boost::asio::yield_context& + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; @@ -126,12 +126,14 @@ bool TemplateQueryHandler::HandleRequest( return true; } - Dictionary::Ptr result = new Dictionary({ - { "results", new Array(std::move(objs)) } - }); + Array::Ptr resultArr = new Array(std::move(objs)); + resultArr->Freeze(); + + Dictionary::Ptr result = new Dictionary{{"results", resultArr}}; + result->Freeze(); response.result(http::status::ok); - HttpUtility::SendJsonBody(response, params, result); + HttpUtility::SendJsonBody(response, params, result, yc); return true; } diff --git a/lib/remote/typequeryhandler.cpp b/lib/remote/typequeryhandler.cpp index 0455d4ccf..203dfbf41 100644 --- a/lib/remote/typequeryhandler.cpp +++ b/lib/remote/typequeryhandler.cpp @@ -4,9 +4,7 @@ #include "remote/typequeryhandler.hpp" #include "remote/httputility.hpp" #include "remote/filterutility.hpp" -#include "base/configtype.hpp" -#include "base/scriptglobal.hpp" -#include "base/logger.hpp" +#include "base/generator.hpp" #include using namespace icinga; @@ -51,7 +49,7 @@ bool TypeQueryHandler::HandleRequest( const WaitGroup::Ptr&, const HttpApiRequest& request, HttpApiResponse& response, - boost::asio::yield_context& + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; @@ -90,23 +88,19 @@ bool TypeQueryHandler::HandleRequest( return true; } - ArrayData results; - - for (Type::Ptr obj : objs) { - Dictionary::Ptr result1 = new Dictionary(); - results.push_back(result1); - + auto generatorFunc = [](const Type::Ptr& obj) -> Value { + Dictionary::Ptr result = new Dictionary(); Dictionary::Ptr resultAttrs = new Dictionary(); - result1->Set("name", obj->GetName()); - result1->Set("plural_name", obj->GetPluralName()); + result->Set("name", obj->GetName()); + result->Set("plural_name", obj->GetPluralName()); if (obj->GetBaseType()) - result1->Set("base", obj->GetBaseType()->GetName()); - result1->Set("abstract", obj->IsAbstract()); - result1->Set("fields", resultAttrs); + result->Set("base", obj->GetBaseType()->GetName()); + result->Set("abstract", obj->IsAbstract()); + result->Set("fields", resultAttrs); Dictionary::Ptr prototype = dynamic_pointer_cast(obj->GetPrototype()); Array::Ptr prototypeKeys = new Array(); - result1->Set("prototype_keys", prototypeKeys); + result->Set("prototype_keys", prototypeKeys); if (prototype) { ObjectLock olock(prototype); @@ -144,14 +138,14 @@ bool TypeQueryHandler::HandleRequest( { "deprecated", static_cast(field.Attributes & FADeprecated) } })); } - } + return result; + }; - Dictionary::Ptr result = new Dictionary({ - { "results", new Array(std::move(results)) } - }); + Dictionary::Ptr result = new Dictionary{{"results", new ValueGenerator{objs, generatorFunc}}}; + result->Freeze(); response.result(http::status::ok); - HttpUtility::SendJsonBody(response, params, result); + HttpUtility::SendJsonBody(response, params, result, yc); return true; } diff --git a/lib/remote/variablequeryhandler.cpp b/lib/remote/variablequeryhandler.cpp index 2e5c55d0b..505983c45 100644 --- a/lib/remote/variablequeryhandler.cpp +++ b/lib/remote/variablequeryhandler.cpp @@ -4,9 +4,8 @@ #include "remote/variablequeryhandler.hpp" #include "remote/httputility.hpp" #include "remote/filterutility.hpp" -#include "base/configtype.hpp" +#include "base/generator.hpp" #include "base/scriptglobal.hpp" -#include "base/logger.hpp" #include "base/serializer.hpp" #include "base/namespace.hpp" #include @@ -63,7 +62,7 @@ bool VariableQueryHandler::HandleRequest( const WaitGroup::Ptr&, const HttpApiRequest& request, HttpApiResponse& response, - boost::asio::yield_context& + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; @@ -99,22 +98,19 @@ bool VariableQueryHandler::HandleRequest( return true; } - ArrayData results; - - for (Dictionary::Ptr var : objs) { - results.emplace_back(new Dictionary({ + auto generatorFunc = [](const Dictionary::Ptr& var) -> Value { + return new Dictionary{ { "name", var->Get("name") }, { "type", var->Get("type") }, { "value", Serialize(var->Get("value"), 0) } - })); - } + }; + }; - Dictionary::Ptr result = new Dictionary({ - { "results", new Array(std::move(results)) } - }); + Dictionary::Ptr result = new Dictionary{{"results", new ValueGenerator{objs, generatorFunc}}}; + result->Freeze(); response.result(http::status::ok); - HttpUtility::SendJsonBody(response, params, result); + HttpUtility::SendJsonBody(response, params, result, yc); return true; } diff --git a/test/base-json.cpp b/test/base-json.cpp index b82e92c02..0f7145fb8 100644 --- a/test/base-json.cpp +++ b/test/base-json.cpp @@ -22,7 +22,7 @@ BOOST_AUTO_TEST_CASE(encode) int emptyGenCounter = 0; std::vector empty; std::vector vec{1, 2, 3}; - auto generate = [](int count) -> std::optional { return Value(count); }; + auto generate = [](int count) -> Value { return Value(count); }; Dictionary::Ptr input (new Dictionary({ { "array", new Array({ new Namespace() }) }, @@ -44,9 +44,9 @@ BOOST_AUTO_TEST_CASE(encode) "empty_generator", new ValueGenerator{ empty, - [&emptyGenCounter](int) -> std::optional { + [&emptyGenCounter](int) -> Value { emptyGenCounter++; - return std::nullopt; + return Empty; } } }, @@ -91,8 +91,8 @@ BOOST_AUTO_TEST_CASE(encode) boost::algorithm::replace_all(output, "\n", ""); input->Set("generator", new ValueGenerator{vec, generate}); - BOOST_CHECK_EQUAL(emptyGenCounter, 0); // Ensure the transformation function was never invoked. BOOST_CHECK(JsonEncode(input, false) == output); + BOOST_CHECK_EQUAL(emptyGenCounter, 0); // Ensure the transformation function was never invoked. } BOOST_AUTO_TEST_CASE(decode)