search

openbmc / bmcweb

A do everything Redfish, KVM, GUI, and DBus webserver for OpenBMC
Apache License 2.0
156 stars 131 forks source link

Bad json utils function #139

Closed edtanous closed 3 years ago

edtanous commented 4 years ago

https://github.com/openbmc/bmcweb/blob/01c6e8581bf44551bbb24d156fc191f87b1cdeea/redfish-core/include/utils/json_utils.hpp#L426

The following function seems to have been injected recently. Aside from it being more code to write than the equivalent code it's attempting to replace, it's wrong in a couple different ways. Given that it's only used in one module thusfar, I suspect that it needs to just be replaced with the equivalent nlohmann code for that places where it's used

  1. jsonData is not passed by const reference.
  2. Key should be passed by std::string_view, not const string&
  3. line 429 is creating a null key in the jsonData structure, then making a complete copy of the value. #1 would've prevented this, and is probably why it was emitted. what I suspect is that this operation was attempting to do a find, not a operator[].
  4. line 430 "is_null()" is not an appropriate check to do to determine presence. Null values are valid input in json.

In all the places where this function is used, the json parser can be written more efficiently as a simple loop.

gtmills commented 3 years ago

https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/37380 is a potential fix