Remove nlohmann::json::items()
nlohmann::json::items() throws an exception if the object in question is
not a json object. This has the potential to cause problems, and isn't
in line with the standard that we code against.
Replace all uses of items with iterating the nlohmann::json::object_t.
This adds a new error check for pulling the object_t out of the
nlohmann::json object before each iteration, to ensure that we're
handling errors.
Tested: Redfish service validator passes.
Change-Id: I2934c9450ec296c76544c2a7c5855c9b519eae7f
Signed-off-by: Ed Tanous <ed@tanous.net>
diff --git a/COMMON_ERRORS.md b/COMMON_ERRORS.md
index e114e64..75871ba 100644
--- a/COMMON_ERRORS.md
+++ b/COMMON_ERRORS.md
@@ -81,6 +81,7 @@
- nlohmann::json::get
- nlohmann::json::get_ref
- nlohmann::json::get_to
+- nlohmann::json::items
- nlohmann::json::operator\<\<
- nlohmann::json::operator\>\>
- std::filesystem::create_directory
diff --git a/include/dbus_monitor.hpp b/include/dbus_monitor.hpp
index db75fce..6ec43d0 100644
--- a/include/dbus_monitor.hpp
+++ b/include/dbus_monitor.hpp
@@ -80,20 +80,32 @@
BMCWEB_LOG_ERROR("convertDBusToJSON failed with {}", r);
return 0;
}
-
- if (!data.is_array())
+ nlohmann::json::array_t* arr = data.get_ptr<nlohmann::json::array_t*>();
+ if (arr == nullptr)
+ {
+ BMCWEB_LOG_ERROR("No data in InterfacesAdded signal");
+ return 0;
+ }
+ if (arr->size() < 2)
{
BMCWEB_LOG_ERROR("No data in InterfacesAdded signal");
return 0;
}
- // data is type oa{sa{sv}} which is an array[2] of string, object
- for (const auto& entry : data[1].items())
+ nlohmann::json::object_t* obj =
+ (*arr)[1].get_ptr<nlohmann::json::object_t*>();
+ if (obj == nullptr)
{
- auto it = thisSession->second.interfaces.find(entry.key());
+ BMCWEB_LOG_ERROR("No data in InterfacesAdded signal");
+ return 0;
+ }
+ // data is type oa{sa{sv}} which is an array[2] of string, object
+ for (const auto& entry : *obj)
+ {
+ auto it = thisSession->second.interfaces.find(entry.first);
if (it != thisSession->second.interfaces.end())
{
- json["interfaces"][entry.key()] = entry.value();
+ json["interfaces"][entry.first] = entry.second;
}
}
}
diff --git a/include/event_service_store.hpp b/include/event_service_store.hpp
index f03bd79..f7f03ab 100644
--- a/include/event_service_store.hpp
+++ b/include/event_service_store.hpp
@@ -26,26 +26,27 @@
std::vector<std::string> metricReportDefinitions;
static std::shared_ptr<UserSubscription>
- fromJson(const nlohmann::json& j, const bool loadFromOldConfig = false)
+ fromJson(const nlohmann::json::object_t& j,
+ const bool loadFromOldConfig = false)
{
std::shared_ptr<UserSubscription> subvalue =
std::make_shared<UserSubscription>();
- for (const auto& element : j.items())
+ for (const auto& element : j)
{
- if (element.key() == "Id")
+ if (element.first == "Id")
{
const std::string* value =
- element.value().get_ptr<const std::string*>();
+ element.second.get_ptr<const std::string*>();
if (value == nullptr)
{
continue;
}
subvalue->id = *value;
}
- else if (element.key() == "Destination")
+ else if (element.first == "Destination")
{
const std::string* value =
- element.value().get_ptr<const std::string*>();
+ element.second.get_ptr<const std::string*>();
if (value == nullptr)
{
continue;
@@ -58,63 +59,68 @@
}
subvalue->destinationUrl = std::move(*url);
}
- else if (element.key() == "Protocol")
+ else if (element.first == "Protocol")
{
const std::string* value =
- element.value().get_ptr<const std::string*>();
+ element.second.get_ptr<const std::string*>();
if (value == nullptr)
{
continue;
}
subvalue->protocol = *value;
}
- else if (element.key() == "DeliveryRetryPolicy")
+ else if (element.first == "DeliveryRetryPolicy")
{
const std::string* value =
- element.value().get_ptr<const std::string*>();
+ element.second.get_ptr<const std::string*>();
if (value == nullptr)
{
continue;
}
subvalue->retryPolicy = *value;
}
- else if (element.key() == "Context")
+ else if (element.first == "Context")
{
const std::string* value =
- element.value().get_ptr<const std::string*>();
+ element.second.get_ptr<const std::string*>();
if (value == nullptr)
{
continue;
}
subvalue->customText = *value;
}
- else if (element.key() == "EventFormatType")
+ else if (element.first == "EventFormatType")
{
const std::string* value =
- element.value().get_ptr<const std::string*>();
+ element.second.get_ptr<const std::string*>();
if (value == nullptr)
{
continue;
}
subvalue->eventFormatType = *value;
}
- else if (element.key() == "SubscriptionType")
+ else if (element.first == "SubscriptionType")
{
const std::string* value =
- element.value().get_ptr<const std::string*>();
+ element.second.get_ptr<const std::string*>();
if (value == nullptr)
{
continue;
}
subvalue->subscriptionType = *value;
}
- else if (element.key() == "MessageIds")
+ else if (element.first == "MessageIds")
{
- const auto& obj = element.value();
- for (const auto& val : obj.items())
+ const nlohmann::json::array_t* obj =
+ element.second.get_ptr<const nlohmann::json::array_t*>();
+ if (obj == nullptr)
+ {
+ continue;
+ }
+ for (const auto& val : *obj)
{
const std::string* value =
- val.value().get_ptr<const std::string*>();
+ val.get_ptr<const std::string*>();
if (value == nullptr)
{
continue;
@@ -122,13 +128,18 @@
subvalue->registryMsgIds.emplace_back(*value);
}
}
- else if (element.key() == "RegistryPrefixes")
+ else if (element.first == "RegistryPrefixes")
{
- const auto& obj = element.value();
- for (const auto& val : obj.items())
+ const nlohmann::json::array_t* obj =
+ element.second.get_ptr<const nlohmann::json::array_t*>();
+ if (obj == nullptr)
+ {
+ continue;
+ }
+ for (const auto& val : *obj)
{
const std::string* value =
- val.value().get_ptr<const std::string*>();
+ val.get_ptr<const std::string*>();
if (value == nullptr)
{
continue;
@@ -136,13 +147,18 @@
subvalue->registryPrefixes.emplace_back(*value);
}
}
- else if (element.key() == "ResourceTypes")
+ else if (element.first == "ResourceTypes")
{
- const auto& obj = element.value();
- for (const auto& val : obj.items())
+ const nlohmann::json::array_t* obj =
+ element.second.get_ptr<const nlohmann::json::array_t*>();
+ if (obj == nullptr)
+ {
+ continue;
+ }
+ for (const auto& val : *obj)
{
const std::string* value =
- val.value().get_ptr<const std::string*>();
+ val.get_ptr<const std::string*>();
if (value == nullptr)
{
continue;
@@ -150,29 +166,39 @@
subvalue->resourceTypes.emplace_back(*value);
}
}
- else if (element.key() == "HttpHeaders")
+ else if (element.first == "HttpHeaders")
{
- const auto& obj = element.value();
- for (const auto& val : obj.items())
+ const nlohmann::json::object_t* obj =
+ element.second.get_ptr<const nlohmann::json::object_t*>();
+ if (obj == nullptr)
+ {
+ continue;
+ }
+ for (const auto& val : *obj)
{
const std::string* value =
- val.value().get_ptr<const std::string*>();
+ val.second.get_ptr<const std::string*>();
if (value == nullptr)
{
BMCWEB_LOG_ERROR("Failed to parse value for key{}",
- val.key());
+ val.first);
continue;
}
- subvalue->httpHeaders.set(val.key(), *value);
+ subvalue->httpHeaders.set(val.first, *value);
}
}
- else if (element.key() == "MetricReportDefinitions")
+ else if (element.first == "MetricReportDefinitions")
{
- const auto& obj = element.value();
- for (const auto& val : obj.items())
+ const nlohmann::json::array_t* obj =
+ element.second.get_ptr<const nlohmann::json::array_t*>();
+ if (obj == nullptr)
+ {
+ continue;
+ }
+ for (const auto& val : *obj)
{
const std::string* value =
- val.value().get_ptr<const std::string*>();
+ val.get_ptr<const std::string*>();
if (value == nullptr)
{
continue;
@@ -184,7 +210,7 @@
{
BMCWEB_LOG_ERROR(
"Got unexpected property reading persistent file: {}",
- element.key());
+ element.first);
continue;
}
}
@@ -210,23 +236,23 @@
uint32_t retryAttempts = 3;
uint32_t retryTimeoutInterval = 30;
- void fromJson(const nlohmann::json& j)
+ void fromJson(const nlohmann::json::object_t& j)
{
- for (const auto& element : j.items())
+ for (const auto& element : j)
{
- if (element.key() == "ServiceEnabled")
+ if (element.first == "ServiceEnabled")
{
- const bool* value = element.value().get_ptr<const bool*>();
+ const bool* value = element.second.get_ptr<const bool*>();
if (value == nullptr)
{
continue;
}
enabled = *value;
}
- else if (element.key() == "DeliveryRetryAttempts")
+ else if (element.first == "DeliveryRetryAttempts")
{
const uint64_t* value =
- element.value().get_ptr<const uint64_t*>();
+ element.second.get_ptr<const uint64_t*>();
if ((value == nullptr) ||
(*value > std::numeric_limits<uint32_t>::max()))
{
@@ -234,10 +260,10 @@
}
retryAttempts = static_cast<uint32_t>(*value);
}
- else if (element.key() == "DeliveryRetryIntervalSeconds")
+ else if (element.first == "DeliveryRetryIntervalSeconds")
{
const uint64_t* value =
- element.value().get_ptr<const uint64_t*>();
+ element.second.get_ptr<const uint64_t*>();
if ((value == nullptr) ||
(*value > std::numeric_limits<uint32_t>::max()))
{
diff --git a/include/openbmc_dbus_rest.hpp b/include/openbmc_dbus_rest.hpp
index a2f68b4..1892213 100644
--- a/include/openbmc_dbus_rest.hpp
+++ b/include/openbmc_dbus_rest.hpp
@@ -914,15 +914,21 @@
}
const std::string& keyType = codes[0];
const std::string& valueType = codes[1];
- for (const auto& it : j->items())
+ const nlohmann::json::object_t* arr =
+ j->get_ptr<const nlohmann::json::object_t*>();
+ if (arr == nullptr)
{
- r = convertJsonToDbus(m, keyType, it.key());
+ return -1;
+ }
+ for (const auto& it : *arr)
+ {
+ r = convertJsonToDbus(m, keyType, it.first);
if (r < 0)
{
return r;
}
- r = convertJsonToDbus(m, valueType, it.value());
+ r = convertJsonToDbus(m, valueType, it.second);
if (r < 0)
{
return r;
@@ -1348,21 +1354,23 @@
// Otherwise, make the results an array with every result
// an entry. Could also just fail in that case, but it
// seems better to get the data back somehow.
-
- if (transaction->methodResponse.is_object() && data.is_object())
+ nlohmann::json::object_t* dataobj =
+ data.get_ptr<nlohmann::json::object_t*>();
+ if (transaction->methodResponse.is_object() && dataobj != nullptr)
{
- for (const auto& obj : data.items())
+ for (auto& obj : *dataobj)
{
// Note: Will overwrite the data for a duplicate key
- transaction->methodResponse.emplace(obj.key(),
- std::move(obj.value()));
+ transaction->methodResponse.emplace(obj.first,
+ std::move(obj.second));
}
return;
}
- if (transaction->methodResponse.is_array() && data.is_array())
+ nlohmann::json::array_t* dataarr = data.get_ptr<nlohmann::json::array_t*>();
+ if (transaction->methodResponse.is_array() && dataarr != nullptr)
{
- for (auto& obj : data)
+ for (auto& obj : *dataarr)
{
transaction->methodResponse.emplace_back(std::move(obj));
}
@@ -1768,7 +1776,13 @@
}
else
{
- for (const auto& prop : properties.items())
+ nlohmann::json::object_t* obj =
+ properties.get_ptr<nlohmann::json::object_t*>();
+ if (obj == nullptr)
+ {
+ return;
+ }
+ for (auto& prop : *obj)
{
// if property name is empty, or
// matches our search query, add it
@@ -1776,12 +1790,12 @@
if (propertyName->empty())
{
- (*response)[prop.key()] =
- std::move(prop.value());
+ (*response)[prop.first] =
+ std::move(prop.second);
}
- else if (prop.key() == *propertyName)
+ else if (prop.first == *propertyName)
{
- *response = std::move(prop.value());
+ *response = std::move(prop.second);
}
}
}
diff --git a/include/persistent_data.hpp b/include/persistent_data.hpp
index 048d987..3bd256d 100644
--- a/include/persistent_data.hpp
+++ b/include/persistent_data.hpp
@@ -61,14 +61,20 @@
}
else
{
- for (const auto& item : data.items())
+ const nlohmann::json::object_t* obj =
+ data.get_ptr<nlohmann::json::object_t*>();
+ if (obj == nullptr)
{
- if (item.key() == "revision")
+ return;
+ }
+ for (const auto& item : *obj)
+ {
+ if (item.first == "revision")
{
fileRevision = 0;
const uint64_t* uintPtr =
- item.value().get_ptr<const uint64_t*>();
+ item.second.get_ptr<const uint64_t*>();
if (uintPtr == nullptr)
{
BMCWEB_LOG_ERROR("Failed to read revision flag");
@@ -78,24 +84,24 @@
fileRevision = *uintPtr;
}
}
- else if (item.key() == "system_uuid")
+ else if (item.first == "system_uuid")
{
const std::string* jSystemUuid =
- item.value().get_ptr<const std::string*>();
+ item.second.get_ptr<const std::string*>();
if (jSystemUuid != nullptr)
{
systemUuid = *jSystemUuid;
}
}
- else if (item.key() == "auth_config")
+ else if (item.first == "auth_config")
{
SessionStore::getInstance()
.getAuthMethodsConfig()
- .fromJson(item.value());
+ .fromJson(item.second);
}
- else if (item.key() == "sessions")
+ else if (item.first == "sessions")
{
- for (const auto& elem : item.value())
+ for (const auto& elem : item.second)
{
std::shared_ptr<UserSession> newSession =
UserSession::fromJson(elem);
@@ -115,10 +121,10 @@
newSession->sessionToken, newSession);
}
}
- else if (item.key() == "timeout")
+ else if (item.first == "timeout")
{
const int64_t* jTimeout =
- item.value().get_ptr<int64_t*>();
+ item.second.get_ptr<const int64_t*>();
if (jTimeout == nullptr)
{
BMCWEB_LOG_DEBUG(
@@ -131,15 +137,25 @@
SessionStore::getInstance().updateSessionTimeout(
sessionTimeoutInseconds);
}
- else if (item.key() == "eventservice_config")
+ else if (item.first == "eventservice_config")
{
+ const nlohmann::json::object_t* esobj =
+ item.second
+ .get_ptr<const nlohmann::json::object_t*>();
+ if (esobj == nullptr)
+ {
+ BMCWEB_LOG_DEBUG(
+ "Problem reading EventService value");
+ continue;
+ }
+
EventServiceStore::getInstance()
.getEventServiceConfig()
- .fromJson(item.value());
+ .fromJson(*esobj);
}
- else if (item.key() == "subscriptions")
+ else if (item.first == "subscriptions")
{
- for (const auto& elem : item.value())
+ for (const auto& elem : item.second)
{
std::shared_ptr<UserSubscription> newSubscription =
UserSubscription::fromJson(elem);
diff --git a/include/sessions.hpp b/include/sessions.hpp
index 10e29c8..5621fff 100644
--- a/include/sessions.hpp
+++ b/include/sessions.hpp
@@ -60,42 +60,43 @@
* @return a shared pointer if data has been loaded properly, nullptr
* otherwise
*/
- static std::shared_ptr<UserSession> fromJson(const nlohmann::json& j)
+ static std::shared_ptr<UserSession>
+ fromJson(const nlohmann::json::object_t& j)
{
std::shared_ptr<UserSession> userSession =
std::make_shared<UserSession>();
- for (const auto& element : j.items())
+ for (const auto& element : j)
{
const std::string* thisValue =
- element.value().get_ptr<const std::string*>();
+ element.second.get_ptr<const std::string*>();
if (thisValue == nullptr)
{
BMCWEB_LOG_ERROR(
"Error reading persistent store. Property {} was not of type string",
- element.key());
+ element.first);
continue;
}
- if (element.key() == "unique_id")
+ if (element.first == "unique_id")
{
userSession->uniqueId = *thisValue;
}
- else if (element.key() == "session_token")
+ else if (element.first == "session_token")
{
userSession->sessionToken = *thisValue;
}
- else if (element.key() == "csrf_token")
+ else if (element.first == "csrf_token")
{
userSession->csrfToken = *thisValue;
}
- else if (element.key() == "username")
+ else if (element.first == "username")
{
userSession->username = *thisValue;
}
- else if (element.key() == "client_id")
+ else if (element.first == "client_id")
{
userSession->clientId = *thisValue;
}
- else if (element.key() == "client_ip")
+ else if (element.first == "client_ip")
{
userSession->clientIp = *thisValue;
}
@@ -104,7 +105,7 @@
{
BMCWEB_LOG_ERROR(
"Got unexpected property reading persistent file: {}",
- element.key());
+ element.first);
continue;
}
}
@@ -141,33 +142,33 @@
bool cookie = BMCWEB_COOKIE_AUTH;
bool tls = BMCWEB_MUTUAL_TLS_AUTH;
- void fromJson(const nlohmann::json& j)
+ void fromJson(const nlohmann::json::object_t& j)
{
- for (const auto& element : j.items())
+ for (const auto& element : j)
{
- const bool* value = element.value().get_ptr<const bool*>();
+ const bool* value = element.second.get_ptr<const bool*>();
if (value == nullptr)
{
continue;
}
- if (element.key() == "XToken")
+ if (element.first == "XToken")
{
xtoken = *value;
}
- else if (element.key() == "Cookie")
+ else if (element.first == "Cookie")
{
cookie = *value;
}
- else if (element.key() == "SessionToken")
+ else if (element.first == "SessionToken")
{
sessionToken = *value;
}
- else if (element.key() == "BasicAuth")
+ else if (element.first == "BasicAuth")
{
basic = *value;
}
- else if (element.key() == "TLS")
+ else if (element.first == "TLS")
{
tls = *value;
}
diff --git a/redfish-core/include/event_service_manager.hpp b/redfish-core/include/event_service_manager.hpp
index 6ad8de3..4c2d0ee 100644
--- a/redfish-core/include/event_service_manager.hpp
+++ b/redfish-core/include/event_service_manager.hpp
@@ -614,17 +614,19 @@
return;
}
- for (const auto& item : jsonData.items())
+ const nlohmann::json::object_t* obj =
+ jsonData.get_ptr<const nlohmann::json::object_t*>();
+ for (const auto& item : *obj)
{
- if (item.key() == "Configuration")
+ if (item.first == "Configuration")
{
persistent_data::EventServiceStore::getInstance()
.getEventServiceConfig()
- .fromJson(item.value());
+ .fromJson(item.second);
}
- else if (item.key() == "Subscriptions")
+ else if (item.first == "Subscriptions")
{
- for (const auto& elem : item.value())
+ for (const auto& elem : item.second)
{
std::shared_ptr<persistent_data::UserSubscription>
newSubscription =
diff --git a/redfish-core/include/utils/json_utils.hpp b/redfish-core/include/utils/json_utils.hpp
index a013847..f6ac70b 100644
--- a/redfish-core/include/utils/json_utils.hpp
+++ b/redfish-core/include/utils/json_utils.hpp
@@ -282,7 +282,9 @@
}
else if constexpr (IsStdArray<Type>::value)
{
- if (!jsonValue.is_array())
+ nlohmann::json::array_t* arr =
+ jsonValue.get_ptr<nlohmann::json::array_t*>();
+ if (arr == nullptr)
{
messages::propertyValueTypeError(res, res.jsonValue, key);
return false;
@@ -293,25 +295,27 @@
return false;
}
size_t index = 0;
- for (const auto& val : jsonValue.items())
+ for (auto& val : *arr)
{
- ret = unpackValue<typename Type::value_type>(val.value(), key, res,
+ ret = unpackValue<typename Type::value_type>(val, key, res,
value[index++]) &&
ret;
}
}
else if constexpr (IsVector<Type>::value)
{
- if (!jsonValue.is_array())
+ nlohmann::json::array_t* arr =
+ jsonValue.get_ptr<nlohmann::json::array_t*>();
+ if (arr == nullptr)
{
messages::propertyValueTypeError(res, res.jsonValue, key);
return false;
}
- for (const auto& val : jsonValue.items())
+ for (auto& val : *arr)
{
value.emplace_back();
- ret = unpackValue<typename Type::value_type>(val.value(), key, res,
+ ret = unpackValue<typename Type::value_type>(val, key, res,
value.back()) &&
ret;
}
@@ -364,7 +368,9 @@
}
else if constexpr (IsStdArray<Type>::value)
{
- if (!jsonValue.is_array())
+ nlohmann::json::array_t* arr =
+ jsonValue.get_ptr<nlohmann::json::array_t*>();
+ if (arr == nullptr)
{
return false;
}
@@ -373,24 +379,26 @@
return false;
}
size_t index = 0;
- for (const auto& val : jsonValue.items())
+ for (const auto& val : *arr)
{
- ret = unpackValue<typename Type::value_type>(val.value(), key,
+ ret = unpackValue<typename Type::value_type>(val, key,
value[index++]) &&
ret;
}
}
else if constexpr (IsVector<Type>::value)
{
- if (!jsonValue.is_array())
+ nlohmann::json::array_t* arr =
+ jsonValue.get_ptr<nlohmann::json::array_t*>();
+ if (arr == nullptr)
{
return false;
}
- for (const auto& val : jsonValue.items())
+ for (const auto& val : *arr)
{
value.emplace_back();
- ret = unpackValue<typename Type::value_type>(val.value(), key,
+ ret = unpackValue<typename Type::value_type>(val, key,
value.back()) &&
ret;
}
diff --git a/redfish-core/src/utils/json_utils.cpp b/redfish-core/src/utils/json_utils.cpp
index 8d12bad..8017b36 100644
--- a/redfish-core/src/utils/json_utils.cpp
+++ b/redfish-core/src/utils/json_utils.cpp
@@ -90,7 +90,7 @@
if (object != nullptr)
{
uint64_t sum = 0;
- for (const auto& [k, v] : root.items())
+ for (const auto& [k, v] : *object)
{
constexpr uint64_t colonQuoteSpaceSize = 4;
sum += k.size() + getEstimatedJsonSize(v) + colonQuoteSpaceSize;