diff options
| -rw-r--r-- | http/http_client.hpp | 161 | ||||
| -rw-r--r-- | http/logging.hpp | 13 | ||||
| -rw-r--r-- | http/utility.hpp | 78 | ||||
| -rw-r--r-- | include/async_resolve.hpp | 5 | ||||
| -rw-r--r-- | include/event_service_store.hpp | 11 | ||||
| -rw-r--r-- | redfish-core/include/event_service_manager.hpp | 43 | ||||
| -rw-r--r-- | redfish-core/include/redfish_aggregator.hpp | 43 | ||||
| -rw-r--r-- | redfish-core/lib/event_service.hpp | 46 | ||||
| -rw-r--r-- | test/http/utility_test.cpp | 45 |
9 files changed, 178 insertions, 267 deletions
diff --git a/http/http_client.hpp b/http/http_client.hpp index e1c4d374eb..2cbdbbcef1 100644 --- a/http/http_client.hpp +++ b/http/http_client.hpp @@ -39,6 +39,7 @@ #include <boost/beast/version.hpp> #include <boost/container/devector.hpp> #include <boost/system/error_code.hpp> +#include <boost/url/url_view.hpp> #include <cstdlib> #include <functional> @@ -133,8 +134,7 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> uint32_t retryCount = 0; std::string subId; std::shared_ptr<ConnectionPolicy> connPolicy; - std::string host; - uint16_t port; + boost::urls::url host; uint32_t connId; // Data buffers @@ -165,10 +165,9 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> void doResolve() { state = ConnState::resolveInProgress; - BMCWEB_LOG_DEBUG("Trying to resolve: {}:{}, id: {}", host, - std::to_string(port), std::to_string(connId)); + BMCWEB_LOG_DEBUG("Trying to resolve: {}, id: {}", host, connId); - resolver.async_resolve(host, std::to_string(port), + resolver.async_resolve(host.encoded_host_address(), host.port(), std::bind_front(&ConnectionInfo::afterResolve, this, shared_from_this())); } @@ -179,18 +178,15 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> { if (ec || (endpointList.empty())) { - BMCWEB_LOG_ERROR("Resolve failed: {} {}:{}", ec.message(), host, - std::to_string(port)); + BMCWEB_LOG_ERROR("Resolve failed: {} {}", ec.message(), host); state = ConnState::resolveFailed; waitAndRetry(); return; } - BMCWEB_LOG_DEBUG("Resolved {}:{}, id: {}", host, std::to_string(port), - std::to_string(connId)); + BMCWEB_LOG_DEBUG("Resolved {}, id: {}", host, connId); state = ConnState::connectInProgress; - BMCWEB_LOG_DEBUG("Trying to connect to: {}:{}, id: {}", host, - std::to_string(port), std::to_string(connId)); + BMCWEB_LOG_DEBUG("Trying to connect to: {}, id: {}", host, connId); timer.expires_after(std::chrono::seconds(30)); timer.async_wait(std::bind_front(onTimeout, weak_from_this())); @@ -216,16 +212,15 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> if (ec) { BMCWEB_LOG_ERROR("Connect {}:{}, id: {} failed: {}", - endpoint.address().to_string(), - std::to_string(endpoint.port()), - std::to_string(connId), ec.message()); + endpoint.address().to_string(), endpoint.port(), + connId, ec.message()); state = ConnState::connectFailed; waitAndRetry(); return; } - BMCWEB_LOG_DEBUG( - "Connected to: {}:{}, id: {}", endpoint.address().to_string(), - std::to_string(endpoint.port()), std::to_string(connId)); + BMCWEB_LOG_DEBUG("Connected to: {}:{}, id: {}", + endpoint.address().to_string(), endpoint.port(), + connId); if (sslConn) { doSslHandshake(); @@ -263,14 +258,13 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> timer.cancel(); if (ec) { - BMCWEB_LOG_ERROR("SSL Handshake failed - id: {} error: {}", - std::to_string(connId), ec.message()); + BMCWEB_LOG_ERROR("SSL Handshake failed - id: {} error: {}", connId, + ec.message()); state = ConnState::handshakeFailed; waitAndRetry(); return; } - BMCWEB_LOG_DEBUG("SSL Handshake successful - id: {}", - std::to_string(connId)); + BMCWEB_LOG_DEBUG("SSL Handshake successful - id: {}", connId); state = ConnState::connected; sendMessage(); } @@ -313,8 +307,7 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> timer.cancel(); if (ec) { - BMCWEB_LOG_ERROR("sendMessage() failed: {} {}:{}", ec.message(), - host, std::to_string(port)); + BMCWEB_LOG_ERROR("sendMessage() failed: {} {}", ec.message(), host); state = ConnState::sendFailed; waitAndRetry(); return; @@ -368,8 +361,8 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> timer.cancel(); if (ec && ec != boost::asio::ssl::error::stream_truncated) { - BMCWEB_LOG_ERROR("recvMessage() failed: {} from {}:{}", - ec.message(), host, std::to_string(port)); + BMCWEB_LOG_ERROR("recvMessage() failed: {} from {}", ec.message(), + host); state = ConnState::recvFailed; waitAndRetry(); return; @@ -392,8 +385,8 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> // The listener failed to receive the Sent-Event BMCWEB_LOG_ERROR( "recvMessage() Listener Failed to " - "receive Sent-Event. Header Response Code: {} from {}:{}", - respCode, host, std::to_string(port)); + "receive Sent-Event. Header Response Code: {} from {}", + respCode, host); state = ConnState::recvFailed; waitAndRetry(); return; @@ -442,8 +435,7 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> if ((retryCount >= connPolicy->maxRetryAttempts) || (state == ConnState::sslInitFailed)) { - BMCWEB_LOG_ERROR("Maximum number of retries reached. {}:{}", host, - std::to_string(port)); + BMCWEB_LOG_ERROR("Maximum number of retries reached. {}", host); BMCWEB_LOG_DEBUG("Retry policy: {}", connPolicy->retryPolicyAction); if (connPolicy->retryPolicyAction == "TerminateAfterRetries") @@ -471,8 +463,7 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> retryCount++; BMCWEB_LOG_DEBUG("Attempt retry after {} seconds. RetryCount = {}", - std::to_string(connPolicy->retryIntervalSecs.count()), - retryCount); + connPolicy->retryIntervalSecs.count(), retryCount); timer.expires_after(connPolicy->retryIntervalSecs); timer.async_wait(std::bind_front(&ConnectionInfo::onTimerDone, this, shared_from_this())); @@ -507,14 +498,12 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> // not_connected happens sometimes so don't bother reporting it. if (ec && ec != boost::beast::errc::not_connected) { - BMCWEB_LOG_ERROR("{}:{}, id: {} shutdown failed: {}", host, - std::to_string(port), std::to_string(connId), + BMCWEB_LOG_ERROR("{}, id: {} shutdown failed: {}", host, connId, ec.message()); } else { - BMCWEB_LOG_DEBUG("{}:{}, id: {} closed gracefully", host, - std::to_string(port), std::to_string(connId)); + BMCWEB_LOG_DEBUG("{}, id: {} closed gracefully", host, connId); } if (retry) @@ -547,14 +536,12 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> { if (ec) { - BMCWEB_LOG_ERROR("{}:{}, id: {} shutdown failed: {}", host, - std::to_string(port), std::to_string(connId), + BMCWEB_LOG_ERROR("{}, id: {} shutdown failed: {}", host, connId, ec.message()); } else { - BMCWEB_LOG_DEBUG("{}:{}, id: {} closed gracefully", host, - std::to_string(port), std::to_string(connId)); + BMCWEB_LOG_DEBUG("{}, id: {} closed gracefully", host, connId); } shutdownConn(retry); } @@ -565,6 +552,7 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> { return; } + std::string hostname(host.encoded_host_address()); // NOTE: The SSL_set_tlsext_host_name is defined in tlsv1.h header // file but its having old style casting (name is cast to void*). // Since bmcweb compiler treats all old-style-cast as error, its @@ -574,15 +562,14 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> // hosts need this to handshake successfully) if (SSL_ctrl(sslConn->native_handle(), SSL_CTRL_SET_TLSEXT_HOSTNAME, TLSEXT_NAMETYPE_host_name, - static_cast<void*>(&host.front())) == 0) + static_cast<void*>(hostname.data())) == 0) { boost::beast::error_code ec{static_cast<int>(::ERR_get_error()), boost::asio::error::get_ssl_category()}; - BMCWEB_LOG_ERROR( - "SSL_set_tlsext_host_name {}:{}, id: {} failed: {}", host, port, - std::to_string(connId), ec.message()); + BMCWEB_LOG_ERROR("SSL_set_tlsext_host_name {}, id: {} failed: {}", + host, connId, ec.message()); // Set state as sslInit failed so that we close the connection // and take appropriate action as per retry configuration. state = ConnState::sslInitFailed; @@ -595,21 +582,20 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> explicit ConnectionInfo( boost::asio::io_context& iocIn, const std::string& idIn, const std::shared_ptr<ConnectionPolicy>& connPolicyIn, - const std::string& destIPIn, uint16_t destPortIn, bool useSSL, - unsigned int connIdIn) : + boost::urls::url_view hostIn, unsigned int connIdIn) : subId(idIn), - connPolicy(connPolicyIn), host(destIPIn), port(destPortIn), - connId(connIdIn), resolver(iocIn), conn(iocIn), timer(iocIn) + connPolicy(connPolicyIn), host(hostIn), connId(connIdIn), + resolver(iocIn), conn(iocIn), timer(iocIn) { - if (useSSL) + if (host.scheme() == "https") { std::optional<boost::asio::ssl::context> sslCtx = ensuressl::getSSLClientContext(); if (!sslCtx) { - BMCWEB_LOG_ERROR("prepareSSLContext failed - {}:{}, id: {}", - host, port, std::to_string(connId)); + BMCWEB_LOG_ERROR("prepareSSLContext failed - {}, id: {}", host, + connId); // Don't retry if failure occurs while preparing SSL context // such as certificate is invalid or set cipher failure or set // host name failure etc... Setting conn state to sslInitFailed @@ -631,9 +617,7 @@ class ConnectionPool : public std::enable_shared_from_this<ConnectionPool> boost::asio::io_context& ioc; std::string id; std::shared_ptr<ConnectionPolicy> connPolicy; - std::string destIP; - uint16_t destPort; - bool useSSL; + boost::urls::url destIP; std::vector<std::shared_ptr<ConnectionInfo>> connections; boost::container::devector<PendingRequest> requestQueue; @@ -654,9 +638,8 @@ class ConnectionPool : public std::enable_shared_from_this<ConnectionPool> conn.req = std::move(nextReq.req); conn.callback = std::move(nextReq.callback); - BMCWEB_LOG_DEBUG("Setting properties for connection {}:{}, id: {}", - conn.host, std::to_string(conn.port), - std::to_string(conn.connId)); + BMCWEB_LOG_DEBUG("Setting properties for connection {}, id: {}", + conn.host, conn.connId); // We can remove the request from the queue at this point requestQueue.pop_front(); @@ -678,9 +661,8 @@ class ConnectionPool : public std::enable_shared_from_this<ConnectionPool> if (!requestQueue.empty()) { BMCWEB_LOG_DEBUG( - "{} requests remaining in queue for {}:{}, reusing connnection {}", - std::to_string(requestQueue.size()), destIP, - std::to_string(destPort), std::to_string(connId)); + "{} requests remaining in queue for {}, reusing connnection {}", + requestQueue.size(), destIP, connId); setConnProps(*conn); @@ -711,15 +693,16 @@ class ConnectionPool : public std::enable_shared_from_this<ConnectionPool> } } - void sendData(std::string&& data, const std::string& destUri, + void sendData(std::string&& data, boost::urls::url_view destUri, const boost::beast::http::fields& httpHeader, const boost::beast::http::verb verb, const std::function<void(Response&)>& resHandler) { // Construct the request to be sent boost::beast::http::request<boost::beast::http::string_body> thisReq( - verb, destUri, 11, "", httpHeader); - thisReq.set(boost::beast::http::field::host, destIP); + verb, destUri.encoded_target(), 11, "", httpHeader); + thisReq.set(boost::beast::http::field::host, + destUri.encoded_host_address()); thisReq.keep_alive(true); thisReq.body() = std::move(data); thisReq.prepare_payload(); @@ -735,8 +718,7 @@ class ConnectionPool : public std::enable_shared_from_this<ConnectionPool> { conn->req = std::move(thisReq); conn->callback = std::move(cb); - std::string commonMsg = std::to_string(i) + " from pool " + - destIP + ":" + std::to_string(destPort); + std::string commonMsg = std::format("{} from pool {}", i, id); if (conn->state == ConnState::idle) { @@ -757,8 +739,7 @@ class ConnectionPool : public std::enable_shared_from_this<ConnectionPool> // the queue if (connections.size() < connPolicy->maxConnections) { - BMCWEB_LOG_DEBUG("Adding new connection to pool {}:{}", destIP, - std::to_string(destPort)); + BMCWEB_LOG_DEBUG("Adding new connection to pool {}", id); auto conn = addConnection(); conn->req = std::move(thisReq); conn->callback = std::move(cb); @@ -766,9 +747,8 @@ class ConnectionPool : public std::enable_shared_from_this<ConnectionPool> } else if (requestQueue.size() < maxRequestQueueSize) { - BMCWEB_LOG_DEBUG( - "Max pool size reached. Adding data to queue.{}:{}", destIP, - std::to_string(destPort)); + BMCWEB_LOG_DEBUG("Max pool size reached. Adding data to queue {}", + id); requestQueue.emplace_back(std::move(thisReq), std::move(cb)); } else @@ -776,7 +756,7 @@ class ConnectionPool : public std::enable_shared_from_this<ConnectionPool> // If we can't buffer the request then we should let the callback // handle a 429 Too Many Requests dummy response BMCWEB_LOG_ERROR("{}:{} request queue full. Dropping request.", - destIP, std::to_string(destPort)); + id); Response dummyRes; dummyRes.result(boost::beast::http::status::too_many_requests); resHandler(dummyRes); @@ -810,11 +790,10 @@ class ConnectionPool : public std::enable_shared_from_this<ConnectionPool> unsigned int newId = static_cast<unsigned int>(connections.size()); auto& ret = connections.emplace_back(std::make_shared<ConnectionInfo>( - ioc, id, connPolicy, destIP, destPort, useSSL, newId)); + ioc, id, connPolicy, destIP, newId)); - BMCWEB_LOG_DEBUG("Added connection {} to pool {}:{}", - std::to_string(connections.size() - 1), destIP, - std::to_string(destPort)); + BMCWEB_LOG_DEBUG("Added connection {} to pool {}", + connections.size() - 1, id); return ret; } @@ -823,13 +802,11 @@ class ConnectionPool : public std::enable_shared_from_this<ConnectionPool> explicit ConnectionPool( boost::asio::io_context& iocIn, const std::string& idIn, const std::shared_ptr<ConnectionPolicy>& connPolicyIn, - const std::string& destIPIn, uint16_t destPortIn, bool useSSLIn) : + boost::urls::url_view destIPIn) : ioc(iocIn), - id(idIn), connPolicy(connPolicyIn), destIP(destIPIn), - destPort(destPortIn), useSSL(useSSLIn) + id(idIn), connPolicy(connPolicyIn), destIP(destIPIn) { - BMCWEB_LOG_DEBUG("Initializing connection pool for {}:{}", destIP, - std::to_string(destPort)); + BMCWEB_LOG_DEBUG("Initializing connection pool for {}", id); // Initialize the pool with a single connection addConnection(); @@ -849,7 +826,7 @@ class HttpClient static void genericResHandler(const Response& res) { BMCWEB_LOG_DEBUG("Response handled with return code: {}", - std::to_string(res.resultInt())); + res.resultInt()); } public: @@ -866,40 +843,34 @@ class HttpClient HttpClient& operator=(HttpClient&&) = delete; ~HttpClient() = default; - // Send a request to destIP:destPort where additional processing of the + // Send a request to destIP where additional processing of the // result is not required - void sendData(std::string&& data, const std::string& destIP, - uint16_t destPort, const std::string& destUri, bool useSSL, + void sendData(std::string&& data, boost::urls::url_view destUri, const boost::beast::http::fields& httpHeader, const boost::beast::http::verb verb) { const std::function<void(Response&)> cb = genericResHandler; - sendDataWithCallback(std::move(data), destIP, destPort, destUri, useSSL, - httpHeader, verb, cb); + sendDataWithCallback(std::move(data), destUri, httpHeader, verb, cb); } - // Send request to destIP:destPort and use the provided callback to + // Send request to destIP and use the provided callback to // handle the response - void sendDataWithCallback(std::string&& data, const std::string& destIP, - uint16_t destPort, const std::string& destUri, - bool useSSL, + void sendDataWithCallback(std::string&& data, boost::urls::url_view destUrl, const boost::beast::http::fields& httpHeader, const boost::beast::http::verb verb, const std::function<void(Response&)>& resHandler) { - std::string clientKey = useSSL ? "https" : "http"; - clientKey += destIP; - clientKey += ":"; - clientKey += std::to_string(destPort); + std::string clientKey = std::format("{}://{}", destUrl.scheme(), + destUrl.encoded_host_and_port()); auto pool = connectionPools.try_emplace(clientKey); if (pool.first->second == nullptr) { pool.first->second = std::make_shared<ConnectionPool>( - ioc, clientKey, connPolicy, destIP, destPort, useSSL); + ioc, clientKey, connPolicy, destUrl); } // Send the data using either the existing connection pool or the newly // created connection pool - pool.first->second->sendData(std::move(data), destUri, httpHeader, verb, + pool.first->second->sendData(std::move(data), destUrl, httpHeader, verb, resHandler); } }; diff --git a/http/logging.hpp b/http/logging.hpp index 1d8e67ffb6..3fdef7e744 100644 --- a/http/logging.hpp +++ b/http/logging.hpp @@ -47,6 +47,19 @@ struct std::formatter<boost::urls::pct_string_view> }; template <> +struct std::formatter<boost::urls::url_view> +{ + constexpr auto parse(std::format_parse_context& ctx) + { + return ctx.begin(); + } + auto format(const boost::urls::url& msg, auto& ctx) const + { + return std::format_to(ctx.out(), "{}", std::string_view(msg.buffer())); + } +}; + +template <> struct std::formatter<boost::urls::url> { constexpr auto parse(std::format_parse_context& ctx) diff --git a/http/utility.hpp b/http/utility.hpp index 572caec710..dc7ea7f1b2 100644 --- a/http/utility.hpp +++ b/http/utility.hpp @@ -445,91 +445,55 @@ inline boost::urls::url replaceUrlSegment(boost::urls::url_view urlView, return url; } -inline std::string setProtocolDefaults(boost::urls::url_view urlView) +inline void setProtocolDefaults(boost::urls::url& url, + std::string_view protocol) { - if (urlView.scheme() == "https") + if (url.has_scheme()) { - return "https"; + return; } - if (urlView.scheme() == "http") + if (protocol == "Redfish" || protocol.empty()) { - if (bmcwebInsecureEnableHttpPushStyleEventing) + if (url.port_number() == 443) { - return "http"; + url.set_scheme("https"); + } + if (url.port_number() == 80) + { + if (bmcwebInsecureEnableHttpPushStyleEventing) + { + url.set_scheme("http"); + } } - return ""; } - if (urlView.scheme() == "snmp") + else if (protocol == "SNMPv2c") { - return "snmp"; + url.set_scheme("snmp"); } - return ""; } -inline uint16_t setPortDefaults(boost::urls::url_view url) +inline void setPortDefaults(boost::urls::url& url) { uint16_t port = url.port_number(); if (port != 0) { - // user picked a port already. - return port; + return; } // If the user hasn't explicitly stated a port, pick one explicitly for them // based on the protocol defaults if (url.scheme() == "http") { - return 80; + url.set_port_number(80); } if (url.scheme() == "https") { - return 443; + url.set_port_number(443); } if (url.scheme() == "snmp") { - return 162; - } - return 0; -} - -inline bool validateAndSplitUrl(std::string_view destUrl, std::string& urlProto, - std::string& host, uint16_t& port, - std::string& path) -{ - boost::urls::result<boost::urls::url_view> url = - boost::urls::parse_uri(destUrl); - if (!url) - { - return false; - } - urlProto = setProtocolDefaults(url.value()); - if (urlProto.empty()) - { - return false; + url.set_port_number(162); } - - port = setPortDefaults(url.value()); - - host = url->encoded_host_address(); - - path = url->encoded_path(); - if (path.empty()) - { - path = "/"; - } - if (url->has_fragment()) - { - path += '#'; - path += url->encoded_fragment(); - } - - if (url->has_query()) - { - path += '?'; - path += url->encoded_query(); - } - - return true; } } // namespace utility diff --git a/include/async_resolve.hpp b/include/async_resolve.hpp index 3f31e1383f..71d2497e26 100644 --- a/include/async_resolve.hpp +++ b/include/async_resolve.hpp @@ -63,7 +63,7 @@ class Resolver // This function is kept using snake case so that it is interoperable with // boost::asio::ip::tcp::resolver // NOLINTNEXTLINE(readability-identifier-naming) - void async_resolve(const std::string& host, std::string_view port, + void async_resolve(std::string_view host, std::string_view port, ResolveHandler&& handler) { BMCWEB_LOG_DEBUG("Trying to resolve: {}:{}", host, port); @@ -82,7 +82,8 @@ class Resolver uint64_t flag = 0; crow::connections::systemBus->async_method_call( - [host, portNum, handler{std::forward<ResolveHandler>(handler)}]( + [host{std::string(host)}, portNum, + handler{std::forward<ResolveHandler>(handler)}]( const boost::system::error_code& ec, const std::vector< std::tuple<int32_t, int32_t, std::vector<uint8_t>>>& resp, diff --git a/include/event_service_store.hpp b/include/event_service_store.hpp index 61f1ac7b0f..8b24fcbe83 100644 --- a/include/event_service_store.hpp +++ b/include/event_service_store.hpp @@ -3,6 +3,7 @@ #include <boost/beast/http/fields.hpp> #include <boost/container/flat_map.hpp> +#include <boost/url/parse.hpp> #include <nlohmann/json.hpp> namespace persistent_data @@ -11,7 +12,7 @@ namespace persistent_data struct UserSubscription { std::string id; - std::string destinationUrl; + boost::urls::url destinationUrl; std::string protocol; std::string retryPolicy; std::string customText; @@ -48,7 +49,13 @@ struct UserSubscription { continue; } - subvalue->destinationUrl = *value; + boost::urls::result<boost::urls::url> url = + boost::urls::parse_absolute_uri(*value); + if (!url) + { + continue; + } + subvalue->destinationUrl = std::move(*url); } else if (element.key() == "Protocol") { diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp index 67f1800bfb..ed02c409eb 100644 --- a/redfish-core/include/event_service_manager.hpp +++ b/redfish-core/include/event_service_manager.hpp @@ -356,13 +356,10 @@ class Subscription : public persistent_data::UserSubscription Subscription(Subscription&&) = delete; Subscription& operator=(Subscription&&) = delete; - Subscription(const std::string& inHost, uint16_t inPort, - const std::string& inPath, const std::string& inUriProto, - boost::asio::io_context& ioc) : - host(inHost), - port(inPort), policy(std::make_shared<crow::ConnectionPolicy>()), - path(inPath), uriProto(inUriProto) + Subscription(boost::urls::url_view url, boost::asio::io_context& ioc) : + policy(std::make_shared<crow::ConnectionPolicy>()) { + destinationUrl = url; client.emplace(ioc, policy); // Subscription constructor policy->invalidResp = retryRespHandler; @@ -384,12 +381,11 @@ class Subscription : public persistent_data::UserSubscription return false; } - bool useSSL = (uriProto == "https"); // A connection pool will be created if one does not already exist if (client) { - client->sendData(std::move(msg), host, port, path, useSSL, - httpHeaders, boost::beast::http::verb::post); + client->sendData(std::move(msg), destinationUrl, httpHeaders, + boost::beast::http::verb::post); return true; } @@ -565,8 +561,7 @@ class Subscription : public persistent_data::UserSubscription private: std::string subId; uint64_t eventSeqNum = 1; - std::string host; - uint16_t port = 0; + boost::urls::url host; std::shared_ptr<crow::ConnectionPolicy> policy; crow::sse_socket::Connection* sseConn = nullptr; std::optional<crow::HttpClient> client; @@ -647,21 +642,17 @@ class EventServiceManager std::shared_ptr<persistent_data::UserSubscription> newSub = it.second; - std::string host; - std::string urlProto; - uint16_t port = 0; - std::string path; - bool status = crow::utility::validateAndSplitUrl( - newSub->destinationUrl, urlProto, host, port, path); + boost::urls::result<boost::urls::url> url = + boost::urls::parse_absolute_uri(newSub->destinationUrl); - if (!status) + if (!url) { BMCWEB_LOG_ERROR( "Failed to validate and split destination url"); continue; } std::shared_ptr<Subscription> subValue = - std::make_shared<Subscription>(host, port, path, urlProto, ioc); + std::make_shared<Subscription>(*url, ioc); subValue->id = newSub->id; subValue->destinationUrl = newSub->destinationUrl; @@ -1012,20 +1003,6 @@ class EventServiceManager return idList; } - bool isDestinationExist(const std::string& destUrl) const - { - for (const auto& it : subscriptionsMap) - { - std::shared_ptr<Subscription> entry = it.second; - if (entry->destinationUrl == destUrl) - { - BMCWEB_LOG_ERROR("Destination exist already{}", destUrl); - return true; - } - } - return false; - } - bool sendTestEventLog() { for (const auto& it : subscriptionsMap) diff --git a/redfish-core/include/redfish_aggregator.hpp b/redfish-core/include/redfish_aggregator.hpp index 2514fae442..e5ad88abe1 100644 --- a/redfish-core/include/redfish_aggregator.hpp +++ b/redfish-core/include/redfish_aggregator.hpp @@ -712,8 +712,9 @@ class RedfishAggregator } // We need to strip the prefix from the request's path - std::string targetURI(thisReq.target()); - size_t pos = targetURI.find(prefix + "_"); + boost::urls::url targetURI(thisReq.target()); + std::string path = thisReq.url().path(); + size_t pos = path.find(prefix + "_"); if (pos == std::string::npos) { // If this fails then something went wrong @@ -722,16 +723,20 @@ class RedfishAggregator messages::internalError(asyncResp->res); return; } - targetURI.erase(pos, prefix.size() + 1); + path.erase(pos, prefix.size() + 1); std::function<void(crow::Response&)> cb = std::bind_front(processResponse, prefix, asyncResp); std::string data = thisReq.req.body(); - client.sendDataWithCallback( - std::move(data), std::string(sat->second.host()), - sat->second.port_number(), targetURI, false /*useSSL*/, - thisReq.fields(), thisReq.method(), cb); + boost::urls::url url(sat->second); + url.set_path(path); + if (targetURI.has_query()) + { + url.set_query(targetURI.query()); + } + client.sendDataWithCallback(std::move(data), url, thisReq.fields(), + thisReq.method(), cb); } // Forward a request for a collection URI to each known satellite BMC @@ -745,12 +750,15 @@ class RedfishAggregator std::function<void(crow::Response&)> cb = std::bind_front( processCollectionResponse, sat.first, asyncResp); - std::string targetURI(thisReq.target()); + boost::urls::url url(sat.second); + url.set_path(thisReq.url().path()); + if (thisReq.url().has_query()) + { + url.set_query(thisReq.url().query()); + } std::string data = thisReq.req.body(); - client.sendDataWithCallback( - std::move(data), std::string(sat.second.host()), - sat.second.port_number(), targetURI, false /*useSSL*/, - thisReq.fields(), thisReq.method(), cb); + client.sendDataWithCallback(std::move(data), url, thisReq.fields(), + thisReq.method(), cb); } } @@ -770,12 +778,13 @@ class RedfishAggregator // is not already supported by the aggregating BMC // TODO: Improve the processing so that we don't have to strip query // params in this specific case - std::string targetURI(thisReq.url().path()); + boost::urls::url url(sat.second); + url.set_path(thisReq.url().path()); + std::string data = thisReq.req.body(); - client.sendDataWithCallback( - std::move(data), std::string(sat.second.host()), - sat.second.port_number(), targetURI, false /*useSSL*/, - thisReq.fields(), thisReq.method(), cb); + + client.sendDataWithCallback(std::move(data), url, thisReq.fields(), + thisReq.method(), cb); } } diff --git a/redfish-core/lib/event_service.hpp b/redfish-core/lib/event_service.hpp index b49e35afe1..89c233702f 100644 --- a/redfish-core/lib/event_service.hpp +++ b/redfish-core/lib/event_service.hpp @@ -24,6 +24,7 @@ #include <boost/beast/http/fields.hpp> #include <boost/system/error_code.hpp> +#include <boost/url/parse.hpp> #include <sdbusplus/unpack_properties.hpp> #include <utils/dbus_utils.hpp> @@ -317,19 +318,30 @@ inline void requestRoutesEventDestinationCollection(App& app) } } - std::string host; - std::string urlProto; - uint16_t port = 0; - std::string path; - - if (!crow::utility::validateAndSplitUrl(destUrl, urlProto, host, port, - path)) + boost::urls::result<boost::urls::url> url = + boost::urls::parse_absolute_uri(destUrl); + if (!url) { BMCWEB_LOG_WARNING("Failed to validate and split destination url"); messages::propertyValueFormatError(asyncResp->res, destUrl, "Destination"); return; } + url->normalize(); + crow::utility::setProtocolDefaults(*url, protocol); + crow::utility::setPortDefaults(*url); + + if (url->path().empty()) + { + url->set_path("/"); + } + + if (url->has_userinfo()) + { + messages::propertyValueFormatError(asyncResp->res, destUrl, + "Destination"); + return; + } if (protocol == "SNMPv2c") { @@ -381,20 +393,22 @@ inline void requestRoutesEventDestinationCollection(App& app) asyncResp->res, "MetricReportDefinitions", "Protocol"); return; } + if (url->scheme() != "snmp") + { + messages::propertyValueConflict(asyncResp->res, "Destination", + "Protocol"); + return; + } - addSnmpTrapClient(asyncResp, host, port); + addSnmpTrapClient(asyncResp, url->host_address(), + url->port_number()); return; } - if (path.empty()) - { - path = "/"; - } - - std::shared_ptr<Subscription> subValue = std::make_shared<Subscription>( - host, port, path, urlProto, app.ioContext()); + std::shared_ptr<Subscription> subValue = + std::make_shared<Subscription>(*url, app.ioContext()); - subValue->destinationUrl = destUrl; + subValue->destinationUrl = std::move(*url); if (subscriptionType) { diff --git a/test/http/utility_test.cpp b/test/http/utility_test.cpp index 0ebba0bebe..cf83410707 100644 --- a/test/http/utility_test.cpp +++ b/test/http/utility_test.cpp @@ -140,51 +140,6 @@ TEST(Utility, readUrlSegments) EXPECT_TRUE(readUrlSegments(*parsed, OrMorePaths())); } -TEST(Utility, ValidateAndSplitUrlPositive) -{ - std::string host; - std::string urlProto; - uint16_t port = 0; - std::string path; - ASSERT_TRUE(validateAndSplitUrl("https://foo.com:18080/bar", urlProto, host, - port, path)); - EXPECT_EQ(host, "foo.com"); - EXPECT_EQ(urlProto, "https"); - EXPECT_EQ(port, 18080); - - EXPECT_EQ(path, "/bar"); - - // query string - ASSERT_TRUE(validateAndSplitUrl("https://foo.com:18080/bar?foobar=1", - urlProto, host, port, path)); - EXPECT_EQ(path, "/bar?foobar=1"); - - // fragment - ASSERT_TRUE(validateAndSplitUrl("https://foo.com:18080/bar#frag", urlProto, - host, port, path)); - EXPECT_EQ(path, "/bar#frag"); - - // Missing port - ASSERT_TRUE( - validateAndSplitUrl("https://foo.com/bar", urlProto, host, port, path)); - EXPECT_EQ(port, 443); - - // Missing path defaults to "/" - ASSERT_TRUE( - validateAndSplitUrl("https://foo.com/", urlProto, host, port, path)); - EXPECT_EQ(path, "/"); - - // If http push eventing is allowed, allow http and pick a default port of - // 80, if it's not, parse should fail. - ASSERT_EQ( - validateAndSplitUrl("http://foo.com/bar", urlProto, host, port, path), - bmcwebInsecureEnableHttpPushStyleEventing); - if constexpr (bmcwebInsecureEnableHttpPushStyleEventing) - { - EXPECT_EQ(port, 80); - } -} - TEST(Router, ParameterTagging) { EXPECT_EQ(1, getParameterTag("<str>")); |