Fix nlohmann::json::dump calls
The nlohmann::json::dump call needs to be called with specific arguments
to avoid throwing in failure cases. http connection already does this
properly, but a bunch of code has snuck in (mostly in redfish) that
ignores this, and calls it incorrectly. This can potentially lead to a
crash if the wrong thing throws on invalid UTF8 characters.
This audits the whole codebase, and replaces every dump() call with the
correct dump(2, ' ', true, nlohmann::json::error_handler_t::replace)
call. For correct output, the callers should expect no change, and in
practice, this would require injecting non-utf8 characters into the
BMC.
Tested:
Ran several of the endpoints/error conditions in question, including
some of the error cases. Observed correct responses. I don't know of a
security issue that would allow injecting invalid utf8 into the BMC, but
in theory if it were possible, this would prevent a crash.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I4a15b8e260e3db129bc20484ade4ed5449f75ad0
diff --git a/redfish-core/lib/managers.hpp b/redfish-core/lib/managers.hpp
index 8953d7a..ea0589e 100644
--- a/redfish-core/lib/managers.hpp
+++ b/redfish-core/lib/managers.hpp
@@ -901,7 +901,10 @@
"PositiveHysteresis", doubles["PositiveHysteresis"],
"NegativeHysteresis", doubles["NegativeHysteresis"]))
{
- BMCWEB_LOG_ERROR << "Illegal Property " << it.value().dump();
+ BMCWEB_LOG_ERROR
+ << "Illegal Property "
+ << it.value().dump(2, ' ', true,
+ nlohmann::json::error_handler_t::replace);
return CreatePIDRet::fail;
}
if (zones)
@@ -1007,7 +1010,10 @@
failSafePercent, "MinThermalOutput",
minThermalOutput))
{
- BMCWEB_LOG_ERROR << "Illegal Property " << it.value().dump();
+ BMCWEB_LOG_ERROR
+ << "Illegal Property "
+ << it.value().dump(2, ' ', true,
+ nlohmann::json::error_handler_t::replace);
return CreatePIDRet::fail;
}
@@ -1018,8 +1024,11 @@
if (!redfish::json_util::readJson(*chassisContainer, response->res,
"@odata.id", chassisId))
{
- BMCWEB_LOG_ERROR << "Illegal Property "
- << chassisContainer->dump();
+ BMCWEB_LOG_ERROR
+ << "Illegal Property "
+ << chassisContainer->dump(
+ 2, ' ', true,
+ nlohmann::json::error_handler_t::replace);
return CreatePIDRet::fail;
}
@@ -1056,7 +1065,10 @@
"NegativeHysteresis", negativeHysteresis, "Direction",
direction))
{
- BMCWEB_LOG_ERROR << "Illegal Property " << it.value().dump();
+ BMCWEB_LOG_ERROR
+ << "Illegal Property "
+ << it.value().dump(2, ' ', true,
+ nlohmann::json::error_handler_t::replace);
return CreatePIDRet::fail;
}
@@ -1089,8 +1101,11 @@
if (!redfish::json_util::readJson(step, response->res, "Target",
target, "Output", out))
{
- BMCWEB_LOG_ERROR << "Illegal Property "
- << it.value().dump();
+ BMCWEB_LOG_ERROR
+ << "Illegal Property "
+ << it.value().dump(
+ 2, ' ', true,
+ nlohmann::json::error_handler_t::replace);
return CreatePIDRet::fail;
}
readings.emplace_back(target);
@@ -1331,7 +1346,10 @@
"FanControllers", fanControllers, "FanZones", fanZones,
"StepwiseControllers", stepwiseControllers, "Profile", profile))
{
- BMCWEB_LOG_ERROR << "Illegal Property " << data.dump();
+ BMCWEB_LOG_ERROR
+ << "Illegal Property "
+ << data.dump(2, ' ', true,
+ nlohmann::json::error_handler_t::replace);
return;
}
configuration.emplace_back("PidControllers", std::move(pidControllers));
@@ -1996,7 +2014,10 @@
std::optional<nlohmann::json> openbmc;
if (!redfish::json_util::readJson(*oem, res, "OpenBmc", openbmc))
{
- BMCWEB_LOG_ERROR << "Illegal Property " << oem->dump();
+ BMCWEB_LOG_ERROR
+ << "Illegal Property "
+ << oem->dump(2, ' ', true,
+ nlohmann::json::error_handler_t::replace);
return;
}
if (openbmc)
@@ -2004,7 +2025,11 @@
std::optional<nlohmann::json> fan;
if (!redfish::json_util::readJson(*openbmc, res, "Fan", fan))
{
- BMCWEB_LOG_ERROR << "Illegal Property " << openbmc->dump();
+ BMCWEB_LOG_ERROR
+ << "Illegal Property "
+ << openbmc->dump(
+ 2, ' ', true,
+ nlohmann::json::error_handler_t::replace);
return;
}
if (fan)