From 37df8437006b7f515484835044f470081c646413 Mon Sep 17 00:00:00 2001 From: Johannes Schmidt Date: Wed, 2 Jul 2025 08:51:29 +0200 Subject: [PATCH 1/7] Add HttpRequest and HttpResponse classes --- lib/remote/CMakeLists.txt | 1 + lib/remote/httpmessage.cpp | 196 ++++++++++++++++++++++++++ lib/remote/httpmessage.hpp | 281 +++++++++++++++++++++++++++++++++++++ 3 files changed, 478 insertions(+) create mode 100644 lib/remote/httpmessage.cpp create mode 100644 lib/remote/httpmessage.hpp diff --git a/lib/remote/CMakeLists.txt b/lib/remote/CMakeLists.txt index 2271abff6ca..d8d3298c50a 100644 --- a/lib/remote/CMakeLists.txt +++ b/lib/remote/CMakeLists.txt @@ -27,6 +27,7 @@ set(remote_SOURCES eventshandler.cpp eventshandler.hpp filterutility.cpp filterutility.hpp httphandler.cpp httphandler.hpp + httpmessage.cpp httpmessage.hpp httpserverconnection.cpp httpserverconnection.hpp httputility.cpp httputility.hpp infohandler.cpp infohandler.hpp diff --git a/lib/remote/httpmessage.cpp b/lib/remote/httpmessage.cpp new file mode 100644 index 00000000000..18e5a301641 --- /dev/null +++ b/lib/remote/httpmessage.cpp @@ -0,0 +1,196 @@ +/* Icinga 2 | (c) 2025 Icinga GmbH | GPLv2+ */ + +#include "remote/httpmessage.hpp" +#include "base/io-engine.hpp" +#include "base/json.hpp" +#include "remote/httputility.hpp" +#include "remote/url.hpp" +#include +#include +#include + +using namespace icinga; + +/** + * This is the buffer size threshold above which to flush to the connection. + * + * This value was determined with a series of measurements in + * [PR #10516](https://github.com/Icinga/icinga2/pull/10516#issuecomment-3232642284). + */ +constexpr std::size_t l_FlushThreshold = 128UL * 1024UL; + +/** + * Adapter class for Boost Beast HTTP messages body to be used with the @c JsonEncoder. + * + * This class implements the @c nlohmann::detail::output_adapter_protocol<> interface and provides + * a way to write JSON data directly into the body of a @c HttpResponse. + * + * @ingroup base + */ +class HttpResponseJsonWriter : public AsyncJsonWriter +{ +public: + explicit HttpResponseJsonWriter(HttpResponse& msg) : m_Message{msg} + { + m_Message.body().Start(); +#if BOOST_VERSION >= 107000 + // We pre-allocate more than the threshold because we always go above the threshold + // at least once. + m_Message.body().Buffer().reserve(l_FlushThreshold + (l_FlushThreshold / 4)); +#endif /* BOOST_VERSION */ + } + + ~HttpResponseJsonWriter() override { m_Message.body().Finish(); } + + void write_character(char c) override { write_characters(&c, 1); } + + void write_characters(const char* s, std::size_t length) override + { + auto buf = m_Message.body().Buffer().prepare(length); + boost::asio::buffer_copy(buf, boost::asio::const_buffer{s, length}); + m_Message.body().Buffer().commit(length); + } + + void MayFlush(boost::asio::yield_context& yield) override + { + if (m_Message.body().Size() >= l_FlushThreshold) { + m_Message.Flush(yield); + } + } + +private: + HttpResponse& m_Message; +}; + +HttpRequest::HttpRequest(Shared::Ptr stream) : m_Stream(std::move(stream)) +{ +} + +void HttpRequest::ParseHeader(boost::beast::flat_buffer& buf, boost::asio::yield_context yc) +{ + boost::beast::http::async_read_header(*m_Stream, buf, m_Parser, yc); + base() = m_Parser.get().base(); +} + +void HttpRequest::ParseBody(boost::beast::flat_buffer& buf, boost::asio::yield_context yc) +{ + boost::beast::http::async_read(*m_Stream, buf, m_Parser, yc); + body() = std::move(m_Parser.release().body()); +} + +ApiUser::Ptr HttpRequest::User() const +{ + return m_User; +} + +void HttpRequest::User(const ApiUser::Ptr& user) +{ + m_User = user; +} + +Url::Ptr HttpRequest::Url() const +{ + return m_Url; +} + +void HttpRequest::DecodeUrl() +{ + m_Url = new icinga::Url(std::string(target())); +} + +Dictionary::Ptr HttpRequest::Params() const +{ + return m_Params; +} + +void HttpRequest::DecodeParams() +{ + if (!m_Url) { + DecodeUrl(); + } + m_Params = HttpUtility::FetchRequestParameters(m_Url, body()); +} + +HttpResponse::HttpResponse(Shared::Ptr stream, HttpServerConnection::Ptr server) + : m_Server(std::move(server)), m_Stream(std::move(stream)) +{ +} + +void HttpResponse::Clear() +{ + ASSERT(!m_SerializationStarted); + boost::beast::http::response::operator=({}); +} + +void HttpResponse::Flush(boost::asio::yield_context yc) +{ + if (!chunked() && !has_content_length()) { + ASSERT(!m_SerializationStarted); + prepare_payload(); + } + + m_SerializationStarted = true; + + if (!m_Serializer.is_header_done()) { + boost::beast::http::write_header(*m_Stream, m_Serializer); + } + + boost::system::error_code ec; + boost::beast::http::async_write(*m_Stream, m_Serializer, yc[ec]); + if (ec && ec != boost::beast::http::error::need_buffer) { + if (yc.ec_) { + *yc.ec_ = ec; + return; + } + BOOST_THROW_EXCEPTION(boost::system::system_error{ec}); + } + m_Stream->async_flush(yc); + + ASSERT(m_Serializer.is_done() || !body().Finished()); +} + +void HttpResponse::StartStreaming(bool checkForDisconnect) +{ + ASSERT(body().Size() == 0 && !m_SerializationStarted); + body().Start(); + chunked(true); + + if (checkForDisconnect) { + ASSERT(m_Server); + m_Server->StartDetectClientSideShutdown(); + } +} + +bool HttpResponse::IsClientDisconnected() const +{ + ASSERT(m_Server); + return m_Server->Disconnected(); +} + +void HttpResponse::SendFile(const String& path, const boost::asio::yield_context& yc) +{ + std::ifstream fp(path.CStr(), std::ifstream::in | std::ifstream::binary | std::ifstream::ate); + fp.exceptions(std::ifstream::badbit | std::ifstream::eofbit); + + std::uint64_t remaining = fp.tellg(); + fp.seekg(0); + + content_length(remaining); + body().Start(); + + while (remaining) { + auto maxTransfer = std::min(remaining, static_cast(l_FlushThreshold)); + + auto buf = *body().Buffer().prepare(maxTransfer).begin(); + fp.read(static_cast(buf.data()), buf.size()); + body().Buffer().commit(buf.size()); + + remaining -= buf.size(); + Flush(yc); + } +} + +JsonEncoder HttpResponse::GetJsonEncoder(bool pretty) +{ + return JsonEncoder{std::make_shared(*this), pretty}; +} diff --git a/lib/remote/httpmessage.hpp b/lib/remote/httpmessage.hpp new file mode 100644 index 00000000000..10d00fd4989 --- /dev/null +++ b/lib/remote/httpmessage.hpp @@ -0,0 +1,281 @@ +/* Icinga 2 | (c) 2025 Icinga GmbH | GPLv2+ */ + +#pragma once + +#include "base/dictionary.hpp" +#include "base/json.hpp" +#include "base/tlsstream.hpp" +#include "remote/apiuser.hpp" +#include "remote/httpserverconnection.hpp" +#include "remote/url.hpp" +#include +#include + +namespace icinga { + +/** + * A custom body_type for a @c boost::beast::http::message + * + * It combines the memory management of @c boost::beast::http::dynamic_body, + * which uses a multi_buffer, with the ability to continue serialization when + * new data arrives of the @c boost::beast::http::buffer_body. + * + * @tparam DynamicBuffer A buffer conforming to the boost::beast interface of the same name + * + * @ingroup remote + */ +template +struct SerializableBody +{ + class writer; + + class value_type + { + public: + template + value_type& operator<<(T&& right) + { + /* Preferably, we would return an ostream object here instead. However + * there seems to be a bug in boost::beast where if the ostream, or rather its + * streambuf object is moved into the return value, the chunked encoding gets + * mangled, leading to the client disconnecting. + * + * A workaround would have been to construct the boost::beast::detail::ostream_helper + * with the last parameter set to false, indicating that the streambuf object is not + * movable, but that is an implementation detail we'd rather not use directly in our + * code. + * + * This version has a certain overhead of the ostream being constructed on every call + * to the operator, which leads to an individual append for each time, whereas if the + * object could be kept until the entire chain of output operators is finished, only + * a single call to prepare()/commit() would have been needed. + * + * However, since this operator is mostly used for small error messages and the big + * responses are handled via a reader instance, this shouldn't be too much of a + * problem. + */ + boost::beast::ostream(m_Buffer) << std::forward(right); + return *this; + } + + [[nodiscard]] std::size_t Size() const { return m_Buffer.size(); } + + void Finish() { m_More = false; } + bool Finished() { return !m_More; } + void Start() { m_More = true; } + DynamicBuffer& Buffer() { return m_Buffer; } + + friend class writer; + + private: + /* This defaults to false so the body does not require any special handling + * for simple messages and can still be written with http::async_write(). + */ + bool m_More = false; + DynamicBuffer m_Buffer; + }; + + static std::uint64_t size(const value_type& body) { return body.Size(); } + + /** + * Implement the boost::beast BodyWriter interface for this body type + * + * This is used (for example) by the @c boost::beast::http::serializer to write out the + * message over the TLS stream. The logic is similar to the writer of the + * @c boost::beast::http::buffer_body. + * + * On the every call, it will free up the buffer range that has previously been written, + * then return a buffer containing data the has become available in the meantime. Otherwise, + * if there is more data expected in the future, for example because a corresponding reader + * has not yet finished filling the body, a `need_buffer` error is returned, to inform the + * serializer to abort writing for now, which in turn leads to the outer call to + * `http::async_write` to call their completion handlers with a `need_buffer` error, to + * notify that more data is required for another call to `http::async_write`. + */ + class writer + { + public: + using const_buffers_type = typename DynamicBuffer::const_buffers_type; + +#if BOOST_VERSION > 106600 + template + explicit writer(const boost::beast::http::header&, value_type& b) : m_Body(b) + { + } +#else + /** + * This constructor is needed specifically for boost-1.66, which was the first version + * the beast library was introduced and is still used on older (supported) distros. + */ + template + explicit writer(const boost::beast::http::message& msg) + : m_Body(const_cast(msg.body())) + { + } +#endif + void init(boost::beast::error_code& ec) { ec = {}; } + + boost::optional> get(boost::beast::error_code& ec) + { + using namespace boost::beast::http; + + if (m_SizeWritten > 0) { + m_Body.m_Buffer.consume(std::exchange(m_SizeWritten, 0)); + } + + if (m_Body.m_Buffer.size()) { + ec = {}; + m_SizeWritten = m_Body.m_Buffer.size(); + return {{m_Body.m_Buffer.data(), m_Body.m_More}}; + } + + if (m_Body.m_More) { + ec = {make_error_code(error::need_buffer)}; + } else { + ec = {}; + } + return boost::none; + } + + private: + value_type& m_Body; + std::size_t m_SizeWritten = 0; + }; +}; + +/** + * A wrapper class for a boost::beast HTTP request + * + * @ingroup remote + */ +class HttpRequest : public boost::beast::http::request +{ +public: + using ParserType = boost::beast::http::request_parser; + + explicit HttpRequest(Shared::Ptr stream); + + /** + * Parse the header of the response using the internal parser object. + * + * This first performs an @f async_read_header() into the parser, then copies + * the parsed header into this object. + */ + void ParseHeader(boost::beast::flat_buffer& buf, boost::asio::yield_context yc); + + /** + * Parse the body of the response using the internal parser object. + * + * This first performs an async_read() into the parser, then moves the parsed body + * into this object. + * + * @param buf The buffer used to track the state of the connection + * @param yc The yield_context for this operation + */ + void ParseBody(boost::beast::flat_buffer& buf, boost::asio::yield_context yc); + + ParserType& Parser() { return m_Parser; } + + [[nodiscard]] ApiUser::Ptr User() const; + void User(const ApiUser::Ptr& user); + + [[nodiscard]] icinga::Url::Ptr Url() const; + void DecodeUrl(); + + [[nodiscard]] Dictionary::Ptr Params() const; + void DecodeParams(); + +private: + ApiUser::Ptr m_User; + Url::Ptr m_Url; + Dictionary::Ptr m_Params; + + ParserType m_Parser; + + Shared::Ptr m_Stream; +}; + +/** + * A wrapper class for a boost::beast HTTP response + * + * @ingroup remote + */ +class HttpResponse : public boost::beast::http::response> +{ +public: + explicit HttpResponse(Shared::Ptr stream, HttpServerConnection::Ptr server = nullptr); + + /* Delete the base class clear() which is inherited from the fields<> class and doesn't + * clear things like the body or obviously our own members. + */ + void clear() = delete; + + /** + * Clear the header and body of the message. + * + * @note This can only be used when nothing has been written to the stream yet. + */ + void Clear(); + + /** + * Writes as much of the response as is currently available. + * + * Uses chunk-encoding if the content_length has not been set by the time this is called + * for the first time. + * + * The caller needs to ensure that the header is finished before calling this for the + * first time as changes to the header afterwards will not have any effect. + * + * @param yc The yield_context for this operation + */ + void Flush(boost::asio::yield_context yc); + + [[nodiscard]] bool HasSerializationStarted() const { return m_SerializationStarted; } + + /** + * Enables chunked encoding. + * + * Optionally starts a coroutine that reads from the stream and checks for client-side + * disconnects. In this case, the stream can not be reused after the response has been + * sent and any further requests sent over the connections will be discarded, even if + * no client-side disconnect occurs. This requires that this object has been constructed + * with a valid HttpServerConnection::Ptr. + * + * @param checkForDisconnect Whether to start a coroutine to detect disconnects + */ + void StartStreaming(bool checkForDisconnect = false); + + /** + * Check if the server has initiated a disconnect. + * + * @note This requires that the message has been constructed with a pointer to the + * @c HttpServerConnection. + */ + [[nodiscard]] bool IsClientDisconnected() const; + + /** + * Sends the contents of a file. + * + * This does not use chunked encoding because the file size is expected to be fixed. + * The message will be flushed to the stream after a certain amount has been loaded into + * the buffer. + * + * @todo Switch the implementation to @c boost::asio::stream_file when we require >=boost-1.78. + * + * @param path A path to the file + * @param yc The yield context for flushing the message. + */ + void SendFile(const String& path, const boost::asio::yield_context& yc); + + JsonEncoder GetJsonEncoder(bool pretty = false); + +private: + using Serializer = boost::beast::http::response_serializer; + Serializer m_Serializer{*this}; + bool m_SerializationStarted = false; + + HttpServerConnection::Ptr m_Server; + Shared::Ptr m_Stream; +}; + +} // namespace icinga From 3832bb4296626dfd31b31d3b56e0c41a0505d6d3 Mon Sep 17 00:00:00 2001 From: Johannes Schmidt Date: Wed, 23 Jul 2025 09:37:05 +0200 Subject: [PATCH 2/7] Use new HTTP message classes in HttpServerConnection and Handlers --- lib/remote/actionshandler.cpp | 10 +- lib/remote/actionshandler.hpp | 7 +- lib/remote/configfileshandler.cpp | 18 +-- lib/remote/configfileshandler.hpp | 7 +- lib/remote/configpackageshandler.cpp | 53 ++++--- lib/remote/configpackageshandler.hpp | 31 +--- lib/remote/configstageshandler.cpp | 53 ++++--- lib/remote/configstageshandler.hpp | 31 +--- lib/remote/consolehandler.cpp | 29 ++-- lib/remote/consolehandler.hpp | 17 +-- lib/remote/createobjecthandler.cpp | 11 +- lib/remote/createobjecthandler.hpp | 7 +- lib/remote/deleteobjecthandler.cpp | 11 +- lib/remote/deleteobjecthandler.hpp | 7 +- lib/remote/eventshandler.cpp | 13 +- lib/remote/eventshandler.hpp | 7 +- lib/remote/httphandler.cpp | 32 +++-- lib/remote/httphandler.hpp | 14 +- lib/remote/httpserverconnection.cpp | 203 +++++++++++++-------------- lib/remote/httpserverconnection.hpp | 4 +- lib/remote/httputility.cpp | 8 +- lib/remote/httputility.hpp | 8 +- lib/remote/infohandler.cpp | 29 ++-- lib/remote/infohandler.hpp | 7 +- lib/remote/mallocinfohandler.cpp | 14 +- lib/remote/mallocinfohandler.hpp | 7 +- lib/remote/modifyobjecthandler.cpp | 11 +- lib/remote/modifyobjecthandler.hpp | 7 +- lib/remote/objectqueryhandler.cpp | 11 +- lib/remote/objectqueryhandler.hpp | 7 +- lib/remote/statushandler.cpp | 11 +- lib/remote/statushandler.hpp | 7 +- lib/remote/templatequeryhandler.cpp | 11 +- lib/remote/templatequeryhandler.hpp | 7 +- lib/remote/typequeryhandler.cpp | 11 +- lib/remote/typequeryhandler.hpp | 7 +- lib/remote/variablequeryhandler.cpp | 11 +- lib/remote/variablequeryhandler.hpp | 7 +- 38 files changed, 325 insertions(+), 421 deletions(-) diff --git a/lib/remote/actionshandler.cpp b/lib/remote/actionshandler.cpp index 5ae5fdc8062..f0fd713b1e5 100644 --- a/lib/remote/actionshandler.cpp +++ b/lib/remote/actionshandler.cpp @@ -18,16 +18,16 @@ REGISTER_URLHANDLER("/v1/actions", ActionsHandler); bool ActionsHandler::HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); if (url->GetPath().size() != 3) return false; diff --git a/lib/remote/actionshandler.hpp b/lib/remote/actionshandler.hpp index fbf716797ec..83132eeecdf 100644 --- a/lib/remote/actionshandler.hpp +++ b/lib/remote/actionshandler.hpp @@ -18,11 +18,8 @@ class ActionsHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) override; diff --git a/lib/remote/configfileshandler.cpp b/lib/remote/configfileshandler.cpp index 6c390e804b8..9a4da43ffe7 100644 --- a/lib/remote/configfileshandler.cpp +++ b/lib/remote/configfileshandler.cpp @@ -16,17 +16,18 @@ REGISTER_URLHANDLER("/v1/config/files", ConfigFilesHandler); bool ConfigFilesHandler::HandleRequest( const WaitGroup::Ptr&, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + if (request.method() != http::verb::get) return false; @@ -78,14 +79,9 @@ bool ConfigFilesHandler::HandleRequest( } try { - std::ifstream fp(path.CStr(), std::ifstream::in | std::ifstream::binary); - fp.exceptions(std::ifstream::badbit); - - String content((std::istreambuf_iterator(fp)), std::istreambuf_iterator()); response.result(http::status::ok); response.set(http::field::content_type, "application/octet-stream"); - response.body() = content; - response.content_length(response.body().size()); + response.SendFile(path, yc); } catch (const std::exception& ex) { HttpUtility::SendJsonError(response, params, 500, "Could not read file.", DiagnosticInformation(ex)); diff --git a/lib/remote/configfileshandler.hpp b/lib/remote/configfileshandler.hpp index a8826d8c1f5..0bb12488d02 100644 --- a/lib/remote/configfileshandler.hpp +++ b/lib/remote/configfileshandler.hpp @@ -16,11 +16,8 @@ class ConfigFilesHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) override; diff --git a/lib/remote/configpackageshandler.cpp b/lib/remote/configpackageshandler.cpp index f24f5b1d200..0f1009bfda2 100644 --- a/lib/remote/configpackageshandler.cpp +++ b/lib/remote/configpackageshandler.cpp @@ -14,42 +14,41 @@ REGISTER_URLHANDLER("/v1/config/packages", ConfigPackagesHandler); bool ConfigPackagesHandler::HandleRequest( const WaitGroup::Ptr&, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + if (url->GetPath().size() > 4) return false; if (request.method() == http::verb::get) - HandleGet(user, request, url, response, params); + HandleGet(request, response); else if (request.method() == http::verb::post) - HandlePost(user, request, url, response, params); + HandlePost(request, response); else if (request.method() == http::verb::delete_) - HandleDelete(user, request, url, response, params); + HandleDelete(request, response); else return false; return true; } -void ConfigPackagesHandler::HandleGet( - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params -) +void ConfigPackagesHandler::HandleGet(const HttpRequest& request, HttpResponse& response) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + FilterUtility::CheckPermission(user, "config/query"); std::vector packages; @@ -90,16 +89,14 @@ void ConfigPackagesHandler::HandleGet( HttpUtility::SendJsonBody(response, params, result); } -void ConfigPackagesHandler::HandlePost( - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params -) +void ConfigPackagesHandler::HandlePost(const HttpRequest& request, HttpResponse& response) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + FilterUtility::CheckPermission(user, "config/modify"); if (url->GetPath().size() >= 4) @@ -142,16 +139,14 @@ void ConfigPackagesHandler::HandlePost( HttpUtility::SendJsonBody(response, params, result); } -void ConfigPackagesHandler::HandleDelete( - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params -) +void ConfigPackagesHandler::HandleDelete(const HttpRequest& request, HttpResponse& response) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + FilterUtility::CheckPermission(user, "config/modify"); if (url->GetPath().size() >= 4) diff --git a/lib/remote/configpackageshandler.hpp b/lib/remote/configpackageshandler.hpp index 2bae0e26576..95bcfacbc64 100644 --- a/lib/remote/configpackageshandler.hpp +++ b/lib/remote/configpackageshandler.hpp @@ -16,37 +16,16 @@ class ConfigPackagesHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) override; private: - void HandleGet( - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params - ); - void HandlePost( - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params - ); - void HandleDelete( - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params - ); + void HandleGet(const HttpRequest& request, HttpResponse& response); + void HandlePost(const HttpRequest& request, HttpResponse& response); + void HandleDelete(const HttpRequest& request, HttpResponse& response); }; diff --git a/lib/remote/configstageshandler.cpp b/lib/remote/configstageshandler.cpp index 451ba1dbff9..8ee99fbddfb 100644 --- a/lib/remote/configstageshandler.cpp +++ b/lib/remote/configstageshandler.cpp @@ -21,42 +21,41 @@ static std::mutex l_RunningPackageUpdatesMutex; // Protects the above two variab bool ConfigStagesHandler::HandleRequest( const WaitGroup::Ptr&, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + if (url->GetPath().size() > 5) return false; if (request.method() == http::verb::get) - HandleGet(user, request, url, response, params); + HandleGet(request, response); else if (request.method() == http::verb::post) - HandlePost(user, request, url, response, params); + HandlePost(request, response); else if (request.method() == http::verb::delete_) - HandleDelete(user, request, url, response, params); + HandleDelete(request, response); else return false; return true; } -void ConfigStagesHandler::HandleGet( - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params -) +void ConfigStagesHandler::HandleGet(const HttpRequest& request, HttpResponse& response) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + FilterUtility::CheckPermission(user, "config/query"); if (url->GetPath().size() >= 4) @@ -95,16 +94,14 @@ void ConfigStagesHandler::HandleGet( HttpUtility::SendJsonBody(response, params, result); } -void ConfigStagesHandler::HandlePost( - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params -) +void ConfigStagesHandler::HandlePost(const HttpRequest& request, HttpResponse& response) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + FilterUtility::CheckPermission(user, "config/modify"); if (url->GetPath().size() >= 4) @@ -208,16 +205,14 @@ void ConfigStagesHandler::HandlePost( HttpUtility::SendJsonBody(response, params, result); } -void ConfigStagesHandler::HandleDelete( - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params -) +void ConfigStagesHandler::HandleDelete(const HttpRequest& request, HttpResponse& response) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + FilterUtility::CheckPermission(user, "config/modify"); if (url->GetPath().size() >= 4) diff --git a/lib/remote/configstageshandler.hpp b/lib/remote/configstageshandler.hpp index a6abb726d8d..f49c2efb19e 100644 --- a/lib/remote/configstageshandler.hpp +++ b/lib/remote/configstageshandler.hpp @@ -16,37 +16,16 @@ class ConfigStagesHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) override; private: - void HandleGet( - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params - ); - void HandlePost( - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params - ); - void HandleDelete( - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params - ); + void HandleGet(const HttpRequest& request, HttpResponse& response); + void HandlePost(const HttpRequest& request, HttpResponse& response); + void HandleDelete(const HttpRequest& request, HttpResponse& response); }; } diff --git a/lib/remote/consolehandler.cpp b/lib/remote/consolehandler.cpp index c48821aaeff..c063e57814b 100644 --- a/lib/remote/consolehandler.cpp +++ b/lib/remote/consolehandler.cpp @@ -56,17 +56,18 @@ static void EnsureFrameCleanupTimer() bool ConsoleHandler::HandleRequest( const WaitGroup::Ptr&, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + if (url->GetPath().size() != 3) return false; @@ -96,17 +97,16 @@ bool ConsoleHandler::HandleRequest( } if (methodName == "execute-script") - return ExecuteScriptHelper(request, response, params, command, session, sandboxed); + return ExecuteScriptHelper(request, response, command, session, sandboxed); else if (methodName == "auto-complete-script") - return AutocompleteScriptHelper(request, response, params, command, session, sandboxed); + return AutocompleteScriptHelper(request, response, command, session, sandboxed); HttpUtility::SendJsonError(response, params, 400, "Invalid method specified: " + methodName); return true; } -bool ConsoleHandler::ExecuteScriptHelper(boost::beast::http::request& request, - boost::beast::http::response& response, - const Dictionary::Ptr& params, const String& command, const String& session, bool sandboxed) +bool ConsoleHandler::ExecuteScriptHelper(const HttpRequest& request, HttpResponse& response, + const String& command, const String& session, bool sandboxed) { namespace http = boost::beast::http; @@ -174,14 +174,13 @@ bool ConsoleHandler::ExecuteScriptHelper(boost::beast::http::request& request, - boost::beast::http::response& response, - const Dictionary::Ptr& params, const String& command, const String& session, bool sandboxed) +bool ConsoleHandler::AutocompleteScriptHelper(const HttpRequest& request, HttpResponse& response, + const String& command, const String& session, bool sandboxed) { namespace http = boost::beast::http; @@ -213,7 +212,7 @@ bool ConsoleHandler::AutocompleteScriptHelper(boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) override; @@ -37,12 +34,10 @@ class ConsoleHandler final : public HttpHandler static std::vector GetAutocompletionSuggestions(const String& word, ScriptFrame& frame); private: - static bool ExecuteScriptHelper(boost::beast::http::request& request, - boost::beast::http::response& response, - const Dictionary::Ptr& params, const String& command, const String& session, bool sandboxed); - static bool AutocompleteScriptHelper(boost::beast::http::request& request, - boost::beast::http::response& response, - const Dictionary::Ptr& params, const String& command, const String& session, bool sandboxed); + static bool ExecuteScriptHelper(const HttpRequest& request, HttpResponse& response, + const String& command, const String& session, bool sandboxed); + static bool AutocompleteScriptHelper(const HttpRequest& request, HttpResponse& response, + const String& command, const String& session, bool sandboxed); }; diff --git a/lib/remote/createobjecthandler.cpp b/lib/remote/createobjecthandler.cpp index 119be1cd92f..447b74c6d23 100644 --- a/lib/remote/createobjecthandler.cpp +++ b/lib/remote/createobjecthandler.cpp @@ -18,17 +18,18 @@ REGISTER_URLHANDLER("/v1/objects", CreateObjectHandler); bool CreateObjectHandler::HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + if (url->GetPath().size() != 4) return false; diff --git a/lib/remote/createobjecthandler.hpp b/lib/remote/createobjecthandler.hpp index 3f6a705c233..317cf023c4e 100644 --- a/lib/remote/createobjecthandler.hpp +++ b/lib/remote/createobjecthandler.hpp @@ -16,11 +16,8 @@ class CreateObjectHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) override; diff --git a/lib/remote/deleteobjecthandler.cpp b/lib/remote/deleteobjecthandler.cpp index 54d31f13db5..d0f49f83c33 100644 --- a/lib/remote/deleteobjecthandler.cpp +++ b/lib/remote/deleteobjecthandler.cpp @@ -18,17 +18,18 @@ REGISTER_URLHANDLER("/v1/objects", DeleteObjectHandler); bool DeleteObjectHandler::HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + if (url->GetPath().size() < 3 || url->GetPath().size() > 4) return false; diff --git a/lib/remote/deleteobjecthandler.hpp b/lib/remote/deleteobjecthandler.hpp index 0f9643277f4..076f7670499 100644 --- a/lib/remote/deleteobjecthandler.hpp +++ b/lib/remote/deleteobjecthandler.hpp @@ -16,11 +16,8 @@ class DeleteObjectHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) override; diff --git a/lib/remote/eventshandler.cpp b/lib/remote/eventshandler.cpp index 2cbee92f39f..813d5f41e75 100644 --- a/lib/remote/eventshandler.cpp +++ b/lib/remote/eventshandler.cpp @@ -42,11 +42,8 @@ const String l_ApiQuery (""); bool EventsHandler::HandleRequest( const WaitGroup::Ptr&, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) @@ -54,6 +51,10 @@ bool EventsHandler::HandleRequest( namespace asio = boost::asio; namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + if (url->GetPath().size() != 2) return false; @@ -101,7 +102,7 @@ bool EventsHandler::HandleRequest( EventsSubscriber subscriber (std::move(eventTypes), HttpUtility::GetLastParameter(params, "filter"), l_ApiQuery); - server.StartStreaming(); + server.StartDetectClientSideShutdown(); response.result(http::status::ok); response.set(http::field::content_type, "application/json"); diff --git a/lib/remote/eventshandler.hpp b/lib/remote/eventshandler.hpp index 49229733a67..68a1f9844e9 100644 --- a/lib/remote/eventshandler.hpp +++ b/lib/remote/eventshandler.hpp @@ -17,11 +17,8 @@ class EventsHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) override; diff --git a/lib/remote/httphandler.cpp b/lib/remote/httphandler.cpp index 79571d760d9..b6d8d0f4b3d 100644 --- a/lib/remote/httphandler.cpp +++ b/lib/remote/httphandler.cpp @@ -49,9 +49,8 @@ void HttpHandler::Register(const Url::Ptr& url, const HttpHandler::Ptr& handler) void HttpHandler::ProcessRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - boost::beast::http::response& response, + HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) @@ -59,8 +58,8 @@ void HttpHandler::ProcessRequest( Dictionary::Ptr node = m_UrlTree; std::vector handlers; - Url::Ptr url = new Url(std::string(request.target())); - auto& path (url->GetPath()); + request.DecodeUrl(); + auto& path (request.Url()->GetPath()); for (std::vector::size_type i = 0; i <= path.size(); i++) { Array::Ptr current_handlers = node->Get("handlers"); @@ -90,12 +89,10 @@ void HttpHandler::ProcessRequest( std::reverse(handlers.begin(), handlers.end()); - Dictionary::Ptr params; - try { - params = HttpUtility::FetchRequestParameters(url, request.body()); + request.DecodeParams(); } catch (const std::exception& ex) { - HttpUtility::SendJsonError(response, params, 400, "Invalid request body: " + DiagnosticInformation(ex, false)); + HttpUtility::SendJsonError(response, request.Params(), 400, "Invalid request body: " + DiagnosticInformation(ex, false)); return; } @@ -109,12 +106,25 @@ void HttpHandler::ProcessRequest( */ try { for (const HttpHandler::Ptr& handler : handlers) { - if (handler->HandleRequest(waitGroup, stream, user, request, url, response, params, yc, server)) { + if (handler->HandleRequest(waitGroup, stream, request, response, yc, server)) { processed = true; break; } } } catch (const std::exception& ex) { + // Errors related to writing the response should be handled in HttpServerConnection. + if (dynamic_cast(&ex)) { + throw; + } + + /* This means we can't send an error response because the exception was thrown + * in the middle of a streaming response. We can't send any error response, so the + * only thing we can do is propagate it up. + */ + if (response.HasSerializationStarted()) { + throw; + } + Log(LogWarning, "HttpServerConnection") << "Error while processing HTTP request: " << ex.what(); @@ -122,7 +132,7 @@ void HttpHandler::ProcessRequest( } if (!processed) { - HttpUtility::SendJsonError(response, params, 404, "The requested path '" + boost::algorithm::join(path, "/") + + HttpUtility::SendJsonError(response, request.Params(), 404, "The requested path '" + boost::algorithm::join(path, "/") + "' could not be found or the request method is not valid for this path."); return; } diff --git a/lib/remote/httphandler.hpp b/lib/remote/httphandler.hpp index ec67ae8a46f..0d6bd12b834 100644 --- a/lib/remote/httphandler.hpp +++ b/lib/remote/httphandler.hpp @@ -4,8 +4,10 @@ #define HTTPHANDLER_H #include "remote/i2-remote.hpp" +#include "base/io-engine.hpp" #include "remote/url.hpp" #include "remote/httpserverconnection.hpp" +#include "remote/httpmessage.hpp" #include "remote/apiuser.hpp" #include "base/registry.hpp" #include "base/tlsstream.hpp" @@ -29,11 +31,8 @@ class HttpHandler : public Object virtual bool HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) = 0; @@ -42,9 +41,8 @@ class HttpHandler : public Object static void ProcessRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - boost::beast::http::response& response, + HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ); diff --git a/lib/remote/httpserverconnection.cpp b/lib/remote/httpserverconnection.cpp index 17e61f16037..cd4ca367b29 100644 --- a/lib/remote/httpserverconnection.cpp +++ b/lib/remote/httpserverconnection.cpp @@ -41,8 +41,7 @@ HttpServerConnection::HttpServerConnection(const WaitGroup::Ptr& waitGroup, cons } HttpServerConnection::HttpServerConnection(const WaitGroup::Ptr& waitGroup, const String& identity, bool authenticated, const Shared::Ptr& stream, boost::asio::io_context& io) - : m_WaitGroup(waitGroup), m_Stream(stream), m_Seen(Utility::GetTime()), m_IoStrand(io), m_ShuttingDown(false), m_HasStartedStreaming(false), - m_CheckLivenessTimer(io) + : m_WaitGroup(waitGroup), m_Stream(stream), m_Seen(Utility::GetTime()), m_IoStrand(io), m_ShuttingDown(false), m_ConnectionReusable(true), m_CheckLivenessTimer(io) { if (authenticated) { m_ApiUser = ApiUser::GetByClientCN(identity); @@ -99,14 +98,40 @@ void HttpServerConnection::Disconnect(boost::asio::yield_context yc) } } -void HttpServerConnection::StartStreaming() +/** + * Starts a coroutine that continually reads from the stream to detect a disconnect from the client. + * + * This can be accessed inside an @c HttpHandler via the HttpResponse::StartStreaming() method by + * passing true as the argument, expressing that disconnect detection is desired. + */ +void HttpServerConnection::StartDetectClientSideShutdown() { namespace asio = boost::asio; - m_HasStartedStreaming = true; + m_ConnectionReusable = false; HttpServerConnection::Ptr keepAlive (this); + /* Technically it would be possible to detect disconnects on the TCP-side by setting the + * socket to non-blocking and then performing a read directly on the socket with the message_peek + * flag. As the TCP FIN message will put the connection into a CLOSE_WAIT even if the kernel + * buffer is full, this would technically be reliable way of detecting a shutdown and free + * of side-effects. + * + * However, for detecting the close_notify on the SSL/TLS-side, an async_fill() would be necessary + * when the check on the TCP level above returns that there are readable bytes (and no FIN/eof). + * If this async_fill() then buffers more application data and not an immediate eof, we could + * attempt to read another message before disconnecting. + * + * This could either be done at the level of the handlers, via the @c HttpResponse class, or + * generally as a separate coroutine here in @c HttpServerConnection, both (mostly) side-effect + * free and without affecting the state of the connection. + * + * However, due to the complexity of this approach, involving several asio operations, message + * flags, synchronous and asynchronous operations in blocking and non-blocking mode, ioctl cmds, + * etc., it was decided to stick with a simple reading loop, started conditionally on request by + * the handler. + */ IoEngine::SpawnCoroutine(m_IoStrand, [this, keepAlive](asio::yield_context yc) { if (!m_ShuttingDown) { char buf[128]; @@ -129,10 +154,9 @@ bool HttpServerConnection::Disconnected() static inline bool EnsureValidHeaders( - AsioTlsStream& stream, boost::beast::flat_buffer& buf, - boost::beast::http::parser& parser, - boost::beast::http::response& response, + HttpRequest& request, + HttpResponse& response, bool& shuttingDown, boost::asio::yield_context& yc ) @@ -147,7 +171,7 @@ bool EnsureValidHeaders( boost::system::error_code ec; - http::async_read_header(stream, buf, parser, yc[ec]); + request.ParseHeader(buf, yc[ec]); if (ec) { if (ec == boost::asio::error::operation_aborted) @@ -156,7 +180,7 @@ bool EnsureValidHeaders( errorMsg = ec.message(); httpError = true; } else { - switch (parser.get().version()) { + switch (request.version()) { case 10: case 11: break; @@ -168,21 +192,16 @@ bool EnsureValidHeaders( if (!errorMsg.IsEmpty() || httpError) { response.result(http::status::bad_request); - if (!httpError && parser.get()[http::field::accept] == "application/json") { - HttpUtility::SendJsonBody(response, nullptr, new Dictionary({ - { "error", 400 }, - { "status", String("Bad Request: ") + errorMsg } - })); + if (!httpError && request[http::field::accept] == "application/json") { + HttpUtility::SendJsonError(response, nullptr, 400, "Bad Request: " + errorMsg); } else { response.set(http::field::content_type, "text/html"); - response.body() = String("

Bad Request

") + errorMsg + "

"; - response.content_length(response.body().size()); + response.body() << "

Bad Request

" << errorMsg << "

"; } response.set(http::field::connection, "close"); - http::async_write(stream, response, yc); - stream.async_flush(yc); + response.Flush(yc); return false; } @@ -192,28 +211,24 @@ bool EnsureValidHeaders( static inline void HandleExpect100( - AsioTlsStream& stream, - boost::beast::http::request& request, + const Shared::Ptr& stream, + const HttpRequest& request, boost::asio::yield_context& yc ) { namespace http = boost::beast::http; if (request[http::field::expect] == "100-continue") { - http::response response; - + HttpResponse response{stream}; response.result(http::status::continue_); - - http::async_write(stream, response, yc); - stream.async_flush(yc); + response.Flush(yc); } } static inline bool HandleAccessControl( - AsioTlsStream& stream, - boost::beast::http::request& request, - boost::beast::http::response& response, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc ) { @@ -240,12 +255,10 @@ bool HandleAccessControl( response.result(http::status::ok); response.set(http::field::access_control_allow_methods, "GET, POST, PUT, DELETE"); response.set(http::field::access_control_allow_headers, "Authorization, Content-Type, X-HTTP-Method-Override"); - response.body() = "Preflight OK"; - response.content_length(response.body().size()); + response.body() << "Preflight OK"; response.set(http::field::connection, "close"); - http::async_write(stream, response, yc); - stream.async_flush(yc); + response.Flush(yc); return false; } @@ -258,9 +271,8 @@ bool HandleAccessControl( static inline bool EnsureAcceptHeader( - AsioTlsStream& stream, - boost::beast::http::request& request, - boost::beast::http::response& response, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc ) { @@ -269,12 +281,10 @@ bool EnsureAcceptHeader( if (request.method() != http::verb::get && request[http::field::accept] != "application/json") { response.result(http::status::bad_request); response.set(http::field::content_type, "text/html"); - response.body() = "

Accept header is missing or not set to 'application/json'.

"; - response.content_length(response.body().size()); + response.body() << "

Accept header is missing or not set to 'application/json'.

"; response.set(http::field::connection, "close"); - http::async_write(stream, response, yc); - stream.async_flush(yc); + response.Flush(yc); return false; } @@ -284,16 +294,14 @@ bool EnsureAcceptHeader( static inline bool EnsureAuthenticatedUser( - AsioTlsStream& stream, - boost::beast::http::request& request, - ApiUser::Ptr& authenticatedUser, - boost::beast::http::response& response, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc ) { namespace http = boost::beast::http; - if (!authenticatedUser) { + if (!request.User()) { Log(LogWarning, "HttpServerConnection") << "Unauthorized request: " << request.method_string() << ' ' << request.target(); @@ -302,18 +310,13 @@ bool EnsureAuthenticatedUser( response.set(http::field::connection, "close"); if (request[http::field::accept] == "application/json") { - HttpUtility::SendJsonBody(response, nullptr, new Dictionary({ - { "error", 401 }, - { "status", "Unauthorized. Please check your user credentials." } - })); + HttpUtility::SendJsonError(response, nullptr, 401, "Unauthorized. Please check your user credentials."); } else { response.set(http::field::content_type, "text/html"); - response.body() = "

Unauthorized. Please check your user credentials.

"; - response.content_length(response.body().size()); + response.body() << "

Unauthorized. Please check your user credentials.

"; } - http::async_write(stream, response, yc); - stream.async_flush(yc); + response.Flush(yc); return false; } @@ -323,11 +326,9 @@ bool EnsureAuthenticatedUser( static inline bool EnsureValidBody( - AsioTlsStream& stream, boost::beast::flat_buffer& buf, - boost::beast::http::parser& parser, - ApiUser::Ptr& authenticatedUser, - boost::beast::http::response& response, + HttpRequest& request, + HttpResponse& response, bool& shuttingDown, boost::asio::yield_context& yc ) @@ -336,7 +337,7 @@ bool EnsureValidBody( { size_t maxSize = 1024 * 1024; - Array::Ptr permissions = authenticatedUser->GetPermissions(); + Array::Ptr permissions = request.User()->GetPermissions(); if (permissions) { ObjectLock olock(permissions); @@ -366,7 +367,7 @@ bool EnsureValidBody( } } - parser.body_limit(maxSize); + request.Parser().body_limit(maxSize); } if (shuttingDown) @@ -374,7 +375,7 @@ bool EnsureValidBody( boost::system::error_code ec; - http::async_read(stream, buf, parser, yc[ec]); + request.ParseBody(buf, yc[ec]); if (ec) { if (ec == boost::asio::error::operation_aborted) @@ -389,21 +390,16 @@ bool EnsureValidBody( response.result(http::status::bad_request); - if (parser.get()[http::field::accept] == "application/json") { - HttpUtility::SendJsonBody(response, nullptr, new Dictionary({ - { "error", 400 }, - { "status", String("Bad Request: ") + ec.message() } - })); + if (request[http::field::accept] == "application/json") { + HttpUtility::SendJsonError(response, nullptr, 400, "Bad Request: " + ec.message()); } else { response.set(http::field::content_type, "text/html"); - response.body() = String("

Bad Request

") + ec.message() + "

"; - response.content_length(response.body().size()); + response.body() << "

Bad Request

" << ec.message() << "

"; } response.set(http::field::connection, "close"); - http::async_write(stream, response, yc); - stream.async_flush(yc); + response.Flush(yc); return false; } @@ -414,52 +410,45 @@ bool EnsureValidBody( static inline bool ProcessRequest( AsioTlsStream& stream, - boost::beast::http::request& request, - ApiUser::Ptr& authenticatedUser, - boost::beast::http::response& response, + HttpRequest& request, + HttpResponse& response, HttpServerConnection& server, - bool& hasStartedStreaming, + bool& connectionReusable, const WaitGroup::Ptr& waitGroup, std::chrono::steady_clock::duration& cpuBoundWorkTime, boost::asio::yield_context& yc ) { - namespace http = boost::beast::http; - try { // Cache the elapsed time to acquire a CPU semaphore used to detect extremely heavy workloads. auto start (std::chrono::steady_clock::now()); CpuBoundWork handlingRequest (yc); cpuBoundWorkTime = std::chrono::steady_clock::now() - start; - HttpHandler::ProcessRequest(waitGroup, stream, authenticatedUser, request, response, yc, server); + HttpHandler::ProcessRequest(waitGroup, stream, request, response, yc, server); } catch (const std::exception& ex) { - if (hasStartedStreaming) { + if (!connectionReusable) { return false; } - auto sysErr (dynamic_cast(&ex)); - - if (sysErr && sysErr->code() == boost::asio::error::operation_aborted) { + /* Since we don't know the state the stream is in, we can't send an error response and + * have to just cause a disconnect here. + */ + if (response.HasSerializationStarted()) { throw; } - http::response response; - - HttpUtility::SendJsonError(response, nullptr, 500, "Unhandled exception" , DiagnosticInformation(ex)); - - http::async_write(stream, response, yc); - stream.async_flush(yc); - + HttpUtility::SendJsonError(response, request.Params(), 500, "Unhandled exception", DiagnosticInformation(ex)); + response.Flush(yc); return true; } - if (hasStartedStreaming) { + if (!connectionReusable) { return false; } - http::async_write(stream, response, yc); - stream.async_flush(yc); + response.body().Finish(); + response.Flush(yc); return true; } @@ -481,23 +470,21 @@ void HttpServerConnection::ProcessMessages(boost::asio::yield_context yc) while (m_WaitGroup->IsLockable()) { m_Seen = Utility::GetTime(); - http::parser parser; - http::response response; + HttpRequest request(m_Stream); + HttpResponse response(m_Stream, this); - parser.header_limit(1024 * 1024); - parser.body_limit(-1); + request.Parser().header_limit(1024 * 1024); + request.Parser().body_limit(-1); response.set(http::field::server, l_ServerHeader); - if (!EnsureValidHeaders(*m_Stream, buf, parser, response, m_ShuttingDown, yc)) { + if (!EnsureValidHeaders(buf, request, response, m_ShuttingDown, yc)) { break; } m_Seen = Utility::GetTime(); auto start (ch::steady_clock::now()); - auto& request (parser.get()); - { auto method (http::string_to_verb(request["X-Http-Method-Override"])); @@ -506,19 +493,19 @@ void HttpServerConnection::ProcessMessages(boost::asio::yield_context yc) } } - HandleExpect100(*m_Stream, request, yc); - - auto authenticatedUser (m_ApiUser); + HandleExpect100(m_Stream, request, yc); - if (!authenticatedUser) { - authenticatedUser = ApiUser::GetByAuthHeader(std::string(request[http::field::authorization])); + if (m_ApiUser) { + request.User(m_ApiUser); + } else { + request.User(ApiUser::GetByAuthHeader(std::string(request[http::field::authorization]))); } Log logMsg (LogInformation, "HttpServerConnection"); logMsg << "Request " << request.method_string() << ' ' << request.target() << " (from " << m_PeerAddress - << ", user: " << (authenticatedUser ? authenticatedUser->GetName() : "") + << ", user: " << (request.User() ? request.User()->GetName() : "") << ", agent: " << request[http::field::user_agent]; //operator[] - Returns the value for a field, or "" if it does not exist. ch::steady_clock::duration cpuBoundWorkTime(0); @@ -531,29 +518,29 @@ void HttpServerConnection::ProcessMessages(boost::asio::yield_context yc) logMsg << " took total " << ch::duration_cast(ch::steady_clock::now() - start).count() << "ms."; }); - if (!HandleAccessControl(*m_Stream, request, response, yc)) { + if (!HandleAccessControl(request, response, yc)) { break; } - if (!EnsureAcceptHeader(*m_Stream, request, response, yc)) { + if (!EnsureAcceptHeader(request, response, yc)) { break; } - if (!EnsureAuthenticatedUser(*m_Stream, request, authenticatedUser, response, yc)) { + if (!EnsureAuthenticatedUser(request, response, yc)) { break; } - if (!EnsureValidBody(*m_Stream, buf, parser, authenticatedUser, response, m_ShuttingDown, yc)) { + if (!EnsureValidBody(buf, request, response, m_ShuttingDown, yc)) { break; } m_Seen = std::numeric_limits::max(); - if (!ProcessRequest(*m_Stream, request, authenticatedUser, response, *this, m_HasStartedStreaming, m_WaitGroup, cpuBoundWorkTime, yc)) { + if (!ProcessRequest(*m_Stream, request, response, *this, m_ConnectionReusable, m_WaitGroup, cpuBoundWorkTime, yc)) { break; } - if (request.version() != 11 || request[http::field::connection] == "close") { + if (!request.keep_alive() || !m_ConnectionReusable) { break; } } diff --git a/lib/remote/httpserverconnection.hpp b/lib/remote/httpserverconnection.hpp index e4f7d257ee6..1f3d5d7f959 100644 --- a/lib/remote/httpserverconnection.hpp +++ b/lib/remote/httpserverconnection.hpp @@ -30,7 +30,7 @@ class HttpServerConnection final : public Object const Shared::Ptr& stream); void Start(); - void StartStreaming(); + void StartDetectClientSideShutdown(); bool Disconnected(); private: @@ -41,7 +41,7 @@ class HttpServerConnection final : public Object String m_PeerAddress; boost::asio::io_context::strand m_IoStrand; bool m_ShuttingDown; - bool m_HasStartedStreaming; + bool m_ConnectionReusable; boost::asio::deadline_timer m_CheckLivenessTimer; HttpServerConnection(const WaitGroup::Ptr& waitGroup, const String& identity, bool authenticated, diff --git a/lib/remote/httputility.cpp b/lib/remote/httputility.cpp index a2142e5d86f..b53a8721b7a 100644 --- a/lib/remote/httputility.cpp +++ b/lib/remote/httputility.cpp @@ -52,16 +52,15 @@ Value HttpUtility::GetLastParameter(const Dictionary::Ptr& params, const String& return arr->Get(arr->GetLength() - 1); } -void HttpUtility::SendJsonBody(boost::beast::http::response& response, const Dictionary::Ptr& params, const Value& val) +void HttpUtility::SendJsonBody(HttpResponse& response, const Dictionary::Ptr& params, const Value& val) { namespace http = boost::beast::http; response.set(http::field::content_type, "application/json"); - response.body() = JsonEncode(val, params && GetLastParameter(params, "pretty")); - response.content_length(response.body().size()); + response.GetJsonEncoder(params && GetLastParameter(params, "pretty")).Encode(val); } -void HttpUtility::SendJsonError(boost::beast::http::response& response, +void HttpUtility::SendJsonError(HttpResponse& response, const Dictionary::Ptr& params, int code, const String& info, const String& diagnosticInformation) { Dictionary::Ptr result = new Dictionary({ { "error", code } }); @@ -74,6 +73,7 @@ void HttpUtility::SendJsonError(boost::beast::http::responseSet("diagnostic_information", diagnosticInformation); } + response.Clear(); response.result(code); HttpUtility::SendJsonBody(response, params, result); diff --git a/lib/remote/httputility.hpp b/lib/remote/httputility.hpp index 6465b4af925..6f64277136e 100644 --- a/lib/remote/httputility.hpp +++ b/lib/remote/httputility.hpp @@ -5,7 +5,7 @@ #include "remote/url.hpp" #include "base/dictionary.hpp" -#include +#include "remote/httpmessage.hpp" #include namespace icinga @@ -23,9 +23,9 @@ class HttpUtility 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(boost::beast::http::response& response, const Dictionary::Ptr& params, const Value& val); - static void SendJsonError(boost::beast::http::response& response, const Dictionary::Ptr& params, const int code, - const String& verbose = String(), const String& diagnosticInformation = String()); + static void SendJsonBody(HttpResponse& response, const Dictionary::Ptr& params, const Value& val); + static void SendJsonError(HttpResponse& response, const Dictionary::Ptr& params, const int code, + const String& info = {}, const String& diagnosticInformation = {}); }; } diff --git a/lib/remote/infohandler.cpp b/lib/remote/infohandler.cpp index 5fc621cd870..9363f7ca0b9 100644 --- a/lib/remote/infohandler.cpp +++ b/lib/remote/infohandler.cpp @@ -11,17 +11,18 @@ REGISTER_URLHANDLER("/", InfoHandler); bool InfoHandler::HandleRequest( const WaitGroup::Ptr&, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + if (url->GetPath().size() > 2) return false; @@ -77,23 +78,23 @@ bool InfoHandler::HandleRequest( } else { response.set(http::field::content_type, "text/html"); - String body = "Icinga 2

Hello from Icinga 2 (Version: " + Application::GetAppVersion() + ")!

"; - body += "

You are authenticated as " + user->GetName() + ". "; + auto& body = response.body(); + body << "Icinga 2

Hello from Icinga 2 (Version: " + << Application::GetAppVersion() << ")!

" + << "

You are authenticated as " << user->GetName() << ". "; if (!permInfo.empty()) { - body += "Your user has the following permissions:

    "; + body << "Your user has the following permissions:

      "; for (const String& perm : permInfo) { - body += "
    • " + perm + "
    • "; + body << "
    • " << perm << "
    • "; } - body += "
    "; + body << "
"; } else - body += "Your user does not have any permissions.

"; + body << "Your user does not have any permissions.

"; - body += R"(

More information about API requests is available in the documentation.

)"; - response.body() = body; - response.content_length(response.body().size()); + body << R"(

More information about API requests is available in the documentation.

)"; } return true; diff --git a/lib/remote/infohandler.hpp b/lib/remote/infohandler.hpp index 7396f5ac9de..f0f6499a305 100644 --- a/lib/remote/infohandler.hpp +++ b/lib/remote/infohandler.hpp @@ -16,11 +16,8 @@ class InfoHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) override; diff --git a/lib/remote/mallocinfohandler.cpp b/lib/remote/mallocinfohandler.cpp index f4c27cac48b..465b47b86aa 100644 --- a/lib/remote/mallocinfohandler.cpp +++ b/lib/remote/mallocinfohandler.cpp @@ -20,17 +20,18 @@ REGISTER_URLHANDLER("/v1/debug/malloc_info", MallocInfoHandler); bool MallocInfoHandler::HandleRequest( const WaitGroup::Ptr&, AsioTlsStream&, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context&, HttpServerConnection& ) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + if (url->GetPath().size() != 3) { return false; } @@ -87,8 +88,7 @@ bool MallocInfoHandler::HandleRequest( response.result(200); response.set(http::field::content_type, "application/xml"); - response.body() = std::string(buf, bufSize); - response.content_length(response.body().size()); + response.body() << std::string_view(buf, bufSize); #endif /* HAVE_MALLOC_INFO */ return true; diff --git a/lib/remote/mallocinfohandler.hpp b/lib/remote/mallocinfohandler.hpp index 9648fac9f08..fc32341fa18 100644 --- a/lib/remote/mallocinfohandler.hpp +++ b/lib/remote/mallocinfohandler.hpp @@ -15,11 +15,8 @@ class MallocInfoHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) override; diff --git a/lib/remote/modifyobjecthandler.cpp b/lib/remote/modifyobjecthandler.cpp index c71be6a9afd..4b9157af899 100644 --- a/lib/remote/modifyobjecthandler.cpp +++ b/lib/remote/modifyobjecthandler.cpp @@ -16,17 +16,18 @@ REGISTER_URLHANDLER("/v1/objects", ModifyObjectHandler); bool ModifyObjectHandler::HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + if (url->GetPath().size() < 3 || url->GetPath().size() > 4) return false; diff --git a/lib/remote/modifyobjecthandler.hpp b/lib/remote/modifyobjecthandler.hpp index f299acd6e38..32ddf176c39 100644 --- a/lib/remote/modifyobjecthandler.hpp +++ b/lib/remote/modifyobjecthandler.hpp @@ -16,11 +16,8 @@ class ModifyObjectHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) override; diff --git a/lib/remote/objectqueryhandler.cpp b/lib/remote/objectqueryhandler.cpp index f6f049e4e2a..edc2824531f 100644 --- a/lib/remote/objectqueryhandler.cpp +++ b/lib/remote/objectqueryhandler.cpp @@ -91,17 +91,18 @@ Dictionary::Ptr ObjectQueryHandler::SerializeObjectAttrs(const Object::Ptr& obje bool ObjectQueryHandler::HandleRequest( const WaitGroup::Ptr&, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + if (url->GetPath().size() < 3 || url->GetPath().size() > 4) return false; diff --git a/lib/remote/objectqueryhandler.hpp b/lib/remote/objectqueryhandler.hpp index 376eb661e7e..d26a9e1ca73 100644 --- a/lib/remote/objectqueryhandler.hpp +++ b/lib/remote/objectqueryhandler.hpp @@ -16,11 +16,8 @@ class ObjectQueryHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) override; diff --git a/lib/remote/statushandler.cpp b/lib/remote/statushandler.cpp index bf14152f870..9c597dd9802 100644 --- a/lib/remote/statushandler.cpp +++ b/lib/remote/statushandler.cpp @@ -71,17 +71,18 @@ class StatusTargetProvider final : public TargetProvider bool StatusHandler::HandleRequest( const WaitGroup::Ptr&, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + if (url->GetPath().size() > 3) return false; diff --git a/lib/remote/statushandler.hpp b/lib/remote/statushandler.hpp index 109fd488191..1d05347d1a1 100644 --- a/lib/remote/statushandler.hpp +++ b/lib/remote/statushandler.hpp @@ -16,11 +16,8 @@ class StatusHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) override; diff --git a/lib/remote/templatequeryhandler.cpp b/lib/remote/templatequeryhandler.cpp index a68ad6dad57..9dceabb7b62 100644 --- a/lib/remote/templatequeryhandler.cpp +++ b/lib/remote/templatequeryhandler.cpp @@ -78,17 +78,18 @@ class TemplateTargetProvider final : public TargetProvider bool TemplateQueryHandler::HandleRequest( const WaitGroup::Ptr&, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + if (url->GetPath().size() < 3 || url->GetPath().size() > 4) return false; diff --git a/lib/remote/templatequeryhandler.hpp b/lib/remote/templatequeryhandler.hpp index 312cf4221e2..c62670610bc 100644 --- a/lib/remote/templatequeryhandler.hpp +++ b/lib/remote/templatequeryhandler.hpp @@ -16,11 +16,8 @@ class TemplateQueryHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) override; diff --git a/lib/remote/typequeryhandler.cpp b/lib/remote/typequeryhandler.cpp index b2184344d73..ce09293e071 100644 --- a/lib/remote/typequeryhandler.cpp +++ b/lib/remote/typequeryhandler.cpp @@ -49,17 +49,18 @@ class TypeTargetProvider final : public TargetProvider bool TypeQueryHandler::HandleRequest( const WaitGroup::Ptr&, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + if (url->GetPath().size() > 3) return false; diff --git a/lib/remote/typequeryhandler.hpp b/lib/remote/typequeryhandler.hpp index 45cbc38ec80..e0567249c87 100644 --- a/lib/remote/typequeryhandler.hpp +++ b/lib/remote/typequeryhandler.hpp @@ -16,11 +16,8 @@ class TypeQueryHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) override; diff --git a/lib/remote/variablequeryhandler.cpp b/lib/remote/variablequeryhandler.cpp index 40552dd7d2b..b8b62bec17e 100644 --- a/lib/remote/variablequeryhandler.cpp +++ b/lib/remote/variablequeryhandler.cpp @@ -59,17 +59,18 @@ class VariableTargetProvider final : public TargetProvider bool VariableQueryHandler::HandleRequest( const WaitGroup::Ptr&, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) { namespace http = boost::beast::http; + auto url = request.Url(); + auto user = request.User(); + auto params = request.Params(); + if (url->GetPath().size() > 3) return false; diff --git a/lib/remote/variablequeryhandler.hpp b/lib/remote/variablequeryhandler.hpp index d145f5b59b7..3b7a522ae95 100644 --- a/lib/remote/variablequeryhandler.hpp +++ b/lib/remote/variablequeryhandler.hpp @@ -16,11 +16,8 @@ class VariableQueryHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, AsioTlsStream& stream, - const ApiUser::Ptr& user, - boost::beast::http::request& request, - const Url::Ptr& url, - boost::beast::http::response& response, - const Dictionary::Ptr& params, + const HttpRequest& request, + HttpResponse& response, boost::asio::yield_context& yc, HttpServerConnection& server ) override; From d32f04a86356b8690f391755fa84b8fe72ea9334 Mon Sep 17 00:00:00 2001 From: Johannes Schmidt Date: Wed, 23 Jul 2025 09:44:13 +0200 Subject: [PATCH 3/7] Refactor EventsHandler to stream responses via chunked encoding --- lib/remote/eventshandler.cpp | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/lib/remote/eventshandler.cpp b/lib/remote/eventshandler.cpp index 813d5f41e75..ac1fecb748c 100644 --- a/lib/remote/eventshandler.cpp +++ b/lib/remote/eventshandler.cpp @@ -102,33 +102,27 @@ bool EventsHandler::HandleRequest( EventsSubscriber subscriber (std::move(eventTypes), HttpUtility::GetLastParameter(params, "filter"), l_ApiQuery); - server.StartDetectClientSideShutdown(); + IoBoundWorkSlot dontLockTheIoThread (yc); response.result(http::status::ok); response.set(http::field::content_type, "application/json"); + response.StartStreaming(true); + // Send response headers before waiting for the first event. + response.Flush(yc); - IoBoundWorkSlot dontLockTheIoThread (yc); - - http::async_write(stream, response, yc); - stream.async_flush(yc); - - asio::const_buffer newLine ("\n", 1); + auto encoder = response.GetJsonEncoder(); for (;;) { auto event (subscriber.GetInbox()->Shift(yc)); - if (event) { - String body = JsonEncode(event); - - boost::algorithm::replace_all(body, "\n", ""); - - asio::const_buffer payload (body.CStr(), body.GetLength()); - - asio::async_write(stream, payload, yc); - asio::async_write(stream, newLine, yc); - stream.async_flush(yc); - } else if (server.Disconnected()) { + if (response.IsClientDisconnected()) { return true; } + + if (event) { + encoder.Encode(event); + response.body() << '\n'; + response.Flush(yc); + } } } From 62b2dadbac828cd7f3da6c8bbe7f7f95a80e716c Mon Sep 17 00:00:00 2001 From: Johannes Schmidt Date: Wed, 23 Jul 2025 09:44:26 +0200 Subject: [PATCH 4/7] Remove extra parameters from HTTP handler signature These parameters are no longer needed since they were only used by EventsHandler which was refactored in an earlier commit. --- lib/remote/actionshandler.cpp | 4 +--- lib/remote/actionshandler.hpp | 4 +--- lib/remote/configfileshandler.cpp | 4 +--- lib/remote/configfileshandler.hpp | 4 +--- lib/remote/configpackageshandler.cpp | 4 +--- lib/remote/configpackageshandler.hpp | 4 +--- lib/remote/configstageshandler.cpp | 4 +--- lib/remote/configstageshandler.hpp | 4 +--- lib/remote/consolehandler.cpp | 4 +--- lib/remote/consolehandler.hpp | 4 +--- lib/remote/createobjecthandler.cpp | 4 +--- lib/remote/createobjecthandler.hpp | 4 +--- lib/remote/deleteobjecthandler.cpp | 4 +--- lib/remote/deleteobjecthandler.hpp | 4 +--- lib/remote/eventshandler.cpp | 4 +--- lib/remote/eventshandler.hpp | 4 +--- lib/remote/httphandler.cpp | 6 ++---- lib/remote/httphandler.hpp | 8 ++------ lib/remote/httpserverconnection.cpp | 25 ++++--------------------- lib/remote/infohandler.cpp | 4 +--- lib/remote/infohandler.hpp | 4 +--- lib/remote/mallocinfohandler.cpp | 4 +--- lib/remote/mallocinfohandler.hpp | 4 +--- lib/remote/modifyobjecthandler.cpp | 4 +--- lib/remote/modifyobjecthandler.hpp | 4 +--- lib/remote/objectqueryhandler.cpp | 4 +--- lib/remote/objectqueryhandler.hpp | 4 +--- lib/remote/statushandler.cpp | 4 +--- lib/remote/statushandler.hpp | 4 +--- lib/remote/templatequeryhandler.cpp | 4 +--- lib/remote/templatequeryhandler.hpp | 4 +--- lib/remote/typequeryhandler.cpp | 4 +--- lib/remote/typequeryhandler.hpp | 4 +--- lib/remote/variablequeryhandler.cpp | 4 +--- lib/remote/variablequeryhandler.hpp | 4 +--- 35 files changed, 40 insertions(+), 127 deletions(-) diff --git a/lib/remote/actionshandler.cpp b/lib/remote/actionshandler.cpp index f0fd713b1e5..b9853945be2 100644 --- a/lib/remote/actionshandler.cpp +++ b/lib/remote/actionshandler.cpp @@ -17,11 +17,9 @@ REGISTER_URLHANDLER("/v1/actions", ActionsHandler); bool ActionsHandler::HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; diff --git a/lib/remote/actionshandler.hpp b/lib/remote/actionshandler.hpp index 83132eeecdf..3ba856f6979 100644 --- a/lib/remote/actionshandler.hpp +++ b/lib/remote/actionshandler.hpp @@ -17,11 +17,9 @@ class ActionsHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) override; }; diff --git a/lib/remote/configfileshandler.cpp b/lib/remote/configfileshandler.cpp index 9a4da43ffe7..2bd54038647 100644 --- a/lib/remote/configfileshandler.cpp +++ b/lib/remote/configfileshandler.cpp @@ -15,11 +15,9 @@ REGISTER_URLHANDLER("/v1/config/files", ConfigFilesHandler); bool ConfigFilesHandler::HandleRequest( const WaitGroup::Ptr&, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; diff --git a/lib/remote/configfileshandler.hpp b/lib/remote/configfileshandler.hpp index 0bb12488d02..3294811c0b0 100644 --- a/lib/remote/configfileshandler.hpp +++ b/lib/remote/configfileshandler.hpp @@ -15,11 +15,9 @@ class ConfigFilesHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) override; }; diff --git a/lib/remote/configpackageshandler.cpp b/lib/remote/configpackageshandler.cpp index 0f1009bfda2..7e0c7b02c5a 100644 --- a/lib/remote/configpackageshandler.cpp +++ b/lib/remote/configpackageshandler.cpp @@ -13,11 +13,9 @@ REGISTER_URLHANDLER("/v1/config/packages", ConfigPackagesHandler); bool ConfigPackagesHandler::HandleRequest( const WaitGroup::Ptr&, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; diff --git a/lib/remote/configpackageshandler.hpp b/lib/remote/configpackageshandler.hpp index 95bcfacbc64..172690f6399 100644 --- a/lib/remote/configpackageshandler.hpp +++ b/lib/remote/configpackageshandler.hpp @@ -15,11 +15,9 @@ class ConfigPackagesHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) override; private: diff --git a/lib/remote/configstageshandler.cpp b/lib/remote/configstageshandler.cpp index 8ee99fbddfb..b08270e56ea 100644 --- a/lib/remote/configstageshandler.cpp +++ b/lib/remote/configstageshandler.cpp @@ -20,11 +20,9 @@ static std::mutex l_RunningPackageUpdatesMutex; // Protects the above two variab bool ConfigStagesHandler::HandleRequest( const WaitGroup::Ptr&, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; diff --git a/lib/remote/configstageshandler.hpp b/lib/remote/configstageshandler.hpp index f49c2efb19e..ec333cc50c8 100644 --- a/lib/remote/configstageshandler.hpp +++ b/lib/remote/configstageshandler.hpp @@ -15,11 +15,9 @@ class ConfigStagesHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) override; private: diff --git a/lib/remote/consolehandler.cpp b/lib/remote/consolehandler.cpp index c063e57814b..e17d7e3c14d 100644 --- a/lib/remote/consolehandler.cpp +++ b/lib/remote/consolehandler.cpp @@ -55,11 +55,9 @@ static void EnsureFrameCleanupTimer() bool ConsoleHandler::HandleRequest( const WaitGroup::Ptr&, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; diff --git a/lib/remote/consolehandler.hpp b/lib/remote/consolehandler.hpp index c2e302ed3a8..30fb98f2eeb 100644 --- a/lib/remote/consolehandler.hpp +++ b/lib/remote/consolehandler.hpp @@ -24,11 +24,9 @@ class ConsoleHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) override; static std::vector GetAutocompletionSuggestions(const String& word, ScriptFrame& frame); diff --git a/lib/remote/createobjecthandler.cpp b/lib/remote/createobjecthandler.cpp index 447b74c6d23..beff9c98707 100644 --- a/lib/remote/createobjecthandler.cpp +++ b/lib/remote/createobjecthandler.cpp @@ -17,11 +17,9 @@ REGISTER_URLHANDLER("/v1/objects", CreateObjectHandler); bool CreateObjectHandler::HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; diff --git a/lib/remote/createobjecthandler.hpp b/lib/remote/createobjecthandler.hpp index 317cf023c4e..972d7b3bdc9 100644 --- a/lib/remote/createobjecthandler.hpp +++ b/lib/remote/createobjecthandler.hpp @@ -15,11 +15,9 @@ class CreateObjectHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) override; }; diff --git a/lib/remote/deleteobjecthandler.cpp b/lib/remote/deleteobjecthandler.cpp index d0f49f83c33..cd99f7b282b 100644 --- a/lib/remote/deleteobjecthandler.cpp +++ b/lib/remote/deleteobjecthandler.cpp @@ -17,11 +17,9 @@ REGISTER_URLHANDLER("/v1/objects", DeleteObjectHandler); bool DeleteObjectHandler::HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; diff --git a/lib/remote/deleteobjecthandler.hpp b/lib/remote/deleteobjecthandler.hpp index 076f7670499..f969facda01 100644 --- a/lib/remote/deleteobjecthandler.hpp +++ b/lib/remote/deleteobjecthandler.hpp @@ -15,11 +15,9 @@ class DeleteObjectHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) override; }; diff --git a/lib/remote/eventshandler.cpp b/lib/remote/eventshandler.cpp index ac1fecb748c..1b7798c04ac 100644 --- a/lib/remote/eventshandler.cpp +++ b/lib/remote/eventshandler.cpp @@ -41,11 +41,9 @@ const String l_ApiQuery (""); bool EventsHandler::HandleRequest( const WaitGroup::Ptr&, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) { namespace asio = boost::asio; diff --git a/lib/remote/eventshandler.hpp b/lib/remote/eventshandler.hpp index 68a1f9844e9..91d5ffe3f0c 100644 --- a/lib/remote/eventshandler.hpp +++ b/lib/remote/eventshandler.hpp @@ -16,11 +16,9 @@ class EventsHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) override; }; diff --git a/lib/remote/httphandler.cpp b/lib/remote/httphandler.cpp index b6d8d0f4b3d..db27da31a77 100644 --- a/lib/remote/httphandler.cpp +++ b/lib/remote/httphandler.cpp @@ -48,11 +48,9 @@ void HttpHandler::Register(const Url::Ptr& url, const HttpHandler::Ptr& handler) void HttpHandler::ProcessRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) { Dictionary::Ptr node = m_UrlTree; @@ -106,7 +104,7 @@ void HttpHandler::ProcessRequest( */ try { for (const HttpHandler::Ptr& handler : handlers) { - if (handler->HandleRequest(waitGroup, stream, request, response, yc, server)) { + if (handler->HandleRequest(waitGroup, request, response, yc)) { processed = true; break; } diff --git a/lib/remote/httphandler.hpp b/lib/remote/httphandler.hpp index 0d6bd12b834..77f7d433717 100644 --- a/lib/remote/httphandler.hpp +++ b/lib/remote/httphandler.hpp @@ -30,21 +30,17 @@ class HttpHandler : public Object virtual bool HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) = 0; static void Register(const Url::Ptr& url, const HttpHandler::Ptr& handler); static void ProcessRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ); private: diff --git a/lib/remote/httpserverconnection.cpp b/lib/remote/httpserverconnection.cpp index cd4ca367b29..d8befd21144 100644 --- a/lib/remote/httpserverconnection.cpp +++ b/lib/remote/httpserverconnection.cpp @@ -408,12 +408,9 @@ bool EnsureValidBody( } static inline -bool ProcessRequest( - AsioTlsStream& stream, +void ProcessRequest( HttpRequest& request, HttpResponse& response, - HttpServerConnection& server, - bool& connectionReusable, const WaitGroup::Ptr& waitGroup, std::chrono::steady_clock::duration& cpuBoundWorkTime, boost::asio::yield_context& yc @@ -425,12 +422,9 @@ bool ProcessRequest( CpuBoundWork handlingRequest (yc); cpuBoundWorkTime = std::chrono::steady_clock::now() - start; - HttpHandler::ProcessRequest(waitGroup, stream, request, response, yc, server); + HttpHandler::ProcessRequest(waitGroup, request, response, yc); + response.body().Finish(); } catch (const std::exception& ex) { - if (!connectionReusable) { - return false; - } - /* Since we don't know the state the stream is in, we can't send an error response and * have to just cause a disconnect here. */ @@ -439,18 +433,9 @@ bool ProcessRequest( } HttpUtility::SendJsonError(response, request.Params(), 500, "Unhandled exception", DiagnosticInformation(ex)); - response.Flush(yc); - return true; } - if (!connectionReusable) { - return false; - } - - response.body().Finish(); response.Flush(yc); - - return true; } void HttpServerConnection::ProcessMessages(boost::asio::yield_context yc) @@ -536,9 +521,7 @@ void HttpServerConnection::ProcessMessages(boost::asio::yield_context yc) m_Seen = std::numeric_limits::max(); - if (!ProcessRequest(*m_Stream, request, response, *this, m_ConnectionReusable, m_WaitGroup, cpuBoundWorkTime, yc)) { - break; - } + ProcessRequest(request, response, m_WaitGroup, cpuBoundWorkTime, yc); if (!request.keep_alive() || !m_ConnectionReusable) { break; diff --git a/lib/remote/infohandler.cpp b/lib/remote/infohandler.cpp index 9363f7ca0b9..52d7c4b2669 100644 --- a/lib/remote/infohandler.cpp +++ b/lib/remote/infohandler.cpp @@ -10,11 +10,9 @@ REGISTER_URLHANDLER("/", InfoHandler); bool InfoHandler::HandleRequest( const WaitGroup::Ptr&, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; diff --git a/lib/remote/infohandler.hpp b/lib/remote/infohandler.hpp index f0f6499a305..e62a497ff86 100644 --- a/lib/remote/infohandler.hpp +++ b/lib/remote/infohandler.hpp @@ -15,11 +15,9 @@ class InfoHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) override; }; diff --git a/lib/remote/mallocinfohandler.cpp b/lib/remote/mallocinfohandler.cpp index 465b47b86aa..4ca37d555b0 100644 --- a/lib/remote/mallocinfohandler.cpp +++ b/lib/remote/mallocinfohandler.cpp @@ -19,11 +19,9 @@ REGISTER_URLHANDLER("/v1/debug/malloc_info", MallocInfoHandler); bool MallocInfoHandler::HandleRequest( const WaitGroup::Ptr&, - AsioTlsStream&, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context&, - HttpServerConnection& + boost::asio::yield_context& ) { namespace http = boost::beast::http; diff --git a/lib/remote/mallocinfohandler.hpp b/lib/remote/mallocinfohandler.hpp index fc32341fa18..10d8b162f42 100644 --- a/lib/remote/mallocinfohandler.hpp +++ b/lib/remote/mallocinfohandler.hpp @@ -14,11 +14,9 @@ class MallocInfoHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) override; }; diff --git a/lib/remote/modifyobjecthandler.cpp b/lib/remote/modifyobjecthandler.cpp index 4b9157af899..9264e3c64d4 100644 --- a/lib/remote/modifyobjecthandler.cpp +++ b/lib/remote/modifyobjecthandler.cpp @@ -15,11 +15,9 @@ REGISTER_URLHANDLER("/v1/objects", ModifyObjectHandler); bool ModifyObjectHandler::HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; diff --git a/lib/remote/modifyobjecthandler.hpp b/lib/remote/modifyobjecthandler.hpp index 32ddf176c39..abc7f973537 100644 --- a/lib/remote/modifyobjecthandler.hpp +++ b/lib/remote/modifyobjecthandler.hpp @@ -15,11 +15,9 @@ class ModifyObjectHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) override; }; diff --git a/lib/remote/objectqueryhandler.cpp b/lib/remote/objectqueryhandler.cpp index edc2824531f..c910a653d8b 100644 --- a/lib/remote/objectqueryhandler.cpp +++ b/lib/remote/objectqueryhandler.cpp @@ -90,11 +90,9 @@ Dictionary::Ptr ObjectQueryHandler::SerializeObjectAttrs(const Object::Ptr& obje bool ObjectQueryHandler::HandleRequest( const WaitGroup::Ptr&, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; diff --git a/lib/remote/objectqueryhandler.hpp b/lib/remote/objectqueryhandler.hpp index d26a9e1ca73..1c7d25afd10 100644 --- a/lib/remote/objectqueryhandler.hpp +++ b/lib/remote/objectqueryhandler.hpp @@ -15,11 +15,9 @@ class ObjectQueryHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) override; private: diff --git a/lib/remote/statushandler.cpp b/lib/remote/statushandler.cpp index 9c597dd9802..8a16ad81e49 100644 --- a/lib/remote/statushandler.cpp +++ b/lib/remote/statushandler.cpp @@ -70,11 +70,9 @@ class StatusTargetProvider final : public TargetProvider bool StatusHandler::HandleRequest( const WaitGroup::Ptr&, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; diff --git a/lib/remote/statushandler.hpp b/lib/remote/statushandler.hpp index 1d05347d1a1..dceb58ac2a9 100644 --- a/lib/remote/statushandler.hpp +++ b/lib/remote/statushandler.hpp @@ -15,11 +15,9 @@ class StatusHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) override; }; diff --git a/lib/remote/templatequeryhandler.cpp b/lib/remote/templatequeryhandler.cpp index 9dceabb7b62..81261f02d0e 100644 --- a/lib/remote/templatequeryhandler.cpp +++ b/lib/remote/templatequeryhandler.cpp @@ -77,11 +77,9 @@ class TemplateTargetProvider final : public TargetProvider bool TemplateQueryHandler::HandleRequest( const WaitGroup::Ptr&, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; diff --git a/lib/remote/templatequeryhandler.hpp b/lib/remote/templatequeryhandler.hpp index c62670610bc..3b3b58cc41c 100644 --- a/lib/remote/templatequeryhandler.hpp +++ b/lib/remote/templatequeryhandler.hpp @@ -15,11 +15,9 @@ class TemplateQueryHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) override; }; diff --git a/lib/remote/typequeryhandler.cpp b/lib/remote/typequeryhandler.cpp index ce09293e071..dda19cd120b 100644 --- a/lib/remote/typequeryhandler.cpp +++ b/lib/remote/typequeryhandler.cpp @@ -48,11 +48,9 @@ class TypeTargetProvider final : public TargetProvider bool TypeQueryHandler::HandleRequest( const WaitGroup::Ptr&, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; diff --git a/lib/remote/typequeryhandler.hpp b/lib/remote/typequeryhandler.hpp index e0567249c87..f065d2471a1 100644 --- a/lib/remote/typequeryhandler.hpp +++ b/lib/remote/typequeryhandler.hpp @@ -15,11 +15,9 @@ class TypeQueryHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) override; }; diff --git a/lib/remote/variablequeryhandler.cpp b/lib/remote/variablequeryhandler.cpp index b8b62bec17e..e96f6abf817 100644 --- a/lib/remote/variablequeryhandler.cpp +++ b/lib/remote/variablequeryhandler.cpp @@ -58,11 +58,9 @@ class VariableTargetProvider final : public TargetProvider bool VariableQueryHandler::HandleRequest( const WaitGroup::Ptr&, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) { namespace http = boost::beast::http; diff --git a/lib/remote/variablequeryhandler.hpp b/lib/remote/variablequeryhandler.hpp index 3b7a522ae95..b6706037e11 100644 --- a/lib/remote/variablequeryhandler.hpp +++ b/lib/remote/variablequeryhandler.hpp @@ -15,11 +15,9 @@ class VariableQueryHandler final : public HttpHandler bool HandleRequest( const WaitGroup::Ptr& waitGroup, - AsioTlsStream& stream, const HttpRequest& request, HttpResponse& response, - boost::asio::yield_context& yc, - HttpServerConnection& server + boost::asio::yield_context& yc ) override; }; From bb75d7301282e7c034abbef78cdacc87234915a7 Mon Sep 17 00:00:00 2001 From: Johannes Schmidt Date: Fri, 27 Jun 2025 12:46:15 +0200 Subject: [PATCH 5/7] Refactor ObjectQueryHandler to use new JSON stream encoder --- lib/remote/objectqueryhandler.cpp | 120 +++++++++++++++++------------- 1 file changed, 68 insertions(+), 52 deletions(-) diff --git a/lib/remote/objectqueryhandler.cpp b/lib/remote/objectqueryhandler.cpp index c910a653d8b..4384abb5576 100644 --- a/lib/remote/objectqueryhandler.cpp +++ b/lib/remote/objectqueryhandler.cpp @@ -1,6 +1,8 @@ /* Icinga 2 | (c) 2012 Icinga GmbH | GPLv2+ */ #include "remote/objectqueryhandler.hpp" +#include "base/generator.hpp" +#include "base/json.hpp" #include "remote/httputility.hpp" #include "remote/filterutility.hpp" #include "base/serializer.hpp" @@ -9,6 +11,7 @@ #include #include #include +#include using namespace icinga; @@ -144,6 +147,22 @@ bool ObjectQueryHandler::HandleRequest( return true; } + bool includeUsedBy = false; + bool includeLocation = false; + if (umetas) { + ObjectLock olock(umetas); + for (String meta : umetas) { + if (meta == "used_by") { + includeUsedBy = true; + } else if (meta == "location") { + includeLocation = true; + } else { + HttpUtility::SendJsonError(response, params, 400, "Invalid field specified for meta: " + meta); + return true; + } + } + } + bool allJoins = HttpUtility::GetLastParameter(params, "all_joins"); params->Set("type", type->GetName()); @@ -165,10 +184,7 @@ bool ObjectQueryHandler::HandleRequest( return true; } - ArrayData results; - results.reserve(objs.size()); - - std::set joinAttrs; + std::set joinAttrs; std::set userJoinAttrs; if (ujoins) { @@ -187,70 +203,63 @@ bool ObjectQueryHandler::HandleRequest( if (!allJoins && userJoinAttrs.find(field.NavigationName) == userJoinAttrs.end()) continue; - joinAttrs.insert(field.Name); + joinAttrs.insert(fid); } std::unordered_map>> typePermissions; std::unordered_map objectAccessAllowed; - for (ConfigObject::Ptr obj : objs) { + auto it = objs.begin(); + auto generatorFunc = [&]() -> std::optional { + if (it == objs.end()) { + return std::nullopt; + } + + ConfigObject::Ptr obj = *it; + ++it; + DictionaryData result1{ { "name", obj->GetName() }, { "type", obj->GetReflectionType()->GetName() } }; DictionaryData metaAttrs; - - if (umetas) { - ObjectLock olock(umetas); - for (String meta : umetas) { - if (meta == "used_by") { - Array::Ptr used_by = new Array(); - metaAttrs.emplace_back("used_by", used_by); - - for (auto& configObj : DependencyGraph::GetChildren(obj)) { - used_by->Add(new Dictionary({ - { "type", configObj->GetReflectionType()->GetName() }, - { "name", configObj->GetName() } - })); - } - } else if (meta == "location") { - metaAttrs.emplace_back("location", obj->GetSourceLocation()); - } else { - HttpUtility::SendJsonError(response, params, 400, "Invalid field specified for meta: " + meta); - return true; - } + if (includeUsedBy) { + Array::Ptr used_by = new Array(); + metaAttrs.emplace_back("used_by", used_by); + + for (auto& configObj : DependencyGraph::GetChildren(obj)) { + used_by->Add(new Dictionary({ + {"type", configObj->GetReflectionType()->GetName()}, + {"name", configObj->GetName()} + })); } } + if (includeLocation) { + metaAttrs.emplace_back("location", obj->GetSourceLocation()); + } + result1.emplace_back("meta", new Dictionary(std::move(metaAttrs))); try { result1.emplace_back("attrs", SerializeObjectAttrs(obj, String(), uattrs, false, false)); } catch (const ScriptError& ex) { - HttpUtility::SendJsonError(response, params, 400, ex.what()); - return true; + return new Dictionary{ + {"type", type->GetName()}, + {"name", obj->GetName()}, + {"code", 400}, + {"status", ex.what()} + }; } DictionaryData joins; - for (const String& joinAttr : joinAttrs) { + for (auto joinAttr : joinAttrs) { Object::Ptr joinedObj; - int fid = type->GetFieldId(joinAttr); - - if (fid < 0) { - HttpUtility::SendJsonError(response, params, 400, "Invalid field specified for join: " + joinAttr); - return true; - } + Field field = type->GetFieldInfo(joinAttr); - Field field = type->GetFieldInfo(fid); - - if (!(field.Attributes & FANavigation)) { - HttpUtility::SendJsonError(response, params, 400, "Not a joinable field: " + joinAttr); - return true; - } - - joinedObj = obj->NavigateField(fid); + joinedObj = obj->NavigateField(joinAttr); if (!joinedObj) continue; @@ -303,22 +312,29 @@ bool ObjectQueryHandler::HandleRequest( try { joins.emplace_back(prefix, SerializeObjectAttrs(joinedObj, prefix, ujoins, true, allJoins)); } catch (const ScriptError& ex) { - HttpUtility::SendJsonError(response, params, 400, ex.what()); - return true; + return new Dictionary{ + {"type", type->GetName()}, + {"name", obj->GetName()}, + {"code", 400}, + {"status", ex.what()} + }; } } result1.emplace_back("joins", new Dictionary(std::move(joins))); - results.push_back(new Dictionary(std::move(result1))); - } - - Dictionary::Ptr result = new Dictionary({ - { "results", new Array(std::move(results)) } - }); + return new Dictionary{std::move(result1)}; + }; response.result(http::status::ok); - HttpUtility::SendJsonBody(response, params, result); + response.set(http::field::content_type, "application/json"); + response.StartStreaming(); + + Dictionary::Ptr results = new Dictionary{{"results", new ValueGenerator{generatorFunc}}}; + results->Freeze(); + + bool pretty = HttpUtility::GetLastParameter(params, "pretty"); + response.GetJsonEncoder(pretty).Encode(results, &yc); return true; } From 4782ea8a75482a0c2d1521e96a4f3149e2bb79fc Mon Sep 17 00:00:00 2001 From: Johannes Schmidt Date: Tue, 15 Jul 2025 11:30:11 +0200 Subject: [PATCH 6/7] Make inherited protected functions of ApiListener public This is needed so it's possible to manually add an ApiListener object for the purpose of unit-testing. --- lib/remote/apilistener.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/remote/apilistener.hpp b/lib/remote/apilistener.hpp index f278c2e9b58..0b0e29f3318 100644 --- a/lib/remote/apilistener.hpp +++ b/lib/remote/apilistener.hpp @@ -161,12 +161,12 @@ class ApiListener final : public ObjectImpl return m_WaitGroup; } -protected: void OnConfigLoaded() override; void OnAllConfigLoaded() override; void Start(bool runtimeCreated) override; void Stop(bool runtimeDeleted) override; +protected: void ValidateTlsProtocolmin(const Lazy& lvalue, const ValidationUtils& utils) override; void ValidateTlsHandshakeTimeout(const Lazy& lvalue, const ValidationUtils& utils) override; From 7373f36cc5651017acff9bb0f13c8cb4b494b885 Mon Sep 17 00:00:00 2001 From: Johannes Schmidt Date: Tue, 22 Jul 2025 13:59:58 +0200 Subject: [PATCH 7/7] Add unit-tests for HttpServerConnection and HTTP message classes --- test/CMakeLists.txt | 70 ++++ test/base-configuration-fixture.hpp | 56 +++ test/base-testloggerfixture.hpp | 127 ++++++ test/base-tlsstream-fixture.hpp | 114 ++++++ test/remote-certificate-fixture.cpp | 42 ++ test/remote-certificate-fixture.hpp | 69 ++++ test/remote-httpmessage.cpp | 351 +++++++++++++++++ test/remote-httpserverconnection.cpp | 558 +++++++++++++++++++++++++++ 8 files changed, 1387 insertions(+) create mode 100644 test/base-configuration-fixture.hpp create mode 100644 test/base-testloggerfixture.hpp create mode 100644 test/base-tlsstream-fixture.hpp create mode 100644 test/remote-certificate-fixture.cpp create mode 100644 test/remote-certificate-fixture.hpp create mode 100644 test/remote-httpmessage.cpp create mode 100644 test/remote-httpserverconnection.cpp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index c87679b0849..5498a6d83ed 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -87,7 +87,10 @@ set(base_test_SOURCES icinga-notification.cpp icinga-perfdata.cpp methods-pluginnotificationtask.cpp + remote-certificate-fixture.cpp remote-configpackageutility.cpp + remote-httpserverconnection.cpp + remote-httpmessage.cpp remote-url.cpp ${base_OBJS} $ @@ -271,6 +274,33 @@ add_boost_test(base icinga_perfdata/parse_edgecases icinga_perfdata/empty_warn_crit_min_max methods_pluginnotificationtask/truncate_long_output + remote_certs_fixture/prepare_directory + remote_certs_fixture/cleanup_certs + remote_httpmessage/request_parse + remote_httpmessage/request_params + remote_httpmessage/response_clear + remote_httpmessage/response_flush_nothrow + remote_httpmessage/response_flush_throw + remote_httpmessage/response_write_empty + remote_httpmessage/response_write_fixed + remote_httpmessage/response_write_chunked + remote_httpmessage/response_sendjsonbody + remote_httpmessage/response_sendjsonerror + remote_httpmessage/response_sendfile + remote_httpserverconnection/expect_100_continue + remote_httpserverconnection/bad_request + remote_httpserverconnection/error_access_control + remote_httpserverconnection/error_accept_header + remote_httpserverconnection/authenticate_cn + remote_httpserverconnection/authenticate_passwd + remote_httpserverconnection/authenticate_error_wronguser + remote_httpserverconnection/authenticate_error_wrongpasswd + remote_httpserverconnection/reuse_connection + remote_httpserverconnection/wg_abort + remote_httpserverconnection/client_shutdown + remote_httpserverconnection/handler_throw_error + remote_httpserverconnection/handler_throw_streaming + remote_httpserverconnection/liveness_disconnect remote_configpackageutility/ValidateName remote_url/id_and_path remote_url/parameters @@ -279,6 +309,46 @@ add_boost_test(base remote_url/illegal_legal_strings ) +if(BUILD_TESTING) + set_tests_properties( + base-remote_httpmessage/request_parse + base-remote_httpmessage/request_params + base-remote_httpmessage/response_clear + base-remote_httpmessage/response_flush_nothrow + base-remote_httpmessage/response_flush_throw + base-remote_httpmessage/response_write_empty + base-remote_httpmessage/response_write_fixed + base-remote_httpmessage/response_write_chunked + base-remote_httpmessage/response_sendjsonbody + base-remote_httpmessage/response_sendjsonerror + base-remote_httpmessage/response_sendfile + base-remote_httpserverconnection/expect_100_continue + base-remote_httpserverconnection/bad_request + base-remote_httpserverconnection/error_access_control + base-remote_httpserverconnection/error_accept_header + base-remote_httpserverconnection/authenticate_cn + base-remote_httpserverconnection/authenticate_passwd + base-remote_httpserverconnection/authenticate_error_wronguser + base-remote_httpserverconnection/authenticate_error_wrongpasswd + base-remote_httpserverconnection/reuse_connection + base-remote_httpserverconnection/wg_abort + base-remote_httpserverconnection/client_shutdown + base-remote_httpserverconnection/handler_throw_error + base-remote_httpserverconnection/handler_throw_streaming + base-remote_httpserverconnection/liveness_disconnect + PROPERTIES FIXTURES_REQUIRED ssl_certs) + + set_tests_properties( + base-remote_certs_fixture/prepare_directory + PROPERTIES FIXTURES_SETUP ssl_certs + ) + + set_tests_properties( + base-remote_certs_fixture/cleanup_certs + PROPERTIES FIXTURES_CLEANUP ssl_certs + ) +endif() + if(ICINGA2_WITH_LIVESTATUS) set(livestatus_test_SOURCES icingaapplication-fixture.cpp diff --git a/test/base-configuration-fixture.hpp b/test/base-configuration-fixture.hpp new file mode 100644 index 00000000000..2639eb3ff1f --- /dev/null +++ b/test/base-configuration-fixture.hpp @@ -0,0 +1,56 @@ +/* Icinga 2 | (c) 2025 Icinga GmbH | GPLv2+ */ + +#ifndef CONFIGURATION_FIXTURE_H +#define CONFIGURATION_FIXTURE_H + +#include "base/configuration.hpp" +#include +#include + +namespace icinga { + +struct ConfigurationDataDirFixture +{ + ConfigurationDataDirFixture() + : m_DataDir(boost::filesystem::current_path() / "data"), m_PrevDataDir(Configuration::DataDir.GetData()) + { + boost::filesystem::create_directories(m_DataDir); + Configuration::DataDir = m_DataDir.string(); + } + + ~ConfigurationDataDirFixture() + { + boost::filesystem::remove_all(m_DataDir); + Configuration::DataDir = m_PrevDataDir.string(); + } + + boost::filesystem::path m_DataDir; + +private: + boost::filesystem::path m_PrevDataDir; +}; + +struct ConfigurationCacheDirFixture +{ + ConfigurationCacheDirFixture() + : m_CacheDir(boost::filesystem::current_path() / "cache"), m_PrevCacheDir(Configuration::CacheDir.GetData()) + { + boost::filesystem::create_directories(m_CacheDir); + Configuration::CacheDir = m_CacheDir.string(); + } + + ~ConfigurationCacheDirFixture() + { + boost::filesystem::remove_all(m_CacheDir); + Configuration::CacheDir = m_PrevCacheDir.string(); + } + + boost::filesystem::path m_CacheDir; + +private: + boost::filesystem::path m_PrevCacheDir; +}; + +} // namespace icinga + +#endif // CONFIGURATION_FIXTURE_H diff --git a/test/base-testloggerfixture.hpp b/test/base-testloggerfixture.hpp new file mode 100644 index 00000000000..69c073b02a6 --- /dev/null +++ b/test/base-testloggerfixture.hpp @@ -0,0 +1,127 @@ +/* Icinga 2 | (c) 2025 Icinga GmbH | GPLv2+ */ + +#ifndef TEST_LOGGER_FIXTURE_H +#define TEST_LOGGER_FIXTURE_H + +#include +#include "base/logger.hpp" +#include +#include +#include +#include + +namespace icinga { + +class TestLogger : public Logger +{ +public: + DECLARE_PTR_TYPEDEFS(TestLogger); + + struct Expect + { + std::string pattern; + std::promise prom; + }; + + auto ExpectLogPattern(const std::string& pattern, + const std::chrono::milliseconds& timeout = std::chrono::seconds(0)) + { + std::unique_lock lock(m_Mutex); + for (const auto& logEntry : m_LogEntries) { + if (boost::regex_match(logEntry.Message.GetData(), boost::regex(pattern))) { + return boost::test_tools::assertion_result{true}; + } + } + + if (timeout == std::chrono::seconds(0)) { + return boost::test_tools::assertion_result{false}; + } + + auto expect = std::make_shared(Expect{pattern, std::promise()}); + m_Expects.emplace_back(expect); + lock.unlock(); + + auto future = expect->prom.get_future(); + auto status = future.wait_for(timeout); + boost::test_tools::assertion_result ret{status == std::future_status::ready && future.get()}; + ret.message() << "Pattern \"" << pattern << "\" in log within " << timeout.count() << "ms"; + + lock.lock(); + m_Expects.erase(boost::range::remove(m_Expects, expect), m_Expects.end()); + + return ret; + } + +private: + void ProcessLogEntry(const LogEntry& entry) override + { + std::unique_lock lock(m_Mutex); + m_LogEntries.push_back(entry); + + auto it = boost::range::remove_if(m_Expects, [&entry](const std::shared_ptr& expect) { + if (boost::regex_match(entry.Message.GetData(), boost::regex(expect->pattern))) { + expect->prom.set_value(true); + return true; + } + return false; + }); + m_Expects.erase(it, m_Expects.end()); + } + + void Flush() override {} + + std::mutex m_Mutex; + std::vector> m_Expects; + std::vector m_LogEntries; +}; + +/** + * A fixture to capture log entries and assert their presence in tests. + * + * Currently, this only supports checking existing entries and waiting for new ones + * using ExpectLogPattern(), but more functionality can easily be added in the future, + * like only asserting on past log messages, only waiting for new ones, asserting log + * entry metadata (severity etc.) and so on. + */ +struct TestLoggerFixture +{ + TestLoggerFixture() + { + testLogger->SetSeverity(testLogger->SeverityToString(LogDebug)); + testLogger->Activate(true); + testLogger->SetActive(true); + } + + ~TestLoggerFixture() + { + testLogger->SetActive(false); + testLogger->Deactivate(true); + } + + /** + * Asserts the presence of a log entry that matches the given regex pattern. + * + * First, the existing log entries are searched for the pattern. If the pattern isn't found, + * until the timeout is reached, the function will wait if a new log message is added that + * matches the pattern. + * + * A boost assertion result object is returned, that evaluates to bool, but contains an + * error message that is printed by the testing framework when the assert failed. + * + * @param pattern The regex pattern the log message needs to match + * @param timeout The maximum amount of time to wait for the log message to arrive + * + * @return A @c boost::test_tools::assertion_result object that can be used in BOOST_REQUIRE + */ + auto ExpectLogPattern(const std::string& pattern, + const std::chrono::milliseconds& timeout = std::chrono::seconds(0)) + { + return testLogger->ExpectLogPattern(pattern, timeout); + } + + TestLogger::Ptr testLogger = new TestLogger; +}; + +} // namespace icinga + +#endif // TEST_LOGGER_FIXTURE_H diff --git a/test/base-tlsstream-fixture.hpp b/test/base-tlsstream-fixture.hpp new file mode 100644 index 00000000000..3d1327b63dd --- /dev/null +++ b/test/base-tlsstream-fixture.hpp @@ -0,0 +1,114 @@ +/* Icinga 2 | (c) 2025 Icinga GmbH | GPLv2+ */ + +#pragma once + +#include "base/io-engine.hpp" +#include "base/tlsstream.hpp" +#include "test/remote-certificate-fixture.hpp" +#include +#include + +namespace icinga { + +/** + * Creates a pair of TLS Streams on a random unused port. + */ +struct TlsStreamFixture : CertificateFixture +{ + TlsStreamFixture() + { + using namespace boost::asio::ip; + using handshake_type = boost::asio::ssl::stream_base::handshake_type; + + auto serverCert = EnsureCertFor("server"); + auto clientCert = EnsureCertFor("client"); + + auto& io = IoEngine::Get().GetIoContext(); + + m_ClientSslContext = SetupSslContext(clientCert.crtFile, clientCert.keyFile, m_CaCrtFile.string(), "", + DEFAULT_TLS_CIPHERS, DEFAULT_TLS_PROTOCOLMIN, DebugInfo()); + client = Shared::Make(io, *m_ClientSslContext); + + m_ServerSslContext = SetupSslContext(serverCert.crtFile, serverCert.keyFile, m_CaCrtFile.string(), "", + DEFAULT_TLS_CIPHERS, DEFAULT_TLS_PROTOCOLMIN, DebugInfo()); + server = Shared::Make(io, *m_ServerSslContext); + + std::promise p; + + tcp::acceptor acceptor{io, tcp::endpoint{address_v4::loopback(), 0}}; + acceptor.listen(); + acceptor.async_accept(server->lowest_layer(), [&](const boost::system::error_code& ec) { + if (ec) { + BOOST_TEST_MESSAGE("Server Accept Error: " + ec.message()); + p.set_exception(std::make_exception_ptr(boost::system::system_error{ec})); + return; + } + server->next_layer().async_handshake(handshake_type::server, [&](const boost::system::error_code& ec) { + if (ec) { + BOOST_TEST_MESSAGE("Server Handshake Error: " + ec.message()); + p.set_exception(std::make_exception_ptr(boost::system::system_error{ec})); + return; + } + + if (!server->next_layer().IsVerifyOK()) { + p.set_exception(std::make_exception_ptr(std::runtime_error{"Verify failed on server-side."})); + } + + p.set_value(); + }); + }); + + auto f = p.get_future(); + boost::system::error_code ec; + if (client->lowest_layer().connect(acceptor.local_endpoint(), ec)) { + BOOST_TEST_MESSAGE("Client Connect error: " + ec.message()); + f.get(); + BOOST_THROW_EXCEPTION(boost::system::system_error{ec}); + } + + if (client->next_layer().handshake(handshake_type::client, ec)) { + BOOST_TEST_MESSAGE("Client Handshake error: " + ec.message()); + f.get(); + BOOST_THROW_EXCEPTION(boost::system::system_error{ec}); + } + + if (!client->next_layer().IsVerifyOK()) { + f.get(); + BOOST_THROW_EXCEPTION(std::runtime_error{"Verify failed on client-side."}); + } + + f.get(); + } + + auto Shutdown(const Shared::Ptr& stream, std::optional yc = {}) + { + boost::system::error_code ec; + if (yc) { + stream->next_layer().async_shutdown((*yc)[ec]); + } else { + stream->next_layer().shutdown(ec); + } +#if BOOST_VERSION < 107000 + /* On boost versions < 1.70, the end-of-file condition was propagated as an error, + * even in case of a successful shutdown. This is information can be found in the + * changelog for the boost Asio 1.14.0 / Boost 1.70 release. + */ + if (ec == boost::asio::error::eof) { + BOOST_TEST_MESSAGE("Shutdown completed successfully with 'boost::asio::error::eof'."); + return boost::test_tools::assertion_result{true}; + } +#endif + boost::test_tools::assertion_result ret{!ec}; + ret.message() << "Error: " << ec.message(); + return ret; + } + + Shared::Ptr client; + Shared::Ptr server; + +private: + Shared::Ptr m_ClientSslContext; + Shared::Ptr m_ServerSslContext; +}; + +} // namespace icinga diff --git a/test/remote-certificate-fixture.cpp b/test/remote-certificate-fixture.cpp new file mode 100644 index 00000000000..adb26074005 --- /dev/null +++ b/test/remote-certificate-fixture.cpp @@ -0,0 +1,42 @@ +/* Icinga 2 | (c) 2025 Icinga GmbH | GPLv2+ */ + +#include "remote-certificate-fixture.hpp" +#include + +using namespace icinga; + +const boost::filesystem::path CertificateFixture::m_PersistentCertsDir = + boost::filesystem::current_path() / "persistent" / "certs"; + +BOOST_AUTO_TEST_SUITE(remote_certs_fixture) + +/** + * Recursively removes the directory that contains the test certificates. + * + * This needs to be done once initially to prepare the directory, in case there are any + * left-overs from previous test runs, and once after all tests using the certificates + * have been completed. + * + * This dependency is expressed as a CTest fixture and not a boost-test one, because that + * is the only way to have persistency between individual test-cases with CTest. + */ +static void CleanupPersistentCertificateDir() +{ + if (boost::filesystem::exists(CertificateFixture::m_PersistentCertsDir)) { + boost::filesystem::remove_all(CertificateFixture::m_PersistentCertsDir); + } +} + +BOOST_FIXTURE_TEST_CASE(prepare_directory, ConfigurationDataDirFixture) +{ + // Remove any existing left-overs of the persistent certificate directory from a previous + // test run. + CleanupPersistentCertificateDir(); +} + +BOOST_FIXTURE_TEST_CASE(cleanup_certs, ConfigurationDataDirFixture) +{ + CleanupPersistentCertificateDir(); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/test/remote-certificate-fixture.hpp b/test/remote-certificate-fixture.hpp new file mode 100644 index 00000000000..1e8ad645a83 --- /dev/null +++ b/test/remote-certificate-fixture.hpp @@ -0,0 +1,69 @@ +/* Icinga 2 | (c) 2025 Icinga GmbH | GPLv2+ */ + +#pragma once + +#include "remote/apilistener.hpp" +#include "remote/pkiutility.hpp" +#include "test/base-configuration-fixture.hpp" +#include + +namespace icinga { + +struct CertificateFixture : ConfigurationDataDirFixture +{ + CertificateFixture() + { + namespace fs = boost::filesystem; + + m_CaDir = ApiListener::GetCaDir(); + m_CertsDir = ApiListener::GetCertsDir(); + m_CaCrtFile = m_CertsDir / "ca.crt"; + + fs::create_directories(m_PersistentCertsDir / "ca"); + fs::create_directories(m_PersistentCertsDir / "certs"); + + if (fs::exists(m_DataDir / "ca")) { + fs::remove(m_DataDir / "ca"); + } + if (fs::exists(m_DataDir / "certs")) { + fs::remove(m_DataDir / "certs"); + } + + fs::create_directory_symlink(m_PersistentCertsDir / "certs", m_DataDir / "certs"); + fs::create_directory_symlink(m_PersistentCertsDir / "ca", m_DataDir / "ca"); + + if (!fs::exists(m_CaCrtFile)) { + PkiUtility::NewCa(); + fs::copy_file(m_CaDir / "ca.crt", m_CaCrtFile); + } + } + + auto EnsureCertFor(const std::string& name) + { + struct Cert + { + String crtFile; + String keyFile; + String csrFile; + }; + + Cert cert; + cert.crtFile = (m_CertsDir / (name + ".crt")).string(); + cert.keyFile = (m_CertsDir / (name + ".key")).string(); + cert.csrFile = (m_CertsDir / (name + ".csr")).string(); + + if (!Utility::PathExists(cert.crtFile)) { + PkiUtility::NewCert(name, cert.keyFile, cert.csrFile, cert.crtFile); + PkiUtility::SignCsr(cert.csrFile, cert.crtFile); + } + + return cert; + } + + boost::filesystem::path m_CaDir; + boost::filesystem::path m_CertsDir; + boost::filesystem::path m_CaCrtFile; + static const boost::filesystem::path m_PersistentCertsDir; +}; + +} // namespace icinga diff --git a/test/remote-httpmessage.cpp b/test/remote-httpmessage.cpp new file mode 100644 index 00000000000..5c79d1cc26c --- /dev/null +++ b/test/remote-httpmessage.cpp @@ -0,0 +1,351 @@ +/* Icinga 2 | (c) 2025 Icinga GmbH | GPLv2+ */ + +#include +#include "base/base64.hpp" +#include "base/json.hpp" +#include "remote/httpmessage.hpp" +#include "remote/httputility.hpp" +#include "test/base-tlsstream-fixture.hpp" +#include +#include + +using namespace icinga; +using namespace boost::beast; + +static std::future SpawnSynchronizedCoroutine(std::function fn) +{ + auto promise = std::make_unique>(); + auto future = promise->get_future(); + auto& io = IoEngine::Get().GetIoContext(); + IoEngine::SpawnCoroutine(io, [promise = std::move(promise), fn = std::move(fn)](boost::asio::yield_context yc) { + try { + fn(std::move(yc)); + } catch (const std::exception&) { + promise->set_exception(std::current_exception()); + return; + } + promise->set_value(); + }); + return future; +} + +BOOST_FIXTURE_TEST_SUITE(remote_httpmessage, TlsStreamFixture) + +BOOST_AUTO_TEST_CASE(request_parse) +{ + http::request requestOut; + requestOut.method(http::verb::get); + requestOut.target("https://localhost:5665/v1/test"); + requestOut.set(http::field::authorization, "Basic " + Base64::Encode("invalid:invalid")); + requestOut.set(http::field::accept, "application/json"); + requestOut.set(http::field::connection, "close"); + requestOut.body() = "test"; + requestOut.prepare_payload(); + + auto future = SpawnSynchronizedCoroutine([this, &requestOut](boost::asio::yield_context yc) { + boost::beast::flat_buffer buf; + HttpRequest request(server); + BOOST_REQUIRE_NO_THROW(request.ParseHeader(buf, yc)); + + for (const auto& field : requestOut.base()) { + BOOST_REQUIRE(request.count(field.name())); + } + + BOOST_REQUIRE_NO_THROW(request.ParseBody(buf, yc)); + BOOST_REQUIRE_EQUAL(request.body(), "test"); + + Shutdown(server, yc); + }); + + http::write(*client, requestOut); + client->flush(); + + Shutdown(client); + future.get(); +} + +BOOST_AUTO_TEST_CASE(request_params) +{ + HttpRequest request(client); + // clang-format off + request.body() = JsonEncode( + new Dictionary{ + {"bool-in-json", true}, + {"bool-in-url-and-json", true}, + {"string-in-json", "json-value"}, + {"string-in-url-and-json", "json-value"} + }); + request.target("https://localhost:1234/v1/test?" + "bool-in-url-and-json=0&" + "bool-in-url=1&" + "string-in-url-and-json=url-value&" + "string-only-in-url=url-value" + ); + // clang-format on + + // Test pointer being valid after decode + request.DecodeParams(); + auto params = request.Params(); + BOOST_REQUIRE(params); + + // Test JSON-only params being parsed as their correct type + BOOST_REQUIRE(params->Get("bool-in-json").IsBoolean()); + BOOST_REQUIRE(params->Get("string-in-json").IsString()); + BOOST_REQUIRE(params->Get("bool-in-url-and-json").IsObjectType()); + BOOST_REQUIRE(params->Get("string-in-url-and-json").IsObjectType()); + + // Test 0/1 string values from URL evaluate to true and false + // These currently get implicitly converted to double and then to bool, but this is an + // implementation we don't need to test for here. + BOOST_REQUIRE_EQUAL(HttpUtility::GetLastParameter(params, "bool-in-url-and-json"), "0"); + BOOST_REQUIRE(!HttpUtility::GetLastParameter(params, "bool-in-url-and-json")); + BOOST_REQUIRE_EQUAL(HttpUtility::GetLastParameter(params, "bool-in-url"), "1"); + BOOST_REQUIRE(HttpUtility::GetLastParameter(params, "bool-in-url")); + + // Test non-existing parameters evaluate to false + BOOST_REQUIRE(HttpUtility::GetLastParameter(params, "does-not-exist").IsEmpty()); + BOOST_REQUIRE(!HttpUtility::GetLastParameter(params, "does-not-exist")); + + // Test precedence of URL params over JSON params + BOOST_REQUIRE_EQUAL(HttpUtility::GetLastParameter(params, "string-in-json"), "json-value"); + BOOST_REQUIRE_EQUAL(HttpUtility::GetLastParameter(params, "string-in-url-and-json"), "url-value"); + BOOST_REQUIRE_EQUAL(HttpUtility::GetLastParameter(params, "string-only-in-url"), "url-value"); +} + +BOOST_AUTO_TEST_CASE(response_clear) +{ + HttpResponse response(server); + response.result(http::status::bad_request); + response.version(10); + response.set(http::field::content_type, "text/html"); + response.body() << "test"; + + response.Clear(); + + BOOST_REQUIRE(response[http::field::content_type].empty()); + BOOST_REQUIRE_EQUAL(response.result(), http::status::ok); + BOOST_REQUIRE_EQUAL(response.version(), 11); + BOOST_REQUIRE_EQUAL(response.body().Size(), 0); +} + +BOOST_AUTO_TEST_CASE(response_flush_nothrow) +{ + auto future = SpawnSynchronizedCoroutine([this](const boost::asio::yield_context& yc) { + HttpResponse response(server); + response.result(http::status::ok); + + server->lowest_layer().close(); + + boost::beast::error_code ec; + BOOST_REQUIRE_NO_THROW(response.Flush(yc[ec])); + BOOST_REQUIRE_EQUAL(ec, boost::system::errc::bad_file_descriptor); + }); + + auto status = future.wait_for(std::chrono::seconds(1)); + BOOST_REQUIRE(status == std::future_status::ready); +} + +BOOST_AUTO_TEST_CASE(response_flush_throw) +{ + auto future = SpawnSynchronizedCoroutine([this](const boost::asio::yield_context& yc) { + HttpResponse response(server); + response.result(http::status::ok); + + server->lowest_layer().close(); + + BOOST_REQUIRE_EXCEPTION(response.Flush(yc), std::exception, [](const std::exception& ex) { + auto se = dynamic_cast(&ex); + return se && se->code() == boost::system::errc::bad_file_descriptor; + }); + }); + + auto status = future.wait_for(std::chrono::seconds(1)); + BOOST_REQUIRE(status == std::future_status::ready); +} + +BOOST_AUTO_TEST_CASE(response_write_empty) +{ + auto future = SpawnSynchronizedCoroutine([this](boost::asio::yield_context yc) { + HttpResponse response(server); + response.result(http::status::ok); + + BOOST_REQUIRE_NO_THROW(response.Flush(yc)); + + Shutdown(server, yc); + }); + + http::response_parser parser; + flat_buffer buf; + boost::system::error_code ec; + http::read(*client, buf, parser, ec); + + Shutdown(client); + + future.get(); + + BOOST_REQUIRE(!ec); + BOOST_REQUIRE_EQUAL(parser.get().result(), http::status::ok); + BOOST_REQUIRE_EQUAL(parser.get().chunked(), false); + BOOST_REQUIRE_EQUAL(parser.get().body(), ""); +} + +BOOST_AUTO_TEST_CASE(response_write_fixed) +{ + auto future = SpawnSynchronizedCoroutine([this](boost::asio::yield_context yc) { + HttpResponse response(server); + response.result(http::status::ok); + response.body() << "test"; + + BOOST_REQUIRE_NO_THROW(response.Flush(yc)); + + Shutdown(server, yc); + }); + + http::response_parser parser; + flat_buffer buf; + boost::system::error_code ec; + http::read(*client, buf, parser, ec); + + Shutdown(client); + + future.get(); + + BOOST_REQUIRE(!ec); + BOOST_REQUIRE_EQUAL(parser.get().result(), http::status::ok); + BOOST_REQUIRE_EQUAL(parser.get().chunked(), false); + BOOST_REQUIRE_EQUAL(parser.get().body(), "test"); +} + +BOOST_AUTO_TEST_CASE(response_write_chunked) +{ + // NOLINTNEXTLINE(readability-function-cognitive-complexity) + auto future = SpawnSynchronizedCoroutine([this](boost::asio::yield_context yc) { + HttpResponse response(server); + response.result(http::status::ok); + + response.StartStreaming(); + BOOST_REQUIRE_NO_THROW(response.Flush(yc)); + BOOST_REQUIRE(response.HasSerializationStarted()); + + response.body() << "test" << 1; + BOOST_REQUIRE_NO_THROW(response.Flush(yc)); + + response.body() << "test" << 2; + BOOST_REQUIRE_NO_THROW(response.Flush(yc)); + + response.body() << "test" << 3; + response.body().Finish(); + BOOST_REQUIRE_NO_THROW(response.Flush(yc)); + + Shutdown(server, yc); + }); + + http::response_parser parser; + flat_buffer buf; + boost::system::error_code ec; + http::read(*client, buf, parser, ec); + + Shutdown(client); + + future.get(); + + BOOST_REQUIRE(!ec); + BOOST_REQUIRE_EQUAL(parser.get().result(), http::status::ok); + BOOST_REQUIRE_EQUAL(parser.get().chunked(), true); + BOOST_REQUIRE_EQUAL(parser.get().body(), "test1test2test3"); +} + +BOOST_AUTO_TEST_CASE(response_sendjsonbody) +{ + auto future = SpawnSynchronizedCoroutine([this](boost::asio::yield_context yc) { + HttpResponse response(server); + response.result(http::status::ok); + + HttpUtility::SendJsonBody(response, nullptr, new Dictionary{{"test", 1}}); + + BOOST_REQUIRE_NO_THROW(response.Flush(yc)); + + Shutdown(server, yc); + }); + + http::response_parser parser; + flat_buffer buf; + boost::system::error_code ec; + http::read(*client, buf, parser, ec); + + Shutdown(client); + + future.get(); + + BOOST_REQUIRE(!ec); + BOOST_REQUIRE_EQUAL(parser.get().result(), http::status::ok); + BOOST_REQUIRE_EQUAL(parser.get().chunked(), false); + Dictionary::Ptr body = JsonDecode(parser.get().body()); + BOOST_REQUIRE_EQUAL(body->Get("test"), 1); +} + +BOOST_AUTO_TEST_CASE(response_sendjsonerror) +{ + auto future = SpawnSynchronizedCoroutine([this](boost::asio::yield_context yc) { + HttpResponse response(server); + + // This has to be overwritten in SendJsonError. + response.result(http::status::ok); + + HttpUtility::SendJsonError(response, nullptr, 404, "Not found."); + + BOOST_REQUIRE_NO_THROW(response.Flush(yc)); + + Shutdown(server, yc); + }); + + http::response_parser parser; + flat_buffer buf; + boost::system::error_code ec; + http::read(*client, buf, parser, ec); + + Shutdown(client); + + future.get(); + + BOOST_REQUIRE(!ec); + BOOST_REQUIRE_EQUAL(parser.get().result(), http::status::not_found); + BOOST_REQUIRE_EQUAL(parser.get().chunked(), false); + Dictionary::Ptr body = JsonDecode(parser.get().body()); + BOOST_REQUIRE_EQUAL(body->Get("error"), 404); + BOOST_REQUIRE_EQUAL(body->Get("status"), "Not found."); +} + +BOOST_AUTO_TEST_CASE(response_sendfile) +{ + auto future = SpawnSynchronizedCoroutine([this](boost::asio::yield_context yc) { + HttpResponse response(server); + + response.result(http::status::ok); + BOOST_REQUIRE_NO_THROW(response.SendFile(m_CaCrtFile.string(), yc)); + BOOST_REQUIRE_NO_THROW(response.Flush(yc)); + + Shutdown(server, yc); + }); + + http::response_parser parser; + flat_buffer buf; + boost::system::error_code ec; + http::read(*client, buf, parser, ec); + + Shutdown(client); + + future.get(); + + BOOST_REQUIRE(!ec); + BOOST_REQUIRE_EQUAL(parser.get().result(), http::status::ok); + BOOST_REQUIRE_EQUAL(parser.get().chunked(), false); + + std::ifstream fp(m_CaCrtFile.string(), std::ifstream::in | std::ifstream::binary); + fp.exceptions(std::ifstream::badbit); + std::stringstream ss; + ss << fp.rdbuf(); + BOOST_REQUIRE_EQUAL(ss.str(), parser.get().body()); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/test/remote-httpserverconnection.cpp b/test/remote-httpserverconnection.cpp new file mode 100644 index 00000000000..e5c20277031 --- /dev/null +++ b/test/remote-httpserverconnection.cpp @@ -0,0 +1,558 @@ +/* Icinga 2 | (c) 2025 Icinga GmbH | GPLv2+ */ + +#include +#include "base/base64.hpp" +#include "base/json.hpp" +#include "remote/httphandler.hpp" +#include "test/base-testloggerfixture.hpp" +#include "test/base-tlsstream-fixture.hpp" +#include +#include +#include + +using namespace icinga; +using namespace boost::beast; +using namespace boost::unit_test_framework; + +struct HttpServerConnectionFixture : TlsStreamFixture, ConfigurationCacheDirFixture, TestLoggerFixture +{ + HttpServerConnection::Ptr m_Connection; + StoppableWaitGroup::Ptr m_WaitGroup; + + HttpServerConnectionFixture() : m_WaitGroup(new StoppableWaitGroup) {} + + static void CreateApiListener(const String& allowOrigin) + { + ScriptGlobal::Set("NodeName", "server"); + ApiListener::Ptr listener = new ApiListener; + listener->OnConfigLoaded(); + listener->SetAccessControlAllowOrigin(new Array{allowOrigin}); + } + + static void CreateTestUsers() + { + ApiUser::Ptr user = new ApiUser; + user->SetName("client"); + user->SetClientCN("client"); + user->SetPermissions(new Array{"*"}); + user->Register(); + + user = new ApiUser; + user->SetName("test"); + user->SetPassword("test"); + user->SetPermissions(new Array{"*"}); + user->Register(); + } + + void SetupHttpServerConnection(bool authenticated) + { + String identity = authenticated ? "client" : "invalid"; + m_Connection = new HttpServerConnection(m_WaitGroup, identity, authenticated, server); + m_Connection->Start(); + } + + template + bool AssertServerDisconnected(const std::chrono::duration& timeout) + { + auto iterations = timeout / std::chrono::milliseconds(50); + for (std::size_t i = 0; i < iterations && !m_Connection->Disconnected(); i++) { + Utility::Sleep(std::chrono::duration(timeout).count() / iterations); + } + return m_Connection->Disconnected(); + } +}; + +class UnitTestHandler final : public HttpHandler +{ +public: + using TestFn = std::function; + + static void RegisterTestFn(std::string handle, TestFn fn) { testFns[std::move(handle)] = std::move(fn); } + +private: + bool HandleRequest(const WaitGroup::Ptr&, const HttpRequest& request, HttpResponse& response, + boost::asio::yield_context& yc) override + { + response.result(boost::beast::http::status::ok); + + auto path = request.Url()->GetPath(); + + if (path.size() == 3) { + if (auto it = testFns.find(path[2].GetData()); it != testFns.end()) { + it->second(response, yc); + return true; + } + } + + response.body() << "test"; + return true; + } + + static inline std::unordered_map testFns; +}; + +REGISTER_URLHANDLER("/v1/test", UnitTestHandler); + +BOOST_FIXTURE_TEST_SUITE(remote_httpserverconnection, HttpServerConnectionFixture) + +BOOST_AUTO_TEST_CASE(expect_100_continue) +{ + CreateTestUsers(); + SetupHttpServerConnection(true); + + http::request request; + request.method(http::verb::get); + request.version(11); + request.target("/v1/test"); + request.set(http::field::expect, "100-continue"); + request.set(http::field::host, "localhost:5665"); + request.set(http::field::accept, "application/json"); + request.set(http::field::connection, "close"); + request.content_length(0); + http::request_serializer sr(request); + http::write_header(*client, sr); + client->flush(); + + flat_buffer buf; + http::response response; + BOOST_REQUIRE_NO_THROW(http::read(*client, buf, response)); + + BOOST_REQUIRE_EQUAL(response.version(), 11); + BOOST_REQUIRE_EQUAL(response.result(), http::status::continue_); + + http::write(*client, sr); + client->flush(); + + BOOST_REQUIRE_NO_THROW(http::read(*client, buf, response)); + BOOST_REQUIRE_EQUAL(response.version(), 11); + BOOST_REQUIRE_EQUAL(response.result(), http::status::ok); + BOOST_REQUIRE_EQUAL(response.body(), "test"); + + BOOST_REQUIRE(Shutdown(client)); +} + +BOOST_AUTO_TEST_CASE(bad_request) +{ + CreateTestUsers(); + SetupHttpServerConnection(true); + + http::request request; + request.method(http::verb::get); + request.version(12); + request.target("/v1/test"); + request.set(http::field::host, "localhost:5665"); + request.set(http::field::accept, "application/json"); + request.set(http::field::connection, "close"); + request.content_length(0); + http::write(*client, request); + client->flush(); + + flat_buffer buf; + http::response response; + BOOST_REQUIRE_NO_THROW(http::read(*client, buf, response)); + + BOOST_REQUIRE_EQUAL(response.result(), http::status::bad_request); + BOOST_REQUIRE_NE(response.body().find("

