From f96e92ae0f099f4ef92547323e9d98d3fe246d1f Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Thu, 9 Oct 2025 12:30:07 -0400 Subject: [PATCH] rgw/beast: disable keepalive for responses before 100 Continue resolves a long-standing category of delays/errors from boto3 for requests like s3:PutObject that contain a request body and use the `Expect: 100-continue` request header. if the server rejects such requests based on their headers alone, it will respond with an http status like `HTTP/1.1 403 Forbidden` without first sending the `HTTP/1.1 100 Continue` status when http keepalive is enabled, this situation creates an ambiguity for the server when trying to read the next request from that connection - subsequent bytes may either correspond to the current request's body or the next request's headers to resolve this ambiguity, disable http keepalive by sending response header `Connection: close` instead of `Connection: keep-alive`. this instructs the client and server to close this connection instead of reusing it for subsequent requests Fixes: https://tracker.ceph.com/issues/42208 Signed-off-by: Casey Bodley --- src/rgw/rgw_asio_client.cc | 17 +++++++++++++++-- src/rgw/rgw_asio_client.h | 5 ++++- src/rgw/rgw_asio_frontend.cc | 12 +++--------- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/rgw/rgw_asio_client.cc b/src/rgw/rgw_asio_client.cc index 354353cde65..a8a9ad420d8 100644 --- a/src/rgw/rgw_asio_client.cc +++ b/src/rgw/rgw_asio_client.cc @@ -18,7 +18,9 @@ ClientIO::ClientIO(parser_type& parser, bool is_ssl, : parser(parser), is_ssl(is_ssl), local_endpoint(local_endpoint), remote_endpoint(remote_endpoint), - txbuf(*this) + txbuf(*this), + keepalive(parser.keep_alive()), + expect100continue(parser.get()[beast::http::field::expect] == "100-continue") { } @@ -108,6 +110,17 @@ void ClientIO::flush() size_t ClientIO::send_status(int status, const char* status_name) { + if (expect100continue && !sent100continue) { + // a client expecting 100-continue is not required to wait for the + // '100 Continue' response before sending the request body. if we + // complete the request before sending 100 Continue (for example, due + // to an authorization error), we don't know whether any following + // bytes on this connection correspond to this request's body or the + // next request's header. so we must disable keepalive and require the + // client to use a new tcp connection for any subsequent requests + keepalive = false; + } + static constexpr size_t STATUS_BUF_SIZE = 128; char statusbuf[STATUS_BUF_SIZE]; @@ -149,7 +162,7 @@ size_t ClientIO::complete_header() sent += txbuf.sputn(timestr, strlen(timestr)); } - if (parser.keep_alive()) { + if (keep_alive()) { constexpr char CONN_KEEP_ALIVE[] = "Connection: Keep-Alive\r\n"; sent += txbuf.sputn(CONN_KEEP_ALIVE, sizeof(CONN_KEEP_ALIVE) - 1); } else { diff --git a/src/rgw/rgw_asio_client.h b/src/rgw/rgw_asio_client.h index 8bfcacbd4ae..aba1c288833 100644 --- a/src/rgw/rgw_asio_client.h +++ b/src/rgw/rgw_asio_client.h @@ -29,6 +29,9 @@ class ClientIO : public io::RestfulClient, RGWEnv env; rgw::io::StaticOutputBufferer<> txbuf; + + bool keepalive = false; + bool expect100continue = false; bool sent100continue = false; public: @@ -55,7 +58,7 @@ class ClientIO : public io::RestfulClient, return env; } - bool sent_100_continue() const { return sent100continue; } + bool keep_alive() const { return keepalive; } }; } // namespace asio diff --git a/src/rgw/rgw_asio_frontend.cc b/src/rgw/rgw_asio_frontend.cc index 7d8aa19adc3..580f82ceb9e 100644 --- a/src/rgw/rgw_asio_frontend.cc +++ b/src/rgw/rgw_asio_frontend.cc @@ -300,8 +300,6 @@ void handle_connection(boost::asio::io_context& context, return; } - bool expect_continue = (message[http::field::expect] == "100-continue"); - { auto lock = pause_mutex.async_lock_shared(yield[ec]); if (ec == boost::asio::error::operation_aborted) { @@ -376,18 +374,14 @@ void handle_connection(boost::asio::io_context& context, return; } - if (real_client.sent_100_continue()) { - expect_continue = false; + if (!real_client.keep_alive()) { + return; } } - if (!parser.keep_alive()) { - return; - } - // if we failed before reading the entire message, discard any remaining // bytes before reading the next - while (!expect_continue && !parser.is_done()) { + while (!parser.is_done()) { static std::array discard_buffer; auto& body = parser.get().body(); -- 2.39.5