Add type safety for NTP server objects
NTPServers is our last usage of nlohmann::json in a readJson unpack.
The capability and unit tests are left in place for that type in case we
need them in the future, but for now, document them as deprecated.
Tested: Redfish service validator passes. Redfish protocol validator
passes most tests (1 known failure in SSE is unrelated to this change).
Change-Id: If4b2ea061a941cc23d47189af7ff453094dc7dca
Signed-off-by: Ed Tanous <ed@tanous.net>
diff --git a/redfish-core/include/utils/json_utils.hpp b/redfish-core/include/utils/json_utils.hpp
index d7d87a6..a013847 100644
--- a/redfish-core/include/utils/json_utils.hpp
+++ b/redfish-core/include/utils/json_utils.hpp
@@ -151,7 +151,7 @@
{
if constexpr (Index < std::variant_size_v<std::variant<Args...>>)
{
- std::variant_alternative_t<Index, std::variant<Args...>> type;
+ std::variant_alternative_t<Index, std::variant<Args...>> type{};
UnpackErrorCode unpack = unpackValueWithErrorCode(j, key, type);
if (unpack == UnpackErrorCode::success)
{
@@ -235,6 +235,23 @@
return UnpackErrorCode::invalidType;
}
}
+ else if constexpr (IsVector<Type>::value)
+ {
+ nlohmann::json::object_t* obj =
+ jsonValue.get_ptr<nlohmann::json::object_t*>();
+ if (obj == nullptr)
+ {
+ return UnpackErrorCode::invalidType;
+ }
+
+ for (const auto& val : *obj)
+ {
+ value.emplace_back();
+ ret = unpackValueWithErrorCode<typename Type::value_type>(
+ val, key, value.back()) &&
+ ret;
+ }
+ }
else
{
using JsonType = std::add_const_t<std::add_pointer_t<Type>>;
@@ -403,7 +420,6 @@
bool*,
double*,
std::string*,
- nlohmann::json*,
nlohmann::json::object_t*,
std::variant<std::string, std::nullptr_t>*,
std::variant<uint8_t, std::nullptr_t>*,
@@ -425,7 +441,6 @@
//std::vector<bool>*,
std::vector<double>*,
std::vector<std::string>*,
- std::vector<nlohmann::json>*,
std::vector<nlohmann::json::object_t>*,
std::optional<uint8_t>*,
std::optional<uint16_t>*,
@@ -437,7 +452,6 @@
std::optional<bool>*,
std::optional<double>*,
std::optional<std::string>*,
- std::optional<nlohmann::json>*,
std::optional<nlohmann::json::object_t>*,
std::optional<std::vector<uint8_t>>*,
std::optional<std::vector<uint16_t>>*,
@@ -449,7 +463,6 @@
//std::optional<std::vector<bool>>*,
std::optional<std::vector<double>>*,
std::optional<std::vector<std::string>>*,
- std::optional<std::vector<nlohmann::json>>*,
std::optional<std::vector<nlohmann::json::object_t>>*,
std::optional<std::variant<std::string, std::nullptr_t>>*,
std::optional<std::variant<uint8_t, std::nullptr_t>>*,
@@ -462,7 +475,15 @@
std::optional<std::variant<double, std::nullptr_t>>*,
std::optional<std::variant<bool, std::nullptr_t>>*,
std::optional<std::vector<std::variant<nlohmann::json::object_t, std::nullptr_t>>>*,
- std::optional<std::variant<nlohmann::json::object_t, std::nullptr_t>>*
+ std::optional<std::vector<std::variant<std::string, nlohmann::json::object_t, std::nullptr_t>>>*,
+
+ // Note, these types are kept for historical completeness, but should not be used,
+ // As they do not provide object type safety. Instead, rely on nlohmann::json::object_t
+ // Will be removed Q2 2025
+ nlohmann::json*,
+ std::optional<std::vector<nlohmann::json>>*,
+ std::vector<nlohmann::json>*,
+ std::optional<nlohmann::json>*
>;
// clang-format on
diff --git a/redfish-core/lib/network_protocol.hpp b/redfish-core/lib/network_protocol.hpp
index 00b5d9e..747abbf 100644
--- a/redfish-core/lib/network_protocol.hpp
+++ b/redfish-core/lib/network_protocol.hpp
@@ -259,17 +259,24 @@
"TimeSyncMethod", "NTP/ProtocolEnabled", timeSyncMethod);
}
+// Redfish states that ip addresses can be
+// string, to set a value
+// null, to delete the value
+// object_t, empty json object, to ignore the value
+using IpAddress =
+ std::variant<std::string, nlohmann::json::object_t, std::nullptr_t>;
+
inline void
handleNTPServersPatch(const std::shared_ptr<bmcweb::AsyncResp>& asyncResp,
- const std::vector<nlohmann::json>& ntpServerObjects,
+ const std::vector<IpAddress>& ntpServerObjects,
std::vector<std::string> currentNtpServers)
{
std::vector<std::string>::iterator currentNtpServer =
currentNtpServers.begin();
for (size_t index = 0; index < ntpServerObjects.size(); index++)
{
- const nlohmann::json& ntpServer = ntpServerObjects[index];
- if (ntpServer.is_null())
+ const IpAddress& ntpServer = ntpServerObjects[index];
+ if (std::holds_alternative<std::nullptr_t>(ntpServer))
{
// Can't delete an item that doesn't exist
if (currentNtpServer == currentNtpServers.end())
@@ -284,22 +291,22 @@
continue;
}
const nlohmann::json::object_t* ntpServerObject =
- ntpServer.get_ptr<const nlohmann::json::object_t*>();
+ std::get_if<nlohmann::json::object_t>(&ntpServer);
if (ntpServerObject != nullptr)
{
if (!ntpServerObject->empty())
{
- messages::propertyValueNotInList(asyncResp->res, ntpServer,
- "NTP/NTPServers/" +
- std::to_string(index));
+ messages::propertyValueNotInList(
+ asyncResp->res, *ntpServerObject,
+ "NTP/NTPServers/" + std::to_string(index));
return;
}
// Can't retain an item that doesn't exist
if (currentNtpServer == currentNtpServers.end())
{
- messages::propertyValueOutOfRange(asyncResp->res, ntpServer,
- "NTP/NTPServers/" +
- std::to_string(index));
+ messages::propertyValueOutOfRange(
+ asyncResp->res, *ntpServerObject,
+ "NTP/NTPServers/" + std::to_string(index));
return;
}
@@ -308,13 +315,10 @@
continue;
}
- const std::string* ntpServerStr =
- ntpServer.get_ptr<const std::string*>();
+ const std::string* ntpServerStr = std::get_if<std::string>(&ntpServer);
if (ntpServerStr == nullptr)
{
- messages::propertyValueTypeError(asyncResp->res, ntpServer,
- "NTP/NTPServers/" +
- std::to_string(index));
+ messages::internalError(asyncResp->res);
return;
}
if (currentNtpServer == currentNtpServers.end())
@@ -470,7 +474,8 @@
return;
}
std::optional<std::string> newHostName;
- std::optional<std::vector<nlohmann::json>> ntpServerObjects;
+
+ std::optional<std::vector<IpAddress>> ntpServerObjects;
std::optional<bool> ntpEnabled;
std::optional<bool> ipmiEnabled;
std::optional<bool> sshEnabled;