diff options
author | Jonathan Doman <jonathan.doman@intel.com> | 2024-04-16 02:56:23 +0300 |
---|---|---|
committer | Ed Tanous <ed@tanous.net> | 2024-05-03 18:24:02 +0300 |
commit | 102a4cdacb0a6d8c8c3c97e10bedbb66000ac5dc (patch) | |
tree | 56eef176e8bd7446995556b90c8216e4a88a701e /include | |
parent | 1aa375b80075c7e1acdc9188440a62bab21b8651 (diff) | |
download | bmcweb-102a4cdacb0a6d8c8c3c97e10bedbb66000ac5dc.tar.xz |
Manage Request with shared_ptr
This is an attempt to solve a class of use-after-move bugs on the
Request objects which have popped up several times. This more clearly
identifies code which owns the Request objects and has a need to keep it
alive. Currently it's just the `Connection` (or `HTTP2Connection`)
(which needs to access Request headers while sending the response), and
the `validatePrivilege()` function (which needs to temporarily own the
Request while doing an asynchronous D-Bus call). Route handlers are
provided a non-owning `Request&` for immediate use and required to not
hold the `Request&` for future use.
Tested: Redfish validator passes (with a few unrelated fails).
Redfish URLs are sent to a browser as HTML instead of raw JSON.
Change-Id: Id581fda90b6bceddd08a5dc7ff0a04b91e7394bf
Signed-off-by: Jonathan Doman <jonathan.doman@intel.com>
Signed-off-by: Ed Tanous <ed@tanous.net>
Diffstat (limited to 'include')
-rw-r--r-- | include/dbus_privileges.hpp | 35 |
1 files changed, 18 insertions, 17 deletions
diff --git a/include/dbus_privileges.hpp b/include/dbus_privileges.hpp index a58f9bebd0..4cafe52ec6 100644 --- a/include/dbus_privileges.hpp +++ b/include/dbus_privileges.hpp @@ -106,19 +106,18 @@ inline bool return true; } -template <typename CallbackFn> -void afterGetUserInfo(Request& req, - const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, - BaseRule& rule, CallbackFn callback, - const boost::system::error_code& ec, - const dbus::utility::DBusPropertiesMap& userInfoMap) +inline bool + afterGetUserInfo(Request& req, + const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, + BaseRule& rule, const boost::system::error_code& ec, + const dbus::utility::DBusPropertiesMap& userInfoMap) { if (ec) { BMCWEB_LOG_ERROR("GetUserInfo failed..."); asyncResp->res.result( boost::beast::http::status::internal_server_error); - return; + return false; } if (!populateUserInfo(req, userInfoMap)) @@ -126,7 +125,7 @@ void afterGetUserInfo(Request& req, BMCWEB_LOG_ERROR("Failed to populate user information"); asyncResp->res.result( boost::beast::http::status::internal_server_error); - return; + return false; } if (!isUserPrivileged(req, asyncResp, rule)) @@ -134,28 +133,30 @@ void afterGetUserInfo(Request& req, // User is not privileged BMCWEB_LOG_ERROR("Insufficient Privilege"); asyncResp->res.result(boost::beast::http::status::forbidden); - return; + return false; } - callback(req); + + return true; } template <typename CallbackFn> -void validatePrivilege(Request& req, +void validatePrivilege(const std::shared_ptr<Request>& req, const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, BaseRule& rule, CallbackFn&& callback) { - if (req.session == nullptr) + if (req->session == nullptr) { return; } - std::string username = req.session->username; + std::string username = req->session->username; crow::connections::systemBus->async_method_call( - [req{std::move(req)}, asyncResp, &rule, - callback = std::forward<CallbackFn>(callback)]( + [req, asyncResp, &rule, callback = std::forward<CallbackFn>(callback)]( const boost::system::error_code& ec, const dbus::utility::DBusPropertiesMap& userInfoMap) mutable { - afterGetUserInfo(req, asyncResp, rule, - std::forward<CallbackFn>(callback), ec, userInfoMap); + if (afterGetUserInfo(*req, asyncResp, rule, ec, userInfoMap)) + { + callback(); + } }, "xyz.openbmc_project.User.Manager", "/xyz/openbmc_project/user", "xyz.openbmc_project.User.Manager", "GetUserInfo", username); |