Bad Request

"), std::string::npos); + + BOOST_REQUIRE(Shutdown(client)); +} + +BOOST_AUTO_TEST_CASE(error_access_control) +{ + CreateTestUsers(); + CreateApiListener("example.org"); + SetupHttpServerConnection(true); + + http::request request; + request.method(http::verb::options); + request.target("/v1/test"); + request.set(http::field::origin, "example.org"); + request.set(http::field::host, "localhost:5665"); + request.set(http::field::access_control_request_method, "GET"); + request.set(http::field::connection, "close"); + request.content_length(0); + http::write(*client, request); + client->flush(); + + flat_buffer buf; + http::response response; + BOOST_REQUIRE_NO_THROW(http::read(*client, buf, response)); + + BOOST_REQUIRE_EQUAL(response.version(), 11); + BOOST_REQUIRE_EQUAL(response.result(), http::status::ok); + BOOST_REQUIRE_EQUAL(response.body(), "Preflight OK"); + + BOOST_REQUIRE_EQUAL(response[http::field::access_control_allow_credentials], "true"); + BOOST_REQUIRE_EQUAL(response[http::field::access_control_allow_origin], "example.org"); + BOOST_REQUIRE_NE(response[http::field::access_control_allow_methods], ""); + BOOST_REQUIRE_NE(response[http::field::access_control_allow_headers], ""); + + BOOST_REQUIRE(Shutdown(client)); +} + +BOOST_AUTO_TEST_CASE(error_accept_header) +{ + CreateTestUsers(); + SetupHttpServerConnection(true); + + http::request request; + request.method(http::verb::post); + request.target("/v1/test"); + request.set(http::field::host, "localhost:5665"); + request.set(http::field::accept, "text/html"); + request.set(http::field::connection, "close"); + request.content_length(0); + http::write(*client, request); + client->flush(); + + flat_buffer buf; + http::response response; + BOOST_REQUIRE_NO_THROW(http::read(*client, buf, response)); + + BOOST_REQUIRE_EQUAL(response.version(), 11); + BOOST_REQUIRE_EQUAL(response.result(), http::status::bad_request); + BOOST_REQUIRE_EQUAL(response.body(), "

