summaryrefslogtreecommitdiff
path: root/redfish-core/include/utils/json_utils.hpp
AgeCommit message (Collapse)AuthorFilesLines
2025-07-25Work around nlohmann changesEd Tanous1-18/+40
https://github.com/nlohmann/json/issues/4475 recently changed behavior that we rely on in a lot of places, and unit tests appear to have caught the failure. Functionally, this changes the readJson* class of values to attempt to read as the type requested first, then attempt to read as the opposite int/uint type requested, with a range check. In addition, the range check functions now need updated to handle comparisons between non-similar value types. Luckily c++20 added cmp_less/greater type functions that we can make use of. Tested: unit tests pass. Change-Id: If114bd55225a3a9948e80a407b19b69f50d048b6 Signed-off-by: Ed Tanous <etanous@nvidia.com>
2025-04-03OEM Route Handling Infrastructurerohitpai1-0/+15
Goal of the MR is to provide infrastructure support in bmcweb to manage the OEM fragment handling separately. OEM schema are vendor defined and per DMTF resource we could have multiple vendor defined OEM schema to be enabled. The feature allows registration of route handler per schema per OEM namespace. Example ``` REDFISH_SUB_ROUTE<"/redfish/v1/Managers/<str>/#/Oem/OpenBmc">(service, HttpVerb::Get)(oemOpenBmcCallback); REDFISH_SUB_ROUTE<"/redfish/v1/Managers/<str>/#/Oem/Nvidia">(service, HttpVerb::Get)(oemNidiaCallback); ``` We can have separate vendor defined route handlers per resource. Each of these route handlers can populate their own vendor specific OEM data. The OEM code can be better organized and enabled/disabled as per the platform needs. The current MR has the code changes related to handling GET requests alone. The feature only supports requests where the response payload is JSON. Tests - All UT cases passes - New UT added for RF OEM router passes - Service Validator passes on qemu - GET Response on Manager/bmc resource contains the OEM fragment ``` curl -c cjar -b cjar -k -X GET https://127.0.0.1:2443/redfish/v1/Managers/bmc { "@odata.id": "/redfish/v1/Managers/bmc", "@odata.type": "#Manager.v1_14_0.Manager", "Oem": { "OpenBmc": { "@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc", "@odata.type": "#OpenBMCManager.v1_0_0.Manager", "Certificates": { "@odata.id": "/redfish/v1/Managers/bmc/Truststore/Certificates" } } }, "UUID": "40575e98-90d7-4c10-9eb5-8d8a7156c9b9" } ``` Change-Id: Ic82aa5fe760eda31e2792fbdfb6884ac3ea613dc Signed-off-by: Rohit PAI <rohitpai77@gmail.com>
2025-02-13Remove nlohmann::json::object helpersEd Tanous1-21/+4
readJsonHelper existed back before the distinction between nlohmann::json and nlohmann::json::object_t was understood. This commit cleans up our sub-value parsing by removing readJsonHelper that accepts a nlohmann::json overload, and always accepting via an object_t overload. Functionally, these two are identical, given that readJsonHelper only did a type check that was redundant. Tested: Unit tests pass. Good coverage of these methods. Change-Id: I734d956dd4bc2ddb14f6e3c735e15adf1f7e00a0 Signed-off-by: Ed Tanous <ed@tanous.net>
2025-02-05Fix Message arg error in JSON Patchrohitpai1-3/+3
When array/vector object is expected in JSON patch the error info does not contain the actual wrong property instead shows "null". Fix is to correct the value in the error info. Tested - add new test case to verify this - unit tests are passing. Change-Id: Ica26ac9e501b5a34a5b118769cc1917eeab30524 Signed-off-by: rohitpai <rohitpai77@gmail.com>
2025-02-04clang-format: update latest spec and reformatPatrick Williams1-2/+2
Copy the latest format file from the docs repository and apply. Change-Id: I2f0b9d0fb6e01ed36a2f34c750ba52de3b6d15d1 Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
2025-01-31Fix includesEd Tanous1-1/+0
Clang-tidy misc-include-cleaner appears to now be enforcing significantly more headers than previously. That is overall a good thing, but forces us to fix some issues. This commit is largely just taking the clang-recommended fixes and checking them in. Subsequent patches will fix the more unique issues. Note, that a number of new ignores are added into the .clang-tidy file. These can be cleaned up over time as they're understood. The majority are places where boost includes a impl/x.hpp and x.hpp, but expects you to use the later. include-cleaner opts for the impl, but it isn't clear why. Change-Id: Id3fdd7ee6df6c33b2fd35626898523048dd51bfb Signed-off-by: Ed Tanous <etanous@nvidia.com> Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
2025-01-26$filter Query parameter support for nested keys addedChandramohan Harkude1-0/+35
Implemented the code to identify the '/' character in the key and perform the level by level search Testing : Tested query parameter with path separated by / example curl -k -u root:0penBmc https://<IP>/redfish/v1/Systems/ Baseboard/LogServices/FaultLog/Entries?$filter=CPER/Oem/ OEM/IpSignature eq 'DRAM-CHANNELS' Results having 'DRAM-CHANNELS' in nested path "CPER/Oem/ OEM/IpSignature" are listed. Change-Id: Ie6cf796026a29ec7a3e8a0366bbfd0c658d0ac7e Signed-off-by: Chandramohan Harkude <chandramohan.harkude@gmail.com>
2025-01-20Use SPDX identifiersEd Tanous1-15/+3
SPDX identifiers are simpler, and reduce the amount of cruft we have in code files. They are recommended by linux foundation, and therefore we should do as they allow. This patchset does not intend to modify any intent on any existing copyrights or licenses, only to standardize their inclusion. [1] https://www.linuxfoundation.org/blog/blog/copyright-notices-in-open-source-software-projects Change-Id: I935c7c0156caa78fc368c929cebd0f068031e830 Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-11-22json utility: fixed core dump during sensor loadJayanth Othayoth1-7/+31
bmcweb replaces underscores with spaces in sensor names for better readability. The existing objectKeyCmp function did not handle this case, leading to core dumps in the sensor load path. Error details are provided below. ``` bmcwebd[1368]: [DEBUG sensors.hpp:507] Added sensor P0_NS_VR_FAN_2 bmcwebd[1368]: terminate called after throwing an instance of 'boost::detail::with_throw_location<boost::system::system_error>' bmcwebd[1368]: what(): leftover [boost.url.grammar:4] ``` Implemented a new algorithm that alphabetically sorts non-URL keys and retains the existing logic for URL-type keys. Tested: Updated and verified the test cases. Change-Id: I39c3f7cc54dec5e7cf9658977e1078acb827afb2 Signed-off-by: Jayanth Othayoth <ojayanth@gmail.com> Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-10-17Handling of OutOfRange in ReadJsonMyung Bae1-2/+2
Currently readJsonPatch returns `PropertValueNotInList` in case when an input integer is out of range. This change is to return `PropertyValueOutOfRange` for the case out-of-range integer input. Tested: - Verify PATCH with an out-of-value integer. e.g. ``` $ curl -k -X PATCH https://${bmc}/redfish/v1/EventService/ -H "Content-Type: application/json" -d '{ "DeliveryRetryIntervalSeconds" : 4294967296}' ``` Before the change, its `MessageId` is `PropertyValueNotInList`. ``` "Message": "The value '4294967296' for the property DeliveryRetryIntervalSeconds is not in the list of acceptable values.", "MessageId": "Base.1.19.0.PropertyValueNotInList", "MessageSeverity": "Warning", "Resolution": "Choose a value from the enumeration list that the implementation can support and resubmit the request if the operation failed." ``` After the change, its `MessageId` will be `PropertyValueOutOfRange`. ``` "Message": "The value '4294967296' for the property DeliveryRetryIntervalSeconds is not in the supported range of acceptable values.", "MessageId": "Base.1.19.0.PropertyValueOutOfRange", "MessageSeverity": "Warning", "Resolution": "Correct the value for the property in the request body and resubmit the request if the operation failed." ``` - Redfish Service Validator passes Change-Id: I0d0c5ecbc9f416b68fa7c0e81a0ea896ec2e50af Signed-off-by: Myung Bae <myungbae@us.ibm.com>
2024-10-03json utility: Update sort algorithmsEd Tanous1-7/+29
Modified sort utility to be able to sort on a specified key. New utility function sortJsonArrayByKey() added. Note: - Function odataObjectCmp() renamed to objectKeyCmp() - New function odataObjectCmp() created which calls objectKeyCmp() with @odata.id key specified. - Comments for odataObjectCmp() didn't match behavior for object without key. These objects are sorted as less than objects with the key. - sortJSONResponse() modified to use the new sortJsonArrayByKey(). Tested: - Added new unit tests. These tests are in addition to the existing tests. So they focus on testing comparing by different keys. The existing tests already cover the different permutations of the basic comparisons. - Redfish Service validator passes Change-Id: I949b7cb868c59a8eeda3798e6a82a1572bbc5792 Signed-off-by: Ed Tanous <etanous@nvidia.com> Signed-off-by: Janet Adkins <janeta@us.ibm.com>
2024-09-11Remove duplicated block commentsEd Tanous1-13/+13
Static analysis flags that these two comments are redundant[1], which seem to be duplicated a lot in copyright headers. Although there is a larger discussion that can likely be had. [1] https://sonarcloud.io/project/issues?issueStatuses=OPEN%2CCONFIRMED&id=edtanous_bmcweb&open=AY9_HYjgKXKyw1ZFwgVP Tested: Comment change only. Code compiles. Change-Id: Ia960317761f558a87842347ca0b5f3da63f8e730 Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-09-10Fix static analysis issuesEd Tanous1-4/+5
These need to implement proper forwarding. [1] https://sonarcloud.io/project/issues?impactSeverities=HIGH&issueStatuses=OPEN%2CCONFIRMED&tags=since-c%2B%2B11&types=CODE_SMELL&id=edtanous_bmcweb&open=AZHJOYheIkxRoiGIkx-E [2] https://sonarcloud.io/project/issues?impactSeverities=HIGH&issueStatuses=OPEN%2CCONFIRMED&tags=since-c%2B%2B11&types=CODE_SMELL&id=edtanous_bmcweb&open=AZHJOYheIkxRoiGIkx-J [3] https://sonarcloud.io/project/issues?impactSeverities=HIGH&issueStatuses=OPEN%2CCONFIRMED&tags=since-c%2B%2B11&types=CODE_SMELL&id=edtanous_bmcweb&open=AZHJOYheIkxRoiGIkx-W Change-Id: I3df2c6765af9020dd278e729f725afffaecbb421 Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-09-04Remove IWYU pragmasEd Tanous1-1/+0
These were added as part of d5c80ad9c07b94465d8ea62d2b6f87c30cac765e: test treewide: iwyu Since then, Nan hasn't been very active on the project, and to my knowledge, since the initial run, we've never used IWYU again. clang-include-cleaner seems to work well without needing these pragmas, and is what we're using, even if it's less useful than IWYU. Remove all mention of IWYU. Tested: Code compiles. Change-Id: I06feedeeac9a114f5bdec81d59ca83223efd8aa7 Signed-off-by: Ed Tanous <etanous@nvidia.com>
2024-08-17clang-format: re-format for clang-18Patrick Williams1-17/+18
clang-format-18 isn't compatible with the clang-format-17 output, so we need to reformat the code with the latest version. The way clang-18 handles lambda formatting also changed, so we have made changes to the organization default style format to better handle lambda formatting. See I5e08687e696dd240402a2780158664b7113def0e for updated style. See Iea0776aaa7edd483fa395e23de25ebf5a6288f71 for clang-18 enablement. Change-Id: Iceec1dc95b6c908ec6c21fb40093de9dd18bf11a Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
2024-06-25Remove nlohmann::json::items()Ed Tanous1-12/+20
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>
2024-04-16Add type safety for NTP server objectsEd Tanous1-6/+27
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>
2024-04-11Give static analysis some hintsEd Tanous1-12/+21
Static analysis tools complain that for certain template types, these comparisons do nothing. Doing nothing is what they're intended to do, as we have other checks. Add some constexpr if hints so static analysis tools don't complain. Change-Id: I60297d094626936d021382ccdc11e4c8b698e3d8 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-11Fix object type in json utilsEd Tanous1-6/+4
This code accidentally makes a copy, given that getJsonObject returns a std::optional<nlohmann::json::object_t> which is then loaded into a std::optional<nlohmann::json>. Because nlohmann::json is implicitly constructible from an object_t, this code works and compiles, but we shouldn't need the intermediate object at all. Change the code to simply load the value as object_t. Tested: Unit tests pass. Good coverage. Change-Id: Ic57953e66958e69a1233e18a5bbd980405cac58e Signed-off-by: Ed Tanous <ed@tanous.net>
2024-04-04Allow parsing null or value classesEd Tanous1-1/+77
In Redfish schema, just about all values can be a type (string, EDM.Numeric, etc) or null. Most APIs don't allow explicitly setting null, but there are a few cases where it is useful, namely in lists, where an an empty object {} keeps the value the same, and null deletes the value from the list. Previously we handled this by unpacking as nlohmann::json, but this allowed things like [1.0, {}] to pass the check for an array of string values. We'd ideally like to reject the 1.0 at the first stage, as well as reduce the number of tiered readJson calls that we make. This commit introducess support for unpacking std::variant types, that allows unpacking a known type, or explicitly allowing null, by unpacking std::nullptr_t. Tested: Unit tests pass. Change-Id: Ic7451877c824ac743faf1951cc2b5d9f8df8019c Signed-off-by: Ed Tanous <edtanous@google.com>
2024-04-01Move where appropriateEd Tanous1-1/+2
Clang-tidy-18 has new checks that can find more cases where we've missed an opportunity to std::move. Fix them. Tested: Logging works, unit tests pass. Change-Id: I0cf58204ce7265828693b787a7b3a16484c3d5e5 Signed-off-by: Ed Tanous <ed@tanous.net>
2024-03-19Make readJson accept object_tEd Tanous1-18/+66
Redfish supports several type systems for json. This makes parsing into proper types a challenge. Nlohmann supports 3 core data types, nlohmann::json, which supports all json types (float, int, array, object). Nlohmann::json::object_t, which is a specific typedef of std::map, and nlohmann::json::array_t, which is a specific typedef of std::map. Redfish allows reading our arrays of complex objects, similar to NtpServers: [null, {}, "string"] Which makes it a challenge to support. This commit allows parsing out objects as a nlohmann::object_t, which gives the ability to later use it in a type safe manner, without having to call get_ptr<nlohmann::json::object_t later>. Tested: Unit tests pass. Change-Id: I4134338951ce27c2f56841a45b56bc64ad1753db Signed-off-by: Ed Tanous <ed@tanous.net>
2023-10-24clang-format: copy latest and re-formatPatrick Williams1-3/+3
clang-format-17 has some backwards incompatible changes that require additional settings for best compatibility and re-running the formatter. Copy the latest .clang-format from the docs repository and reformat the repository. Change-Id: I2f9540cf0d545a2da4d6289fc87b754f684bc9a7 Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
2023-08-21Use rangesEd Tanous1-1/+2
C++20 brought us std::ranges for a lot of algorithms. Most of these conversions were done using comby, similar to: ``` comby -verbose 'std::lower_bound(:[a].begin(),:[b].end(),:[c])' 'std::ranges::lower_bound(:[a], :[c])' $(git ls-files | grep "\.[hc]\(pp\)\?$") -in-place ``` Change-Id: I0c99c04e9368312555c08147d474ca93a5959e8d Signed-off-by: Ed Tanous <edtanous@google.com>
2023-08-07Fix bugprone-unchecked-optional-access findingsEd Tanous1-1/+1
Clang-tidy has the aforementioned check, which shows a few places in the core where we ignored the required optional checks. Fix all uses. Note, we cannot enable the check that this time because of some weird code in health.hpp that crashes tidy[1]. That will need to be a future improvement. There are tests that call something like ASSERT(optional) EXPECT(optional->foo()) While this isn't an actual violation, clang-tidy doesn't seem to be smart enough to deal with it, so add some explicit checks. [1] https://github.com/llvm/llvm-project/issues/55530 Tested: Redfish service validator passes. Change-Id: Ied579cd0b957efc81aff5d5d1091a740a7a2d7e3 Signed-off-by: Ed Tanous <edtanous@google.com>
2023-07-20Replace logging with std::formatEd Tanous1-12/+11
std::format is a much more modern logging solution, and gives us a lot more flexibility, and better compile times when doing logging. Unfortunately, given its level of compile time checks, it needs to be a method, instead of the stream style logging we had before. This requires a pretty substantial change. Fortunately, this change can be largely automated, via the script included in this commit under scripts/replace_logs.py. This is to aid people in moving their patchsets over to the new form in the short period where old patches will be based on the old logging. The intention is that this script eventually goes away. The old style logging (stream based) looked like. BMCWEB_LOG_DEBUG << "Foo " << foo; The new equivalent of the above would be: BMCWEB_LOG_DEBUG("Foo {}", foo); In the course of doing this, this also cleans up several ignored linter errors, including macro usage, and array to pointer deconstruction. Note, This patchset does remove the timestamp from the log message. In practice, this was duplicated between journald and bmcweb, and there's no need for both to exist. One design decision of note is the addition of logPtr. Because the compiler can't disambiguate between const char* and const MyThing*, it's necessary to add an explicit cast to void*. This is identical to how fmt handled it. Tested: compiled with logging meson_option enabled, and launched bmcweb Saw the usual logging, similar to what was present before: ``` [Error include/webassets.hpp:60] Unable to find or open /usr/share/www/ static file hosting disabled [Debug include/persistent_data.hpp:133] Restored Session Timeout: 1800 [Debug redfish-core/include/event_service_manager.hpp:671] Old eventService config not exist [Info src/webserver_main.cpp:59] Starting webserver on port 18080 [Error redfish-core/include/event_service_manager.hpp:1301] inotify_add_watch failed for redfish log file. [Info src/webserver_main.cpp:137] Start Hostname Monitor Service... ``` Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: I86a46aa2454be7fe80df608cb7e5573ca4029ec8
2023-06-21json utils: add getEstimatedJsonSizeNan Zhou1-0/+14
Add a utility function which estimates the size of the JSON tree. It is used in the children change to limit the reponse size of expand query. Tested: 1. unit test passed; 2. tested on hardware, the following are real sizes and estimation ``` Real payload size, Estimation, query 15.69 KB, 10.21 KB, redfish/v1?$expand=.($levels=1) 95.76 KB, 62.11 KB, redfish/v1?$expand=.($levels=2) 117.14 KB, 72.71 KB, redfish/v1?$expand=.($levels=3) 127.65 KB, 77.64 KB, redfish/v1?$expand=.($levels=4) ``` Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: Iae26d6732a6ec63ecc59eacf657b4bf33c07c046
2023-06-13Make propertyValueTypeError more typesafeEd Tanous1-20/+4
Similar to the prior patchset in this series, propertyValueTypeError can be moved to safer constructs. This ensures that we are minimizing how many places we are calling dump() from, and allows us to reduce the amount of code written for error handling. Tested: PATCH /redfish/v1/SessionService {"SessionTimeout": "foo"} Returns PropertyValueTypeError in the same behavior as prior to this patch. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Iddff4b787f35c49bf923663d61bba156687f358c
2023-06-13Make propertyValueNotInList typesafeEd Tanous1-5/+1
The error codes for this function accept a string_view, which has caused a number of cases of users of this function to call dump() to_string() and all manner of other conversions. Considering that dump() is something that's difficult to call correctly, and overly wordy, it would be ideal if the message code just handled that for us. Therefore, this commit changes the prototype to include a nlohmann::json object as an argument instead of string_view, then audits the codebase for all uses, and moves them to a more normalized usage, which allows the calling code to call "dump" for them. Tested: PATCH /redfish/v1/SessionService {"SessionTimeout": 1} Returns the PropertyValueNotInList error as it did before. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: If62909072db1f067ad1f8aa590bb716c84181219
2023-05-26json utility: add sortNan Zhou1-0/+103
This commit adds a utility function |sortJsonArrayByKey|. It can sort an json array by value of a given key of each element. Use cases includes: 1. sort the MemberCollection by @odata.id Tested: 1. unit test passed; Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Idc175fab3af5c6102a5a3439b712b659ecb76468
2023-05-25Fix some includesEd Tanous1-1/+2
System includes should be included with <>, in-tree includes should be included with "". This was found manually, with the help of the following grep statement[1]. git grep -o -h "#include .*" | sort | uniq Tested: Code compiles Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I1a6b2a5ba35ccbbb61c67b7c4b036a2d7b3a36a3
2023-05-23Switched bmcweb to use new telemetry service APIKrzysztof Grobelny1-1/+0
Added support for multiple MetricProperties. Added support for new parameters: CollectionTimeScope, CollectionDuration. ReadingParameters was not yet changed in telemetry backend, instead temporary property ReadingParametersFutureVersion was introduced. Once bmcweb is adapted to use ReadingParametersFutureVersion this property will be renamed in backend to ReadingParameters. Then bmcweb will change to use ReadingParameters. Then ReadingParametersFutureVersion will be removed from backend and everything will be exactly like described in phosphor-dbus-interfaces without introducing breaking changes. Related change in phosphor-dbus-interfaces [1], [2]. This change needs to be bumped together with [3]. Tested: - It is possible to create MetricReportDefinitions with multiple MetricProperties. - Stub values for new parameters are correctly passed to telemetry service. - All existing telemetry service functionalities remain unchanged. [1]: https://github.com/openbmc/phosphor-dbus-interfaces/commit/4f9c09144b60edc015291d2c120fc5b33aa0bec2 [2]: https://gerrit.openbmc.org/c/openbmc/phosphor-dbus-interfaces/+/60750 [3]: https://gerrit.openbmc.org/c/openbmc/telemetry/+/58229 Change-Id: I2cd17069e3ea015c8f5571c29278f1d50536272a Signed-off-by: Krzysztof Grobelny <krzysztof.grobelny@intel.com> Signed-off-by: Lukasz Kazmierczak <lukasz.kazmierczak@intel.com>
2023-05-11clang-format: copy latest and re-formatPatrick Williams1-2/+1
clang-format-16 has some backwards incompatible changes that require additional settings for best compatibility and re-running the formatter. Copy the latest .clang-format from the docs repository and reformat the repository. Change-Id: I75f89d2959b0f1338c20d72ad669fbdc1d720835 Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
2023-02-10Change readJson from is_object to get_ptrEd Tanous1-6/+8
While is_object is useful in contexts where we don't need the underlying value, in cases where we do, nlohmann::json::begin() can throw when generating the nlohmann::iterator_proxy object. This causes unwind tables to be generated, so as a general rule, we should be prefering get_ptr in these cases. Tested: Good unit test coverage for readJson. Unit tests only. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I5ecb2dca95cc06246e983f9e4190b2dd46d1a6b9
2022-09-09clang-tidy: fix misc warningsPatrick Williams1-1/+1
The following error reports have started to be reported by clang-tidy: * readability-qualified-auto - add 'const' to `auto&` iterators * bugprone-use-after-move - add break in loop after element is found Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: I5314559f62f58aa032d4c74946b8e3e4ce6be808
2022-08-30json utils: remove forward_declareNan Zhou1-5/+1
It was accidentally missed in previous round of iwyu clean up. Tested: build. Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: I6857409848a99594d5c91168cf4a760e456e8c90
2022-07-23test treewide: iwyuNan Zhou1-5/+26
These changes are done by running iwyu manually under clang14. Suppressed some obvious impl or details headers. Kept the recommended public headers. IWYU can increase readability, make maintenance easier, and avoid errors in some cases. See details in https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/WhyIWYU.md. This commit also uses its best effort to correct obvious errors through iwyu pragma. See reference here: https://github.com/include-what-you-use/include-what-you-use#how-to-correct-iwyu-mistakes Tested: unit test passed. Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: I983b6f75601707cbb0f2f04546c3362ff4ba7fee
2022-07-07Handle NTPServers list per the specificationEd Tanous1-8/+0
The Redfish specification for PATCH of arrays defines a number of requirements. - Setting a value to null, should remove it from the list. - Setting a value to empty object "{}" should leave the value unmodified - Values at indexes larger than whats included in the PATCH request shall be removed. This commit attempts to fix this behavior for NTPServers and make it correct. It does this by first getting the list of NTP servers, then walking the list in parallel with the list given in the patch, and either modifying or changing the list as the spec requires before setting the setting across the system. It also turns out that the current behavior of unpacking nlohmann::json objects requires an object to be an array, object, or null, which doesn't allow unpacking the strings required in this case, so that check is removed. A quick inspection shows that we don't unpack nlohmann objects very often, and this should have no impact. Tested: Redfish-protocol-validator tests for NTPServers now pass ''' curl -vvvv --insecure --user root:0penBmc https://192.168.7.2/redfish/v1/Managers/bmc/NetworkProtocol -X PATCH -d '{"NTP": {"NTPServers": []}}' ''' Used to patch values succeeds with various "good" values; ["time-a-b.nist.gov", "time-b-b.nist.gov"] [{}, {}] ["time-a-b.nist.gov", null] [] Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I23a8febde34817bb0b934e46e2b77ff391b52a57
2022-06-01Try to fix the lambda formatting issueEd Tanous1-14/+13
clang-tidy has a setting, LambdaBodyIndentation, which it says: "For callback-heavy code, it may improve readability to have the signature indented two levels and to use OuterScope." bmcweb is very callback heavy code. Try to enable it and see if that improves things. There are many cases where the length of a lambda call will change, and reindent the entire lambda function. This is really bad for code reviews, as it's difficult to see the lines changed. This commit should resolve it. This does have the downside of reindenting a lot of functions, which is unfortunate, but probably worth it in the long run. All changes except for the .clang-format file were made by the robot. Tested: Code compiles, whitespace changes only. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ib4aa2f1391fada981febd25b67dcdb9143827f43
2022-04-19Implement odata annotations ignoringEd Tanous1-2/+16
From the quoted section of the spec in the patchset, we should be ignoring odata annotations on PATCH requests. This commit implements a preliminary loop through the json object, and removes the odata items before processing begins. Tested: curl -vvvv --insecure --user root:0penBmc -X PATCH -d '{"@odata.etag": "my_etag"}' https://192.168.7.2/redfish/v1/AccountService/Accounts/root returns: Base.1.11.0.NoOperation Redfish protocol validator now passes the REQ_PATCH_ODATA_PROPS test. Included unit tests passing. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I62be75342681d147b8536fd122bbc793eeaa3788
2022-03-12Enable readability checksEd Tanous1-2/+2
clang-tidy readability checks are overall a good thing, and help us to write consistent and readable code, even if it doesn't change the result. All changes done by the robot. Tested: Code compiles, inspection only (changes made by robot) Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Iee4a0c74a11eef9f158f0044eae675ebc518b549
2022-03-01json_utils: Add support for multiple level json readWilly Tu1-93/+191
Added support for multiple level direct read. For example, we can now access `abc/xyz` directly instead of getting `abc` and then abc[`xyz`]. For extra element error, it will only be triggered if the element at the root level is not a parent of any of the requested elements. For example, { "abc": { "xyz": 12 } } Getting "abc/xyz" will satisfy the condition so it does not throw an error. This is accomplished in a reasonable way by moving the previously variadic templated code to a std::span<variant> that contains all possible types. This is a trick learned from the fmt library to reduce compile sizes, as the majority of the code doesn't get duplicated at template level, and is instead operating on the fixed variant type. This commit drops 7316 bytes (about half a percent of total) from the bmcweb binary size from the reduction in template usage. Later patches build on this patchset to simplify call sites even more and reduce the binary size further, but as is, this is still a win. Note: now that the UnpackVariant lists all possible unpack types, it was found that readJson would fail to compile for vector<bool>. This is a well known C++ deficiency in the std::vector<bool> type when compared to all other types, and will need special handling in the future. The two types for vector<bool> are left commented out in the typelist. Tested: Unit tests passing with reasonable coverage. Functional use in next commit in this series. Change-Id: Ifb247c9121c41ad8f1de26eb4bfc3d71484e6bd6 Signed-off-by: Willy Tu <wltu@google.com> Signed-off-by: Ed Tanous <edtanous@google.com>
2022-02-25json_utils: Add support jsonRead Patch/ActionWilly Tu1-9/+28
Added support for readJson for Patch and Action. The only difference is that Patch does not allow empty json input while Action does. Action with empty input will use the default value based on the implementation and return 200 OK response code. readJsonPatch will replace the existing readJson and be used for path requests. It will not allow empty json input and all requested keys are required in the json input. readJsonAction will be used for Action requests where it is possible for all of the properties to be optional and allow empty request. The optional properties are determined by the requested values type. All current Action readJson are replaced with readJsonAction. It does not change the existing behavior since it needs `std::optional`. This will have to be updated later as we define the default behavior. Tested: Added unit tests and readJsonAction allows empty empty json object. No Change to Redfish Tree. Change-Id: Ia5e1f81695c528a20f1dc985aee19c920d8adaea Signed-off-by: Willy Tu <wltu@google.com>
2022-02-07Enable readability-named-parameter checksEd Tanous1-4/+5
We don't have too many violations here, probably because we don't have many optional parameters. Fix the existing instances, and enable the check. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I4d512f0ec90b060fb60a42fe3cd6ba72fb6c6bcb
2022-01-12Enable pointer devolution checksEd Tanous1-3/+8
Enable cpp core guidelines checks for pointer deevolution. For the moment, simply ignore the uses, although ideally these should be cleaned up at some point. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I9a8aae94cc7a59529eab89225a37e89628c17597
2021-02-19Fix nlohmann::json::dump callsEd Tanous1-5/+25
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
2021-01-07Omit using subscripting operator in json utilsEd Tanous1-4/+3
Using operator[] as a result of expression is a common mistake in c++. First it creates a key in container with empty value than execute an expression and assign result from it to the key in container. The result of it is that object is created two times. Fixes https://github.com/openbmc/bmcweb/issues/139 Tested: Used: curl -vvv -X POST -d "{ \"EventType\": \"Alert\", \"EventId\": \"TestEventId\", \"EventTimestamp\": \"2017-08-08T08:24:00Z\", \"Severity\": \"TestSeverity\", \"Message\": \"TestMessage\", \"MessageId\": \"TestMessageId\", \"MessageArgs\": [ \"TestMessageArg\" ], \"OriginOfCondition\": \"/redfish/v1/\" }" --insecure --user root:0penBmc https://192.168.7.2/redfish/v1/EventService/Actions/EventService.SubmitTestEven To send test event. Call returned 204 as expected. Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: Idf44829bfb25daf216003f591d354df65ccecb18
2020-10-23fix include namesEd Tanous1-3/+2
cppcheck isn't smart enough to recognize these are c++ headers, not c headers. Considering we're already inconsistent about our naming, it's easier to just be consistent, and move the last few files to use .hpp instead of .h. Tested: Code builds, no changes. Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: Ic348d695f8527fa4a0ded53f433e1558c319db40
2020-09-29Fix naming conventionsEd Tanous1-22/+13
Lots of code has been checked in that doesn't match the naming conventions. Lets fix that. Tested: Code compiles. Variable/function renames only. Signed-off-by: Ed Tanous <ed@tanous.net> Change-Id: I6bd107811d0b724f1fad990016113cdf035b604b
2020-08-17Enable unused variable warnings and resolveEd Tanous1-9/+8
This commit enables the "unused variables" warning in clang. Throughout this, it did point out several issues that would've been functional bugs, so I think it was worthwhile. It also cleaned up several unused variable from old constructs that no longer exist. Tested: Built with clang. Code no longer emits warnings. Downloaded bmcweb to system and pulled up the webui, observed webui loads and logs in properly. Change-Id: I51505f4222cc147d6f2b87b14d7e2ac4a74cafa8 Signed-off-by: Ed Tanous <ed@tanous.net>