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/complete_response_fields.hpp b/http/complete_response_fields.hpp
index 8323dca..1ac75ad 100644
--- a/http/complete_response_fields.hpp
+++ b/http/complete_response_fields.hpp
@@ -10,7 +10,6 @@
#include "utils/hex_utils.hpp"
#include <boost/beast/http/message.hpp>
-#include <boost/beast/http/string_body.hpp>
#include <nlohmann/json.hpp>
#include <array>
diff --git a/http/http2_connection.hpp b/http/http2_connection.hpp
index 95d184e..a9e91d7 100644
--- a/http/http2_connection.hpp
+++ b/http/http2_connection.hpp
@@ -21,7 +21,6 @@
#include <boost/beast/http/parser.hpp>
#include <boost/beast/http/read.hpp>
#include <boost/beast/http/serializer.hpp>
-#include <boost/beast/http/string_body.hpp>
#include <boost/beast/http/write.hpp>
#include <boost/beast/ssl/ssl_stream.hpp>
#include <boost/beast/websocket.hpp>
@@ -35,9 +34,9 @@
struct Http2StreamData
{
- crow::Request req{};
- crow::Response res{};
- size_t sentSofar = 0;
+ Request req{};
+ Response res{};
+ std::optional<bmcweb::FileBody::writer> writer;
};
template <typename Adaptor, typename Handler>
@@ -48,14 +47,10 @@
public:
HTTP2Connection(Adaptor&& adaptorIn, Handler* handlerIn,
- std::function<std::string()>& getCachedDateStrF
-
- ) :
+ std::function<std::string()>& getCachedDateStrF) :
adaptor(std::move(adaptorIn)),
-
- ngSession(initializeNghttp2Session()),
-
- handler(handlerIn), getCachedDateStr(getCachedDateStrF)
+ ngSession(initializeNghttp2Session()), handler(handlerIn),
+ getCachedDateStr(getCachedDateStrF)
{}
void start()
@@ -103,59 +98,46 @@
}
Http2StreamData& stream = streamIt->second;
BMCWEB_LOG_DEBUG("File read callback length: {}", length);
- crow::Response& res = stream.res;
- Response::string_response* body =
- boost::variant2::get_if<Response::string_response>(&res.response);
- Response::file_response* fbody =
- boost::variant2::get_if<Response::file_response>(&res.response);
-
- size_t size = res.size();
- BMCWEB_LOG_DEBUG("total: {} send_sofar: {}", size, stream.sentSofar);
-
- size_t toSend = std::min(size - stream.sentSofar, length);
- BMCWEB_LOG_DEBUG("Copying {} bytes to buf", toSend);
-
- if (body != nullptr)
- {
- std::string::const_iterator bodyBegin = body->body().begin();
- std::advance(bodyBegin, stream.sentSofar);
-
- memcpy(buf, &*bodyBegin, toSend);
- }
- else if (fbody != nullptr)
- {
- boost::system::error_code ec;
-
- size_t nread = fbody->body().file().read(buf, toSend, ec);
- if (ec || nread != toSend)
- {
- return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
- }
- }
- else
+ boost::beast::error_code ec;
+ boost::optional<std::pair<boost::asio::const_buffer, bool>> out =
+ stream.writer->getWithMaxSize(ec, length);
+ if (ec)
{
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
}
- stream.sentSofar += toSend;
-
- if (stream.sentSofar >= size)
+ if (!out)
{
- BMCWEB_LOG_DEBUG("Setting OEF flag");
+ *dataFlags |= NGHTTP2_DATA_FLAG_EOF;
+ return 0;
+ }
+
+ BMCWEB_LOG_DEBUG("Send chunk of size: {}", out->first.size());
+ if (length < out->first.size())
+ {
+ // Should never happen because of length limit on get() above
+ return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
+ }
+
+ BMCWEB_LOG_DEBUG("Copying {} bytes to buf", out->first.size());
+ memcpy(buf, out->first.data(), out->first.size());
+
+ if (!out->second)
+ {
+ BMCWEB_LOG_DEBUG("Setting OEF and nocopy flag");
*dataFlags |= NGHTTP2_DATA_FLAG_EOF;
//*dataFlags |= NGHTTP2_DATA_FLAG_NO_COPY;
}
- return static_cast<ssize_t>(toSend);
+ return static_cast<ssize_t>(out->first.size());
}
nghttp2_nv headerFromStringViews(std::string_view name,
- std::string_view value)
+ std::string_view value, uint8_t flags)
{
uint8_t* nameData = std::bit_cast<uint8_t*>(name.data());
uint8_t* valueData = std::bit_cast<uint8_t*>(value.data());
- return {nameData, valueData, name.size(), value.size(),
- NGHTTP2_NV_FLAG_NONE};
+ return {nameData, valueData, name.size(), value.size(), flags};
}
int sendResponse(Response& completedRes, int32_t streamId)
@@ -178,14 +160,18 @@
boost::beast::http::fields& fields = thisRes.fields();
std::string code = std::to_string(thisRes.resultInt());
- hdr.emplace_back(headerFromStringViews(":status", code));
+ hdr.emplace_back(
+ headerFromStringViews(":status", code, NGHTTP2_NV_FLAG_NONE));
for (const boost::beast::http::fields::value_type& header : fields)
{
- hdr.emplace_back(
- headerFromStringViews(header.name_string(), header.value()));
+ hdr.emplace_back(headerFromStringViews(
+ header.name_string(), header.value(),
+ NGHTTP2_NV_FLAG_NO_COPY_VALUE | NGHTTP2_NV_FLAG_NO_COPY_NAME));
}
Http2StreamData& stream = it->second;
- stream.sentSofar = 0;
+ crow::Response& res = stream.res;
+ http::response<bmcweb::FileBody>& fbody = res.response;
+ stream.writer.emplace(fbody.base(), fbody.body());
nghttp2_data_provider dataPrd{
.source = {.fd = 0},
@@ -480,7 +466,7 @@
static ssize_t onSendCallbackStatic(nghttp2_session* session,
const uint8_t* data, size_t length,
- int flags /* flags */, void* userData)
+ int flags, void* userData)
{
return userPtrToSelf(userData).onSendCallback(session, data, length,
flags);
@@ -519,36 +505,39 @@
return;
}
inBuffer.commit(bytesTransferred);
-
- size_t consumed = 0;
- for (const auto bufferIt : inBuffer.data())
+ // NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+ for (const auto* it =
+ boost::asio::buffer_sequence_begin(inBuffer.data());
+ it != boost::asio::buffer_sequence_end(inBuffer.data()); it++)
{
- std::span<const uint8_t> bufferSpan{
- std::bit_cast<const uint8_t*>(bufferIt.data()),
- bufferIt.size()};
- BMCWEB_LOG_DEBUG("http2 is getting {} bytes",
- bufferSpan.size());
- ssize_t readLen = ngSession.memRecv(bufferSpan);
- if (readLen <= 0)
+ // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+ while (inBuffer.size() > 0)
{
- BMCWEB_LOG_ERROR("nghttp2_session_mem_recv returned {}",
- readLen);
- close();
- return;
+ std::span<const uint8_t> bufferSpan{
+ std::bit_cast<const uint8_t*>(it->data()), it->size()};
+ BMCWEB_LOG_DEBUG("http2 is getting {} bytes",
+ bufferSpan.size());
+ ssize_t readLen = ngSession.memRecv(bufferSpan);
+ if (readLen <= 0)
+ {
+ BMCWEB_LOG_ERROR("nghttp2_session_mem_recv returned {}",
+ readLen);
+ close();
+ return;
+ }
+ inBuffer.consume(static_cast<size_t>(readLen));
}
- consumed += static_cast<size_t>(readLen);
}
- inBuffer.consume(consumed);
doRead();
});
}
// A mapping from http2 stream ID to Stream Data
- boost::container::flat_map<int32_t, Http2StreamData> streams;
+ std::map<int32_t, Http2StreamData> streams;
boost::beast::multi_buffer sendBuffer;
- boost::beast::multi_buffer inBuffer;
+ boost::beast::flat_static_buffer<8192> inBuffer;
Adaptor adaptor;
bool isWriting = false;
diff --git a/http/http_client.hpp b/http/http_client.hpp
index 076d852..6f42f3e 100644
--- a/http/http_client.hpp
+++ b/http/http_client.hpp
@@ -33,7 +33,6 @@
#include <boost/beast/http/message.hpp>
#include <boost/beast/http/parser.hpp>
#include <boost/beast/http/read.hpp>
-#include <boost/beast/http/string_body.hpp>
#include <boost/beast/http/write.hpp>
#include <boost/beast/ssl/ssl_stream.hpp>
#include <boost/beast/version.hpp>
@@ -117,10 +116,10 @@
struct PendingRequest
{
- boost::beast::http::request<boost::beast::http::string_body> req;
+ boost::beast::http::request<bmcweb::FileBody> req;
std::function<void(bool, uint32_t, Response&)> callback;
PendingRequest(
- boost::beast::http::request<boost::beast::http::string_body>&& reqIn,
+ boost::beast::http::request<bmcweb::FileBody>&& reqIn,
const std::function<void(bool, uint32_t, Response&)>& callbackIn) :
req(std::move(reqIn)),
callback(callbackIn)
@@ -139,8 +138,8 @@
uint32_t connId;
// Data buffers
- http::request<http::string_body> req;
- using parser_type = http::response_parser<http::string_body>;
+ http::request<bmcweb::FileBody> req;
+ using parser_type = http::response_parser<bmcweb::FileBody>;
std::optional<parser_type> parser;
boost::beast::flat_static_buffer<httpReadBufferSize> buffer;
Response res;
@@ -376,7 +375,7 @@
{
return;
}
- BMCWEB_LOG_DEBUG("recvMessage() data: {}", parser->get().body());
+ BMCWEB_LOG_DEBUG("recvMessage() data: {}", parser->get().body().str());
unsigned int respCode = parser->get().result_int();
BMCWEB_LOG_DEBUG("recvMessage() Header Response Code: {}", respCode);
@@ -669,7 +668,7 @@
return;
}
- auto nextReq = requestQueue.front();
+ PendingRequest& nextReq = requestQueue.front();
conn.req = std::move(nextReq.req);
conn.callback = std::move(nextReq.callback);
@@ -734,12 +733,12 @@
const std::function<void(Response&)>& resHandler)
{
// Construct the request to be sent
- boost::beast::http::request<boost::beast::http::string_body> thisReq(
+ boost::beast::http::request<bmcweb::FileBody> thisReq(
verb, destUri.encoded_target(), 11, "", httpHeader);
thisReq.set(boost::beast::http::field::host,
destUri.encoded_host_address());
thisReq.keep_alive(true);
- thisReq.body() = std::move(data);
+ thisReq.body().str() = std::move(data);
thisReq.prepare_payload();
auto cb = std::bind_front(&ConnectionPool::afterSendData,
weak_from_this(), resHandler);
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;
diff --git a/http/http_file_body.hpp b/http/http_file_body.hpp
index eaafd5d..17554a4 100644
--- a/http/http_file_body.hpp
+++ b/http/http_file_body.hpp
@@ -1,19 +1,24 @@
#pragma once
+#include "logging.hpp"
#include "utility.hpp"
+#include <unistd.h>
+
+#include <boost/beast/core/buffers_range.hpp>
#include <boost/beast/core/file_posix.hpp>
#include <boost/beast/http/message.hpp>
#include <boost/system/error_code.hpp>
+#include <string_view>
+
namespace bmcweb
{
struct FileBody
{
class writer;
+ class reader;
class value_type;
-
- static std::uint64_t size(const value_type& body);
};
enum class EncodingType
@@ -25,8 +30,8 @@
class FileBody::value_type
{
boost::beast::file_posix fileHandle;
-
- std::uint64_t fileSize = 0;
+ std::optional<size_t> fileSize;
+ std::string strBody;
public:
EncodingType encodingType = EncodingType::Raw;
@@ -34,40 +39,122 @@
~value_type() = default;
value_type() = default;
explicit value_type(EncodingType enc) : encodingType(enc) {}
- value_type(value_type&& other) = default;
- value_type& operator=(value_type&& other) = default;
- value_type(const value_type& other) = delete;
- value_type& operator=(const value_type& other) = delete;
+ explicit value_type(std::string_view str) : strBody(str) {}
- boost::beast::file_posix& file()
+ value_type(value_type&& other) noexcept :
+ fileHandle(std::move(other.fileHandle)), fileSize(other.fileSize),
+ strBody(std::move(other.strBody)), encodingType(other.encodingType)
+ {}
+
+ value_type& operator=(value_type&& other) noexcept
+ {
+ fileHandle = std::move(other.fileHandle);
+ fileSize = other.fileSize;
+ strBody = std::move(other.strBody);
+ encodingType = other.encodingType;
+
+ return *this;
+ }
+
+ // Overload copy constructor, because posix doesn't have dup(), but linux
+ // does
+ value_type(const value_type& other) :
+ fileSize(other.fileSize), strBody(other.strBody),
+ encodingType(other.encodingType)
+ {
+ fileHandle.native_handle(dup(other.fileHandle.native_handle()));
+ }
+
+ value_type& operator=(const value_type& other)
+ {
+ if (this != &other)
+ {
+ fileSize = other.fileSize;
+ strBody = other.strBody;
+ encodingType = other.encodingType;
+ fileHandle.native_handle(dup(other.fileHandle.native_handle()));
+ }
+ return *this;
+ }
+
+ const boost::beast::file_posix& file()
{
return fileHandle;
}
- std::uint64_t size() const
+ std::string& str()
{
+ return strBody;
+ }
+
+ const std::string& str() const
+ {
+ return strBody;
+ }
+
+ std::optional<size_t> payloadSize() const
+ {
+ if (!fileHandle.is_open())
+ {
+ return strBody.size();
+ }
+ if (fileSize)
+ {
+ if (encodingType == EncodingType::Base64)
+ {
+ return crow::utility::Base64Encoder::encodedSize(*fileSize);
+ }
+ }
return fileSize;
}
+ void clear()
+ {
+ strBody.clear();
+ strBody.shrink_to_fit();
+ fileHandle = boost::beast::file_posix();
+ fileSize = std::nullopt;
+ }
+
void open(const char* path, boost::beast::file_mode mode,
boost::system::error_code& ec)
{
fileHandle.open(path, mode, ec);
- fileSize = fileHandle.size(ec);
+ if (ec)
+ {
+ return;
+ }
+ boost::system::error_code ec2;
+ uint64_t size = fileHandle.size(ec2);
+ if (!ec2)
+ {
+ BMCWEB_LOG_INFO("File size was {} bytes", size);
+ fileSize = static_cast<size_t>(size);
+ }
+ else
+ {
+ BMCWEB_LOG_WARNING("Failed to read file size on {}", path);
+ }
+ ec = {};
}
void setFd(int fd, boost::system::error_code& ec)
{
fileHandle.native_handle(fd);
- fileSize = fileHandle.size(ec);
+
+ boost::system::error_code ec2;
+ uint64_t size = fileHandle.size(ec2);
+ if (!ec2)
+ {
+ if (size != 0 && size < std::numeric_limits<size_t>::max())
+ {
+ fileSize = static_cast<size_t>(size);
+ }
+ }
+ ec = {};
}
};
-inline std::uint64_t FileBody::size(const value_type& body)
-{
- return body.size();
-}
-
class FileBody::writer
{
public:
@@ -78,7 +165,7 @@
crow::utility::Base64Encoder encoder;
value_type& body;
- std::uint64_t remain;
+ size_t sent = 0;
constexpr static size_t readBufSize = 4096;
std::array<char, readBufSize> fileReadBuf{};
@@ -86,8 +173,7 @@
template <bool IsRequest, class Fields>
writer(boost::beast::http::header<IsRequest, Fields>& /*header*/,
value_type& bodyIn) :
- body(bodyIn),
- remain(body.size())
+ body(bodyIn)
{}
static void init(boost::beast::error_code& ec)
@@ -98,22 +184,38 @@
boost::optional<std::pair<const_buffers_type, bool>>
get(boost::beast::error_code& ec)
{
- size_t toRead = fileReadBuf.size();
- if (remain < toRead)
+ return getWithMaxSize(ec, std::numeric_limits<size_t>::max());
+ }
+
+ boost::optional<std::pair<const_buffers_type, bool>>
+ getWithMaxSize(boost::beast::error_code& ec, size_t maxSize)
+ {
+ std::pair<const_buffers_type, bool> ret;
+ if (!body.file().is_open())
{
- toRead = static_cast<size_t>(remain);
+ size_t remain = body.str().size() - sent;
+ size_t toReturn = std::min(maxSize, remain);
+ ret.first = const_buffers_type(&body.str()[sent], toReturn);
+
+ sent += toReturn;
+ ret.second = sent < body.str().size();
+ BMCWEB_LOG_INFO("Returning {} bytes more={}", ret.first.size(),
+ ret.second);
+ return ret;
}
- size_t read = body.file().read(fileReadBuf.data(), toRead, ec);
- if (read != toRead || ec)
+ size_t readReq = std::min(fileReadBuf.size(), maxSize);
+ size_t read = body.file().read(fileReadBuf.data(), readReq, ec);
+ if (ec)
{
+ BMCWEB_LOG_CRITICAL("Failed to read from file");
return boost::none;
}
- remain -= read;
std::string_view chunkView(fileReadBuf.data(), read);
-
- std::pair<const_buffers_type, bool> ret;
- ret.second = remain > 0;
+ BMCWEB_LOG_INFO("Read {} bytes from file", read);
+ // If the number of bytes read equals the amount requested, we haven't
+ // reached EOF yet
+ ret.second = read == readReq;
if (body.encodingType == EncodingType::Base64)
{
buf.clear();
@@ -133,4 +235,49 @@
return ret;
}
};
+
+class FileBody::reader
+{
+ value_type& value;
+
+ public:
+ template <bool IsRequest, class Fields>
+ reader(boost::beast::http::header<IsRequest, Fields>& /*headers*/,
+ value_type& body) :
+ value(body)
+ {}
+
+ void init(const boost::optional<std::uint64_t>& contentLength,
+ boost::beast::error_code& ec)
+ {
+ if (contentLength)
+ {
+ if (!value.file().is_open())
+ {
+ value.str().reserve(static_cast<size_t>(*contentLength));
+ }
+ }
+ ec = {};
+ }
+
+ template <class ConstBufferSequence>
+ std::size_t put(const ConstBufferSequence& buffers,
+ boost::system::error_code& ec)
+ {
+ size_t extra = boost::beast::buffer_bytes(buffers);
+ for (const auto b : boost::beast::buffers_range_ref(buffers))
+ {
+ const char* ptr = static_cast<const char*>(b.data());
+ value.str() += std::string_view(ptr, b.size());
+ }
+ ec = {};
+ return extra;
+ }
+
+ static void finish(boost::system::error_code& ec)
+ {
+ ec = {};
+ }
+};
+
} // namespace bmcweb
diff --git a/http/http_request.hpp b/http/http_request.hpp
index 7cefb7d..9cd4a0c 100644
--- a/http/http_request.hpp
+++ b/http/http_request.hpp
@@ -6,9 +6,9 @@
#include <boost/asio/io_context.hpp>
#include <boost/asio/ip/address.hpp>
#include <boost/beast/http/message.hpp>
-#include <boost/beast/http/string_body.hpp>
#include <boost/beast/websocket.hpp>
#include <boost/url/url.hpp>
+#include <http_file_body.hpp>
#include <string>
#include <string_view>
@@ -19,7 +19,7 @@
struct Request
{
- boost::beast::http::request<boost::beast::http::string_body> req;
+ boost::beast::http::request<bmcweb::FileBody> req;
private:
boost::urls::url urlBase{};
@@ -33,7 +33,7 @@
std::shared_ptr<persistent_data::UserSession> session;
std::string userRole{};
- Request(boost::beast::http::request<boost::beast::http::string_body> reqIn,
+ Request(boost::beast::http::request<bmcweb::FileBody> reqIn,
std::error_code& ec) :
req(std::move(reqIn))
{
@@ -65,6 +65,17 @@
req.insert(key, value);
}
+ void clear()
+ {
+ req.clear();
+ urlBase.clear();
+ isSecure = false;
+ ioService = nullptr;
+ ipAddress = boost::asio::ip::address();
+ session = nullptr;
+ userRole = "";
+ }
+
boost::beast::http::verb method() const
{
return req.method();
@@ -102,7 +113,7 @@
const std::string& body() const
{
- return req.body();
+ return req.body().str();
}
bool target(std::string_view target)
diff --git a/http/http_response.hpp b/http/http_response.hpp
index 4015899..123a7e1 100644
--- a/http/http_response.hpp
+++ b/http/http_response.hpp
@@ -4,9 +4,6 @@
#include "utils/hex_utils.hpp"
#include <boost/beast/http/message.hpp>
-#include <boost/beast/http/message_generator.hpp>
-#include <boost/beast/http/string_body.hpp>
-#include <boost/variant2/variant.hpp>
#include <nlohmann/json.hpp>
#include <optional>
@@ -26,24 +23,18 @@
template <typename Adaptor, typename Handler>
friend class crow::Connection;
- using string_response = http::response<http::string_body>;
- using file_response = http::response<bmcweb::FileBody>;
-
- // Use boost variant2 because it doesn't have valueless by exception
- boost::variant2::variant<string_response, file_response> response;
+ http::response<bmcweb::FileBody> response;
nlohmann::json jsonValue;
using fields_type = http::header<false, http::fields>;
fields_type& fields()
{
- return boost::variant2::visit(
- [](auto&& r) -> fields_type& { return r.base(); }, response);
+ return response.base();
}
const fields_type& fields() const
{
- return boost::variant2::visit(
- [](auto&& r) -> const fields_type& { return r.base(); }, response);
+ return response.base();
}
void addHeader(std::string_view key, std::string_view value)
@@ -61,7 +52,7 @@
fields().erase(key);
}
- Response() : response(string_response()) {}
+ Response() = default;
Response(Response&& res) noexcept :
response(std::move(res.response)), jsonValue(std::move(res.jsonValue)),
completed(res.completed)
@@ -72,8 +63,6 @@
completeRequestHandler = std::move(res.completeRequestHandler);
res.completeRequestHandler = nullptr;
}
- isAliveHelper = res.isAliveHelper;
- res.isAliveHelper = nullptr;
}
~Response() = default;
@@ -107,8 +96,6 @@
completeRequestHandler = nullptr;
}
completed = r.completed;
- isAliveHelper = std::move(r.isAliveHelper);
- r.isAliveHelper = nullptr;
return *this;
}
@@ -124,20 +111,7 @@
void copyBody(const Response& res)
{
- const string_response* s =
- boost::variant2::get_if<string_response>(&(res.response));
- if (s == nullptr)
- {
- BMCWEB_LOG_ERROR("Unable to copy a file");
- return;
- }
- string_response* myString =
- boost::variant2::get_if<string_response>(&response);
- if (myString == nullptr)
- {
- myString = &response.emplace<string_response>();
- }
- myString->body() = s->body();
+ response.body() = res.response.body();
}
http::status result() const
@@ -162,13 +136,7 @@
const std::string* body()
{
- string_response* body =
- boost::variant2::get_if<string_response>(&response);
- if (body == nullptr)
- {
- return nullptr;
- }
- return &body->body();
+ return &response.body().str();
}
std::string_view getHeaderValue(std::string_view key) const
@@ -178,27 +146,31 @@
void keepAlive(bool k)
{
- return boost::variant2::visit([k](auto&& r) { r.keep_alive(k); },
- response);
+ response.keep_alive(k);
}
bool keepAlive() const
{
- return boost::variant2::visit([](auto&& r) { return r.keep_alive(); },
- response);
+ return response.keep_alive();
}
- uint64_t getContentLength(boost::optional<uint64_t> pSize)
+ std::optional<uint64_t> size()
+ {
+ return response.body().payloadSize();
+ }
+
+ void preparePayload()
{
// This code is a throw-free equivalent to
// beast::http::message::prepare_payload
+ std::optional<uint64_t> pSize = response.body().payloadSize();
+ if (!pSize)
+ {
+ return;
+ }
using http::status;
using http::status_class;
using http::to_status_class;
- if (!pSize)
- {
- return 0;
- }
bool is1XXReturn = to_status_class(result()) ==
status_class::informational;
if (*pSize > 0 && (is1XXReturn || result() == status::no_content ||
@@ -208,30 +180,17 @@
"no-content or not_modified, which aren't "
"allowed to have a body",
logPtr(this));
- return 0;
+ response.content_length(0);
+ return;
}
- return *pSize;
- }
-
- uint64_t size()
- {
- return boost::variant2::visit(
- [](auto&& res) -> uint64_t { return res.body().size(); }, response);
- }
-
- void preparePayload()
- {
- boost::variant2::visit(
- [this](auto&& r) {
- r.content_length(getContentLength(r.payload_size()));
- },
- response);
+ response.content_length(*pSize);
}
void clear()
{
BMCWEB_LOG_DEBUG("{} Clearing response containers", logPtr(this));
- response.emplace<string_response>();
+ response.clear();
+
jsonValue = nullptr;
completed = false;
expectedHash = std::nullopt;
@@ -255,17 +214,7 @@
void write(std::string&& bodyPart)
{
- string_response* str =
- boost::variant2::get_if<string_response>(&response);
- if (str != nullptr)
- {
- str->body() += bodyPart;
- return;
- }
- http::header<false> headTemp = std::move(fields());
- string_response& stringResponse =
- response.emplace<string_response>(std::move(headTemp));
- stringResponse.body() = std::move(bodyPart);
+ response.body().str() = std::move(bodyPart);
}
void end()
@@ -289,11 +238,6 @@
}
}
- bool isAlive() const
- {
- return isAliveHelper && isAliveHelper();
- }
-
void setCompleteRequestHandler(std::function<void(Response&)>&& handler)
{
BMCWEB_LOG_DEBUG("{} setting completion handler", logPtr(this));
@@ -314,18 +258,6 @@
return ret;
}
- void setIsAliveHelper(std::function<bool()>&& handler)
- {
- isAliveHelper = std::move(handler);
- }
-
- std::function<bool()> releaseIsAliveHelper()
- {
- std::function<bool()> ret = std::move(isAliveHelper);
- isAliveHelper = nullptr;
- return ret;
- }
-
void setHashAndHandleNotModified()
{
// Can only hash if we have content that's valid
@@ -348,56 +280,36 @@
expectedHash = hash;
}
- using message_generator = http::message_generator;
- message_generator generator()
- {
- return boost::variant2::visit(
- [](auto& r) -> message_generator { return std::move(r); },
- response);
- }
-
bool openFile(const std::filesystem::path& path,
bmcweb::EncodingType enc = bmcweb::EncodingType::Raw)
{
- file_response::body_type::value_type body(enc);
boost::beast::error_code ec;
- body.open(path.c_str(), boost::beast::file_mode::read, ec);
+ response.body().open(path.c_str(), boost::beast::file_mode::read, ec);
+ response.body().encodingType = enc;
if (ec)
{
+ BMCWEB_LOG_ERROR("Failed to open file {}", path.c_str());
return false;
}
- updateFileBody(std::move(body));
return true;
}
bool openFd(int fd, bmcweb::EncodingType enc = bmcweb::EncodingType::Raw)
{
- file_response::body_type::value_type body(enc);
boost::beast::error_code ec;
- body.setFd(fd, ec);
+ response.body().encodingType = enc;
+ response.body().setFd(fd, ec);
if (ec)
{
BMCWEB_LOG_ERROR("Failed to set fd");
return false;
}
- updateFileBody(std::move(body));
return true;
}
private:
- void updateFileBody(file_response::body_type::value_type file)
- {
- // store the headers on stack temporarily so we can reconstruct the new
- // base with the old headers copied in.
- http::header<false> headTemp = std::move(fields());
- file_response& fileResponse =
- response.emplace<file_response>(std::move(headTemp));
- fileResponse.body() = std::move(file);
- }
-
std::optional<std::string> expectedHash;
bool completed = false;
std::function<void(Response&)> completeRequestHandler;
- std::function<bool()> isAliveHelper;
};
} // namespace crow
diff --git a/http/websocket.hpp b/http/websocket.hpp
index 4f5b3c1..388a3e4 100644
--- a/http/websocket.hpp
+++ b/http/websocket.hpp
@@ -128,8 +128,7 @@
}));
// Make a pointer to keep the req alive while we accept it.
- using Body =
- boost::beast::http::request<boost::beast::http::string_body>;
+ using Body = boost::beast::http::request<bmcweb::FileBody>;
std::unique_ptr<Body> mobile = std::make_unique<Body>(req.req);
Body* ptr = mobile.get();
// Perform the websocket upgrade
@@ -227,8 +226,8 @@
}
void acceptDone(const std::shared_ptr<Connection>& /*self*/,
- const std::unique_ptr<boost::beast::http::request<
- boost::beast::http::string_body>>& /*req*/,
+ const std::unique_ptr<
+ boost::beast::http::request<bmcweb::FileBody>>& /*req*/,
const boost::system::error_code& ec)
{
if (ec)
diff --git a/redfish-core/include/utils/query_param.hpp b/redfish-core/include/utils/query_param.hpp
index 03b5f53..881b090 100644
--- a/redfish-core/include/utils/query_param.hpp
+++ b/redfish-core/include/utils/query_param.hpp
@@ -511,7 +511,6 @@
BMCWEB_LOG_DEBUG("setting completion handler on {}",
logPtr(&asyncResp->res));
asyncResp->res.setCompleteRequestHandler(std::move(completionHandler));
- asyncResp->res.setIsAliveHelper(res.releaseIsAliveHelper());
app.handle(newReq, asyncResp);
return true;
}
diff --git a/test/http/http_response_test.cpp b/test/http/http_response_test.cpp
index 1ee3853..f61d6de 100644
--- a/test/http/http_response_test.cpp
+++ b/test/http/http_response_test.cpp
@@ -1,3 +1,4 @@
+#include "boost/beast/core/buffers_to_string.hpp"
#include "boost/beast/core/flat_buffer.hpp"
#include "boost/beast/http/serializer.hpp"
#include "http/http_response.hpp"
@@ -35,71 +36,40 @@
return stringPath;
}
-void readHeader(boost::beast::http::serializer<false, bmcweb::FileBody>& sr)
+std::string getData(boost::beast::http::response<bmcweb::FileBody>& m)
{
+ std::string ret;
+
+ boost::beast::http::response_serializer<bmcweb::FileBody> sr{m};
+ sr.split(true);
+
+ auto reader = [&sr, &ret](const boost::system::error_code& ec2,
+ const auto& buffer) {
+ EXPECT_FALSE(ec2);
+ std::string ret2 = boost::beast::buffers_to_string(buffer);
+ sr.consume(ret2.size());
+ ret += ret2;
+ };
+ boost::system::error_code ec;
+
+ // Read headers
while (!sr.is_header_done())
{
- boost::system::error_code ec;
- sr.next(ec, [&sr](const boost::system::error_code& ec2,
- const auto& buffer) {
- ASSERT_FALSE(ec2);
- sr.consume(boost::beast::buffer_bytes(buffer));
- });
- ASSERT_FALSE(ec);
+ sr.next(ec, reader);
+ EXPECT_FALSE(ec);
}
-}
+ ret.clear();
-std::string collectFromBuffers(
- const auto& buffer,
- boost::beast::http::serializer<false, bmcweb::FileBody>& sr)
-{
- std::string ret;
-
- for (auto iter = boost::asio::buffer_sequence_begin(buffer);
- iter != boost::asio::buffer_sequence_end(buffer); ++iter)
- {
- const auto& innerBuf = *iter;
- auto view = std::string_view(static_cast<const char*>(innerBuf.data()),
- innerBuf.size());
- ret += view;
- sr.consume(innerBuf.size());
- }
- return ret;
-}
-
-std::string
- readBody(boost::beast::http::serializer<false, bmcweb::FileBody>& sr)
-{
- std::string ret;
+ // Read body
while (!sr.is_done())
{
- boost::system::error_code ec;
- sr.next(ec, [&sr, &ret](const boost::system::error_code& ec2,
- const auto& buffer) {
- ASSERT_FALSE(ec2);
- ret += collectFromBuffers(buffer, sr);
- });
+ sr.next(ec, reader);
EXPECT_FALSE(ec);
}
return ret;
}
-std::string getData(crow::Response::file_response& m)
-{
- boost::beast::http::serializer<false, bmcweb::FileBody> sr{m};
- std::stringstream ret;
- sr.split(true);
- readHeader(sr);
- return readBody(sr);
-}
-TEST(HttpResponse, Defaults)
-{
- crow::Response res;
- EXPECT_EQ(
- boost::variant2::holds_alternative<crow::Response::string_response>(
- res.response),
- true);
-}
+
TEST(HttpResponse, Headers)
{
crow::Response res;
@@ -156,26 +126,16 @@
std::string path = makeFile("sample text");
res.openFile(path);
- EXPECT_EQ(boost::variant2::holds_alternative<crow::Response::file_response>(
- res.response),
- true);
-
verifyHeaders(res);
res.write("body text");
- EXPECT_EQ(
- boost::variant2::holds_alternative<crow::Response::string_response>(
- res.response),
- true);
-
verifyHeaders(res);
std::filesystem::remove(path);
}
void testFileData(crow::Response& res, const std::string& data)
{
- auto& fb =
- boost::variant2::get<crow::Response::file_response>(res.response);
+ auto& fb = res.response;
EXPECT_EQ(getData(fb), data);
}