commit 608ad2cc0e17bf62ff34bae7ceb499816fe13fbb
author Ed Tanous <ed@tanous.net> Mon May 20 19:14:50 2024 -0700
committer Ed Tanous <ed@tanous.net> Thu Apr 03 16:19:49 2025 +0000
tree 238dce2b2358306cd29b408de650bbafc7e85df6
parent c1a75ebc267a78853fb26a3da8c6b3388e6ee07c

Make Request copy explicit

It is currently too easy to accidentally make copies of the Request
object.  Ideally code would parse out the Request in the first handler,
then no longer require an async copy.  There is one case in the redfish
query things where we actually need a copy of the request object, so we
need these constructors, but we should make them explicit.

This commit moves the Request constructor to be private, and adds a new
method called copy() for explicitly making a copy.  Ironcially, this
finds one place where we were actually making a copy of the request
object unintentionally, so fix that to only capture the value
required,the user session.

Tested:
- Compiles
- Run GET/PATCH related curl or If-Match like PATCH Account
- Redfish Service Validator runs and passes

Change-Id: I19255981f42757ed736112c003201e3f758735ac
Signed-off-by: Ed Tanous <ed@tanous.net>
Signed-off-by: Myung Bae <myungbae@us.ibm.com>

http/http_request.hpp

diff --git a/http/http_request.hpp b/http/http_request.hpp
index bfbe2e3..c2778ca 100644
--- a/http/http_request.hpp
+++ b/http/http_request.hpp
@@ -32,6 +32,8 @@
   private:
     boost::urls::url urlBase;
 
+    Request(const Request& other) = default;
+
   public:
     boost::asio::ip::address ipAddress;
 
@@ -51,13 +53,17 @@
 
     Request() = default;
 
-    Request(const Request& other) = default;
     Request(Request&& other) = default;
 
-    Request& operator=(const Request&) = default;
+    Request& operator=(const Request&) = delete;
     Request& operator=(Request&&) = default;
     ~Request() = default;
 
+    Request copy() const
+    {
+        return {*this};
+    }
+
     void addHeader(std::string_view key, std::string_view value)
     {
         req.insert(key, value);

http/server_sent_event_impl.hpp

diff --git a/http/server_sent_event_impl.hpp b/http/server_sent_event_impl.hpp
index fe9ed2e..5da3a8e 100644
--- a/http/server_sent_event_impl.hpp
+++ b/http/server_sent_event_impl.hpp
@@ -70,7 +70,7 @@
         boost::beast::http::async_write_header(
             adaptor, serial,
             std::bind_front(&ConnectionImpl::sendSSEHeaderCallback, this,
-                            shared_from_this(), req));
+                            shared_from_this(), req.copy()));
     }
 
     void close(const std::string_view msg) override

redfish-core/include/query.hpp

diff --git a/redfish-core/include/query.hpp b/redfish-core/include/query.hpp
index cbe84ab..9629f03 100644
--- a/redfish-core/include/query.hpp
+++ b/redfish-core/include/query.hpp
@@ -106,7 +106,7 @@
     // a full copy to restart it.
     getReqAsyncResp->res.setCompleteRequestHandler(std::bind_front(
         afterIfMatchRequest, std::ref(app), asyncResp,
-        std::make_shared<crow::Request>(req), std::move(ifMatch)));
+        std::make_shared<crow::Request>(req.copy()), std::move(ifMatch)));
 
     app.handle(getReq, getReqAsyncResp);
     return false;

redfish-core/lib/account_service.hpp

diff --git a/redfish-core/lib/account_service.hpp b/redfish-core/lib/account_service.hpp
index 8b312b1..d7f686c 100644
--- a/redfish-core/lib/account_service.hpp
+++ b/redfish-core/lib/account_service.hpp
@@ -2288,7 +2288,8 @@
     crow::connections::systemBus->async_method_call(
         [asyncResp, username, password(std::move(password)),
          roleId(std::move(roleId)), enabled, newUser{std::string(*newUserName)},
-         locked, userSelf, req, accountTypes(std::move(accountTypes))](
+         locked, userSelf, session = req.session,
+         accountTypes(std::move(accountTypes))](
             const boost::system::error_code& ec, sdbusplus::message_t& m) {
             if (ec)
             {
@@ -2298,7 +2299,7 @@
             }
 
             updateUserProperties(asyncResp, newUser, password, enabled, roleId,
-                                 locked, accountTypes, userSelf, req.session);
+                                 locked, accountTypes, userSelf, session);
         },
         "xyz.openbmc_project.User.Manager", "/xyz/openbmc_project/user",
         "xyz.openbmc_project.User.Manager", "RenameUser", username,