diff options
author | Ed Tanous <ed@tanous.net> | 2024-03-28 10:35:13 +0300 |
---|---|---|
committer | Ed Tanous <ed@tanous.net> | 2024-04-01 19:06:10 +0300 |
commit | 93cf0ac2c7921d3344fa112f667c47a838e3b7d6 (patch) | |
tree | baf607a094de6dd4dd165e29b50c81085f6d059d | |
parent | 06fc9bebd8c9146636b53f2a04d340230bf21e29 (diff) | |
download | bmcweb-93cf0ac2c7921d3344fa112f667c47a838e3b7d6.tar.xz |
Fix SSE sockets
Redfish protocol validatator has SSE tests that expose some bad coding
practies in SSE handlers, namely, that there are several cases where we
don't check for nullptr.
Fix them.
This appears to have been introduced in:
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/41319
Tested: Redfish service validator passes more tests.
Change-Id: Id980725f007d044b7d120dbe0f4b625865cab6ba
Signed-off-by: Ed Tanous <ed@tanous.net>
-rw-r--r-- | http/routing/sserule.hpp | 8 | ||||
-rw-r--r-- | http/server_sent_event.hpp | 9 | ||||
-rw-r--r-- | redfish-core/include/event_service_manager.hpp | 5 | ||||
-rw-r--r-- | test/http/server_sent_event_test.cpp | 2 |
4 files changed, 15 insertions, 9 deletions
diff --git a/http/routing/sserule.hpp b/http/routing/sserule.hpp index 1a422a8268..c0a4e504b3 100644 --- a/http/routing/sserule.hpp +++ b/http/routing/sserule.hpp @@ -31,7 +31,7 @@ class SseSocketRule : public BaseRule } #ifndef BMCWEB_ENABLE_SSL - void handleUpgrade(const Request& req, + void handleUpgrade(const Request& /*req*/, const std::shared_ptr<bmcweb::AsyncResp>& /*asyncResp*/, boost::asio::ip::tcp::socket&& adaptor) override { @@ -39,11 +39,11 @@ class SseSocketRule : public BaseRule crow::sse_socket::ConnectionImpl<boost::asio::ip::tcp::socket>> myConnection = std::make_shared< crow::sse_socket::ConnectionImpl<boost::asio::ip::tcp::socket>>( - *req.ioService, std::move(adaptor), openHandler, closeHandler); + std::move(adaptor), openHandler, closeHandler); myConnection->start(); } #else - void handleUpgrade(const Request& req, + void handleUpgrade(const Request& /*req*/, const std::shared_ptr<bmcweb::AsyncResp>& /*asyncResp*/, boost::beast::ssl_stream<boost::asio::ip::tcp::socket>&& adaptor) override @@ -52,7 +52,7 @@ class SseSocketRule : public BaseRule boost::beast::ssl_stream<boost::asio::ip::tcp::socket>>> myConnection = std::make_shared<crow::sse_socket::ConnectionImpl< boost::beast::ssl_stream<boost::asio::ip::tcp::socket>>>( - *req.ioService, std::move(adaptor), openHandler, closeHandler); + std::move(adaptor), openHandler, closeHandler); myConnection->start(); } #endif diff --git a/http/server_sent_event.hpp b/http/server_sent_event.hpp index a8bccdb450..0e1cd62a9d 100644 --- a/http/server_sent_event.hpp +++ b/http/server_sent_event.hpp @@ -18,7 +18,7 @@ namespace crow namespace sse_socket { -struct Connection : std::enable_shared_from_this<Connection> +struct Connection : public std::enable_shared_from_this<Connection> { public: Connection() = default; @@ -38,11 +38,13 @@ template <typename Adaptor> class ConnectionImpl : public Connection { public: - ConnectionImpl(boost::asio::io_context& ioIn, Adaptor&& adaptorIn, + ConnectionImpl(Adaptor&& adaptorIn, std::function<void(Connection&)> openHandlerIn, std::function<void(Connection&)> closeHandlerIn) : adaptor(std::move(adaptorIn)), - ioc(ioIn), timer(ioc), openHandler(std::move(openHandlerIn)), + timer(static_cast<boost::asio::io_context&>( + adaptor.get_executor().context())), + openHandler(std::move(openHandlerIn)), closeHandler(std::move(closeHandlerIn)) { @@ -276,7 +278,6 @@ class ConnectionImpl : public Connection using BodyType = bmcweb::HttpBody; boost::beast::http::response<BodyType> res; std::optional<boost::beast::http::response_serializer<BodyType>> serializer; - boost::asio::io_context& ioc; boost::asio::steady_timer timer; bool doingWrite = false; diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp index cac6b8b2b2..679c4f6918 100644 --- a/redfish-core/include/event_service_manager.hpp +++ b/redfish-core/include/event_service_manager.hpp @@ -532,6 +532,11 @@ class Subscription : public persistent_data::UserSubscription void updateRetryConfig(uint32_t retryAttempts, uint32_t retryTimeoutInterval) { + if (policy == nullptr) + { + BMCWEB_LOG_DEBUG("Retry policy was nullptr, ignoring set"); + return; + } policy->maxRetryAttempts = retryAttempts; policy->retryIntervalSecs = std::chrono::seconds(retryTimeoutInterval); } diff --git a/test/http/server_sent_event_test.cpp b/test/http/server_sent_event_test.cpp index 3e740cb525..74007797ef 100644 --- a/test/http/server_sent_event_test.cpp +++ b/test/http/server_sent_event_test.cpp @@ -34,7 +34,7 @@ TEST(ServerSentEvent, SseWorks) std::shared_ptr<ConnectionImpl<boost::beast::test::stream>> conn = std::make_shared<ConnectionImpl<boost::beast::test::stream>>( - io, std::move(stream), openHandler, closeHandler); + std::move(stream), openHandler, closeHandler); conn->start(); // Connect { |