diff options
author | Ed Tanous <edtanous@google.com> | 2023-02-07 02:09:46 +0300 |
---|---|---|
committer | Ed Tanous <ed@tanous.net> | 2023-03-11 03:50:32 +0300 |
commit | 39662a3be1877afd893b70d78a449f91ba36c260 (patch) | |
tree | d2f28439ca44d16d292f7879efbf8a8577295514 /http/http_request.hpp | |
parent | 33c6b58026e5dd3bc600d152a07b95f863bfa1a7 (diff) | |
download | bmcweb-39662a3be1877afd893b70d78a449f91ba36c260.tar.xz |
Make url by value in Request
There's some tough-to-track-down safety problems in http Request. This
commit is an attempt to make things more safe, even if it isn't clear
how the old code was wrong.
Previously, the old code took a url_view from the target() string for a
given URI. This was effectively a pointer, and needed to be updated in
custom move/copy constructors that were error prone to write.
This commit moves to taking the URI by non-view, which involves a copy,
but allows us to use the default move and copy constructors, as well as
have no internal references within Request, which should improve the
safety and reviewability.
There's already so many string copies in bmcweb, that this is unlikely
to show up as any sort of performance regression, and simple code is
much better in this case.
Note, because of a bug in boost::url, we have to explicitly construct a
url_view in any case where we want to use segments() or query() on a
const Request. This has been reported to the boost maintainers, and is
being worked for a long term solution.
https://github.com/boostorg/url/pull/704
Tested: Redfish service validator passed on last commit in series.
Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I49a7710e642dff624d578ec1dde088428f284627
Diffstat (limited to 'http/http_request.hpp')
-rw-r--r-- | http/http_request.hpp | 33 |
1 files changed, 13 insertions, 20 deletions
diff --git a/http/http_request.hpp b/http/http_request.hpp index e59f84b810..3fd9d4d753 100644 --- a/http/http_request.hpp +++ b/http/http_request.hpp @@ -8,7 +8,7 @@ #include <boost/beast/http/message.hpp> #include <boost/beast/http/string_body.hpp> #include <boost/beast/websocket.hpp> -#include <boost/url/url_view.hpp> +#include <boost/url/url.hpp> #include <string> #include <string_view> @@ -20,9 +20,11 @@ namespace crow struct Request { boost::beast::http::request<boost::beast::http::string_body> req; - std::string_view url{}; - boost::urls::url_view urlView{}; + private: + boost::urls::url urlBase{}; + + public: bool isSecure{false}; boost::asio::io_context* ioService{}; @@ -49,21 +51,8 @@ struct Request } } - Request(const Request& other) : - req(other.req), isSecure(other.isSecure), ioService(other.ioService), - ipAddress(other.ipAddress), session(other.session), - userRole(other.userRole) - { - setUrlInfo(); - } - - Request(Request&& other) noexcept : - req(std::move(other.req)), isSecure(other.isSecure), - ioService(other.ioService), ipAddress(std::move(other.ipAddress)), - session(std::move(other.session)), userRole(std::move(other.userRole)) - { - setUrlInfo(); - } + Request(const Request& other) = default; + Request(Request&& other) = default; Request& operator=(const Request&) = delete; Request& operator=(const Request&&) = delete; @@ -94,6 +83,11 @@ struct Request return req.target(); } + boost::urls::url_view url() const + { + return {urlBase}; + } + const boost::beast::http::fields& fields() const { return req.base(); @@ -134,8 +128,7 @@ struct Request { return false; } - urlView = *result; - url = urlView.encoded_path(); + urlBase = *result; return true; } }; |