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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...

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 ...