Rework logger to create compile time errors

The logger changes to move to std::format incidentally caused format
errors to no longer be flagged at compile time.

The example here[1] shows that the l

Rework logger to create compile time errors

The logger changes to move to std::format incidentally caused format
errors to no longer be flagged at compile time.

The example here[1] shows that the latest gcc/c++ gave us a way to solve
this, using std::format_string.

This incidentally shows two places where we got the format arguments
wrong, so fix them.

[1] https://stackoverflow.com/questions/72795189/how-can-i-wrap-stdformat-with-my-own-template-function

Change-Id: Id884200e2c98eeaf5ef8db6c1d6362ede2ffb858
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Fix moves/forward

Clang has new checks for std::move/std::forward correctness, which
catches quite a few "wrong" things where we were making copies of
callback handlers.

Unfortunately, the lambda s

Fix moves/forward

Clang has new checks for std::move/std::forward correctness, which
catches quite a few "wrong" things where we were making copies of
callback handlers.

Unfortunately, the lambda syntax of

callback{std::forward<Callback>(callback)}

in a capture confuses it, so change usages to
callback = std::forward<Callback>(callback)

to be consistent.

Tested: Redfish service validator passes.

Change-Id: I7a111ec00cf78ecb7d5f5b102c786c1c14d74384
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Make flush explicit

A more in depth explanation of this is here[1], but being explicit
flushing the buffer (using std::flush) captures intent better here,
which we intend a newline, then a flush. K

Make flush explicit

A more in depth explanation of this is here[1], but being explicit
flushing the buffer (using std::flush) captures intent better here,
which we intend a newline, then a flush. Keeping those as separate
captures the intended behavior.

Tested: Launching bmcweb shows logging statements still function.

[1] https://clang.llvm.org/extra/clang-tidy/checks/performance/avoid-endl.html

Change-Id: I6aa427f4e4134ce30ce552cbfdd5d7be3df56c47
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Fix minor error in Request::method

Minor syntax error injected in:
1873a04f Reduce multi-level calls of req.req members

Tested: Code compiles.

Change-Id: Iec26273349df6063eb425e4a75b1250c17bc6f3f

Fix minor error in Request::method

Minor syntax error injected in:
1873a04f Reduce multi-level calls of req.req members

Tested: Code compiles.

Change-Id: Iec26273349df6063eb425e4a75b1250c17bc6f3f
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Fix http2 payloads

d547d8d2c30a7d00852855da8ecc15c0cc424b0e caused a minor regression in a
poorly executed merge conflict. The reqReader instance never gets
initialized, so all payloads now fail.

Fix http2 payloads

d547d8d2c30a7d00852855da8ecc15c0cc424b0e caused a minor regression in a
poorly executed merge conflict. The reqReader instance never gets
initialized, so all payloads now fail.

Tested: Redfish service validator modified to use http2 now succeeds
when using http/2.

Change-Id: If00f5bc1d1a70396171eddd6a643fbd860e8508c
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Reduce multi-level calls of req.req members

Several places access the members of `req` indirectly like
`req.req.method()`. This can be simplified as `req.method()` .

This would also make the code

Reduce multi-level calls of req.req members

Several places access the members of `req` indirectly like
`req.req.method()`. This can be simplified as `req.method()` .

This would also make the code clearer.

Tested:
- Compiles
- Redfish service validator passes

Change-Id: Ie129564ff907cdea7ac224b1e3d80cc0dedfbd7b
Signed-off-by: Myung Bae <myungbae@us.ibm.com>

show more ...

Up the max connectionCount to 200

Have seen defects where hitting the max connection limit with multiple
server managers attached. Although not common to exceed 100, can hit
this when using 2 or 3 w

Up the max connectionCount to 200

Have seen defects where hitting the max connection limit with multiple
server managers attached. Although not common to exceed 100, can hit
this when using 2 or 3 webui-vue GUIs and a server manager attached.
webui-vue can use ~30 of these on its own; this isn't that hard to hit.

Nginx by default sets 512 connections[1] , so 200 for an embedded
target doesn't seem that unreasonable:

Apache sets 256 by default [2]

lighttpd sets 1024 [3]

We're in line for the defaults for other webservers.

Tested: Sent 180 basic auth requests seen bmcweb memory at
2189 2178 root R 29080 4% 49% ./bmcweb
This was on a AST2600 (p10bmc)

The connections open got to:
[DEBUG "http_connection.hpp":79] 0x19bb5c8 Connection open, total 161

Came back down as expected:
[DEBUG "http_connection.hpp":89] 0x1a41440 Connection closed, total 1

Didn't see this with multiple webui-vues / server managers.

[1] https://nginx.org/en/docs/ngx_core_module.html#worker_connections
[2] https://httpd.apache.org/docs/2.4/mod/mpm_common.html#maxrequestworkers
[3] https://redmine.lighttpd.net/projects/1/wiki/Server_max-connectionsDetails

Change-Id: I807302e32e61e31212850a480d721d89d484593f
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>

show more ...

Move where appropriate

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: I0cf58204

Move where appropriate

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>

