dbus_rest: Fix dangling reference of crow::Response
The openbmc_dbus_reset was holding reference of `crow::Response`, set
the response in `~InProgressActionData()`, and call res.end() to
complete the result of the response.
The bmcweb code now uses `std::shared_ptr<AsyncResp>` for the response
and the `res.end()` is handled in `~AsyncResp()`.
By using the reference of `crow::Response`, the `InProgressActionData`
is actually using a dangling reference because the
`std::shared_ptr<AsyncResp>` is already destructed, and bmcweb will
crash on `action` calls, or not crash but get invalid response, as it's
undefined behavior.
Fix the above issue by using `std::shared_ptr<AsyncResp>` to make sure
the response is correctly handled.
Tested:
1. Without the fix, bmcweb crashes, or get no json output response on
the below method call, be noted that it's an invalid call:
```
$ curl -k -H "X-Auth-Token: $token" -x POST -d '{"data": []}' https://${bmc}/xyz/openbmc_project/logging/action/deleteAll
```
2. With the fix, bmcweb gives expected response:
```
$ curl -k -H "X-Auth-Token: $token" -x POST -d '{"data": []}' https://${bmc}/xyz/openbmc_project/logging/action/deleteAll
{
"data": {
"description": "The specified method cannot be found"
},
"message": "404 Not Found",
"status": "error"
}
$ curl -k -H "X-Auth-Token: $token" -x POST -d '{"data": []}' https://${bmc}/xyz/openbmc_project/logging/action/DeleteAll
{
"data": null,
"message": "200 OK",
"status": "ok"
}
```
Signed-off-by: Lei YU <yulei.sh@bytedance.com>
Change-Id: I38ef34fe8ff18e4e127664c853c6792461f6edf8
diff --git a/include/openbmc_dbus_rest.hpp b/include/openbmc_dbus_rest.hpp
index 98dfb84..5bdc4ec 100644
--- a/include/openbmc_dbus_rest.hpp
+++ b/include/openbmc_dbus_rest.hpp
@@ -477,7 +477,9 @@
// Structure for storing data on an in progress action
struct InProgressActionData
{
- explicit InProgressActionData(crow::Response& resIn) : res(resIn)
+ explicit InProgressActionData(
+ const std::shared_ptr<bmcweb::AsyncResp>& res) :
+ asyncResp(res)
{}
~InProgressActionData()
{
@@ -492,13 +494,14 @@
// * if output processing didn't fail, return the data
// Only deal with method returns if nothing failed earlier
- if (res.result() == boost::beast::http::status::ok)
+ if (asyncResp->res.result() == boost::beast::http::status::ok)
{
if (!methodPassed)
{
if (!methodFailed)
{
- setErrorResponse(res, boost::beast::http::status::not_found,
+ setErrorResponse(asyncResp->res,
+ boost::beast::http::status::not_found,
methodNotFoundDesc, notFoundMsg);
}
}
@@ -507,19 +510,18 @@
if (outputFailed)
{
setErrorResponse(
- res, boost::beast::http::status::internal_server_error,
+ asyncResp->res,
+ boost::beast::http::status::internal_server_error,
"Method output failure", methodOutputFailedMsg);
}
else
{
- res.jsonValue["status"] = "ok";
- res.jsonValue["message"] = "200 OK";
- res.jsonValue["data"] = methodResponse;
+ asyncResp->res.jsonValue["status"] = "ok";
+ asyncResp->res.jsonValue["message"] = "200 OK";
+ asyncResp->res.jsonValue["data"] = methodResponse;
}
}
}
-
- res.end();
}
InProgressActionData(const InProgressActionData&) = delete;
InProgressActionData(InProgressActionData&&) = delete;
@@ -528,10 +530,11 @@
void setErrorStatus(const std::string& desc)
{
- setErrorResponse(res, boost::beast::http::status::bad_request, desc,
+ setErrorResponse(asyncResp->res,
+ boost::beast::http::status::bad_request, desc,
badReqMsg);
}
- crow::Response& res;
+ std::shared_ptr<bmcweb::AsyncResp> asyncResp;
std::string path;
std::string methodName;
std::string interfaceName;
@@ -1505,14 +1508,14 @@
if (e != nullptr)
{
setErrorResponse(
- transaction->res,
+ transaction->asyncResp->res,
boost::beast::http::status::bad_request,
e->name, e->message);
}
else
{
setErrorResponse(
- transaction->res,
+ transaction->asyncResp->res,
boost::beast::http::status::bad_request,
"Method call failed", methodFailedMsg);
}
@@ -1574,7 +1577,7 @@
badReqMsg);
return;
}
- auto transaction = std::make_shared<InProgressActionData>(asyncResp->res);
+ auto transaction = std::make_shared<InProgressActionData>(asyncResp);
transaction->path = objectPath;
transaction->methodName = methodName;
@@ -1588,7 +1591,7 @@
if (ec || interfaceNames.empty())
{
BMCWEB_LOG_ERROR << "Can't find object";
- setErrorResponse(transaction->res,
+ setErrorResponse(transaction->asyncResp->res,
boost::beast::http::status::not_found,
notFoundDesc, notFoundMsg);
return;
@@ -1625,8 +1628,7 @@
return;
}
- auto transaction =
- std::make_shared<InProgressActionData>(asyncResp->res);
+ auto transaction = std::make_shared<InProgressActionData>(asyncResp);
transaction->path = objectPath;
transaction->methodName = "Delete";
transaction->interfaceName = "xyz.openbmc_project.Object.Delete";
@@ -2416,8 +2418,7 @@
asyncResp->res.result(boost::beast::http::status::bad_request);
return;
}
- auto transaction =
- std::make_shared<InProgressActionData>(asyncResp->res);
+ auto transaction = std::make_shared<InProgressActionData>(asyncResp);
transaction->path = objectPath;
transaction->methodName = methodName;