Move http client to URL

Type safety is a good thing.  In:
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/65606

It was found that splitting out the URI into encoded pieces in the early
phase removed some information we needed, namely whether or not a URI
was ipv6.  This commit changes http client such that it passes all the
information through, with the correct type, rather than passing in
hostname, port, path, and ssl separately.

Opportunistically, because a number of log lines are changing, this uses
the opportunity to remove a number of calls to std::to_string, and rely
on std::format instead.

Now that we no longer use custom URI splitting code, the
ValidateAndSplitUrl() method can be removed, given that our validation
now happens in the URI class.

Tested: Aggregation works properly when satellite URIs are queried.

Change-Id: I9f605863179af54c5af2719bc5ce9d29cbfffab7
Signed-off-by: Ed Tanous <edtanous@google.com>

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>"));