show more ...

Fix redundant inline operators

inline is not required on member methods. Clang-tidy has a check for
this. Enable the check and fix the two bad usages.

Tested: Code compiles.

Change-Id: I3115b7c0

Fix redundant inline operators

inline is not required on member methods. Clang-tidy has a check for
this. Enable the check and fix the two bad usages.

Tested: Code compiles.

Change-Id: I3115b7c0c4005e1082e0005b818fbe6569511f49
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Use no-switch-default on clang

clang-18 improves this check so that we can actually use it. Enable it
and fix all violations.

Change-Id: Ibe4ce19c423d447a4cbe593d1abba948362426af
Signed-off-by: Ed

Use no-switch-default on clang

clang-18 improves this check so that we can actually use it. Enable it
and fix all violations.

Change-Id: Ibe4ce19c423d447a4cbe593d1abba948362426af
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Fix SSE sockets

Redfish protocol validatator has SSE tests that expose some bad coding
practies in SSE handlers, namely, that there are several cases where we
don't check for nullptr.

Fix them.

Th

Fix SSE sockets

Redfish protocol validatator has SSE tests that expose some bad coding
practies in SSE handlers, namely, that there are several cases where we
don't check for nullptr.

Fix them.

This appears to have been introduced in:
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/41319

Tested: Redfish service validator passes more tests.

Change-Id: Id980725f007d044b7d120dbe0f4b625865cab6ba
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Fix regression in http_file_body

The commit:
b5f288d Make use of filebody for dump offload

Caused a minor failure in clearing responses, where open file handles
wouldn't be closed in between querie

Fix regression in http_file_body

The commit:
b5f288d Make use of filebody for dump offload

Caused a minor failure in clearing responses, where open file handles
wouldn't be closed in between queries, resulting in the next read to
return empty content. This caused redfish protocol validator to fail.

boost::beast::http::response::clear() documentation shows that it only
clears the headers, not the file body. Now normally, this doesn't
matter, because bmcweb completely replaces the response body when a new
response is driven, but not in the case of files.

Add response.body().clear() during the clear to ensure the response is
cleared.

In addition, add encodingType to the clear() call, to ensure that it is
reset as well. This is a bug, but I don't know the reproduction steps.

Tested: Redfish protocol validator now completes (with SSE failures)

Change-Id: Ice6d5085003034a1bed48397ddc6316e9cd0536f
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Rename http2 unpacker

These classes accidentally overlapped in naming with the nghttp2
classes. This is because this class, unlike most nghttp2 classes
doesn't end in _ptr for a type. This changes

Rename http2 unpacker

These classes accidentally overlapped in naming with the nghttp2
classes. This is because this class, unlike most nghttp2 classes
doesn't end in _ptr for a type. This changes the class name to add a
_ex to differentiate the two classes, and avoid a warning in clang.

Tested: Unit tests pass. Code only used in unit test.

Change-Id: I91a6982264df69bc65166ab38feddc21f72cd223
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Check optionals in tidy

clang-tidy-18 makes this feature stable enough for us to use in general.
Enable the check, and fix the couple of regressions that have snuck in
since we last ran the check.

Check optionals in tidy

clang-tidy-18 makes this feature stable enough for us to use in general.
Enable the check, and fix the couple of regressions that have snuck in
since we last ran the check.

Tidy seems to not be able to understand that ASSERT will not continue,
so if we ASSERT a std::optional, it's not a bug. Add explicit checks to
keep tidy happy.

Tested: clang-tidy passes.

Change-Id: I0986453851da5471056a7b47b8ad57a9801df259
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Fix redundant init issues

clang-tidy-18 must've fixed their checking for these in headers.
Resolve as the robot commands.

Tested: Noop changes made by tidy. Code compiles.

Change-Id: I1de7686c597

Fix redundant init issues

clang-tidy-18 must've fixed their checking for these in headers.
Resolve as the robot commands.

Tested: Noop changes made by tidy. Code compiles.

Change-Id: I1de7686c597deffb0df91c30dae1a29f9ba7900e
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Add nolint for naming violations

Change-Id: I0133bbd0a7573bd3d1e3c3c99382442b286696f6
Signed-off-by: Ed Tanous <ed@tanous.net>

Don't use switch for single conditional http2

This code was copied from one of the nghttp2 examples, and makes things
more complicated than they should be. We only handle one case here, so
a patter

Don't use switch for single conditional http2

This code was copied from one of the nghttp2 examples, and makes things
more complicated than they should be. We only handle one case here, so
a pattern of returning early is easier.

