diff options
author | Ed Tanous <ed@tanous.net> | 2023-08-07 22:02:40 +0300 |
---|---|---|
committer | Ed Tanous <ed@tanous.net> | 2023-10-31 20:43:04 +0300 |
commit | 27b0cf90f6cba207837f5c263a45c6ea5651975b (patch) | |
tree | 314cda7432edfe44fdd1297664db1399a1d6ba22 | |
parent | 522377dcb85082da598403e104a44d621b4c2bb4 (diff) | |
download | bmcweb-27b0cf90f6cba207837f5c263a45c6ea5651975b.tar.xz |
Move to file_body in boost
As is, it reads the whole file into memory before sending it. While
fairly fast for the user, this wastes ram, and makes bmcweb less useful
on less capable systems.
This patch enables using the boost::beast::http::file_body type, which
has more efficient serialization semantics than using a std::string. To
do this, it adds a openFile() handler to http::Response, which can be
used to properly open a file. Once the file is opened, the existing
string body is ignored, and the file payload is sent instead.
openFile() also returns success or failure, to allow users to properly
handle 404s and other errors.
To prove that it works, I moved over every instance of direct use of the
body() method over to using this, including the webasset handler. The
webasset handler specifically should help with system load when doing an
initial page load of the webui.
Tested:
Redfish service validator passes.
Change-Id: Ic7ea9ffefdbc81eb985de7edc0fac114822994ad
Signed-off-by: Ed Tanous <ed@tanous.net>
-rw-r--r-- | http/complete_response_fields.hpp | 11 | ||||
-rw-r--r-- | http/http2_connection.hpp | 41 | ||||
-rw-r--r-- | http/http_client.hpp | 42 | ||||
-rw-r--r-- | http/http_connection.hpp | 79 | ||||
-rw-r--r-- | http/http_response.hpp | 202 | ||||
-rw-r--r-- | include/forward_unauthorized.hpp | 4 | ||||
-rw-r--r-- | include/openbmc_dbus_rest.hpp | 5 | ||||
-rw-r--r-- | include/webassets.hpp | 6 | ||||
-rw-r--r-- | redfish-core/include/redfish_aggregator.hpp | 24 | ||||
-rw-r--r-- | redfish-core/lib/log_services.hpp | 23 | ||||
-rw-r--r-- | src/json_html_serializer.cpp | 4 | ||||
-rw-r--r-- | test/redfish-core/include/redfish_aggregator_test.cpp | 28 |
12 files changed, 286 insertions, 183 deletions
diff --git a/http/complete_response_fields.hpp b/http/complete_response_fields.hpp index 66a4bb79af..a5b3eec3d0 100644 --- a/http/complete_response_fields.hpp +++ b/http/complete_response_fields.hpp @@ -27,8 +27,7 @@ inline void completeResponseFields(const Request& req, Response& res) authentication::cleanupTempSession(req); res.setHashAndHandleNotModified(); - - if (res.body().empty() && res.jsonValue.is_structured()) + if (res.jsonValue.is_structured()) { using http_helpers::ContentType; std::array<ContentType, 3> allowed{ContentType::CBOR, ContentType::JSON, @@ -44,7 +43,9 @@ inline void completeResponseFields(const Request& req, Response& res) { res.addHeader(boost::beast::http::field::content_type, "application/cbor"); - nlohmann::json::to_cbor(res.jsonValue, res.body()); + std::string cbor; + nlohmann::json::to_cbor(res.jsonValue, cbor); + res.write(std::move(cbor)); } else { @@ -53,8 +54,8 @@ inline void completeResponseFields(const Request& req, Response& res) // backward compatibility. res.addHeader(boost::beast::http::field::content_type, "application/json"); - res.body() = res.jsonValue.dump( - 2, ' ', true, nlohmann::json::error_handler_t::replace); + res.write(res.jsonValue.dump( + 2, ' ', true, nlohmann::json::error_handler_t::replace)); } } } diff --git a/http/http2_connection.hpp b/http/http2_connection.hpp index d815119e96..ee3a218807 100644 --- a/http/http2_connection.hpp +++ b/http/http2_connection.hpp @@ -106,19 +106,42 @@ class HTTP2Connection : BMCWEB_LOG_DEBUG("File read callback length: {}", length); crow::Response& res = stream.res; - BMCWEB_LOG_DEBUG("total: {} send_sofar: {}", res.body().size(), - stream.sentSofar); + 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 toSend = std::min(res.body().size() - stream.sentSofar, length); + 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); - std::string::iterator bodyBegin = res.body().begin(); - std::advance(bodyBegin, stream.sentSofar); + 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 + { + return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; + } - memcpy(buf, &*bodyBegin, toSend); stream.sentSofar += toSend; - if (stream.sentSofar >= res.body().size()) + if (stream.sentSofar >= size) { BMCWEB_LOG_DEBUG("Setting OEF flag"); *dataFlags |= NGHTTP2_DATA_FLAG_EOF; @@ -154,8 +177,8 @@ class HTTP2Connection : completeResponseFields(thisReq, thisRes); thisRes.addHeader(boost::beast::http::field::date, getCachedDateStr()); - boost::beast::http::fields& fields = thisRes.stringResponse.base(); - std::string code = std::to_string(thisRes.stringResponse.result_int()); + boost::beast::http::fields& fields = thisRes.fields(); + std::string code = std::to_string(thisRes.resultInt()); hdr.emplace_back(headerFromStringViews(":status", code)); for (const boost::beast::http::fields::value_type& header : fields) { diff --git a/http/http_client.hpp b/http/http_client.hpp index 046df2e0f0..8c27d6d61a 100644 --- a/http/http_client.hpp +++ b/http/http_client.hpp @@ -39,6 +39,8 @@ #include <boost/beast/version.hpp> #include <boost/container/devector.hpp> #include <boost/system/error_code.hpp> +#include <boost/url/format.hpp> +#include <boost/url/url.hpp> #include <boost/url/url_view.hpp> #include <cstdlib> @@ -50,9 +52,8 @@ namespace crow { - -// With Redfish Aggregation it is assumed we will connect to another instance -// of BMCWeb which can handle 100 simultaneous connections. +// With Redfish Aggregation it is assumed we will connect to another +// instance of BMCWeb which can handle 100 simultaneous connections. constexpr size_t maxPoolSize = 20; constexpr size_t maxRequestQueueSize = 500; constexpr unsigned int httpReadBodyLimit = 131072; @@ -96,8 +97,8 @@ static inline boost::system::error_code return boost::system::errc::make_error_code(boost::system::errc::success); }; -// We need to allow retry information to be set before a message has been sent -// and a connection pool has been created +// We need to allow retry information to be set before a message has been +// sent and a connection pool has been created struct ConnectionPolicy { uint32_t maxRetryAttempts = 5; @@ -402,7 +403,7 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> // Copy the response into a Response object so that it can be // processed by the callback function. - res.stringResponse = parser->release(); + res.response = parser->release(); callback(parser->keep_alive(), connId, res); res.clear(); } @@ -419,8 +420,8 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> if (ec) { BMCWEB_LOG_ERROR("async_wait failed: {}", ec.message()); - // If the timer fails, we need to close the socket anyway, same as - // if it expired. + // If the timer fails, we need to close the socket anyway, same + // as if it expired. } std::shared_ptr<ConnectionInfo> self = weakSelf.lock(); if (self == nullptr) @@ -454,8 +455,8 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> callback(false, connId, res); res.clear(); - // Reset the retrycount to zero so that client can try connecting - // again if needed + // Reset the retrycount to zero so that client can try + // connecting again if needed retryCount = 0; return; } @@ -604,10 +605,11 @@ class ConnectionInfo : public std::enable_shared_from_this<ConnectionInfo> BMCWEB_LOG_ERROR("prepareSSLContext failed - {}, id: {}", host, connId); // Don't retry if failure occurs while preparing SSL context - // such as certificate is invalid or set cipher failure or set - // host name failure etc... Setting conn state to sslInitFailed - // and connection state will be transitioned to next state - // depending on retry policy set by subscription. + // such as certificate is invalid or set cipher failure or + // set host name failure etc... Setting conn state to + // sslInitFailed and connection state will be transitioned + // to next state depending on retry policy set by + // subscription. state = ConnState::sslInitFailed; waitAndRetry(); return; @@ -742,8 +744,8 @@ class ConnectionPool : public std::enable_shared_from_this<ConnectionPool> } } - // All connections in use so create a new connection or add request to - // the queue + // All connections in use so create a new connection or add request + // to the queue if (connections.size() < connPolicy->maxConnections) { BMCWEB_LOG_DEBUG("Adding new connection to pool {}", id); @@ -760,8 +762,8 @@ class ConnectionPool : public std::enable_shared_from_this<ConnectionPool> } else { - // If we can't buffer the request then we should let the callback - // handle a 429 Too Many Requests dummy response + // If we can't buffer the request then we should let the + // callback handle a 429 Too Many Requests dummy response BMCWEB_LOG_ERROR("{}:{} request queue full. Dropping request.", id); Response dummyRes; @@ -875,8 +877,8 @@ class HttpClient pool.first->second = std::make_shared<ConnectionPool>( ioc, clientKey, connPolicy, destUrl); } - // Send the data using either the existing connection pool or the newly - // created connection pool + // Send the data using either the existing connection pool or the + // newly created connection pool pool.first->second->sendData(std::move(data), destUrl, httpHeader, verb, resHandler); } diff --git a/http/http_connection.hpp b/http/http_connection.hpp index 86cc49a2f7..1ec80ae77a 100644 --- a/http/http_connection.hpp +++ b/http/http_connection.hpp @@ -17,11 +17,11 @@ #include <boost/asio/ip/tcp.hpp> #include <boost/asio/ssl/stream.hpp> #include <boost/asio/steady_timer.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> #include <boost/beast/http/read.hpp> -#include <boost/beast/http/serializer.hpp> #include <boost/beast/http/write.hpp> #include <boost/beast/ssl/ssl_stream.hpp> #include <boost/beast/websocket.hpp> @@ -532,47 +532,50 @@ class Connection : }); } - void doWrite(crow::Response& thisRes) + void afterDoWrite(const std::shared_ptr<self_type>& /*self*/, + const boost::system::error_code& ec, + std::size_t bytesTransferred) { - BMCWEB_LOG_DEBUG("{} doWrite", logPtr(this)); - thisRes.preparePayload(); - serializer.emplace(thisRes.stringResponse); - startDeadline(); - boost::beast::http::async_write(adaptor, *serializer, - [this, self(shared_from_this())]( - const boost::system::error_code& ec, - std::size_t bytesTransferred) { - BMCWEB_LOG_DEBUG("{} async_write {} bytes", logPtr(this), - bytesTransferred); + BMCWEB_LOG_DEBUG("{} async_write {} bytes", logPtr(this), + bytesTransferred); - cancelDeadlineTimer(); + cancelDeadlineTimer(); - if (ec) - { - BMCWEB_LOG_DEBUG("{} from write(2)", logPtr(this)); - return; - } - if (!keepAlive) - { - close(); - BMCWEB_LOG_DEBUG("{} from write(1)", logPtr(this)); - return; - } + if (ec) + { + BMCWEB_LOG_DEBUG("{} from write(2)", logPtr(this)); + return; + } + if (!keepAlive) + { + close(); + BMCWEB_LOG_DEBUG("{} from write(1)", logPtr(this)); + return; + } - serializer.reset(); - BMCWEB_LOG_DEBUG("{} Clearing response", logPtr(this)); - res.clear(); - parser.emplace(std::piecewise_construct, std::make_tuple()); - parser->body_limit(httpReqBodyLimit); // reset body limit for - // newly created parser - buffer.consume(buffer.size()); + BMCWEB_LOG_DEBUG("{} Clearing response", logPtr(this)); + res.clear(); + parser.emplace(std::piecewise_construct, std::make_tuple()); + parser->body_limit(httpReqBodyLimit); // reset body limit for + // newly created parser + buffer.consume(buffer.size()); - userSession = nullptr; + userSession = nullptr; - // Destroy the Request via the std::optional - req.reset(); - doReadHeaders(); - }); + // Destroy the Request via the std::optional + req.reset(); + doReadHeaders(); + } + + void doWrite(crow::Response& thisRes) + { + BMCWEB_LOG_DEBUG("{} doWrite", logPtr(this)); + thisRes.preparePayload(); + + startDeadline(); + boost::beast::async_write(adaptor, thisRes.generator(), + std::bind_front(&self_type::afterDoWrite, + this, shared_from_this())); } void cancelDeadlineTimer() @@ -637,10 +640,6 @@ class Connection : boost::beast::flat_static_buffer<8192> buffer; - std::optional<boost::beast::http::response_serializer< - boost::beast::http::string_body>> - serializer; - std::optional<crow::Request> req; crow::Response res; diff --git a/http/http_response.hpp b/http/http_response.hpp index cb07a83635..a5f95a90b2 100644 --- a/http/http_response.hpp +++ b/http/http_response.hpp @@ -2,8 +2,11 @@ #include "logging.hpp" #include "utils/hex_utils.hpp" +#include <boost/beast/http/file_body.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> @@ -16,37 +19,52 @@ namespace crow template <typename Adaptor, typename Handler> class Connection; +namespace http = boost::beast::http; + struct Response { template <typename Adaptor, typename Handler> friend class crow::Connection; - using response_type = - boost::beast::http::response<boost::beast::http::string_body>; - response_type stringResponse; + using string_response = http::response<http::string_body>; + using file_response = http::response<http::file_body>; + + // Use boost variant2 because it doesn't have valueless by exception + boost::variant2::variant<string_response, file_response> 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); + } - void addHeader(std::string_view key, std::string_view value) + const fields_type& fields() const { - stringResponse.insert(key, value); + return boost::variant2::visit( + [](auto&& r) -> const fields_type& { return r.base(); }, response); } - void addHeader(boost::beast::http::field key, std::string_view value) + void addHeader(std::string_view key, std::string_view value) { - stringResponse.insert(key, value); + fields().insert(key, value); } - void clearHeader(boost::beast::http::field key) + void addHeader(http::field key, std::string_view value) { - stringResponse.erase(key); + fields().insert(key, value); } - Response() = default; + void clearHeader(http::field key) + { + fields().erase(key); + } + Response() : response(string_response()) {} Response(Response&& res) noexcept : - stringResponse(std::move(res.stringResponse)), - jsonValue(std::move(res.jsonValue)), completed(res.completed) + response(std::move(res.response)), jsonValue(std::move(res.jsonValue)), + completed(res.completed) { // See note in operator= move handler for why this is needed. if (!res.completed) @@ -61,7 +79,6 @@ struct Response ~Response() = default; Response(const Response&) = delete; - Response& operator=(const Response& r) = delete; Response& operator=(Response&& r) noexcept @@ -72,8 +89,7 @@ struct Response { return *this; } - stringResponse = std::move(r.stringResponse); - r.stringResponse.clear(); + response = std::move(r.response); jsonValue = std::move(r.jsonValue); // Only need to move completion handler if not already completed @@ -98,27 +114,45 @@ struct Response void result(unsigned v) { - stringResponse.result(v); + fields().result(v); } - void result(boost::beast::http::status v) + void result(http::status v) { - stringResponse.result(v); + fields().result(v); } - boost::beast::http::status result() const + void copyBody(const Response& res) { - return stringResponse.result(); + 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(); + } + + http::status result() const + { + return fields().result(); } unsigned resultInt() const { - return stringResponse.result_int(); + return fields().result_int(); } std::string_view reason() const { - return stringResponse.reason(); + return fields().reason(); } bool isCompleted() const noexcept @@ -126,75 +160,87 @@ struct Response return completed; } - std::string& body() + const std::string* body() { - return stringResponse.body(); + string_response* body = + boost::variant2::get_if<string_response>(&response); + if (body == nullptr) + { + return nullptr; + } + return &body->body(); } std::string_view getHeaderValue(std::string_view key) const { - return stringResponse.base()[key]; + return fields()[key]; } void keepAlive(bool k) { - stringResponse.keep_alive(k); + return boost::variant2::visit([k](auto&& r) { r.keep_alive(k); }, + response); } bool keepAlive() const { - return stringResponse.keep_alive(); + return boost::variant2::visit([](auto&& r) { return r.keep_alive(); }, + response); } - void preparePayload() + uint64_t getContentLength(boost::optional<uint64_t> pSize) { // This code is a throw-free equivalent to // beast::http::message::prepare_payload - boost::optional<uint64_t> pSize = stringResponse.payload_size(); - using boost::beast::http::status; - using boost::beast::http::status_class; - using boost::beast::http::to_status_class; + using http::status; + using http::status_class; + using http::to_status_class; if (!pSize) { - pSize = 0; + return 0; } - else + bool is1XXReturn = to_status_class(result()) == + status_class::informational; + if (*pSize > 0 && (is1XXReturn || result() == status::no_content || + result() == status::not_modified)) { - bool is1XXReturn = to_status_class(stringResponse.result()) == - status_class::informational; - if (*pSize > 0 && - (is1XXReturn || stringResponse.result() == status::no_content || - stringResponse.result() == status::not_modified)) - { - BMCWEB_LOG_CRITICAL( - "{} Response content provided but code was no-content or not_modified, which aren't allowed to have a body", - logPtr(this)); - pSize = 0; - body().clear(); - } + BMCWEB_LOG_CRITICAL("{} Response content provided but code was " + "no-content or not_modified, which aren't " + "allowed to have a body", + logPtr(this)); + return 0; } - stringResponse.content_length(*pSize); + 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); } void clear() { BMCWEB_LOG_DEBUG("{} Clearing response containers", logPtr(this)); - stringResponse.clear(); - stringResponse.body().shrink_to_fit(); + response.emplace<string_response>(); jsonValue = nullptr; completed = false; expectedHash = std::nullopt; } - void write(std::string_view bodyPart) - { - stringResponse.body() += std::string(bodyPart); - } - std::string computeEtag() const { // Only set etag if this request succeeded - if (result() != boost::beast::http::status::ok) + if (result() != http::status::ok) { return ""; } @@ -207,12 +253,24 @@ struct Response return "\"" + intToHexString(hashval, 8) + "\""; } + void write(std::string&& bodyPart) + { + string_response* str = + boost::variant2::get_if<string_response>(&response); + if (str != nullptr) + { + str->body() += bodyPart; + return; + } + response.emplace<string_response>(result(), 11, std::move(bodyPart)); + } + void end() { std::string etag = computeEtag(); if (!etag.empty()) { - addHeader(boost::beast::http::field::etag, etag); + addHeader(http::field::etag, etag); } if (completed) { @@ -268,17 +326,17 @@ struct Response void setHashAndHandleNotModified() { // Can only hash if we have content that's valid - if (jsonValue.empty() || result() != boost::beast::http::status::ok) + if (jsonValue.empty() || result() != http::status::ok) { return; } size_t hashval = std::hash<nlohmann::json>{}(jsonValue); std::string hexVal = "\"" + intToHexString(hashval, 8) + "\""; - addHeader(boost::beast::http::field::etag, hexVal); + addHeader(http::field::etag, hexVal); if (expectedHash && hexVal == *expectedHash) { jsonValue = nullptr; - result(boost::beast::http::status::not_modified); + result(http::status::not_modified); } } @@ -287,6 +345,32 @@ struct Response 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) + { + http::file_body::value_type file; + boost::beast::error_code ec; + file.open(path.c_str(), boost::beast::file_mode::read, ec); + if (ec) + { + return false; + } + // 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); + return true; + } + private: std::optional<std::string> expectedHash; bool completed = false; diff --git a/include/forward_unauthorized.hpp b/include/forward_unauthorized.hpp index 5d5e744f47..07836cefd1 100644 --- a/include/forward_unauthorized.hpp +++ b/include/forward_unauthorized.hpp @@ -34,8 +34,8 @@ inline void sendUnauthorized(std::string_view url, // If we don't have a webui installed, just return an unauthorized // body res.result(boost::beast::http::status::unauthorized); - res.body() = - "No authentication provided, and no login UI present to forward to."; + res.write( + "No authentication provided, and no login UI present to forward to."); return; } diff --git a/include/openbmc_dbus_rest.hpp b/include/openbmc_dbus_rest.hpp index bade59e11b..e0c16d9e8f 100644 --- a/include/openbmc_dbus_rest.hpp +++ b/include/openbmc_dbus_rest.hpp @@ -2534,8 +2534,7 @@ inline void requestRoutes(App& app) for (const auto& file : files) { - std::ifstream readFile(file.path()); - if (!readFile.good()) + if (!asyncResp->res.openFile(file)) { continue; } @@ -2564,8 +2563,6 @@ inline void requestRoutes(App& app) boost::beast::http::field::content_disposition, contentDispositionParam); - asyncResp->res.body() = {std::istreambuf_iterator<char>(readFile), - std::istreambuf_iterator<char>()}; return; } asyncResp->res.result(boost::beast::http::status::not_found); diff --git a/include/webassets.hpp b/include/webassets.hpp index 4deb7a78ee..5ab03231e4 100644 --- a/include/webassets.hpp +++ b/include/webassets.hpp @@ -165,17 +165,13 @@ inline void requestRoutes(App& app) } // res.set_header("Cache-Control", "public, max-age=86400"); - std::ifstream inf(absolutePath); - if (!inf) + if (!asyncResp->res.openFile(absolutePath)) { BMCWEB_LOG_DEBUG("failed to read file"); asyncResp->res.result( boost::beast::http::status::internal_server_error); return; } - - asyncResp->res.body() = {std::istreambuf_iterator<char>(inf), - std::istreambuf_iterator<char>()}; }); } } diff --git a/redfish-core/include/redfish_aggregator.hpp b/redfish-core/include/redfish_aggregator.hpp index 335c64e694..680e1b0c05 100644 --- a/redfish-core/include/redfish_aggregator.hpp +++ b/redfish-core/include/redfish_aggregator.hpp @@ -728,7 +728,7 @@ class RedfishAggregator std::function<void(crow::Response&)> cb = std::bind_front(processResponse, prefix, asyncResp); - std::string data = thisReq.req.body(); + std::string data = thisReq.body(); boost::urls::url url(sat->second); url.set_path(path); if (targetURI.has_query()) @@ -756,7 +756,7 @@ class RedfishAggregator { url.set_query(thisReq.url().query()); } - std::string data = thisReq.req.body(); + std::string data = thisReq.body(); client.sendDataWithCallback(std::move(data), url, thisReq.fields(), thisReq.method(), cb); } @@ -781,7 +781,7 @@ class RedfishAggregator boost::urls::url url(sat.second); url.set_path(thisReq.url().path()); - std::string data = thisReq.req.body(); + std::string data = thisReq.body(); client.sendDataWithCallback(std::move(data), url, thisReq.fields(), thisReq.method(), cb); @@ -874,8 +874,8 @@ class RedfishAggregator if (boost::iequals(contentType, "application/json") || boost::iequals(contentType, "application/json; charset=utf-8")) { - nlohmann::json jsonVal = nlohmann::json::parse(resp.body(), nullptr, - false); + nlohmann::json jsonVal = nlohmann::json::parse(*resp.body(), + nullptr, false); if (jsonVal.is_discarded()) { BMCWEB_LOG_ERROR("Error parsing satellite response as JSON"); @@ -898,7 +898,7 @@ class RedfishAggregator { // We allow any Content-Type that is not "application/json" now asyncResp->res.result(resp.result()); - asyncResp->res.write(resp.body()); + asyncResp->res.copyBody(resp); } addAggregatedHeaders(asyncResp->res, resp, prefix); } @@ -927,7 +927,7 @@ class RedfishAggregator if (asyncResp->res.resultInt() != 200) { asyncResp->res.result(resp.result()); - asyncResp->res.write(resp.body()); + asyncResp->res.copyBody(resp); } return; } @@ -938,8 +938,8 @@ class RedfishAggregator if (boost::iequals(contentType, "application/json") || boost::iequals(contentType, "application/json; charset=utf-8")) { - nlohmann::json jsonVal = nlohmann::json::parse(resp.body(), nullptr, - false); + nlohmann::json jsonVal = nlohmann::json::parse(*resp.body(), + nullptr, false); if (jsonVal.is_discarded()) { BMCWEB_LOG_ERROR("Error parsing satellite response as JSON"); @@ -1061,7 +1061,7 @@ class RedfishAggregator if (asyncResp->res.resultInt() != 200) { asyncResp->res.result(resp.result()); - asyncResp->res.write(resp.body()); + asyncResp->res.copyBody(resp); } return; } @@ -1073,8 +1073,8 @@ class RedfishAggregator boost::iequals(contentType, "application/json; charset=utf-8")) { bool addedLinks = false; - nlohmann::json jsonVal = nlohmann::json::parse(resp.body(), nullptr, - false); + nlohmann::json jsonVal = nlohmann::json::parse(*resp.body(), + nullptr, false); if (jsonVal.is_discarded()) { BMCWEB_LOG_ERROR("Error parsing satellite response as JSON"); diff --git a/redfish-core/lib/log_services.hpp b/redfish-core/lib/log_services.hpp index 116bc8acb8..54284cfef2 100644 --- a/redfish-core/lib/log_services.hpp +++ b/redfish-core/lib/log_services.hpp @@ -766,8 +766,9 @@ inline void return; } - asyncResp->res.body().resize(static_cast<size_t>(size), '\0'); - rc = read(fd, asyncResp->res.body().data(), asyncResp->res.body().size()); + std::string body; + body.resize(static_cast<size_t>(size), '\0'); + rc = read(fd, body.data(), body.size()); if ((rc == -1) || (rc != size)) { BMCWEB_LOG_ERROR("Failed to read in file"); @@ -776,16 +777,17 @@ inline void return; } close(fd); - if (downloadEntryType == "System") { - // We need to encode the data. Doing it this way saves the other paths - // from having to make a copy into a temporary variable. - asyncResp->res.body() = - crow::utility::base64encode(asyncResp->res.body()); + // Base64 encode response. + asyncResp->res.write(crow::utility::base64encode(body)); asyncResp->res.addHeader( boost::beast::http::field::content_transfer_encoding, "Base64"); } + else + { + asyncResp->res.write(std::move(body)); + } asyncResp->res.addHeader(boost::beast::http::field::content_type, "application/octet-stream"); @@ -3545,14 +3547,11 @@ inline void requestRoutesCrashdumpFile(App& app) return; } - if (!std::filesystem::exists(dbusFilepath)) + if (!asyncResp->res.openFile(dbusFilepath)) { messages::resourceNotFound(asyncResp->res, "LogEntry", logID); return; } - std::ifstream ifs(dbusFilepath, std::ios::in | std::ios::binary); - asyncResp->res.body() = - std::string(std::istreambuf_iterator<char>{ifs}, {}); // Configure this to be a file download when accessed // from a browser @@ -4314,7 +4313,7 @@ inline void requestRoutesPostCodesEntryAdditionalData(App& app) "application/octet-stream"); asyncResp->res.addHeader( boost::beast::http::field::content_transfer_encoding, "Base64"); - asyncResp->res.body() = crow::utility::base64encode(strData); + asyncResp->res.write(crow::utility::base64encode(strData)); }, "xyz.openbmc_project.State.Boot.PostCode0", "/xyz/openbmc_project/State/Boot/PostCode0", diff --git a/src/json_html_serializer.cpp b/src/json_html_serializer.cpp index 405061beee..8322f1dd8a 100644 --- a/src/json_html_serializer.cpp +++ b/src/json_html_serializer.cpp @@ -562,8 +562,10 @@ void dumpHtml(std::string& out, const nlohmann::json& json) void prettyPrintJson(crow::Response& res) { - json_html_util::dumpHtml(res.body(), res.jsonValue); + std::string html; + json_html_util::dumpHtml(html, res.jsonValue); + res.write(std::move(html)); res.addHeader(boost::beast::http::field::content_type, "text/html;charset=UTF-8"); } diff --git a/test/redfish-core/include/redfish_aggregator_test.cpp b/test/redfish-core/include/redfish_aggregator_test.cpp index 83e4b4ec23..26015f7096 100644 --- a/test/redfish-core/include/redfish_aggregator_test.cpp +++ b/test/redfish-core/include/redfish_aggregator_test.cpp @@ -248,8 +248,8 @@ void assertProcessResponse(unsigned result) jsonResp["Name"] = "Test"; crow::Response resp; - resp.body() = jsonResp.dump(2, ' ', true, - nlohmann::json::error_handler_t::replace); + resp.write( + jsonResp.dump(2, ' ', true, nlohmann::json::error_handler_t::replace)); resp.addHeader("Content-Type", "application/json"); resp.addHeader("Allow", "GET"); resp.addHeader("Location", "/redfish/v1/Chassis/TestChassis"); @@ -351,8 +351,8 @@ void populateCollectionNotFound(crow::Response& resp) // from a satellite which will not have a json component void convertToSat(crow::Response& resp) { - resp.body() = resp.jsonValue.dump(2, ' ', true, - nlohmann::json::error_handler_t::replace); + resp.write(resp.jsonValue.dump(2, ' ', true, + nlohmann::json::error_handler_t::replace)); resp.jsonValue.clear(); } @@ -494,7 +494,7 @@ TEST(processCollectionResponse, preserveHeaders) void assertProcessResponseContentType(std::string_view contentType) { crow::Response resp; - resp.body() = "responseBody"; + resp.write("responseBody"); resp.addHeader("Content-Type", contentType); resp.addHeader("Location", "/redfish/v1/Chassis/TestChassis"); resp.addHeader("Link", "metadataLink"); @@ -507,7 +507,7 @@ void assertProcessResponseContentType(std::string_view contentType) "/redfish/v1/Chassis/prefix_TestChassis"); EXPECT_EQ(asyncResp->res.getHeaderValue("Link"), ""); EXPECT_EQ(asyncResp->res.getHeaderValue("Retry-After"), "120"); - EXPECT_EQ(asyncResp->res.body(), "responseBody"); + EXPECT_EQ(*asyncResp->res.body(), "responseBody"); } TEST(processResponse, DifferentContentType) @@ -658,8 +658,8 @@ TEST(processContainsSubordinateResponse, addLinks) jsonValue["Test"]["@odata.id"] = "/redfish/v1/Test"; jsonValue["TelemetryService"]["@odata.id"] = "/redfish/v1/TelemetryService"; jsonValue["UpdateService"]["@odata.id"] = "/redfish/v1/UpdateService"; - resp.body() = jsonValue.dump(2, ' ', true, - nlohmann::json::error_handler_t::replace); + resp.write( + jsonValue.dump(2, ' ', true, nlohmann::json::error_handler_t::replace)); auto asyncResp = std::make_shared<bmcweb::AsyncResp>(); asyncResp->res.result(200); @@ -701,8 +701,8 @@ TEST(processContainsSubordinateResponse, localNotOK) jsonValue["Test"]["@odata.id"] = "/redfish/v1/Test"; jsonValue["TelemetryService"]["@odata.id"] = "/redfish/v1/TelemetryService"; jsonValue["UpdateService"]["@odata.id"] = "/redfish/v1/UpdateService"; - resp.body() = jsonValue.dump(2, ' ', true, - nlohmann::json::error_handler_t::replace); + resp.write( + jsonValue.dump(2, ' ', true, nlohmann::json::error_handler_t::replace)); RedfishAggregator::processContainsSubordinateResponse("prefix", asyncResp, resp); @@ -769,8 +769,8 @@ TEST(processContainsSubordinateResponse, noValidLinks) nlohmann::json jsonValue; resp.addHeader("Content-Type", "application/json"); jsonValue["@odata.id"] = "/redfish/v1"; - resp.body() = jsonValue.dump(2, ' ', true, - nlohmann::json::error_handler_t::replace); + resp.write( + jsonValue.dump(2, ' ', true, nlohmann::json::error_handler_t::replace)); RedfishAggregator::processContainsSubordinateResponse("prefix", asyncResp, resp); @@ -788,8 +788,8 @@ TEST(processContainsSubordinateResponse, noValidLinks) jsonValue["Test"]["@odata.id"] = "/redfish/v1/Test"; jsonValue["TelemetryService"]["@odata.id"] = "/redfish/v1/TelemetryService"; jsonValue["UpdateService"]["@odata.id"] = "/redfish/v1/UpdateService"; - resp.body() = jsonValue.dump(2, ' ', true, - nlohmann::json::error_handler_t::replace); + resp.write( + jsonValue.dump(2, ' ', true, nlohmann::json::error_handler_t::replace)); RedfishAggregator::processContainsSubordinateResponse("prefix", asyncResp, resp); |