| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
TemporaryFileHandle class is used to create temp files in the
filesystem, and hold a handle to them until the class goes out of scope,
at which time they can be removed. It replaces makeFile(), which was
not RAII safe if an exception gets thrown, and could potentially leave
files in the filesystem if the tests fail.
Tested: Unit tests pass
Change-Id: I03eb0d342a6cd7b78115a8c42be9175f30c4ccd0
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Change-Id: I147664c3d181ba8ec535c7cddcb5c714e05616ea
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
And fix the includes that are wrong.
Note, there is a very large ignore list included in the .clang-tidy
configcfile. These are things that clang-tidy doesn't yet handle
well, like knowing about a details include.
Change-Id: Ie3744f2c8cba68a8700b406449d6c2018a736952
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
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>
|
|
File body needs some unit tests for managing the move constructors.
Tested: Unit tests pass.
Change-Id: Ia640aec75a6f3f85640a50f5dd492638871f9eca
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Now that we have a custom boost http body class, we can use it in more
cases. There's some significant overhead and code when switching to a
file body, namely removing all the headers. Making the body class
support strings would allow us to completely avoid that inefficiency.
At the same time, it would mean that we can now use that class for all
cases, including HttpClient, and http::Request. This leads to some code
reduction overall, and means we're reliant on fewer beast structures.
As an added benefit, we no longer have to take a dependency on
boost::variant2.
Tested: Redfish service validator passes, with the exception of
badNamespaceInclude, which is showing warnings prior to this commit.
Change-Id: I061883a73230d6085d951c15891465c2c8445969
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Logservice has been rewritten to use file_body to offload dump files
from BMC.
There are two kind of dump files, BMC dump and System dump.While BMC
dump just requires default support from beast::file_body, System dump
requires base64 encoding support from beast. But beast::file_body do not
have ready-made support for base64 encoding. So a custom file_body has
been written for the base64 encoding.
The openFile apis in crow::Response do not have support for unix file
descriptor. Since dump files are accesses via descriptors, added new
openFile api that accepts descriptors.
Tested:
Functionality test have been executed to verify the bmc dump offload.
Did sanity test by invoking bmcweb pages via browser.
Change-Id: I24192657c03d8b2f0394d31e7424c6796ba3227a
Signed-off-by: Abhilash Raju <abhilash.kollam@gmail.com>
|
|
As part of https://gerrit.openbmc.org/c/openbmc/bmcweb/+/67667, it would
be desirable if we could incrementally encode base64 in chunks. Given
that base64 encoding requires encoding 3 characters to 4, there's a
possibility that a chunk might not be mod 3 length.
This commit moves the base64 encoder into a class that can run
incrementally.
Tested: Unit tests pass. More tests in next commit.
Change-Id: Ic7da3fd4db865c99fcbd96ae06fdecb87628f94c
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
These were found with:
codespell -w $(git ls-files | grep "\.[hc]\(pp\)\?$")
At some point in the future, we might want to get this enabled in CI.
Change-Id: Iccb57b2adfd06a2e177e99db2923fe4e8e329118
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Meta Inc's client certificates use an internal Subject CN format
which AFAIK is specific to Meta and don't adhere to a known standard:
Subject: CN = <type>:<entity>/<hostname>
Commit adds the `mutual-tls-common-name-parsing=meta` option to, on
Meta builds, parse the Subject CN field and map either the <entity>
to a local user.
The <type> field determines what kind of client identity the cert
represents. Only type="user" is supported for now with <entity> being
the unixname of a Meta employee. For example, the Subject CN string
below maps to a local BMC user named "kawmarco":
Subject CN = "user:kawmarco/dev123.facebook.com"
Tested: Unit tests, built and tested on romulus using the script below:
https://gist.github.com/kawmarco/87170a8250020023d913ed5f7ed5c01f
Flags used in meta-ibm/meta-romulus/conf/layer.conf :
```
-Dbmcweb-logging='enabled'
-Dmutual-tls-common-name-parsing='meta'
```
Change-Id: I35ee9b92d163ce56815a5bd9cce5296ba1a44eef
Signed-off-by: Marco Kawajiri <kawajiri@meta.com>
|
|
bmcweb should be using the openssl primitives for these checks. There
are examples where we've known to have gotten the behavior incorrect, so
given that OpenSSL clearly should know these things better than we do,
use it.
Tested: unit tests pass.
Change-Id: I0bcd381a9e3c9a1e8e6dc39534e81fa698570689
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Mutual TLS paths were not tested. Add some unit tests.
Because CI doesn't actually compile dependent libraries with ASAN
enabled, and these tests call into openssl, we need to add a check for
if we're compiling with asan enabled.
Tested: unit tests pass.
Change-Id: I02dcb69708619cc00fffd840738c608db3ae8bdf
Signed-off-by: Ed Tanous <ed@tanous.net>
|
|
Write function in http_response.hpp missing implementation for first
write after changing from filebody.
Usecase:
Current resonse type is filebody. Developer tries to change the body
type to stringbody by calling write function.
Observed:
The write fails to update the body type.
Expected:
Write should succeed and body should change to string body.
Tested:
Unit test has been added for crow::Response.
Manual sanity test done for file offloads using curl.
Change-Id: Icbf8585b5b04c3ac5120d7b334c13d89ed3eb4aa
Signed-off-by: Abhilash Raju <abhilash.kollam@gmail.com>
|
|
At some point, the date got removed from http1 requests. HTTP2 does not
show this issue, but this showed up in unit tests (which is why the
prior commit is adding unit tests).
The Date Header is useful for synchronizing things like
Cache-Control-Policy, with the actual server time, instead of the local
system time.
Tested: Unit tests pass.
Change-Id: I8f105f0cbb6c816c5ec6b14cbeae587d728a20d2
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
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>
|
|
In boost 1.83.0, the boost::url maintainers deprecated the header only
usage of the library without warning. A discussion with the
maintainers[1] made it clear that they removed the abiliy on purpose,
and they're not going to add it back or add a deprecation strategy (they
did say they would update the documentation to actually match the
intent), and that from here on in we should be using the cmake boost
project to pull in the non-header-only boost libraries we use (which at
this point is ONLY boost url).
This commit updates to remove the usage of boost::urls::result typedef,
which was deprecated in this release (which causes a compile error) and
moves it to boost::system::result.
In addition, it updates our meson files to pull in the boost project as
a cmake dependency.
[1] https://cpplang.slack.com/archives/C01JR6C9C4U/p1696441238739129
Tested: Not yet.
Change-Id: Ia7adfc0348588915440687c3ab83a1de3e6b845a
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
Type safety is a good thing. In:
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/65606
It was found that splitting out the URI into encoded pieces in the early
phase removed some information we needed, namely whether or not a URI
was ipv6. This commit changes http client such that it passes all the
information through, with the correct type, rather than passing in
hostname, port, path, and ssl separately.
Opportunistically, because a number of log lines are changing, this uses
the opportunity to remove a number of calls to std::to_string, and rely
on std::format instead.
Now that we no longer use custom URI splitting code, the
ValidateAndSplitUrl() method can be removed, given that our validation
now happens in the URI class.
Tested: Aggregation works properly when satellite URIs are queried.
Change-Id: I9f605863179af54c5af2719bc5ce9d29cbfffab7
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
Clang-tidy has the aforementioned check, which shows a few places in the
core where we ignored the required optional checks. Fix all uses.
Note, we cannot enable the check that this time because of some weird
code in health.hpp that crashes tidy[1]. That will need to be a future
improvement.
There are tests that call something like
ASSERT(optional)
EXPECT(optional->foo())
While this isn't an actual violation, clang-tidy doesn't seem to be
smart enough to deal with it, so add some explicit checks.
[1] https://github.com/llvm/llvm-project/issues/55530
Tested: Redfish service validator passes.
Change-Id: Ied579cd0b957efc81aff5d5d1091a740a7a2d7e3
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
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>
|
|
Boost 1.82 dropped a lovely new toy, boost::urls::format, which is a lot
like our urlFromPieces method, but better in that it makes the resulting
uris more readable, and allows doing things like fragments in a single
line instead of multiple. We should prefer it in some cases.
Tested:
Redfish service validator passes.
Spot checks of URLs work as expected.
Unit tests pass.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ia7b38f0a95771c862507e7d5b4aa68aa1c98403c
|
|
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>
|
|
The router historically came from crow. Crow supported wildcards of
<int>, <float>, and <double>. bmcweb doesn't use them, nor should it in
basically any case, as we now have explicit 404 handling.
This commit removes them. This amounts to about -450 lines of code, but
it's some of the scarier code we have, some of it existing in the
namespace "black_magic". Reducing the brain debt for people working in
this subsystem seems worthwhile. There is no case in the future where
we would use integer based url parameters.
Tested: Redfish service validator passes. Should be good enough
coverage for a code removal.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I34add8df7d3486952474ca7ec3dc6be990c50ed0
|
|
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
|
|
Only id in event_service and account_service have not been updated due
to the risk of it breaking the username/id. It will require further
testing to verify.
Use urlFromPieces wherever that is needed to insert a variable in the
URI. Don't use urlFromPieces when it is hardcoded values. This allow us
to control all resource URIs that is dynamically added and to sync with
the current recommanded method for `@odata.id`. The goal is to have a
common place to manage the url created from dbus-paths in order to
manage/update it easily when needed.
Tested:
RedfishValidtor Passed for all resource including the sensors with the
fragments.
Change-Id: I95cdfaaee58fc7f21c95f5944e1e5c813b3215f2
Signed-off-by: Willy Tu <wltu@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
The new boost URL now interops properly with std::string_view, which is
great, and cleans up a bunch of mediocre code to convert one to another.
It has also been pulled into boost-proper, so we no longer need a
boost-url dependency that's separate.
Unfortunately, boost url makes these improvements by changing
boost::string_view for boost::urls::const_string, which causes us to
have some compile errors on the missing type.
The bulk of these changes fall into a couple categories, and have to be
executed in one commit.
string() is replaced with buffer() on the url and url_view types
boost::string_view is replaced by std::string_view for many times, in
many cases removing a temporary that we had in the code previously.
Tested: Code compiles with boost 1.81.0 beta.
Redfish service validator passes.
Pretty good unit test coverage for URL-specific use cases.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8d3dc89b53d1cc390887fe53605d4867f75f76fd
|
|
As is, the router designates routes for every possible boost verb, of
which there are 31. In bmcweb, we only make use of 6 of those verbs, so
that ends up being quite a bit of wasted space and cache non-locality.
This commit invents a new enum class for declaring a subset of boost
verbs that we support, and a mapping between bmcweb verbs and boost
verbs.
Then it walks through and updates the router to support converting one
to another.
Tested:
Unit Tested
Redfish Service Validator performed on future commit
Signed-off-by: Ed Tanous <edtanous@google.com>
Signed-off-by: Edward Lee <edwarddl@google.com>
Change-Id: I3c89e896c632a5d4134dbd08a30b313c12a60de6
|
|
Add helper function to append pieces to existing url to allow more
flexible control over the url. This allows us to avoid have each
resource append the pieces outside of the utility functions and help
maintain all url modifications in a central place for easy management.
Tested: Does not affect Redfish Tree. Unit Test passed.
Change-Id: I751f3c120cbadb465915b12aa253edd53ef32123
Signed-off-by: Willy Tu <wltu@google.com>
|
|
Like other C++ projects, unit tests normally are in a separate repo and
respect the folder structure of the file under test.
This commit deleted all "ut" folder and move tests to a "test" folder.
The test folder also has similar structure as the main folder.
This commit also made neccessary include changes to make codes compile.
Unused tests are untouched.
Tested: unit test passed.
Reference:
[1] https://github.com/grpc/grpc/tree/master/test
[2] https://github.com/boostorg/core/tree/414dfb466878af427d33b36e6ccf84d21c0e081b/test
[3] Many other OpenBMC repos: https://github.com/openbmc/entity-manager/tree/master/test
[4] https://stackoverflow.com/questions/2360734/whats-a-good-directory-structure-for-larger-c-projects-using-makefile
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I4521c7ef5fa03c47cca5c146d322bbb51365ee96
|
