Implement Chunking for unix sockets
Response::openFd was added recently to allow handlers to pass in a file
descriptor to be used to read. This worked great for files, but had
some trouble with unix sockets. First, unix sockets have no known
length that we can get. They are fed by another client until that
client decides to stop sending data and sends an EOF. HTTP in general
needs to set the Content-Length header before starting a reply, so the
previous code just passes an error back.
HTTP has a concept of HTTP chunking, where a payload might not have a
known size, but can still be downloaded in chunks. Beast has handling
for this that we can enable that just deals with this at the protocol
layer silently. This patch enables that.
In addition, a unix socket very likely might not have data and will
block on the read call. Blocking in an async reactor is bad, and
especially bad when you don't know how large a payload is to be
expected, so it's possible those bytes will never come. This commit
sets all FDs into O_NONBLOCK[1] mode when they're sent to a response,
then handles the subsequent EWOULDBLOCK and EAGAIN messages when beast
propagates them to the http connection class. When these messages are
received, the doWrite loop is simply re-executed directly, attempting to
read from the socket again. For "slow" unix sockets, this very likely
results in some wasted cycles where we read 0 bytes from the socket, so
shouldn't be used for eventing purposes, given that bmcweb is
essentially in a spin loop while waiting for data, but given that this
is generally used for handling chunking of large payloads being
generated, and while spinning, other reactor operations can still
progress, this seems like a reasonable compromise.
[1] https://www.gnu.org/software/libc/manual/html_node/Open_002dtime-Flags.html
Tested:
The next patch in this series includes an example of explicitly adding a
unix socket as a response target, using the CredentialsPipe that bmcweb
already has. When this handler is present, curl shows the response
data, including the newlines (when dumped to a file)
```
curl -vvvv -k --user "root:0penBmc" https://192.168.7.2/testpipe -o output.txt
```
Loading the webui works as expected, logging in produces the overview
page as expected, and network console shows no failed requests.
Redfish service validator passes.
Change-Id: I8bd8586ae138f5b55033b78df95c798aa1d014db
Signed-off-by: Ed Tanous <ed@tanous.net>
diff --git a/http/http_body.hpp b/http/http_body.hpp
index f4bfed5..7be23f0 100644
--- a/http/http_body.hpp
+++ b/http/http_body.hpp
@@ -220,11 +220,19 @@
return ret;
}
size_t readReq = std::min(fileReadBuf.size(), maxSize);
- size_t read = body.file().read(fileReadBuf.data(), readReq, ec);
- if (ec)
+ BMCWEB_LOG_INFO("Reading {}", readReq);
+ boost::system::error_code readEc;
+ size_t read = body.file().read(fileReadBuf.data(), readReq, readEc);
+ if (readEc)
{
- BMCWEB_LOG_CRITICAL("Failed to read from file");
- return boost::none;
+ if (readEc != boost::system::errc::operation_would_block &&
+ readEc != boost::system::errc::resource_unavailable_try_again)
+ {
+ BMCWEB_LOG_CRITICAL("Failed to read from file {}",
+ readEc.message());
+ ec = readEc;
+ return boost::none;
+ }
}
std::string_view chunkView(fileReadBuf.data(), read);
diff --git a/http/http_connection.hpp b/http/http_connection.hpp
index 4d9c980..fdbecd1 100644
--- a/http/http_connection.hpp
+++ b/http/http_connection.hpp
@@ -614,11 +614,17 @@
const boost::system::error_code& ec,
std::size_t bytesTransferred)
{
- BMCWEB_LOG_DEBUG("{} async_write wrote {} bytes, ec=", logPtr(this),
+ BMCWEB_LOG_DEBUG("{} async_write wrote {} bytes, ec={}", logPtr(this),
bytesTransferred, ec);
cancelDeadlineTimer();
+ if (ec == boost::system::errc::operation_would_block ||
+ ec == boost::system::errc::resource_unavailable_try_again)
+ {
+ doWrite();
+ return;
+ }
if (ec)
{
BMCWEB_LOG_DEBUG("{} from write(2)", logPtr(this));
diff --git a/http/http_response.hpp b/http/http_response.hpp
index 18266ec..a4c8375 100644
--- a/http/http_response.hpp
+++ b/http/http_response.hpp
@@ -3,6 +3,8 @@
#include "logging.hpp"
#include "utils/hex_utils.hpp"
+#include <fcntl.h>
+
#include <boost/beast/http/message.hpp>
#include <nlohmann/json.hpp>
@@ -10,6 +12,7 @@
#include <string>
#include <string_view>
#include <utility>
+
namespace crow
{
@@ -170,17 +173,21 @@
// 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;
bool is1XXReturn = to_status_class(result()) ==
status_class::informational;
- if (*pSize > 0 && (is1XXReturn || result() == status::no_content ||
- result() == status::not_modified))
+ if (!pSize)
+ {
+ response.chunked(true);
+ return;
+ }
+ response.content_length(*pSize);
+
+ if (is1XXReturn || result() == status::no_content ||
+ result() == status::not_modified)
{
BMCWEB_LOG_CRITICAL("{} Response content provided but code was "
"no-content or not_modified, which aren't "
@@ -189,7 +196,6 @@
response.content_length(0);
return;
}
- response.content_length(*pSize);
}
void clear()
@@ -299,6 +305,12 @@
bool openFd(int fd, bmcweb::EncodingType enc = bmcweb::EncodingType::Raw)
{
boost::beast::error_code ec;
+ // NOLINTNEXTLINE(cppcoreguidelines-pro-type-vararg)
+ int retval = fcntl(fd, F_SETFL, fcntl(fd, F_GETFL) | O_NONBLOCK);
+ if (retval == -1)
+ {
+ BMCWEB_LOG_ERROR("Setting O_NONBLOCK failed");
+ }
response.body().encodingType = enc;
response.body().setFd(fd, ec);
if (ec)