From f0f28d07e19e93de03633c26b6e2738e04c89b9d Mon Sep 17 00:00:00 2001 From: Nadav Har'El Date: Sun, 23 May 2021 21:40:38 +0300 Subject: [PATCH] httpd: allow handler to not read an empty content Starting in commit 4fa0e4b4394b6293555af197b1835418df80398a, an HTTP handler can read a request's body using an input stream instead of as a string. If the handler does not read the entire request body, the client can not send any further requests on this socket, so the server needs to close it. Conversely, if the handler *did* read the content, the server may keep the socket open. To check whether the handler read the content we checked the content stream's eof() state. However, this may only become true after a read() noticed the content has reached its end (specified by content-length or chunked encoding). But one interesting case we missed is a simple handler which assumes an empty request body, and doesn't bother to read it at all. In this case, even though the request is empty, and the handler didn't miss any content by not reading it, we close the connection. The solution in this patch is to replace the check for eof() by an attempt to read() from the content stream. In the typical case the handler either read the entire content or there was nothing to read in the first place, so this read() will return nothing immediately - and we allow reusing the connection. If anything is returned by the read(), it is ignored, and the connection is closed (we may still not read the entire content). Fixes #907 Refs scylladb/scylla#8691 Signed-off-by: Nadav Har'El Message-Id: <20210523184038.425954-1-nyh@scylladb.com> --- src/http/httpd.cc | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/http/httpd.cc b/src/http/httpd.cc index f6255f706..774bb84fa 100644 --- a/src/http/httpd.cc +++ b/src/http/httpd.cc @@ -279,12 +279,16 @@ future<> connection::read_one() { return generate_reply(std::move(req)); }).then([this, &content_stream](bool done) { _done = done; - - // If the handler did not read the entire request content, this connection cannot be - // reused so we need to close it. "_done = true" does that. - if (!content_stream.eof()) { - _done = true; - } + // If the handler did not read the entire request + // content, this connection cannot be reused so we + // need to close it (via "_done = true"). But we can't + // just check content_stream.eof(): It may only become + // true after read(). Issue #907. + return content_stream.read().then([this] (temporary_buffer buf) { + if (!buf.empty()) { + _done = true; + } + }); }); }).handle_exception_type([this, &version] (const base_exception& e) mutable { // If the request had a "Transfer-Encoding: chunked" header and content streaming wasn't enabled, we might have failed