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 ...

ServiceRoot Support Link header

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 referenc

ServiceRoot Support Link header

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

show more ...

test treewide: iwyu

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 readabili

test treewide: iwyu

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

show more ...

Remove usages of boost::starts/ends_with

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. Thi

Remove usages of boost::starts/ends_with

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

show more ...

HTTP Client: Fix handling on connection timeout

If a destination is not reachable, then the connection will timeout
in doConnect(). This causes issues later when the client attempts to
resend the m

HTTP Client: Fix handling on connection timeout

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

show more ...

Simplify logic in router matcher

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

Simplify logic in router matcher

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

show more ...

Fix const correctness issues

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>
Cha

Fix const correctness issues

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

show more ...

Fix const correctness on http Response object

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

Fix const correctness on http Response object

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

show more ...

Remove magic numbers

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

Remove magic numbers

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

show more ...

Remove redfish message from http client

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 ret

Remove redfish message from http client

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

show more ...

Make resHandler const

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,

Make resHandler const

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

show more ...

Remove unused variable

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:

Remove unused variable

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

show more ...

Fix unused branches in http_client

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 chec

Fix unused branches in http_client

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

show more ...

Fix #includes on http_client.hpp

Include what you use.

Tested: Code compiles

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I23c89fee6f3e39d2f2a7dc1c3ba2db8819e8adc7

Require explicit decorator on one arg constructors

We essentially follow this rule already, not relying on implicit
operators, although there are a number of cases where in theory we
could've implic

Require explicit decorator on one arg constructors

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

show more ...

Fix shadowed variable issues

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, the

Fix shadowed variable issues

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

show more ...

http: router_test: fix namespace

Add using directives and namespaces.

Teseted: unit test passed.

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I4c04f07484a2c1ce29ae26b523d62071ea0399

http: router_test: fix namespace

Add using directives and namespaces.

Teseted: unit test passed.

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I4c04f07484a2c1ce29ae26b523d62071ea03996c

show more ...

http: router_test: fix headers

IWYU. Use <> for dependency headers and "" for bmcweb headers.

Reference:
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf12-prefer-the-quoted-form-of-

http: router_test: fix headers

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

show more ...

http: utility_test: fix namespace

Removed unnecessary using directives. Add necessary using directives.

Tested: unit test passed.

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I36948

http: utility_test: fix namespace

Removed unnecessary using directives. Add necessary using directives.

Tested: unit test passed.

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: I36948b531c4e75dbae6837601894839b23b2d3d1

show more ...

http: utility_test: fix headers

IWYU. Use <> for dependency headers and "" for bmcweb headers.

Reference:
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#sf12-prefer-the-quoted-form-of

http: utility_test: fix headers

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

show more ...

HttpClient: Increase httpReadBodyLimit

Testing of https://gerrit.openbmc.org/c/openbmc/bmcweb/+/53310 revealed
that recvMessage() was failing because the body limit was being
exceeded. httpReadBody

HttpClient: Increase httpReadBodyLimit

Testing of https://gerrit.openbmc.org/c/openbmc/bmcweb/+/53310 revealed
that recvMessage() was failing because the body limit was being
exceeded. httpReadBodyLimit was being used to set both the body limit
as well as the buffer size. Add a different variable to set the buffer
size so that the body limit can be doubled to 16K while the fixed
buffer can be set as a smaller size of 4K.

Tested:
Queries using the mentioned patch stopped failing after applying this
change.

Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: I33b62dfc12ff078fba9fb92b2f95a05f41acce3d

show more ...

bmcweb: Set Retry Policy Valid Response Codes

Allows individual retry policies to specify what HTTP response codes
are considered valid. Sets functions for the EventService and
Redfish Aggregation

bmcweb: Set Retry Policy Valid Response Codes

Allows individual retry policies to specify what HTTP response codes
are considered valid. Sets functions for the EventService and
Redfish Aggregation retry policies. Those functions expect a
response code and return an error code based on what the response
code is.

This change is needed because EventService only considers 2XX codes
to be valid. Any code outside of that range would trigger a retry
attempt. Redfish Aggregation by design will need to return
errors outside of that range such as 404. It should not retry to
send a message when it receives a 404 from a satellite BMC.

Right now 404 is the only error code that is handled differently
between the services. Going forward, Redfish Aggregation will
likely want to allow other error codes as its functionality is
expanded.

Tested:
Used Redfish-Event-Listener with ssh port forwarding to create 3
subscriptions. I then closed the ssh connection and sent a test
event. Bmcweb made 3 retry attempts for each subscription. At
that point the max retry amount (as defined by EventService) was
reached and bmcweb stop attempting to resend the messages.

There were no errors when the Redfish-Event-Listener was correctly
connected. Test events resulted in messages being sent for each
subscription.

Signed-off-by: Carson Labrado <clabrado@google.com>
Change-Id: Ifdfaf638d28982ed18998f3ca05280a288e0020a

show more ...

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 ...

http_server: remove unused variable

|useSsl| was not referenced anywhere else, so delete it.

Tested: code compiles.

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ie6ba9c122ab0897254e

http_server: remove unused variable

|useSsl| was not referenced anywhere else, so delete it.

Tested: code compiles.

Signed-off-by: Nan Zhou <nanzhoumails@gmail.com>
Change-Id: Ie6ba9c122ab0897254ed3cb0e278d43e8b6a283c

show more ...

Fix www-authenticate behavior

bmcweb is in a weird position where, on the one hand, we would like to
support Redfish to the specification, while also supporting a secure
webui. For better or worse,

Fix www-authenticate behavior

bmcweb is in a weird position where, on the one hand, we would like to
support Redfish to the specification, while also supporting a secure
webui. For better or worse, the webui can't currently use non-cookie
auth because of the impacts to things outside of Redfish like
websockets.

This has lead to some odd code in bmcweb that tries to "detect" whether
the browser is present, so we don't accidentally pop up the basic auth
window if a user happens to get logged out on an xhr request. Basic
auth in a browser actually causes CSRF vulnerabilities, as the browser
caches the credentials, so we don't want to make that auth method
available at all.

Previously, this detection was based on the presence of the user-agent
header, but in the years since this code was originally written, a
majority of implementations have moved to sending a user-agent by
default, which makes this check pretty much useless for its purpose. To
work around that, this patchset relies on the X-Requested-With header,
to determine if a json payload request was done by xhr. In theory, all
browsers will set this header when doing xhr requests, so this should
provide a "more correct" solution to this issue.

Background:
https://en.wikipedia.org/wiki/List_of_HTTP_header_fields
"X-Requested-With Mainly used to identify Ajax requests (most JavaScript
frameworks send this field with value of XMLHttpRequest)"

Tested:
curl -vvvv --insecure https://192.168.7.2/redfish/v1/SessionService/Sessions
Now returns a WWW-Authenticate header

Redfish-protocol-validator now passes 7 more tests from the
RESP_HEADERS_WWW_AUTHENTICATE category.

Launched webui-vue and logged in. Responses in network tab appear to
work, and data populates the page as expected.
Used curl to delete redfish session from store with
DELETE /redfish/v1/SessionService/Sessions/<SessionId>
Then clicked an element on the webui, page forwarded to login page as
expected.

Opened https://localhost:8000/redfish/v1/CertificateService in a
browser, and observed that page forwarded to the login page as it
should.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I60345caa41e520c23fe57792bf2e8c16ef144a7a

show more ...