#
102a4cda |
| 15-Apr-2024 |
Jonathan Doman <jonathan.doman@intel.com> |
Manage Request with shared_ptr
This is an attempt to solve a class of use-after-move bugs on the Request objects which have popped up several times. This more clearly identifies code which owns the
Manage Request with shared_ptr
This is an attempt to solve a class of use-after-move bugs on the Request objects which have popped up several times. This more clearly identifies code which owns the Request objects and has a need to keep it alive. Currently it's just the `Connection` (or `HTTP2Connection`) (which needs to access Request headers while sending the response), and the `validatePrivilege()` function (which needs to temporarily own the Request while doing an asynchronous D-Bus call). Route handlers are provided a non-owning `Request&` for immediate use and required to not hold the `Request&` for future use.
Tested: Redfish validator passes (with a few unrelated fails). Redfish URLs are sent to a browser as HTML instead of raw JSON.
Change-Id: Id581fda90b6bceddd08a5dc7ff0a04b91e7394bf Signed-off-by: Jonathan Doman <jonathan.doman@intel.com> Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
25b54dba |
| 17-Apr-2024 |
Ed Tanous <ed@tanous.net> |
Bring consistency to config options
The configuration options that exist in bmcweb are an amalgimation of CROW options, CMAKE options using #define, pre-bmcweb ifdef mechanisms and meson options usi
Bring consistency to config options
The configuration options that exist in bmcweb are an amalgimation of CROW options, CMAKE options using #define, pre-bmcweb ifdef mechanisms and meson options using a config file. This history has led to a lot of different ways to configure code in the codebase itself, which has led to problems, and issues in consistency.
ifdef options do no compile time checking of code not within the branch. This is good when you have optional dependencies, but not great when you're trying to ensure both options compile.
This commit moves all internal configuration options to: 1. A namespace called bmcweb 2. A naming scheme matching the meson option. hyphens are replaced with underscores, and the option is uppercased. This consistent transform allows matching up option keys with their code counterparts, without naming changes. 3. All options are bool true = enabled, and any options with _ENABLED or _DISABLED postfixes have those postfixes removed. (note, there are still some options with disable in the name, those are left as-is) 4. All options are now constexpr booleans, without an explicit compare.
To accomplish this, unfortunately an option list in config/meson.build is required, given that meson doesn't provide a way to dump all options, as is a manual entry in bmcweb_config.h.in, in addition to the meson_options. This obsoletes the map in the main meson.build, which helps some of the complexity.
Now that we've done this, we have some rules that will be documented. 1. Runtime behavior changes should be added as a constexpr bool to bmcweb_config.h 2. Options that require optionally pulling in a dependency shall use an ifdef, defined in the primary meson.build. (note, there are no options that currently meet this class, but it's included for completeness.)
Note, that this consolidation means that at configure time, all options are printed. This is a good thing and allows direct comparison of configs in log files.
Tested: Code compiles Server boots, and shows options configured in the default build. (HTTPS, log level, etc)
Change-Id: I94e79a56bcdc01755036e4e7278c7e69e25809ce Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
003301a2 |
| 16-Apr-2024 |
Ed Tanous <ed@tanous.net> |
Change ssl stream implementations
Boost beast ssl_stream is just a wrapper around asio ssl_stream, and aims to optimize the case where we're writing small payloads (one or two bytes.) which needs to
Change ssl stream implementations
Boost beast ssl_stream is just a wrapper around asio ssl_stream, and aims to optimize the case where we're writing small payloads (one or two bytes.) which needs to be optimized in SSL.
bmcweb never writes one or two bytes, we almost always write the full payload of what we received, so there's no reason to take the binary size overhead, and additional boost headers that this implementation requires.
Tested: This drops the on-target binary size by 2.6%
Redfish service validator passes.
Change-Id: Ie1ae6f197f8e5ed70cf4abc6be9b1b382c42d64d Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
8db83747 |
| 13-Apr-2024 |
Ed Tanous <ed@tanous.net> |
Clean up BMCWEB_ENABLE_SSL
This macro came originally from CROW_ENABLE_SSL, and was used as a macro to optionally compile without openssl being required.
OpenSSL has been pulled into many other dep
Clean up BMCWEB_ENABLE_SSL
This macro came originally from CROW_ENABLE_SSL, and was used as a macro to optionally compile without openssl being required.
OpenSSL has been pulled into many other dependencies, and has been functionally required to be included for a long time, so there's no reason to hold onto this macro.
Remove most uses of the macro, and for the couple functional places the macro is used, transition to a constexpr if to enable the TLS paths.
This allows a large simplification of code in some places.
Tested: Redfish service validator passes.
Change-Id: Iebd46a68e5e417b6031479e24be3c21bef782f4c Signed-off-by: Ed Tanous <ed@tanous.net>
show more ...
|
#
8cb2c024 |
| 27-Mar-2024 |
Ed Tanous <ed@tanous.net> |
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 ...
|
#
4fa45dff |
| 01-Sep-2023 |
Ed Tanous <edtanous@google.com> |
Unit test Connection
Boost asio provides a test stream object that we can use to begin unit testing the connection object. This patchset uses it to re-enable some simple http1.1 tests. There's som
Unit test Connection
Boost asio provides a test stream object that we can use to begin unit testing the connection object. This patchset uses it to re-enable some simple http1.1 tests. There's some features that have snuck into the connection class that aren't compatible with a stream (like ip address getting), so unfortunately we do need the connection class to be aware if it's in test mode, but that tradeoff seems worthwhile.
Tested: Unit test pass.
Change-Id: Id8b1f8866582b58502dbafe6139f841bf64b8ef3 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 ...
|
#
47488a98 |
| 26-Jun-2023 |
Ed Tanous <edtanous@google.com> |
Remove the black_magic namespace
The black_magic namespace has been eradicated of what most would call "black magic" and while there's some non-trivial stuff in there, it's far from the most complic
Remove the black_magic namespace
The black_magic namespace has been eradicated of what most would call "black magic" and while there's some non-trivial stuff in there, it's far from the most complicated part of this stack.
This commit takes the two remaining things in the black_magic namespace, namely the parameter tagging functionality, and moves them into the utility namespace.
Tested: Redfish service validator passes
Change-Id: I9e2686fff5ef498cafc4cb83d4d808ea849f7737 Signed-off-by: Ed Tanous <edtanous@google.com>
show more ...
|
#
21b4aba4 |
| 05-Jun-2023 |
Ed Tanous <edtanous@google.com> |
Remove this->
this-> is redundant when operating within a class member scope. We should aim to be consistent.
This change was done automatically with sed replace
Tested: Code compiles
Signed-off
Remove this->
this-> is redundant when operating within a class member scope. We should aim to be consistent.
This change was done automatically with sed replace
Tested: Code compiles
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: If791e83e0bd5f84031e65156b7ea082ded8e158c
show more ...
|
#
6fde95fa |
| 01-Jun-2023 |
Ed Tanous <edtanous@google.com> |
Server-sent-event fixes
This makes several changes to server-sent events to allow it to merge to master. The routing system has been removed in leiu of using content-type eventstream detection. Ti
Server-sent-event fixes
This makes several changes to server-sent events to allow it to merge to master. The routing system has been removed in leiu of using content-type eventstream detection. Timers have been added to the sse connections, and sse connections now rely on async_wait, rather than a full read.
Tested: WIP
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Id0ff0ebc2b3a795b3dba008e440556a9fdd882c2
show more ...
|
#
88ada3bc |
| 13-Apr-2023 |
V-Sanjana <sanjana.v@intel.com> |
Add Server-Sent-Event support
Server-Sent-Event is a standard describing how servers can initiate data transmission towards clients once an initial client connection has been established. Unlike web
Add Server-Sent-Event support
Server-Sent-Event is a standard describing how servers can initiate data transmission towards clients once an initial client connection has been established. Unlike websockets (which are bidirectional), Server-Sent-Events(SSE) are unidirectional and commonly used to send message updates or continuous data streams to a browser client.
This is base patch for adding Server-Sent-Events routing support to bmcweb. Redfish EventService SSE style subscription uses SSE route for sending the Events/MetricReports to client which establishes the connection.
Tested this patch with along with EventService SSE support patches and verified the functionalty on browser.
Tested: - Tested using follow-up patches on top which adds support for Redfish EventService SSE style subscription and observed events are getting sent periodically. - Created SSE subscription from the browser by visiting https://<BMC IP>/redfish/v1/EventService/SSE
Change-Id: I36956565cbba30c2007852c9471f477f6d1736e9 Signed-off-by: AppaRao Puli <apparao.puli@linux.intel.com> Signed-off-by: P Dheeraj Srujan Kumar <p.dheeraj.srujan.kumar@intel.com> Signed-off-by: V-Sanjana <sanjana.v@intel.com>
show more ...
|
#
f8ca6d79 |
| 28-Jun-2022 |
Ed Tanous <edtanous@google.com> |
Allow async resolver to be optional
This commit adds a meson option to allow selecting which dns resolver bmcweb uses. There are use cases, like Open Compute Project Inband Management Agent, that w
Allow async resolver to be optional
This commit adds a meson option to allow selecting which dns resolver bmcweb uses. There are use cases, like Open Compute Project Inband Management Agent, that would require not using dbus, which would require us to fall back to the asio resolver. This commit makes the existing asio resolver constructor, and async_resolve methods match the equivalents in asio (which we intended to do anyway), then adds a macro and configure option for being able to select which resolver backend to rely on.
Tested: Code can now compile without sdbusplus.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I3220214367179f131a60082bdfaf7e725d35c125
show more ...
|
#
a232343d |
| 12-May-2023 |
Patrick Williams <patrick@stwcx.xyz> |
http-app: fix unused macro clang-tidy warning
``` /data0/jenkins/workspace/ci-repository/openbmc/bmcweb/http/app.hpp:27:9: error: macro is not used [clang-diagnostic-unused-macros,-warnings-as-error
http-app: fix unused macro clang-tidy warning
``` /data0/jenkins/workspace/ci-repository/openbmc/bmcweb/http/app.hpp:27:9: error: macro is not used [clang-diagnostic-unused-macros,-warnings-as-errors] #define BMCWEB_ROUTE(app, url) ```
Signed-off-by: Patrick Williams <patrick@stwcx.xyz> Change-Id: I49b6239d7d5f31c52af6f6e5b0bc57be22a299b5
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 ...
|
#
7e9093e6 |
| 17-Sep-2021 |
P Dheeraj Srujan Kumar <p.dheeraj.srujan.kumar@intel.com> |
Add Support for privilege check in handleUpgrade
This commit enables privilege check for user(s) in case of upgraded connections. Currently users with no privileges will also be able to access Webso
Add Support for privilege check in handleUpgrade
This commit enables privilege check for user(s) in case of upgraded connections. Currently users with no privileges will also be able to access Websockets connections (Ex: KVM).
The privilege check was already in place for normal connections (i.e. router->handle()). This commit lifts off the privilege check code and moves it into a common function (validatePrivilege()), which can be used both by handle() and handleUpgrade() and register required callback to be called.
Also, the const qualifier for Request in the handleUpgrade() function's signature is removed to enable setting "isConfigureSelf" field of request. The signature of handleUpgrade() is made identical to handle()
Tested: - websocket_test.py Passed - Admin and Operator users are able to access KVM on WebUI - Readonly User was unable to access KVM on WebUI
Signed-off-by: P Dheeraj Srujan Kumar <p.dheeraj.srujan.kumar@intel.com> Change-Id: I6f743c27e7e6077f1c6c56e6958922027e4404e8
show more ...
|
#
a9f076e5 |
| 18-Oct-2021 |
P Dheeraj Srujan Kumar <p.dheeraj.srujan.kumar@intel.com> |
Add asyncResp support to handleUpgrade
This commit enables passing down the asyncResp (of the connection) to the handler of upgraded connections. This is already in place for normal requests (i.e. C
Add asyncResp support to handleUpgrade
This commit enables passing down the asyncResp (of the connection) to the handler of upgraded connections. This is already in place for normal requests (i.e. Class Router -> handle())
This change would enable any async calls that would be required before upgrade of the connection. For example, as on today, we have only Authentication of user in place for upgraded connection, but not Authorization. So, this asyncResp could further be used for such dbus calls to return informative response.
This commit updates the signature of all the handleUpgrade() functions present in router.hpp to take in asyncResp object instead of normal response.
Tested : - websocket_test.py Passed - KVM was functional in WebUI.
Change-Id: I1c6c91f126b734e1b5573d5ef204fe2bf6ed6c26 Signed-off-by: P Dheeraj Srujan Kumar <p.dheeraj.srujan.kumar@intel.com>
show more ...
|
#
d9049df1 |
| 02-Aug-2022 |
Nan Zhou <nanzhoumails@gmail.com> |
app: fix -Wpessimizing-move
clang14 doesn't compile because of "moving a temporary object prevents copy elision".
This also alligns the plaintext socket with style of SSL socket.
Tested: trivial c
app: fix -Wpessimizing-move
clang14 doesn't compile because of "moving a temporary object prevents copy elision".
This also alligns the plaintext socket with style of SSL socket.
Tested: trivial change. It builds.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: I9203cf162d738290306f9ba73ec0ab8f2ca5033c
show more ...
|
#
cec58fe3 |
| 14-Jun-2022 |
Nan Zhou <nanzhoumails@gmail.com> |
http/app: iwyu
While working on tests, I found that |app.hpp| is missing some boost headers. I added them manually in this commit.
Tested: code compiles.
Signed-off-by: Nan Zhou <nanzhoumails@gmai
http/app: iwyu
While working on tests, I found that |app.hpp| is missing some boost headers. I added them manually in this commit.
Tested: code compiles.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com> Change-Id: I1d2fb0f312e1810d836c986e320263a9581f13f2
show more ...
|
#
1c825279 |
| 28-Mar-2022 |
Jiaqing Zhao <jiaqing.zhao@intel.com> |
Drop unused App.sslFile() method
App.sslFile() method is never called, the sslContext is generated in ensuressl::getSslContext() function now. So remove these unused code.
Tested: Build pass.
Chan
Drop unused App.sslFile() method
App.sslFile() method is never called, the sslContext is generated in ensuressl::getSslContext() function now. So remove these unused code.
Tested: Build pass.
Change-Id: I2737462a3a2ec2e0dc792e5070e9e5a7244bc889 Signed-off-by: Jiaqing Zhao <jiaqing.zhao@intel.com>
show more ...
|
#
e05aec50 |
| 25-Jan-2022 |
Ed Tanous <edtanous@google.com> |
Add readability-redundant-* checks
There's a number of redundancies in our code that clang can sanitize out. Fix the existing problems, and enable the checks.
Signed-off-by: Ed Tanous <edtanous@go
Add readability-redundant-* checks
There's a number of redundancies in our code that clang can sanitize out. Fix the existing problems, and enable the checks.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ie63d7b7f0777b702fbf1b23a24e1bed7b4f5183b
show more ...
|
#
600d2394 |
| 07-Jan-2022 |
Ed Tanous <edtanous@google.com> |
Enable cpp core guidelines macro checks
We only use a couple macros. Ignore them in the checks.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I38feb10f76f6aaea8899617f081c9be68c88b3eb
|
#
ecd6a3a2 |
| 07-Jan-2022 |
Ed Tanous <edtanous@google.com> |
Enable cppcoreguidelines-special-member-functions checks
Part of enforcing cpp core guidelines involves explicitly including all constructors required on a non-trivial class. We were missing quite
Enable cppcoreguidelines-special-member-functions checks
Part of enforcing cpp core guidelines involves explicitly including all constructors required on a non-trivial class. We were missing quite a few. In all cases, the copy/move/and operator= methods are simply deleted.
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ie8d6e8bf2bc311fa21a9ae48b0d61ee5c1940999
show more ...
|
#
f94c4ecf |
| 06-Jan-2022 |
Ed Tanous <edtanous@google.com> |
Enable clang-tidy forward reference checks
Clang-13 adds new checks we can turn on, which find quite a few errors.
Tested: Code compiles
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I
Enable clang-tidy forward reference checks
Clang-13 adds new checks we can turn on, which find quite a few errors.
Tested: Code compiles
Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: I74b780760014c898cc440b37aea640b33e91c439
show more ...
|
#
8d1b46d7 |
| 31-Mar-2021 |
zhanghch05 <zhanghch05@inspur.com> |
Using AsyncResp everywhere Get the core using AsyncResp everywhere, and not have each individual handler creating its own object.We can call app.handle() without fear of the response
Using AsyncResp everywhere Get the core using AsyncResp everywhere, and not have each individual handler creating its own object.We can call app.handle() without fear of the response getting ended after the first tree is done populating. Don't use res.end() anymore. Tested: 1. Validator passed. Signed-off-by: zhanghaicheng <zhanghch05@inspur.com> Change-Id: I867367ce4a0caf8c4b3f4e07e06c11feed0782e8
show more ...
|
#
c7b9cb35 |
| 11-Feb-2021 |
Ed Tanous <edtanous@google.com> |
Bind dev server to ipv4 only On systems that don't support ipv6, or systems that don't have an ipv6 address, binding to all ipv6 addresses can fail. Because this is just the dev ser
Bind dev server to ipv4 only On systems that don't support ipv6, or systems that don't have an ipv6 address, binding to all ipv6 addresses can fail. Because this is just the dev server, it's perfectly reasonable to limit to ipv4 addresses only. This failure has been reported by several people over time, but it was only recently that I root caused this as their problem. This should have no effect on the BMC itself, as the bmc is using socket activation, and completely bypasses this code path. Tested: Launched bmcweb on a system that was previously failing because of a bind error, and observed that I could launch bmcweb and have it work correctly. Signed-off-by: Ed Tanous <edtanous@google.com> Change-Id: Ife6b051aa62d62e1691c5221d8ddee0b9bd012c0
show more ...
|