Simplify body
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>
diff --git a/http/http_connection.hpp b/http/http_connection.hpp
index 196dc5f..ed3dc07 100644
--- a/http/http_connection.hpp
+++ b/http/http_connection.hpp
@@ -18,7 +18,6 @@
#include <boost/asio/ssl/stream.hpp>
#include <boost/asio/steady_timer.hpp>
#include <boost/beast/_experimental/test/stream.hpp>
-#include <boost/beast/core/buffers_generator.hpp>
#include <boost/beast/core/flat_static_buffer.hpp>
#include <boost/beast/http/error.hpp>
#include <boost/beast/http/parser.hpp>
@@ -222,7 +221,7 @@
{
return;
}
- crow::Request& thisReq = req.emplace(parser->release(), reqEc);
+ req = crow::Request(parser->release(), reqEc);
if (reqEc)
{
BMCWEB_LOG_DEBUG("Request failed to construct{}", reqEc.message());
@@ -230,15 +229,15 @@
completeRequest(res);
return;
}
- thisReq.session = userSession;
+ req.session = userSession;
// Fetch the client IP address
readClientIp();
// Check for HTTP version 1.1.
- if (thisReq.version() == 11)
+ if (req.version() == 11)
{
- if (thisReq.getHeaderValue(boost::beast::http::field::host).empty())
+ if (req.getHeaderValue(boost::beast::http::field::host).empty())
{
res.result(boost::beast::http::status::bad_request);
completeRequest(res);
@@ -247,13 +246,11 @@
}
BMCWEB_LOG_INFO("Request: {} HTTP/{}.{} {} {} {}", logPtr(this),
- thisReq.version() / 10, thisReq.version() % 10,
- thisReq.methodString(), thisReq.target(),
- thisReq.ipAddress.to_string());
+ req.version() / 10, req.version() % 10,
+ req.methodString(), req.target(),
+ req.ipAddress.to_string());
- res.isAliveHelper = [this]() -> bool { return isAlive(); };
-
- thisReq.ioService = static_cast<decltype(thisReq.ioService)>(
+ req.ioService = static_cast<decltype(req.ioService)>(
&adaptor.get_executor().context());
if (res.completed)
@@ -261,19 +258,19 @@
completeRequest(res);
return;
}
- keepAlive = thisReq.keepAlive();
+ keepAlive = req.keepAlive();
if constexpr (!std::is_same_v<Adaptor, boost::beast::test::stream>)
{
#ifndef BMCWEB_INSECURE_DISABLE_AUTHX
- if (!crow::authentication::isOnAllowlist(req->url().path(),
- req->method()) &&
- thisReq.session == nullptr)
+ if (!crow::authentication::isOnAllowlist(req.url().path(),
+ req.method()) &&
+ req.session == nullptr)
{
BMCWEB_LOG_WARNING("Authentication failed");
forward_unauthorized::sendUnauthorized(
- req->url().encoded_path(),
- req->getHeaderValue("X-Requested-With"),
- req->getHeaderValue("Accept"), res);
+ req.url().encoded_path(),
+ req.getHeaderValue("X-Requested-With"),
+ req.getHeaderValue("Accept"), res);
completeRequest(res);
return;
}
@@ -286,11 +283,11 @@
self->completeRequest(thisRes);
});
bool isSse =
- isContentTypeAllowed(req->getHeaderValue("Accept"),
+ isContentTypeAllowed(req.getHeaderValue("Accept"),
http_helpers::ContentType::EventStream, false);
std::string_view upgradeType(
- thisReq.getHeaderValue(boost::beast::http::field::upgrade));
- if ((thisReq.isUpgrade() &&
+ req.getHeaderValue(boost::beast::http::field::upgrade));
+ if ((req.isUpgrade() &&
bmcweb::asciiIEquals(upgradeType, "websocket")) ||
isSse)
{
@@ -308,33 +305,18 @@
return;
}
});
- handler->handleUpgrade(thisReq, asyncResp, std::move(adaptor));
+ handler->handleUpgrade(req, asyncResp, std::move(adaptor));
return;
}
std::string_view expected =
- req->getHeaderValue(boost::beast::http::field::if_none_match);
+ req.getHeaderValue(boost::beast::http::field::if_none_match);
if (!expected.empty())
{
res.setExpectedHash(expected);
}
- handler->handle(thisReq, asyncResp);
+ handler->handle(req, asyncResp);
}
- bool isAlive()
- {
- if constexpr (IsTls<Adaptor>::value)
- {
- return adaptor.next_layer().is_open();
- }
- else if constexpr (std::is_same_v<Adaptor, boost::beast::test::stream>)
- {
- return true;
- }
- else
- {
- return adaptor.is_open();
- }
- }
void close()
{
if constexpr (IsTls<Adaptor>::value)
@@ -356,22 +338,13 @@
void completeRequest(crow::Response& thisRes)
{
- if (!req)
- {
- return;
- }
res = std::move(thisRes);
res.keepAlive(keepAlive);
- completeResponseFields(*req, res);
+ completeResponseFields(req, res);
res.addHeader(boost::beast::http::field::date, getCachedDateStr());
- if (!isAlive())
- {
- res.setCompleteRequestHandler(nullptr);
- return;
- }
- doWrite(res);
+ doWrite();
// delete lambda with self shared_ptr
// to enable connection destruction
@@ -386,11 +359,7 @@
{
return;
}
- if (!req)
- {
- return;
- }
- req->ipAddress = ip;
+ req.ipAddress = ip;
}
boost::system::error_code getClientIp(boost::asio::ip::address& ip)
@@ -432,10 +401,11 @@
std::size_t bytesTransferred) {
BMCWEB_LOG_DEBUG("{} async_read_header {} Bytes", logPtr(this),
bytesTransferred);
- bool errorWhileReading = false;
+
if (ec)
{
- errorWhileReading = true;
+ cancelDeadlineTimer();
+
if (ec == boost::beast::http::error::end_of_stream)
{
BMCWEB_LOG_WARNING("{} Error while reading: {}",
@@ -446,22 +416,6 @@
BMCWEB_LOG_ERROR("{} Error while reading: {}", logPtr(this),
ec.message());
}
- }
- else
- {
- // if the adaptor isn't open anymore, and wasn't handed to a
- // websocket, treat as an error
- if (!isAlive() &&
- !boost::beast::websocket::is_upgrade(parser->get()))
- {
- errorWhileReading = true;
- }
- }
-
- cancelDeadlineTimer();
-
- if (errorWhileReading)
- {
close();
BMCWEB_LOG_DEBUG("{} from read(1)", logPtr(this));
return;
@@ -584,19 +538,21 @@
userSession = nullptr;
// Destroy the Request via the std::optional
- req.reset();
+ req.clear();
doReadHeaders();
}
- void doWrite(crow::Response& thisRes)
+ void doWrite()
{
BMCWEB_LOG_DEBUG("{} doWrite", logPtr(this));
- thisRes.preparePayload();
+ res.preparePayload();
startDeadline();
- boost::beast::async_write(adaptor, thisRes.generator(),
- std::bind_front(&self_type::afterDoWrite,
- this, shared_from_this()));
+ serializer.emplace(res.response);
+ boost::beast::http::async_write(
+ adaptor, *serializer,
+ std::bind_front(&self_type::afterDoWrite, this,
+ shared_from_this()));
}
void cancelDeadlineTimer()
@@ -655,13 +611,13 @@
Handler* handler;
// Making this a std::optional allows it to be efficiently destroyed and
// re-created on Connection reset
- std::optional<
- boost::beast::http::request_parser<boost::beast::http::string_body>>
- parser;
+ std::optional<boost::beast::http::request_parser<bmcweb::FileBody>> parser;
+ std::optional<boost::beast::http::response_serializer<bmcweb::FileBody>>
+ serializer;
boost::beast::flat_static_buffer<8192> buffer;
- std::optional<crow::Request> req;
+ crow::Request req;
crow::Response res;
std::shared_ptr<persistent_data::UserSession> userSession;