Accept header is missing or not set to 'application/json'.

"); + + BOOST_REQUIRE(Shutdown(client)); +} + +BOOST_AUTO_TEST_CASE(authenticate_cn) +{ + CreateTestUsers(); + SetupHttpServerConnection(true); + + http::request request; + request.method(http::verb::get); + request.target("/v1/test"); + request.set(http::field::host, "localhost:5665"); + request.set(http::field::accept, "application/json"); + request.set(http::field::connection, "close"); + request.content_length(0); + http::write(*client, request); + client->flush(); + + flat_buffer buf; + http::response response; + BOOST_REQUIRE_NO_THROW(http::read(*client, buf, response)); + + BOOST_REQUIRE_EQUAL(response.version(), 11); + BOOST_REQUIRE_EQUAL(response.result(), http::status::ok); + + BOOST_REQUIRE(Shutdown(client)); +} + +BOOST_AUTO_TEST_CASE(authenticate_passwd) +{ + CreateTestUsers(); + SetupHttpServerConnection(false); + + http::request request; + request.method(http::verb::get); + request.target("/v1/test"); + request.set(http::field::authorization, "Basic " + Base64::Encode("test:test")); + request.set(http::field::host, "localhost:5665"); + request.set(http::field::accept, "application/json"); + request.set(http::field::connection, "close"); + request.content_length(0); + http::write(*client, request); + client->flush(); + + flat_buffer buf; + http::response response; + BOOST_REQUIRE_NO_THROW(http::read(*client, buf, response)); + + BOOST_REQUIRE_EQUAL(response.version(), 11); + BOOST_REQUIRE_EQUAL(response.result(), http::status::ok); + + BOOST_REQUIRE(Shutdown(client)); +} + +BOOST_AUTO_TEST_CASE(authenticate_error_wronguser) +{ + CreateTestUsers(); + SetupHttpServerConnection(false); + + http::request request; + request.method(http::verb::get); + request.target("/v1/test"); + request.set(http::field::authorization, "Basic " + Base64::Encode("invalid:invalid")); + request.set(http::field::host, "localhost:5665"); + request.set(http::field::accept, "application/json"); + request.set(http::field::connection, "close"); + request.content_length(0); + http::write(*client, request); + client->flush(); + + flat_buffer buf; + http::response response; + BOOST_REQUIRE_NO_THROW(http::read(*client, buf, response)); + + BOOST_REQUIRE_EQUAL(response.version(), 11); + BOOST_REQUIRE_EQUAL(response.result(), http::status::unauthorized); + Dictionary::Ptr body = JsonDecode(response.body()); + BOOST_REQUIRE(body); + BOOST_REQUIRE_EQUAL(body->Get("error"), 401); + + BOOST_REQUIRE(Shutdown(client)); +} + +BOOST_AUTO_TEST_CASE(authenticate_error_wrongpasswd) +{ + CreateTestUsers(); + SetupHttpServerConnection(false); + + http::request request; + request.method(http::verb::get); + request.target("/v1/test"); + request.set(http::field::authorization, "Basic " + Base64::Encode("test:invalid")); + request.set(http::field::host, "localhost:5665"); + request.set(http::field::accept, "application/json"); + request.set(http::field::connection, "close"); + request.content_length(0); + http::write(*client, request); + client->flush(); + + flat_buffer buf; + http::response response; + BOOST_REQUIRE_NO_THROW(http::read(*client, buf, response)); + + BOOST_REQUIRE_EQUAL(response.version(), 11); + BOOST_REQUIRE_EQUAL(response.result(), http::status::unauthorized); + Dictionary::Ptr body = JsonDecode(response.body()); + BOOST_REQUIRE(body); + BOOST_REQUIRE_EQUAL(body->Get("error"), 401); + + BOOST_REQUIRE(Shutdown(client)); +} + +BOOST_AUTO_TEST_CASE(reuse_connection) +{ + CreateTestUsers(); + SetupHttpServerConnection(true); + + http::request request; + request.method(http::verb::get); + request.target("/v1/test"); + request.set(http::field::host, "localhost:5665"); + request.set(http::field::accept, "application/json"); + request.keep_alive(true); + http::write(*client, request); + client->flush(); + + flat_buffer buf; + http::response response; + BOOST_REQUIRE_NO_THROW(http::read(*client, buf, response)); + + BOOST_REQUIRE_EQUAL(response.version(), 11); + BOOST_REQUIRE_EQUAL(response.result(), http::status::ok); + BOOST_REQUIRE_EQUAL(response.body(), "test"); + + request.keep_alive(false); + http::write(*client, request); + client->flush(); + + boost::system::error_code ec; + http::response_parser parser; + BOOST_REQUIRE_NO_THROW(http::read(*client, buf, parser)); + + BOOST_REQUIRE(parser.is_header_done()); + BOOST_REQUIRE(parser.is_done()); + BOOST_REQUIRE_EQUAL(parser.get().version(), 11); + BOOST_REQUIRE_EQUAL(parser.get().result(), http::status::ok); + BOOST_REQUIRE_EQUAL(parser.get().body(), "test"); + + // Second read to get the end of stream error; + http::read(*client, buf, response, ec); + BOOST_REQUIRE_EQUAL(ec, boost::system::error_code{boost::beast::http::error::end_of_stream}); + + BOOST_REQUIRE(AssertServerDisconnected(std::chrono::seconds(5))); + BOOST_REQUIRE(Shutdown(client)); + BOOST_REQUIRE(ExpectLogPattern("HTTP client disconnected .*", std::chrono::seconds(5))); +} + +BOOST_AUTO_TEST_CASE(wg_abort) +{ + CreateTestUsers(); + SetupHttpServerConnection(true); + + UnitTestHandler::RegisterTestFn("wgjoin", [this](HttpResponse& response, const boost::asio::yield_context&) { + response.body() << "test"; + m_WaitGroup->Join(); + }); + + http::request request; + request.method(http::verb::get); + request.target("/v1/test/wgjoin"); + request.set(http::field::host, "localhost:5665"); + request.set(http::field::accept, "application/json"); + request.keep_alive(true); + http::write(*client, request); + client->flush(); + + flat_buffer buf; + http::response_parser parser; + BOOST_REQUIRE_NO_THROW(http::read(*client, buf, parser)); + + BOOST_REQUIRE(parser.is_header_done()); + BOOST_REQUIRE(parser.is_done()); + BOOST_REQUIRE_EQUAL(parser.get().version(), 11); + BOOST_REQUIRE_EQUAL(parser.get().result(), http::status::ok); + BOOST_REQUIRE_EQUAL(parser.get().body(), "test"); + + // Second read to get the end of stream error; + http::response response{}; + boost::system::error_code ec; + http::read(*client, buf, response, ec); + BOOST_REQUIRE_EQUAL(ec, boost::system::error_code{boost::beast::http::error::end_of_stream}); + + BOOST_REQUIRE(AssertServerDisconnected(std::chrono::seconds(5))); + BOOST_REQUIRE(Shutdown(client)); + BOOST_REQUIRE(ExpectLogPattern("HTTP client disconnected .*", std::chrono::seconds(5))); +} + +BOOST_AUTO_TEST_CASE(client_shutdown) +{ + CreateTestUsers(); + SetupHttpServerConnection(true); + + UnitTestHandler::RegisterTestFn("stream", [](HttpResponse& response, const boost::asio::yield_context& yc) { + response.StartStreaming(); + response.Flush(yc); + + boost::asio::deadline_timer dt{IoEngine::Get().GetIoContext()}; + for (;;) { + dt.expires_from_now(boost::posix_time::seconds(1)); + dt.async_wait(yc); + + if (!response.IsClientDisconnected()) { + return; + } + + response.body() << "test"; + response.Flush(yc); + } + }); + + http::request request; + request.method(http::verb::get); + request.target("/v1/test/stream"); + request.set(http::field::host, "localhost:5665"); + request.set(http::field::accept, "application/json"); + request.keep_alive(true); + http::write(*client, request); + client->flush(); + + flat_buffer buf; + http::response_parser parser; + BOOST_REQUIRE_NO_THROW(http::read_header(*client, buf, parser)); + BOOST_REQUIRE(parser.is_header_done()); + + /* Unlike the other test cases we don't require success here, because with the request + * above, UnitTestHandler simulates a HttpHandler that is constantly writing. + * That may cause the shutdown to fail on the client-side with "application data after + * close notify", but the important part is that HttpServerConnection actually closes + * the connection on its own side, which we check with the BOOST_REQUIRE() below. + */ + BOOST_WARN(Shutdown(client)); + + BOOST_REQUIRE(AssertServerDisconnected(std::chrono::seconds(5))); + BOOST_REQUIRE(ExpectLogPattern("HTTP client disconnected .*", std::chrono::seconds(5))); +} + +BOOST_AUTO_TEST_CASE(handler_throw_error) +{ + CreateTestUsers(); + SetupHttpServerConnection(true); + + UnitTestHandler::RegisterTestFn("throw", [](HttpResponse& response, const boost::asio::yield_context&) { + response.StartStreaming(); + response.body() << "test"; + + boost::system::error_code ec{}; + throw boost::system::system_error(ec); + }); + + http::request request; + request.method(http::verb::get); + request.target("/v1/test/throw"); + request.set(http::field::host, "localhost:5665"); + request.set(http::field::accept, "application/json"); + request.keep_alive(false); + http::write(*client, request); + client->flush(); + + flat_buffer buf; + http::response response; + BOOST_REQUIRE_NO_THROW(http::read(*client, buf, response)); + + BOOST_REQUIRE_EQUAL(response.version(), 11); + BOOST_REQUIRE_EQUAL(response.result(), http::status::internal_server_error); + Dictionary::Ptr body = JsonDecode(response.body()); + BOOST_REQUIRE(body); + BOOST_REQUIRE_EQUAL(body->Get("error"), 500); + BOOST_REQUIRE_EQUAL(body->Get("status"), "Unhandled exception"); + + BOOST_REQUIRE(Shutdown(client)); + BOOST_REQUIRE(ExpectLogPattern("HTTP client disconnected .*", std::chrono::seconds(5))); + BOOST_REQUIRE(!ExpectLogPattern("Exception while processing HTTP request.*")); +} + +BOOST_AUTO_TEST_CASE(handler_throw_streaming) +{ + CreateTestUsers(); + SetupHttpServerConnection(true); + + UnitTestHandler::RegisterTestFn("throw", [](HttpResponse& response, const boost::asio::yield_context& yc) { + response.StartStreaming(); + response.body() << "test"; + + response.Flush(yc); + + boost::system::error_code ec{}; + throw boost::system::system_error(ec); + }); + + http::request request; + request.method(http::verb::get); + request.target("/v1/test/throw"); + request.set(http::field::host, "localhost:5665"); + request.set(http::field::accept, "application/json"); + request.keep_alive(true); + + http::write(*client, request); + client->flush(); + + flat_buffer buf; + http::response_parser parser; + boost::system::error_code ec; + http::read(*client, buf, parser, ec); + + /* Since the handler threw in the middle of sending the message we shouldn't be able + * to read a complete message here. + */ + BOOST_REQUIRE_EQUAL(ec, boost::system::error_code{boost::beast::http::error::partial_message}); + + /* The body should only contain the single "test" the handler has written, without any + * attempts made to additionally write some json error message. + */ + BOOST_REQUIRE_EQUAL(parser.get().body(), "test"); + + /* We then expect the server to initiate a shutdown, which we then complete below. + */ + BOOST_REQUIRE(AssertServerDisconnected(std::chrono::seconds(5))); + BOOST_REQUIRE(Shutdown(client)); + BOOST_REQUIRE(ExpectLogPattern("HTTP client disconnected .*", std::chrono::seconds(5))); + BOOST_REQUIRE(ExpectLogPattern("Exception while processing HTTP request.*")); +} + +BOOST_AUTO_TEST_CASE(liveness_disconnect) +{ + SetupHttpServerConnection(false); + + BOOST_REQUIRE(AssertServerDisconnected(std::chrono::seconds(11))); + BOOST_REQUIRE(ExpectLogPattern("HTTP client disconnected .*")); + BOOST_REQUIRE(ExpectLogPattern("No messages for HTTP connection have been received in the last 10 seconds.")); + BOOST_REQUIRE(Shutdown(client)); +} + +BOOST_AUTO_TEST_SUITE_END()