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>
diff --git a/include/dbus_privileges.hpp b/include/dbus_privileges.hpp
index a58f9be..4cafe52 100644
--- a/include/dbus_privileges.hpp
+++ b/include/dbus_privileges.hpp
@@ -106,19 +106,18 @@
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 @@
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 @@
// 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);