diff options
author | Lei YU <yulei.sh@bytedance.com> | 2023-03-17 08:17:05 +0300 |
---|---|---|
committer | Lei YU <yulei.sh@bytedance.com> | 2023-03-17 08:48:06 +0300 |
commit | 28dd5ca15a3008e50e5046602464966720d6b5f3 (patch) | |
tree | 35450188f069e1f164ef55fc32e160ca210d14de | |
parent | f5892d0d56accc51deb5e4ad1bbdad6028784c16 (diff) | |
download | bmcweb-28dd5ca15a3008e50e5046602464966720d6b5f3.tar.xz |
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
-rw-r--r-- | include/openbmc_dbus_rest.hpp | 39 |
1 files changed, 20 insertions, 19 deletions
diff --git a/include/openbmc_dbus_rest.hpp b/include/openbmc_dbus_rest.hpp index 98dfb843fa..5bdc4ec0b7 100644 --- a/include/openbmc_dbus_rest.hpp +++ b/include/openbmc_dbus_rest.hpp @@ -477,7 +477,9 @@ inline void getObjectAndEnumerate( // 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 @@ struct InProgressActionData // * 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 @@ struct InProgressActionData 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 @@ struct InProgressActionData 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 @@ inline void findActionOnInterface( 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 @@ inline void handleAction(const crow::Request& req, 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 @@ inline void handleAction(const crow::Request& req, 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 @@ inline void handleDelete(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp, 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 @@ inline void 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; |