Also, this resolves a possible clang-tidy bugprone warning (that we
don't yet enable).

Tested: Http2 unit tests pass (good coverage for this case).

Change-Id: Ie8606872f3a96f1bb0329bf22a4f7429f431bbef
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Allow reading http2 bodies

This allows http2 connections to now host authenticated endpoints.
Note, this work exposed that the http2 path was not calling
preparePayload() and responses were therefor

Allow reading http2 bodies

This allows http2 connections to now host authenticated endpoints.
Note, this work exposed that the http2 path was not calling
preparePayload() and responses were therefore missing the
Content-Length header. preparePayload is now called, and Content-Length
is added to the unit tests.

This commit also allows a full Redfish Service Validator test to pass
entirely using HTTP2.

Tested: Unit tests pass.

Curl /redfish/v1/Managers/bmc/LogServices/Journal/Entries
(which returns a payload larger than 16kB) succeeds and returns the
data.

Manually logging in with both basic and session authentication succeeds
over http2.

A modified Redfish-Service-Validator, changed to use httpx as its
backend, (thus using http2) succeeds.

Change-Id: I956f3ff8f442e9826312c6147d7599ab136a8e7c
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Allow routes with 5 wildcards

We don't have any routes that use 5 wildcards, but clearly someone uses
it because of the bug #270. There's no reason not to fix this.

Ideally we would support an arb

Allow routes with 5 wildcards

We don't have any routes that use 5 wildcards, but clearly someone uses
it because of the bug #270. There's no reason not to fix this.

Ideally we would support an arbitrary number of wildcards, but that's a
template problem for another day.

Tested: No way to test, inspection only.

Change-Id: I5de75f5288124e84c153518966d658e1c899f6d5
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Allow no spaces in content-type

For the content type header

application/json;charset=utf-8

The Redfish specification DSP0266 shows no space between the ; and
charset. Sites like mozilla show the

Allow no spaces in content-type

For the content type header

application/json;charset=utf-8

The Redfish specification DSP0266 shows no space between the ; and
charset. Sites like mozilla show the space included [1]

Considering the discrepancy, we should just accept both.

Resolves #271

[1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Type

Tested: Submitter reports issue fixed.

Change-Id: I77b7db91d65acc84f2221ec50985d4b942fbe77f
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Rename FileBody to HttpBody

Now that our custom body type does things more than files, it makes
sense to rename it. This commit renames the header itself, then all
instances of the class.

Tested:

Rename FileBody to HttpBody

Now that our custom body type does things more than files, it makes
sense to rename it. This commit renames the header itself, then all
instances of the class.

Tested: Basic GET requests succeed.
Change-Id: If4361ac8992fc7c268f48a336707f96e68d3576c
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Add unit test for SSE

Writing this test exposed some bugs in SSE that got merged.
sendSSEHeader was never called, leading to a connection that starts
and immediately closes with no error code.

This

Add unit test for SSE

Writing this test exposed some bugs in SSE that got merged.
sendSSEHeader was never called, leading to a connection that starts
and immediately closes with no error code.

This issue has been corrected in code, such that the sockets start.

To allow for unit tests, the io_service needs to be passed into the
class, previously, the SSE connection was pulling the io_context from
the DBus connection, which is odd, given that the SSE connection has
no other dependencies on DBus.

Unit tests should help keep it working.

Tested: Unit tests pass.

Change-Id: I48080d2a94b6349989f556cd1c7b103bad498526
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Fix duplicated etag generation

bmcweb generates the duplicated etags on GET and it may cause the
PATCH failure if etag is used - e.g. redfishtool..

This commit is to put only one Etag on http_respo

Fix duplicated etag generation

bmcweb generates the duplicated etags on GET and it may cause the
PATCH failure if etag is used - e.g. redfishtool..

This commit is to put only one Etag on http_response on GET.

Tested:

- Check the duplicated Etag on GET, and check it after fix

Before:
```
$ curl -v -k -X GET https://admin:${password}@${bmc}:18080/redfish/v1/AccountService/Accounts/readonly
...
< ETag: "D4B30BB4"
< ETag: "D4B30BB4"
```

After:
```
$ curl -v -k -X GET https://admin:${password}@${bmc}:18080/redfish/v1/AccountService/Accounts/readonly
...
< ETag: "D4B30BB4"
```

Change-Id: I5361919c5671b546181a26de792bc57de2c4f670
Signed-off-by: Myung Bae <myungbae@us.ibm.com>

show more ...

Write unit tests for http2 connection

This unit test currently only tests a simple connect and settings frame
transfer, but should form the basis for more complex testing in the
future.

Tested: Uni

Write unit tests for http2 connection

This unit test currently only tests a simple connect and settings frame
transfer, but should form the basis for more complex testing in the
future.

Tested: Unit tests pass

Change-Id: Ieb803dbe490129ec5fe99fb3d4505a06202e282e
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...

Simplify HTTP/2 buffering

Using the mem_send methods of nghttp2 can reduce the amount of buffering
we need to do. This is recommended by the nghttp2 docs.

Tested: Enabled experimental-http. Curl

Simplify HTTP/2 buffering

Using the mem_send methods of nghttp2 can reduce the amount of buffering
we need to do. This is recommended by the nghttp2 docs.

Tested: Enabled experimental-http. Curl succeeds on /redfish/v1, and
shows:

* using HTTP/2
* [HTTP/2] [1] OPENED stream for https://localhost:18080/redfish/v1

Change-Id: I287d8c956f064d244116fac853055a17fca915a2
Signed-off-by: Ed Tanous <ed@tanous.net>

show more ...