commit f79b7a502db253f6a862aa2cdf5308a7bf2b9070
author Ed Tanous <edtanous@google.com> Wed Sep 22 19:04:29 2021 -0700
committer Ed Tanous <edtanous@google.com> Mon Oct 18 10:23:50 2021 -0700
tree 3ab9fcd35ae5477ac78cacd3cbacdce749ac260d
parent 5edbe947bc474743763b34a89bacf973645fefab

Deduplicate url parsing code

In the current model, URLs get parsed twice, once to satisfy the
authenticate method, and once again later to satisfy the handle() call.
This commit deduplicates the parsing.  This is wasteful, and as of the
previous commit, unnecessary.

Specifically, it moves the actual parsing into the request object, and
adds a target() method to explicitly set a url.  This deduplicates the
code that was in http_connection, and centralizes it in request, where
it should really belong.

Tested:
curl --insecure "https://192.168.7.2/redfish/v1"
Returns the redfish v1 resource

curl --insecure "https://192.168.7.2/redfish/v1/Systems"
Returns 401 unauthorized

curl --insecure --user root:0penBmc "https://192.168.7.2/redfish/v1/Systems"
returns the SystemsCollection

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

http/http_connection.hpp

diff --git a/http/http_connection.hpp b/http/http_connection.hpp
index 8e53afa..c296782 100644
--- a/http/http_connection.hpp
+++ b/http/http_connection.hpp
@@ -314,8 +314,13 @@
     void handle()
     {
         cancelDeadlineTimer();
-
-        crow::Request& thisReq = req.emplace(parser->release());
+        std::error_code reqEc;
+        crow::Request& thisReq = req.emplace(parser->release(), reqEc);
+        if (reqEc)
+        {
+            BMCWEB_LOG_DEBUG << "Request failed to construct" << reqEc;
+            return;
+        }
         thisReq.session = userSession;
 
         // Fetch the client IP address
@@ -338,18 +343,6 @@
                         << thisReq.methodString() << " " << thisReq.target()
                         << " " << thisReq.ipAddress;
 
-        boost::urls::error_code ec;
-        req->urlView = boost::urls::parse_relative_ref(
-            boost::urls::string_view(req->target().data(),
-                                     req->target().size()),
-            ec);
-        if (ec)
-        {
-            return;
-        }
-        req->url = std::string_view(req->urlView.encoded_path().data(),
-                                    req->urlView.encoded_path().size());
-
         res.setCompleteRequestHandler(nullptr);
         res.isAliveHelper = [this]() -> bool { return isAlive(); };
 
@@ -432,6 +425,10 @@
 
     void completeRequest()
     {
+        if (!req)
+        {
+            return;
+        }
         BMCWEB_LOG_INFO << "Response: " << this << ' ' << req->url << ' '
                         << res.resultInt() << " keepalive=" << req->keepAlive();
 
@@ -563,21 +560,6 @@
 
                 boost::beast::http::verb method = parser->get().method();
                 readClientIp();
-                boost::urls::error_code uriEc;
-                boost::urls::string_view uriStringView(
-                    parser->get().target().data(),
-                    parser->get().target().size());
-                BMCWEB_LOG_DEBUG << "Parsing URI: " << uriStringView;
-                req->urlView =
-                    boost::urls::parse_relative_ref(uriStringView, uriEc);
-                if (uriEc)
-                {
-                    BMCWEB_LOG_ERROR << "Failed to parse URI "
-                                     << uriEc.message();
-                    return;
-                }
-                req->url = std::string_view(req->urlView.encoded_path().data(),
-                                            req->urlView.encoded_path().size());
 
                 boost::asio::ip::address ip;
                 if (getClientIp(ip))

http/http_request.hpp

diff --git a/http/http_request.hpp b/http/http_request.hpp
index da3d4e0..be84f18 100644
--- a/http/http_request.hpp
+++ b/http/http_request.hpp
@@ -12,6 +12,7 @@
 
 #include <string>
 #include <string_view>
+#include <system_error>
 
 namespace crow
 {
@@ -33,11 +34,16 @@
     std::shared_ptr<persistent_data::UserSession> session;
 
     std::string userRole{};
-    Request(
-        boost::beast::http::request<boost::beast::http::string_body> reqIn) :
+    Request(boost::beast::http::request<boost::beast::http::string_body> reqIn,
+            std::error_code& ec) :
         req(std::move(reqIn)),
         fields(req.base()), body(req.body())
-    {}
+    {
+        if (!setUrlInfo())
+        {
+            ec = std::make_error_code(std::errc::invalid_argument);
+        }
+    }
 
     boost::beast::http::verb method() const
     {
@@ -64,6 +70,12 @@
         return req.target();
     }
 
+    bool target(const std::string_view target)
+    {
+        req.target(target);
+        return setUrlInfo();
+    }
+
     unsigned version() const
     {
         return req.version();
@@ -78,6 +90,22 @@
     {
         return req.keep_alive();
     }
+
+  private:
+    bool setUrlInfo()
+    {
+        boost::urls::error_code ec;
+        urlView = boost::urls::parse_relative_ref(
+            boost::urls::string_view(target().data(), target().size()), ec);
+        if (ec)
+        {
+            return false;
+        }
+        url = std::string_view(urlView.encoded_path().data(),
+                               urlView.encoded_path().size());
+        urlParams = urlView.query_params();
+        return true;
+    }
 };
 
 } // namespace crow