#
0bdda665 |
| 03-Aug-2023 |
Ed Tanous <edtanous@google.com> |
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 stand
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>
show more ...
|
#
20fa6a2c |
| 20-May-2024 |
Ed Tanous <ed@tanous.net> |
Remove the last instances of json pattern
In the past, we've tried to erradicate the use of nlohmann::json(initiatlizer_list<...>) because it bloats binary sizes, as every type is given a new nlohma
Remove the last instances of json pattern
In the past, we've tried to erradicate the use of nlohmann::json(initiatlizer_list<...>) because it bloats binary sizes, as every type is given a new nlohmann constructor.
This commit hunts down the last few places where we call this. There is still 2 remaining in openbmc_dbus_rest after this, but those are variant accesses that are difficult to triage, and considering it's a less used api, they're left as is.
Tested: WIP
Change-Id: Iaac24584bb78bb238da69010b511c1d598bd38bc Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
07900817 |
| 06-May-2024 |
Ed Tanous <ed@tanous.net> |
Check return codes
Static analysis finds these two places that we don't check error codes. Check them.
Tested: Deprecated code. Inspection only.
Change-Id: I92c238c5a4b1f51c5c6855c59f4f943855c4e5
Check return codes
Static analysis finds these two places that we don't check error codes. Check them.
Tested: Deprecated code. Inspection only.
Change-Id: I92c238c5a4b1f51c5c6855c59f4f943855c4e50f Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
95c6307a |
| 26-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Break out formatters
In the change made to move to std::format, we defined some custom type formatters in logging.hpp. This had the unintended effect of making all compile units pull in the majorit
Break out formatters
In the change made to move to std::format, we defined some custom type formatters in logging.hpp. This had the unintended effect of making all compile units pull in the majority of boost::url, and nlohmann::json as includes.
This commit breaks out boost and json formatters into their own separate includes.
Tested: Code compiles. Logging changes only.
Change-Id: I6a788533169f10e19130a1910cd3be0cc729b020 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
5b90429a |
| 16-Apr-2024 |
Ed Tanous <ed@tanous.net> |
Add missing headers
Most of these were found by breaking every redfish class handler into its own compile unit:
When that's done, these missing headers become compile errors. We should just fix the
Add missing headers
Most of these were found by breaking every redfish class handler into its own compile unit:
When that's done, these missing headers become compile errors. We should just fix them.
In addition, this allows us to enable automatic header checking in clang-tidy using misc-header-cleaner. Because the compiler can now "see" all the defines, it no longer tries to remove headers that it thinks are unused.
[1] https://github.com/openbmc/bmcweb/commit/4fdee9e39e9f03122ee16a6fb251a380681f56ac
Tested: Code compiles.
Change-Id: Ifa27ac4a512362b7ded7cc3068648dc4aea6ad7b Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
296579be |
| 11-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Call dump() less
nlohmann::json::dump() is not an easy function to get the call parameters correct on. We should limit the places we use it.
Luckily, both logging and redfish::messages support pri
Call dump() less
nlohmann::json::dump() is not an easy function to get the call parameters correct on. We should limit the places we use it.
Luckily, both logging and redfish::messages support printing json values directly. Use them where appropriate.
Tested: Error logging and out of range calls only of heavily used messages and logging calls. Inspection only.
Change-Id: I57521d8791dd95250c93e8e3b2d4a959740ac713 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
f79ce6a8 |
| 20-Mar-2024 |
Ed Tanous <ed@tanous.net> |
Enable bugprone clang check
bugprone-multi-level-implicit-pointer-conversion is something that we pass currently, with one exception in the deprecated rest API. Ignore the one exception, as it's no
Enable bugprone clang check
bugprone-multi-level-implicit-pointer-conversion is something that we pass currently, with one exception in the deprecated rest API. Ignore the one exception, as it's not clear how to fix it, and enable the check.
Tested: Clang tidy passes.
Change-Id: Idc10e0bb7b876e1c70afa28f9c27cc7bef1db0d7 Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
18f8f608 |
| 18-Jul-2023 |
Ed Tanous <edtanous@google.com> |
Remove some boost includes
The less we rely on boost, and more on std algorithms, the less people have to look up, and the more likely that our code will deduplicate.
Replace all uses of boost::alg
Remove some boost includes
The less we rely on boost, and more on std algorithms, the less people have to look up, and the more likely that our code will deduplicate.
Replace all uses of boost::algorithms with std alternatives.
Tested: Redfish Service Validator passes.
Change-Id: I8a26f39b5709adc444b4178e92f5f3c7b988b05b Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|
#
f3477566 |
| 25-Jul-2023 |
Mikhail Zhvakin <striker_1993@mail.ru> |
DBus REST: Fix array and dict handling
The bmcweb DBus REST API cannot currently handle array or dictionary data types correctly. This commit is meant to fix that.
Tested: get/set DBus attributes (
DBus REST: Fix array and dict handling
The bmcweb DBus REST API cannot currently handle array or dictionary data types correctly. This commit is meant to fix that.
Tested: get/set DBus attributes (consisting of array and/or dictionary) via bmcweb DBus REST API.
Change-Id: I9694cb888375c90d7a8fb1a10e53bdb5c0bce3bb Signed-off-by: Mikhail Zhvakin <striker_1993@mail.ru>
show more ...
|
#
27b0cf90 |
| 07-Aug-2023 |
Ed Tanous <ed@tanous.net> |
Move to file_body in boost
As is, it reads the whole file into memory before sending it. While fairly fast for the user, this wastes ram, and makes bmcweb less useful on less capable systems.
This
Move to file_body in boost
As is, it reads the whole file into memory before sending it. While fairly fast for the user, this wastes ram, and makes bmcweb less useful on less capable systems.
This patch enables using the boost::beast::http::file_body type, which has more efficient serialization semantics than using a std::string. To do this, it adds a openFile() handler to http::Response, which can be used to properly open a file. Once the file is opened, the existing string body is ignored, and the file payload is sent instead. openFile() also returns success or failure, to allow users to properly handle 404s and other errors.
To prove that it works, I moved over every instance of direct use of the body() method over to using this, including the webasset handler. The webasset handler specifically should help with system load when doing an initial page load of the webui.
Tested: Redfish service validator passes.
Change-Id: Ic7ea9ffefdbc81eb985de7edc0fac114822994ad Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
5a39f77a |
| 20-Oct-2023 |
Patrick Williams <patrick@stwcx.xyz> |
clang-format: copy latest and re-format
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: copy latest and re-format
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>
show more ...
|
#
3544d2a7 |
| 06-Aug-2023 |
Ed Tanous <edtanous@google.com> |
Use ranges
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])' 's
Use ranges
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>
show more ...
|
#
62598e31 |
| 17-Jul-2023 |
Ed Tanous <ed@tanous.net> |
Replace logging with std::format
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
Replace logging with std::format
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
show more ...
|
#
8b24275d |
| 27-Jun-2023 |
Ed Tanous <edtanous@google.com> |
Rename all error_code instances to ec
We're not consistent here, which leads to people copying and pasting code all over, which has lead to a bunch of different names for error codes.
This commit c
Rename all error_code instances to ec
We're not consistent here, which leads to people copying and pasting code all over, which has lead to a bunch of different names for error codes.
This commit changes to coerce them all to "ec", because that's what boost uses for a naming convention.
Tested: Rename only, code compiles.
Change-Id: I7053cc738faa9f7a82f55fc46fc78618bdf702a5 Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|
#
5eb468da |
| 20-Jun-2023 |
George Liu <liuxiwei@inspur.com> |
Refactor getManagedObjects method
Since the getManagedObjects method has been implemented in dbus_utility and this commit is to integrate all the places where the GetManagedObjects method is obtaine
Refactor getManagedObjects method
Since the getManagedObjects method has been implemented in dbus_utility and this commit is to integrate all the places where the GetManagedObjects method is obtained, and use the method in dbus_utility uniformly.
Signed-off-by: George Liu <liuxiwei@inspur.com> Change-Id: Ic13f2bef7b30f805cd3444a75d7df17b031f2eb0
show more ...
|
#
4b242749 |
| 11-May-2023 |
Ed Tanous <edtanous@google.com> |
Make all std::regex instances static
Per [1] we really shouldn't be using regex. In the cases we do, it's a HUUUUUGE performance benefit to be compiling the regex ONCE.
The only downside is a slig
Make all std::regex instances static
Per [1] we really shouldn't be using regex. In the cases we do, it's a HUUUUUGE performance benefit to be compiling the regex ONCE.
The only downside is a slight increase in memory usage.
[1]: https://github.com/openbmc/bmcweb/issues/176
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I8644b8a07810349fb60bfa0258a13e815912a38e
show more ...
|
#
b2ba3072 |
| 12-May-2023 |
Patrick Williams <patrick@stwcx.xyz> |
fix more push vs emplace calls
It seems like clang-tidy doesn't catch every place that an emplace could be used instead of a push. Use a few grep/sed pairs to find and fix up some common patterns.
fix more push vs emplace calls
It seems like clang-tidy doesn't catch every place that an emplace could be used instead of a push. Use a few grep/sed pairs to find and fix up some common patterns.
Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: I93eaec26b8e3be240599e92b66cf54947073dc4c
show more ...
|
#
89492a15 |
| 10-May-2023 |
Patrick Williams <patrick@stwcx.xyz> |
clang-format: copy latest and re-format
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: copy latest and re-format
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>
show more ...
|
#
28dd5ca1 |
| 17-Mar-2023 |
Lei YU <yulei.sh@bytedance.com> |
dbus_rest: Fix dangling reference of crow::Response
The openbmc_dbus_reset was holding reference of `crow::Response`, set the response in `~InProgressActionData()`, and call res.end() to complete th
dbus_rest: Fix dangling reference of crow::Response
The openbmc_dbus_reset was holding reference of `crow::Response`, set the response in `~InProgressActionData()`, and call res.end() to complete the result of the response.
The bmcweb code now uses `std::shared_ptr<AsyncResp>` for the response and the `res.end()` is handled in `~AsyncResp()`. By using the reference of `crow::Response`, the `InProgressActionData` is actually using a dangling reference because the `std::shared_ptr<AsyncResp>` is already destructed, and bmcweb will crash on `action` calls, or not crash but get invalid response, as it's undefined behavior.
Fix the above issue by using `std::shared_ptr<AsyncResp>` to make sure the response is correctly handled.
Tested: 1. Without the fix, bmcweb crashes, or get no json output response on the below method call, be noted that it's an invalid call: ``` $ curl -k -H "X-Auth-Token: $token" -x POST -d '{"data": []}' https://${bmc}/xyz/openbmc_project/logging/action/deleteAll ``` 2. With the fix, bmcweb gives expected response: ``` $ curl -k -H "X-Auth-Token: $token" -x POST -d '{"data": []}' https://${bmc}/xyz/openbmc_project/logging/action/deleteAll { "data": { "description": "The specified method cannot be found" }, "message": "404 Not Found", "status": "error" }
$ curl -k -H "X-Auth-Token: $token" -x POST -d '{"data": []}' https://${bmc}/xyz/openbmc_project/logging/action/DeleteAll { "data": null, "message": "200 OK", "status": "ok" } ```
Signed-off-by: Lei YU <yulei.sh@bytedance.com> Change-Id: I38ef34fe8ff18e4e127664c853c6792461f6edf8
show more ...
|
#
5e7e2dc5 |
| 16-Feb-2023 |
Ed Tanous <edtanous@google.com> |
Take boost error_code by reference
By convention, we should be following boost here, and passing error_code by reference, not by value. This makes our code consistent, and removes the need for a co
Take boost error_code by reference
By convention, we should be following boost here, and passing error_code by reference, not by value. This makes our code consistent, and removes the need for a copy in some cases.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Id42ea4a90b6685a84818b87d1506c11256b3b9ae
show more ...
|
#
26ccae32 |
| 16-Feb-2023 |
Ed Tanous <edtanous@google.com> |
Pass string views by value
string_view should always be passed by value; This commit is a sed replace of the code to make all string_views pass by value, per general coding guidelines[1].
[1] http
Pass string views by value
string_view should always be passed by value; This commit is a sed replace of the code to make all string_views pass by value, per general coding guidelines[1].
[1] https://quuxplusone.github.io/blog/2021/11/09/pass-string-view-by-value/
Tested: Code compiles.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I55b342a29a0fbfce0a4ed9ea63db6014d03b134c
show more ...
|
#
1aa0c2b8 |
| 08-Feb-2022 |
Ed Tanous <edtanous@google.com> |
Add option for validating content-type header
For systems implementing to the OWASP security guidelines[1] (of which all should ideally) we should be checking the content-type header all times that
Add option for validating content-type header
For systems implementing to the OWASP security guidelines[1] (of which all should ideally) we should be checking the content-type header all times that we parse a request as JSON.
This commit adds an option for parsing content-type, and sets a default of "must get content-type". Ideally this would not be a breaking change, but given the number of guides and scripts that omit the content type, it seems worthwhile to add a trapdoor, such that people can opt into their own model on how they would like to see this checking work.
Tested: ``` curl --insecure -H "Content-Type: application/json" -X POST -D headers.txt https://${bmc}/redfish/v1/SessionService/Sessions -d '{"UserName":"root", "Password":"0penBmc"}' ```
Succeeds.
Removing Content-Type argument causes bmc to return Base.1.13.0.UnrecognizedRequestBody.
[1] cheatsheetseries.owasp.org/cheatsheets/REST_Security_Cheat_Sheet.html
Change-Id: Iaa47dd563b40036ff2fc2cacb70d941fd8853038 Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|
#
50ebd4af |
| 19-Jan-2023 |
Ed Tanous <edtanous@google.com> |
Implement alternative to on boost::split
boost::split has a documented false-positive in clang-tidy. While normally we'd handle this with NOLINTNEXTLINE, this doesn't appear to work in all cases.
Implement alternative to on boost::split
boost::split has a documented false-positive in clang-tidy. While normally we'd handle this with NOLINTNEXTLINE, this doesn't appear to work in all cases. Unclear why, but seems to be due to some of our lambda callback complexity.
Each of these uses is a case where we should be using a more specific check, rather than split, but for the moment, this is the best we have.
Tested: clang-tidy passes.
[1] https://github.com/llvm/llvm-project/issues/40486
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I144c6610cb740287b7225e2be03b4142a64f9563
show more ...
|
#
e99073f5 |
| 08-Dec-2022 |
George Liu <liuxiwei@inspur.com> |
Refactor GetSubTree method
Since the GetSubTree method has been implemented in dbus_utility and this commit is to integrate all the places where the GetSubTree method is called, and use the method i
Refactor GetSubTree method
Since the GetSubTree method has been implemented in dbus_utility and this commit is to integrate all the places where the GetSubTree method is called, and use the method in dbus_utility uniformly.
Tested: Redfish Validator Passed
Signed-off-by: George Liu <liuxiwei@inspur.com> Change-Id: If3852b487d74e7cd8f123e0efffbd4affe92743c
show more ...
|
#
2b73119c |
| 11-Jan-2023 |
George Liu <liuxiwei@inspur.com> |
Add the GetObject method to dbus_utility
There are currently many files that use the GetObject method. Since they are a general method, they are defined in the dbus_utility.hpp file and refactors th
Add the GetObject method to dbus_utility
There are currently many files that use the GetObject method. Since they are a general method, they are defined in the dbus_utility.hpp file and refactors them.
Tested: Built bmcweb successfully and Validator passes.
Signed-off-by: George Liu <liuxiwei@inspur.com> Change-Id: If2af77294389b023b611987252ee6149906fcd25
show more ...
|