| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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
|
|
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>
|
|
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
|
|
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
|
|
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>
|
|
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
|
|
```
/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
|
|
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>
|
|
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
|
|
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>
|
|
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
|
|
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
|
|
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>
|
|
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
|
|
We only use a couple macros. Ignore them in the checks.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I38feb10f76f6aaea8899617f081c9be68c88b3eb
|
|
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
|
|
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
|
|
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
|
|
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
|
|
camelLower is not a type, camelBack is.
Changes were made automatically with clang-tidy --fix-errors
To be able to apply changes automatically, the only way I've found that
works was to build the version of clang/clang-tidy that yocto has, and
run the fix script within bitbake -c devshell bmcweb. Unfortunately,
yocto has clang-tidy 11, which can apparently find a couple extra errors
in tests we already had enabled. As such, a couple of those are also
included.
Tested:
Ran clang-tidy-11 and got a clean result.
Signed-off-by: Ed Tanous <ed@tanous.net>
Change-Id: I9d1080b67f0342229c2f267160849445c065ca51
|
|
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
|
