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