| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
If given a bad URI, e.g. "https://$bmc/?a=id;" return http bad request
(400) like we do other places, e.g. a few lines below.
Certain scanners expect to see bad request when crawling and probing for
vulnerabilities and using not valid URIs. Just dropping the connection
causes errors with these scanners. They think connection problem or
worse the server was taken down.
Tested: Tested downstream https://$bmc/?a=id; returns bad request.
Change-Id: I99a52d4c5e7f366046c5de1cf22c4db95ab39e13
Signed-off-by: Gunnar Mills <gmills@us.ibm.com>
|
|
cppcheck correctly notes that a lot of our variables can be declared at
more specific scopes, and in every case, it seems to be correct.
Tested: Redfish service validator passes. Unit test coverage on others.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ia4414410d0e8f74a3bd40fdc0e0232450d1a6416
|
|
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
|
|
If-Match is a header in the HTTP specification[1] designed for handling
atomic operations within a given HTTP tree. It allows a mechanism for
an implementation to explicitly declare "only take this action if the
resource has not been changed". While most things within the Redfish
tree don't require this level of interlocking, it continues to round out
our redfish support for the specific use cases that might require it.
Redfish specification 6.5 states:
If a service supports the return of the ETag header on a resource, the
service may respond with HTTP 428 status code if the If-Match or
If-None-Match header is missing from the PUT or PATCH request for the
same resource, as specified in RFC6585
This commit implements that behavior for all handlers to follow the
following flow.
If If-Match is present
Repeat the same request as a GET
Compare the ETag produced by the GET, to the one provided by If-Match
If they don't match, return 428
if they do match, re-run the query.
[1] https://www.rfc-editor.org/rfc/rfc2616#section-14.24
As a consequence, this requires declaring copy and move constructors
onto the Request object, so the request object can have its lifetime
extended through a request, which is very uncommon.
Tested:
Tests run on /redfish/v1/AccountService/Accounts/root
PATCH with correct If-Match returns 200 success
PATCH with an incorrect If-Match returns 419 precondition required
GET returns the resource as expected
Redfish service validator passes
Redfish protocol validator passes! ! ! ! !
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I530ab255259c32fe4402eb8e5104bd091925c77b
|
|
While implementing https://gerrit.openbmc.org/c/openbmc/bmcweb/+/57932,
there has been an issue where there is a circular dependency between
routing.hpp and privileges.hpp. This code move predates this change
to resolve it before implementing the heart of redfish authz.
Circular dependency will occur when we try to use the http verb
index variables in privilege.hpp. If this occurs routing.hpp
and privilege.hpp will co-depend on each other
and this code move prevents this from occuring.
Tested:
bitbake bmcweb
Code compiles (code move only)
Redfish Validator passed on next commit
Signed-off-by: Edward Lee <edwarddl@google.com>
Change-Id: I46551d9fe222e702d239ed3ea6d3d7e505d488c8
|
|
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
|
|
This commit implements support for the If-None-Match header on http
requests. This can be combined with the
89f180089bce9cc431d0b1053410f262f157b987 commit for producing ETag to
allow a client to have a highly efficient cache, while still pulling
data from the BMC.
This behavior is documented several places, in W3C produced docs[1], as
well as section 7.1 of the Redfish specification:
'''
A service only returns the resource if the current ETag of that resource
does not match the ETag sent in this header.
If the ETag in this header matches the resource's current ETag, the GET
operation returns the HTTP 304 status code.
'''
Inside bmcweb, this behavior is accomplished in a relatively naive way,
by creating the complete request, then doing a direct ETag comparison
between the generated data and the request header. In the event the two
match, 304 not-modified is returned, in-line with both the Redfish
specification and the HTTP RFC.
[1] https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-None-Match
Tested (on previous rebase):
First, request ServiceRoot
curl --insecure -vvvv --user root:0penBmc https://192.168.7.2/redfish/v1
This returns a header similar to:
< ETag: "ECE52663"
Taking that ETag, and putting it into an If-None-Match header:
```
curl --insecure -vvvv -H "If-None-Match: \"ECE52663\"" \
--user root:0penBmc https://192.168.7.2/redfish/v1
```
Returns:
< HTTP/1.1 304 Not Modified
...
< Content-Length: 0
Showing that the payload was not repeated, and the response size was
much.... much smaller on the wire. Performance was not measured as part
of this testing, but even if it has no performance impact (which is
unlikely), this change is still worthwhile to implement more of the
Redfish specification.
Redfish-service-validator passes.
Redfish-protocol-validator passes 1 more atom in comparison to previous.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I1e7d41738884593bf333e4b9b53d318838808008
|
|
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>
|
|
Now that we are using timer.async_wait() with the async http
operations we need to account for the scenario where the timer fails
before the operation fails. When that occurs we need to abort the
operation once its callback gets called.
Currently we proceed as if the timer doesn't exist. This causes a
fault if one of the operations times out. This patch adds a check
to the start of each async operation so that we do not continue with
the normal message sending flow when an operation times out.
Tested:
In order to create a connection timeout I created a dummy interface
and set the IP of my satellite BMC to route to the interface:
ip link add dummy0 type dummy
ip link set dev dummy0 up
ip route add 120.60.30.15 dev dummy0
All packets sent to 120.60.30.15 will get dropped and thus connection
attempts will timeout. This does not cause bmcweb to crash.
To make the satellite reachable again I used
this command to delete the routing:
ip route del 120.60.31.15 dev dummy0
After doing so messages were once again able to be forwarded correctly
to the satellite.
Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: Ie8d022c2195838e383eefcd0e12ae8cfab76e3e1
|
|
This commit fixed several places (but not all) where wrong include
directory is specified and prevent the clean up in the chidren changes.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ibbba62e2c0cfe3583a65f1befa1b233bd3eebf19
|
|
In 2022.2, Redfish added support for the Context parameter on the
Session Resource. This parameter has the same function that the
OemSession.ClientId field served. This commit moves all the existing
ClientId code to produce Context as well.
Functionally, this has one important difference, in that Context in
Redfish is optionally provided by the user, which means we need to omit
it if not given by the user. The old implementation left it set to
empty string ("").
Because of this, a few minor interfaces need to change to use
std::optional. Existing uses of clientId are moved to using
value_or("") to keep the same behavior as before.
Tested:
curl --insecure -X POST -d "{\"UserName\": \"root\", \"Password\":
\"0penBmc\"}" https://192.168.7.2/redfish/v1/SessionService/Sessions
Returns a Session object with no Context key present
curl --insecure -X POST -d "{\"UserName\": \"root\", \"Password\":
\"0penBmc\", \"Context\": \"Foobar\"}"
https://192.168.7.2/redfish/v1/SessionService/Sessions
Returns a Session object with:
"Context": "Foobar"
Subsequent Gets of /redfish/v1/SessionService/Sessions/<sid>
return the same session objects, both with and without Context.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I4df358623f93f3e6cb659e99970ad909cefebc62
|
|
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
|
|
The existing code doesn't propogate errors of subqueries correctly.
This commit corrects the behavior, so that the final response gets all
error message of subqueries and the "highest priority" HTTP code.
DMTF doesn't specify how expand queries handle error codes, since using
subqueries is an implementation choice that we made in this project.
What we did here follows existing behavior of this project, and follows
the error message section of the Redfish spec;
[1] https://redfish.dmtf.org/schemas/DSP0266_1.15.1.html#error-responses
As for now, this commit uses the worst HTTP code among all the error
code. See query_param.hpp, function |propogateErrorCode| for detailed
order of the errror codes.
Tested:
1. this is difficult to test, but I hijacked the code so it returns
errors in TaskServices, then I verified that "/redfish/v1?$expand=."
correctly returns 500 and the gets the error message set.
2. unit test so that when there are multiple errors, the final response
gets a generate error message.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I0c1ebdd9015f389801db9150d687027485f1203c
|
|
There are apparently libraries that use an Accepts header of */*,
instead of simply omitting the Accepts header, or providing a more
correct value for that header.
99351cd856038475cac146029e5db03767a1459c Improve content type
The above commit attempted to refine our handling of this header, and
changed the behavior for */* to default to HTML. While this is arguably
"correct" to the HTTP RFC, and the clients that do this are definitely
wrong in their implementation, we can try to shield them a little from
their incorrectness, and we can certainly avoid compatibility issues
with these clients, without effecting the clients that implement the
spec correctly.
This commit changes the priority order of Accepts header to JSON then
HTML, instead of the other way around.
Tested:
curl --insecure -H "Accept: */*" --user root:0penBmc \
https://192.168.7.2/redfish/v1
Now returns a json payload.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I7850a3afb0b5d635f8d632fb0a9f790e53fe4466
|
|
We have a number of specialized content-type functions for varying
levels of degree, and most of them rely on quite a few strings. This
commit changes them to consolidate on two APIs.
isContentTypeSupported, which as the name implies, takes a single
content type, and returns a bool about whether or not that content type
is allowed.
getPreferedContentType, which takes an array of multiple options, and
fine the first one in the list that matches the clients expected string.
These two functions makes these functions more able to be reused in the
future, and don't require specialized entries for each possible type or
combination of types that we need to check for.
Tested: Unit tests passing. Pretty good coverage.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8b976d0cefec5f24e62fbbfae33d12cc803cb373
|
|
There's a few last calls that are inline lambdas. As we've been moving
away from inline lambdas due to the problems with debug logging and
usability, this file is the next cleanup.
Tested:
Ran Redfish-Event-Listener, and set an event into /var/log/redfish with
```
echo "2022-08-04T21:37:49.156798+00:00 OpenBMC.0.1.ServiceFailure,com.intel.DomainMapper.service" > /var/log/redfish
```
Simulating a crash. Saw the event get sent to Redfish-Event-Listener.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I61361eef03d1301dc126a28c695fdd74e504f891
|
|
Similar to the 404 handler, add a 405 handler for registering custom 405
handlers for a given tree. The primary use case is for protocols like
redfish that support specific messages for 405 handlers that don't have
an empty body.
Tested: Unit tests pass.
PATCH /redfish/v1 returns 405 Method Not Allowed
POST /redfish/v1/Chassis returns 405 Method Not Allowed
POST /redfish/v1/Chassis/foo returns 405 Method Not Allowed
PATCH /redfish/v1/foo/bar returns 404 Not Found
GET /redfish/v1 returns ServiceRoot
GET /redfish/v1/Chassis returns Chassis collection
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib0afd23d46bb5b88f89cf1e3f4e0606a48ae47ca
Signed-off-by: Carson Labrado <clabrado@google.com>
|
|
Different HTTP protocols have different http responses for 404. This
commit adds support for registering a route designed to host a handler
meant for when a response would otherwise return. This allows
registering a custom 404 handler for Redfish, for which all routes will
now return a Redfish response.
This was in response to the 404 handler not working in all cases (in the
case of POST/PATCH/DELETE). Allowing an explicit registration helps to
give the intended behavior in all cases.
Tested:
GET /redfish/v1/foo returns 404 Not found
PATCH /redfish/v1/foo returns 404 Not found
GET /redfish/v1 returns 200 OK, and content
PATCH /redfish/v1 returns 405 Method Not Allowed
With Redfish Aggregation:
GET /redfish/v1/foo gets forwarded to satellite BMC
PATCH /redfish/v1/foo does not get forwarded and returns 404
PATCH /redfish/v1/foo/5B247A_bar gets forwarded
Unit tests pass
Redfish-service-validator passes
Redfish-Protocol-Validator fails 7 tests (same as before)
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I731a5b4e736a2480700d8f3e81f9c9c6cbe6efca
Signed-off-by: Carson Labrado <clabrado@google.com>
|
|
tcp_stream has several problems in its current implementation. First,
it takes up a significant amount of binary size, for features that we
don't use. Next, it has some implied guarantees about timeouts that we
erronously rely on, namely that async_connect will timeout
appropriately given the tcp_stream timeout (it doesn't).
We already have a timer present in the ConnectionInfo class anyway, this
commit just makes use of it for ALL timeout operations, rather than just
the connect timeout operations. This flow is roughly analogous to what
we do in http_connection.hpp already, so the pattern is well troden.
This saves 2.8% of the binary size of bmcweb, and solves several race
conditions.
Tested:
Relying on Carson: Aggregated a sub-bmc, and ensured that top level
collections returned correctly under GET /redfish/v1/Chassis
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I72e8e97b667826f272bb4921afc2b16907f3b271
|
|
This commit adds the initial SSL support for http_client which can be
used for sending asynchronous Events/MetricReports to subscribed Event
Listener servers over secure channel.
Current implementation of http client only works for http protocol.
With current implementation, http client can be configured to work
with secure http (HTTPS). As part of implementation it adds the SSL
handshake mechanism and enforces the peer ceritificate verification.
The http-client uses the cipher suites which are supported by mozilla
browser and as recommended by OWASP. For better security enforcement
its disables the SSLv2, SSLv3, TLSv1, TLSv1.1 as described in below
OWASP cheetsheet.
It is validated with RootCA certificate(PEM) for now. Adding support
for different certificates can be looked in future as need arises.
[1]: https://cheatsheetseries.owasp.org/cheatsheets/TLS_Cipher_String_Cheat_Sheet.html
Tested:
- Created new subscription with SSL destination(https) and confirmed
that events are seen on EventListener side.
URI: /redfish/v1/EventService/Subscriptions
Method: POST
Body:
{
"Context": "CustomText",
"Destination": "https://<IP>:4000/service/collector/event_logs",
"EventFormatType": "Event",
"DeliveryRetryPolicy": "RetryForever",
"Protocol": "Redfish"
}
- Unit tested the non-SSL connection by disabling the check in code
(Note: EventService blocks all Non-SSL destinations). Verified that
all events are properly shown on EventListener.
URI: /redfish/v1/EventService/Subscriptions
Method: POST
Body:
{
"Context": "CustomText",
"Destination": "http://<IP>:4001/service/collector/event_logs",
"EventFormatType": "Event",
"Protocol": "Redfish"
}
- Combined above two tests and verified both SSL & Non-SSL work fine in
congention.
- Created subscription with different URI paths on same IP, Port and
protocol and verified that events sent as expected.
Change-Id: I13b2fc942c9ce6c55cd7348aae1e088a3f3d7fd9
Signed-off-by: AppaRao Puli <apparao.puli@intel.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
|
|
Adds aggregation support for resource collections that take the form
of "/redfish/v1/<resource collection>". Collection URIs are
identified by the precense of a "Members" array in the response.
Resources from satellite BMCs are added to the "Members" array of
the response and the "Members@odata.count" value is updated to
denote the new array size.
These satellite resource URIs that are added also include the
prefix associated with that satellite.
Note that as a first step this patch assumes a single satellite BMC.
There are some potential race conditions that could occur for setups
with multiple satellite BMCs. This has been commented in the code
and is better left to its own patch.
Tested:
Queried various collection URIs and the aggregated resources
appeared in the response's "Members" array.
Querying 'localhost:80/redfish/v1/Chassis?$expand=.($levels=1)'
resulted in $expand correctly returning the outputs from querying
the URIs of all local and satellite Chassis resources. This would
have failed if the satellite Chassis resources were omitted from the
"Members" array or the satellite's prefix was not correctly added to
the URI.
Also queried a collection URI that only existed on the satellite BMC.
The AsyncResp was completely overwritten by the response from the
satellite BMC.
Queries to non-collection URIs resulted in no attempts to add
satellite responses to the AsyncResp.
Signed-off-by: Carson Labrado <clabrado@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I3b379cd57e5a121eb4a344d88fc8e43170ca78a6
|
|
URIs in the responses returned with Redfish Aggregation enabled will
potentially be incorrect since ones from satellite BMCs will not
include the associated prefix such as "5B247A_" in the resource ID
portion of the URIs.
This patch fixes those links so that they include their BMC's
associated prefix. Note that a future patch will be needed to add
prefixes to aggregated resources that would appear under collection
URIs such as "/redfish/v1/Chassis".
Tested:
Requests were sent to URIs associated with the aggregating BMC and a
satellite BMC denoted as "5B247A". The URIs in the responses
were successfully updated such that "5B247A_" was added for
satellite resources.
Signed-off-by: Carson Labrado <clabrado@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib4f976fab1ca1e8603f7cf55292732ffb71cd03e
|
|
Adds ability to route requests to either native resources or
resources that belong to a satellite BMC as part of Redfish
Aggregation. A prefix in the URI denotes if the resource is actually
from a satellite BMC. Prefixes are only used to denote satellite
resources. The URI of resources on the local/aggregating BMC will
remain unchanged.
Prefixes are separated from the resource ID by an underscore. This
means that underscores cannot be used in the prefix name itself.
The prefixes used by satellite BMCs are revealed via D-Bus as well as
the config information needed to connect to that BMC.
Requests for satellite resources will not be handled locally.
Care should be taken to not name any local resources in a way that
could cause a collision (e.g. having a Chassis object named
"aggregated0_1U" on the aggregating BMC).
The patch only covers routing requests. Requests to collection URIs
like /redfish/v1/Chassis will only return resources on the local BMC.
A future patch will cover adding satellite resources to collections.
Also note that URIs returned in the responses will not have the proper
prefix included. Fixing these URIs will be addressed in future
patches.
A number of TODO comments are included in the code to indicate that
this functionality (collections and URI fixup) still needs to be
implemented.
Example URIs w/o Redfish Aggregation:
/redfish/v1/Chassis/1U/
/redfish/v1/Systems/system/
/redfish/v1/Managers/bmc/
Example URIs after enabling Redfish Aggregation if the associated
resources are located on the local/aggregating BMC:
/redfish/v1/Chassis/1U/
/redfish/v1/Systems/system/
/redfish/v1/Managers/bmc/
Example URIs if resources are instead located on a satellite BMC
named "aggregated0":
/redfish/v1/Chassis/aggregated0_1U/
/redfish/v1/Systems/aggregated0_system/
/redfish/v1/Managers/aggregated0_bmc/
Tested:
I was able to query supported resources located on the local BMC
as well as on a satellite BMC. Requests with unknown prefixes return
a 404. Requests to resource collections only return the resources
that are located on the aggregating BMC.
Signed-off-by: Carson Labrado <clabrado@google.com>
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I87a3deb730bda95e72ecd3144ea40b0e5ee7d491
|
|
Sometimes it is desirable to only parse a portion of a URL, and allow
any elements at the end. This comes up significantly in aggregation
where parsing "/redfish/v1/<collection>/anything is pretty common.
This commit adds a new class, OrMorePaths, that can be used as a
placeholder to indicate that more paths should be accepted.
Tested: Unit tests pass.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: If4fb3991a91560fd3b8b838f912aa36e79ddd2b3
|
|
A continuation of 4d94272fe54c147974f86788092bbbdd950406aa,
increases the size of httpReadBodyLimit to 2^17 bytes (about 128 KB).
This is because with Redfish Aggregation we need to be able to handle
larger response sizes such as json schema files. The ComputerSystem
schema in particular is over 120 KB.
Going forward we will not be able to keep increasing the limit.
We need a better solution for handling larger response that are
larger than httpReadBodyLimit.
Tested:
Used https://gerrit.openbmc.org/c/openbmc/bmcweb/+/53310/68 to
successfully retrieve
/redfish/v1/JsonSchemas/ComputerSystem/ComputerSystem.json from a
satellite BMC.
Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: Ida80b8fddbd1df1310d18a77ecfb44b7fdf292ef
|
|
We've accumulated several time utility functions in the http classes.
Time isn't a core HTTP primitive, so http is not where those functions
below.
This commit moves all the time functions from the crow::utility
namespace into the redfish::time_utils namespace, as well as moves the
unit tests.
No code changes where made to the individual functions, with the
exception of changing the namespace on the unit tests.
Tested: Unit tests pass.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8493375f60aea31899c84ae703e0f71a17dbdb73
|
|
Supporting higher precision on our timestamps seems worthwhile, and
useful to logging implementations for getting more accurate numbers from
redfish LogService interfaces.
This commit updates the code to add millisecond and microsecond
precision to timestamps.
Tested: Unit tests pass, pretty good coverage here.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I0914908966702a6cf1bcb59f22761dc24c46b4c3
|
|
Found these when I went through the code path of authx.
Tested: comment only changes.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Id80725f4bf5f3972e975347dcac8598e2ffab332
|
|
There are two overloads of addHeader, one that takes a string, and one
that takes a boost enum. For most common headers, boost contains a
string table with all of those entries anyway, so there's no point in
duplicating the strings, and ensures that we don't make trivial
mistakes, like capitalization or - versus underscore that aren't caught
at compile time.
Tested:
This saves a trivial amount (572 bytes) of compressed binary size.
curl --insecure -vvv --user root:0penBmc https://192.168.7.2/redfish/v1
returns < Content-Type: application/json
curl --insecure -vvv -H "Accept: text/html" --user root:0penBmc https://192.168.7.2/redfish/v1
Returns
< Content-Type: text/html;charset=UTF-8
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I34c198b4f9e219247fcfe719f9b3616d35aea3dc
|
|
Per the coding standard, if we can support what we need to do with
std variants of something, we should prefer that. This commit adds an
iso8160 to string method that supports any arbitrary
std::chrono::duration object, which allows doing the full range of all
of our integer types, and reduces the complexity (and presumably compile
times) not pulling in a complex library.
Despite the heavy templating, this only appears to add 108 bytes of
compressed binary size to bmcweb. This is likely due to the decreased
complexity compared to the boost variant (that likely pulls in
boost::locale). (Ie 3 template instantiations of the simple one take
about the same binary space as 1 complex instantiation).
Tested:
Unit tests pass (pretty good coverage here)
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I78200fb391b601eba8d2bfd2de0dd868e4390d6b
|
|
There is a bug where, when running an expand query, headers from the
response object get dropped. These headers include OData.type, and the
newly minted Link header, as well as possible others.
This was actually noted in a TODO, although the author of the TODO,
didn't fully understand the consequences at the time, and thought there
was no functional impact.
To resolve this, this commit resolves the TODO, and allows the Response
object to be moved out, instead of having to create a new one, which
preserves all the response state. To do this, it creates a move
constructor on the Response object for this use. The move constructor
is relatively benign, with one caveat, that we might be moving while in
a completion handler (as is the most common use). So both the existing
operator= and Response() move constructor are amended to handle this
case, and simply null out the response object in the copied object,
which would be correct behavior, given that each callback handler should
only be called once per Response object.
Tested:
curl --insecure --user root:0penBmc -vvvv
https://192.168.7.2/redfish/v1\?\$expand\=\*\(\$levels\=2\)
returns the same body as previously, now with the included:
OData-Version: 4.0
Allow: Get
headers in the response.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I221364dd4304903b37cacb1386f621b073a0a891
|
|
The jsonMode method is a single line, that basically only sets the
content type, and is only called from one place. In all other cases we
set the content-type directly from a header, so we should do the same
here.
Tested:
curl --insecure -vvv --user root:0penBmc https://192.168.7.2/redfish/v1
returns < Content-Type: application/json
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I82f6fcf1b574192620ac7b29f97cdd01334c18c4
|
|
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
|
|
The Redfish standard in section 8.2 states:
A Link header containing rel=describedby shall be returned on GET and
HEAD requests for Redfish resources. If the referenced JSON Schema is a
versioned schema, it shall match the version contained in the value of
the @odata.type property returned in this resource.
This commit attempts to add this capability to ServiceRoot. Future
similar patches will start adding this across the tree.
To do this, a few things happen. First, this removes the implicit HEAD
handling in the router. Because we now need explicit HEAD handling
per-route with specific headers, there's no good way to make this
generic.
Next, it rearranges the code such that handleServiceRootGet can first
call handleServiceRootHead, to avoid duplicating the addHeader call.
Tested: Redfish protocol validator passes the
RESP_HEADERS_REL_LINK_DESCRIBED_BY check for ServiceRoot.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I92248089a3545432c14f551309ea62332e554647
|
|
These changes are done by running iwyu manually under clang14.
Suppressed some obvious impl or details headers. Kept the recommended
public headers.
IWYU can increase readability, make maintenance easier, and avoid errors
in some cases. See details in
https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/WhyIWYU.md.
This commit also uses its best effort to correct obvious errors through
iwyu pragma.
See reference here:
https://github.com/include-what-you-use/include-what-you-use#how-to-correct-iwyu-mistakes
Tested: unit test passed.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I983b6f75601707cbb0f2f04546c3362ff4ba7fee
|
|
Per the coding standard, now that C++ supports std::string::starts_with
and std::string::ends_with, we should be using them over the boost
alternatives. This commit goes through and updates all usages.
Arguably some of these are incorrect, and instances of common error 13,
but because this is mostly a mechanical it intentionally doesn't try to
handle it.
Tested: Unit tests pass.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ic4c6e5d0da90f7442693199dc691a47d2240fa4f
|
|
If a destination is not reachable, then the connection will timeout
in doConnect(). This causes issues later when the client attempts to
resend the message.
The error check in doClose() should not exit early since that will
result in the connection's status not being marked as closed and
thus it will never get reused.
Similarly, doCloseAndRetry() should not exit early since that will
cause the retry flow to hang and the connection's callback function
will not get deleted.
Tested:
Used Redfish Aggregation patches in the chain through
https://gerrit.openbmc.org/c/openbmc/bmcweb/+/54896 to verify that
requests to collections such as /redfish/v1/Chassis no longer hang
when the specified Satellite BMC does not exist
Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: Ic8369b0de8efb00ff168bc1ed43f1d7fd6c7366a
|
|
cppcheck takes a little issue with this logic having some redundancies
in it. Regardless of that, it's kind of hard to read; Rearrange the
logic so it's easier to read and add comments.
Tested: Redfish service validator passes.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I0251ceb511e1bc62260b68c430b272d02b90fcb7
|
|
cppcheck correctly notes that a lot of variables in the new code can be
const. Make most of them const.
Tested: WIP
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8f37b6353fd707923f533e1d61c5b5419282bf23
|
|
A number of methods in http::Response were not marked const when they
should've been. This is generally not an issue, as most usages of
Response are in a non-const context, but as we start using const
Response objects more, we need to be more careful about const
correctness.
Tested: Unit tests pass.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8b31e71b6594d9328f106e1367084db42b783b6c
|
|
There's lots of magic numbers in this file that we inherited from crow.
This commit Adds a TypeCode enum class that can be used in place of the
magic numbers. It keeps the same values as were present previously
(0-6) in case there are places where this abstraction leaked out, but I
believe this catches all of them.
Tested: Redfish service validator passes.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I063955adb8bf75d9bb6298e29e6e44c210ee9cc3
|
|
HttpClient these days is intended to be a generic feature. It should
not be relying directly on Redfish messages. In this case, we only ever
rely on the return code internally, so replace messages with a simple
bad_gateway.
Tested: Code compiles. Unit tests pass.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ie4bd65dc0b90b75f61ab0e31ca535eefbf0f4ebb
|
|
cppcheck correctly notes this parameter can be const.
Unfortunately we've making a copy here, but looking at the ownership,
that can't be avoided simply without a shared_ptr, and that's far more
invasive.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ib70c89a7a1bc7210219052ba2a44bc7a1c20c7f2
|
|
This variable was clearly for the print statement below, but then got
abandoned at some point. Clean it up.
cppcheck found this as well.
Tested: Unused code
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I2d143f00dc6956c87ba77d5cbe1b96be631ca795
|
|
cppcheck correctly finds that these branches will always be hit, so the
branch is unneeded. Let's start by getting the code cleaned up.
Tested: CI only, cpp check.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I7b89337a81e676915243d5bc9d26c24a89c74aef
|
|
Include what you use.
Tested: Code compiles
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I23c89fee6f3e39d2f2a7dc1c3ba2db8819e8adc7
|
|
We essentially follow this rule already, not relying on implicit
operators, although there are a number of cases where in theory we
could've implicitly constructed an object.
This commit enables the clang-tidy check.
Tested: Code compiles, passes clang-tidy.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ia428463313b075c69614fdb326e8c5c094e7adde
|
|
This patchset is the conclusion of a multi-year effort to try to fix
shadowed variable names. Variables seem to be shadowed all over, and in
most places they exist, there's a "code smell" of things that aren't
doing what the author intended.
This commit attempts to clean up these in several ways by:
1. Renaming variables where appropriate.
2. Preferring to refer to member variables directly when operating
within a class
3. Rearranging code so that pass through variables are handled in the
calling scope, rather than passing them through.
These patterns are applied throughout the codebase, to the point where
-Wshadow can be enabled in meson.build.
Tested: Code compiles, unit tests pass. Still need to run redfish
service validator.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: If703398c2282f9e096ca2694fd94515de36a098b
|
|
Add using directives and namespaces.
Teseted: unit test passed.
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I4c04f07484a2c1ce29ae26b523d62071ea03996c
|
|
IWYU. Use <> for dependency headers and "" for bmcweb headers.
Reference:
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf12-prefer-the-quoted-form-of-include-for-files-relative-to-the-including-file-and-the-angle-bracket-form-everywhere-else
Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I02537f16974ab96a1e4e4e1ee6e1db1a9c679c68
|
