From 0d3e2eb98527b21f9603950003bdc4aae1422f59 Mon Sep 17 00:00:00 2001 From: Eduard Bagdasaryan Date: Mon, 2 Sep 2024 17:57:11 +0300 Subject: [PATCH 1/9] Do not log an aborted transaction with bodyPipe twice This extra logging happened because the current inBuf cleanup in ConnStateData::terminateAll() does not account for the case when the pipeline is empty (the stream object already deleted in ConnStateData::afterClientWrite()). --- src/client_side.cc | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/client_side.cc b/src/client_side.cc index fe45fc97497..af76fe9d72b 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -3914,32 +3914,32 @@ ConnStateData::terminateAll(const Error &rawError, const LogTagsErrors <e) debugs(33, 3, pipeline.count() << '/' << pipeline.nrequests << " after " << error); - if (pipeline.empty()) { - bareError.update(error); // XXX: bareLogTagsErrors - } else { - // We terminate the current CONNECT/PUT/etc. context below, logging any - // error details, but that context may leave unparsed bytes behind. - // Consume them to stop checkLogging() from logging them again later. - const auto intputToConsume = + // We terminate the current CONNECT/PUT/etc. context below, logging any + // error details, but that context may leave unparsed bytes behind. + // Consume them to stop checkLogging() from logging them again later. + const auto intputToConsume = #if USE_OPENSSL - parsingTlsHandshake ? "TLS handshake" : // more specific than CONNECT + parsingTlsHandshake ? "TLS handshake" : // more specific than CONNECT #endif - bodyPipe ? "HTTP request body" : - pipeline.back()->mayUseConnection() ? "HTTP CONNECT" : - nullptr; + bodyPipe ? "HTTP request body" : + (!pipeline.empty() && pipeline.back()->mayUseConnection()) ? "HTTP CONNECT" : + nullptr; + if (!pipeline.empty()) { while (const auto context = pipeline.front()) { context->noteIoError(error, lte); context->finished(); // cleanup and self-deregister assert(context != pipeline.front()); } + } - if (intputToConsume && !inBuf.isEmpty()) { - debugs(83, 5, "forgetting client " << intputToConsume << " bytes: " << inBuf.length()); - inBuf.clear(); - } + if (intputToConsume && !inBuf.isEmpty()) { + debugs(83, 5, "forgetting client " << intputToConsume << " bytes: " << inBuf.length()); + inBuf.clear(); } + bareError.update(error); // XXX: bareLogTagsErrors + clientConnection->close(); } From 2d64f739a64b1b4be61f4e637ac95d6dce716fec Mon Sep 17 00:00:00 2001 From: Eduard Bagdasaryan Date: Tue, 3 Sep 2024 23:07:07 +0300 Subject: [PATCH 2/9] Restored a condition in ConnStateData::terminateAll() on which bareError is updated. --- src/client_side.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/client_side.cc b/src/client_side.cc index af76fe9d72b..e29fe0cddea 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -3914,6 +3914,9 @@ ConnStateData::terminateAll(const Error &rawError, const LogTagsErrors <e) debugs(33, 3, pipeline.count() << '/' << pipeline.nrequests << " after " << error); + if (pipeline.empty()) + bareError.update(error); // XXX: bareLogTagsErrors + // We terminate the current CONNECT/PUT/etc. context below, logging any // error details, but that context may leave unparsed bytes behind. // Consume them to stop checkLogging() from logging them again later. @@ -3938,8 +3941,6 @@ ConnStateData::terminateAll(const Error &rawError, const LogTagsErrors <e) inBuf.clear(); } - bareError.update(error); // XXX: bareLogTagsErrors - clientConnection->close(); } From 77b70a1ad0e6c772d4054bbbb6efcbd8f30d8b25 Mon Sep 17 00:00:00 2001 From: Eduard Bagdasaryan Date: Mon, 16 Sep 2024 15:22:07 +0300 Subject: [PATCH 3/9] Covered more cases These cases include various invalid requests, detected by Server::processParsedRequest(), such as requests with invalid 'Expect' header, Content-Length exceeding request_body_max_size and others. --- src/client_side.cc | 39 +++++++++++++++++++++++++-------------- src/client_side.h | 6 ++++++ src/http/Stream.cc | 2 +- 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/src/client_side.cc b/src/client_side.cc index e29fe0cddea..94e41288572 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -1635,10 +1635,8 @@ clientProcessRequest(ConnStateData *conn, const Http1::RequestParserPointer &hp, clientSetKeepaliveFlag(http); // Let tunneling code be fully responsible for CONNECT requests - if (http->request->method == Http::METHOD_CONNECT) { - context->mayUseConnection(true); + if (http->request->method == Http::METHOD_CONNECT) conn->flags.readMore = false; - } #if USE_OPENSSL if (conn->switchedToHttps() && conn->serveDelayedError(context)) { @@ -1650,7 +1648,7 @@ clientProcessRequest(ConnStateData *conn, const Http1::RequestParserPointer &hp, /* Do we expect a request-body? */ const auto chunked = request->header.chunked(); expectBody = chunked || request->content_length > 0; - if (!context->mayUseConnection() && expectBody) { + if (expectBody && (http->request->method != Http::METHOD_CONNECT)) { request->body_pipe = conn->expectRequestBody( chunked ? -1 : request->content_length); @@ -1680,7 +1678,6 @@ clientProcessRequest(ConnStateData *conn, const Http1::RequestParserPointer &hp, if (!request->body_pipe->productionEnded()) { debugs(33, 5, "need more request body"); - context->mayUseConnection(true); assert(conn->flags.readMore); } } @@ -1705,6 +1702,21 @@ ConnStateData::add(const Http::StreamPointer &context) bareError.clear(); } pipeline.add(context); + if (pipeline.count() == 1) { + context->mayUseConnection(true); + connLeftovers_ = false; + } +} + +void +ConnStateData::remove(const Http::StreamPointer &context) +{ + debugs(33, 3, context << " from " << pipeline.count() << '/' << pipeline.nrequests); + pipeline.popMe(context); + if (!pipeline.empty()) + pipeline.front()->mayUseConnection(true); + else if (!inBuf.isEmpty()) + connLeftovers_ = true; } int @@ -1962,6 +1974,7 @@ ConnStateData::handleRequestBodyData() consumeInput(putSize); if (!bodyPipe->mayNeedMoreData()) { + pipeline.front()->mayUseConnection(false); // BodyPipe will clear us automagically when we produced everything bodyPipe = nullptr; } @@ -3173,7 +3186,6 @@ ConnStateData::buildFakeRequest(SBuf &useHost, const AnyP::KnownPort usePort, co clientSocketDetach, newClient, tempBuffer); stream->flags.parsed_ok = 1; // Do we need it? - stream->mayUseConnection(true); extendLifetime(); stream->registerWithConn(); @@ -3594,9 +3606,11 @@ ConnStateData::finishDechunkingRequest(bool withSuccess) Must(!bodyPipe); // we rely on it being nil after we are done with body if (withSuccess) { Must(myPipe->bodySizeKnown()); - Http::StreamPointer context = pipeline.front(); - if (context != nullptr && context->http && context->http->request) - context->http->request->setContentLength(myPipe->bodySize()); + if (auto context = pipeline.front()) { + context->mayUseConnection(false); + if (context->http && context->http->request) + context->http->request->setContentLength(myPipe->bodySize()); + } } } @@ -3921,11 +3935,8 @@ ConnStateData::terminateAll(const Error &rawError, const LogTagsErrors <e) // error details, but that context may leave unparsed bytes behind. // Consume them to stop checkLogging() from logging them again later. const auto intputToConsume = -#if USE_OPENSSL - parsingTlsHandshake ? "TLS handshake" : // more specific than CONNECT -#endif - bodyPipe ? "HTTP request body" : - (!pipeline.empty() && pipeline.back()->mayUseConnection()) ? "HTTP CONNECT" : + connLeftovers_ ? "extra connection bytes" : + (!pipeline.empty() && pipeline.back()->mayUseConnection()) ? "still using connection" : nullptr; if (!pipeline.empty()) { diff --git a/src/client_side.h b/src/client_side.h index 4a67203f75a..e0676631d77 100644 --- a/src/client_side.h +++ b/src/client_side.h @@ -314,6 +314,9 @@ class ConnStateData: /// registers a newly created stream void add(const Http::StreamPointer &context); + /// unregisters the front stream from the pipeline + void remove(const Http::StreamPointer &context); + /// handle a control message received by context from a peer and call back virtual bool writeControlMsgAndCall(HttpReply *rep, AsyncCall::Pointer &call) = 0; @@ -433,6 +436,9 @@ class ConnStateData: /// whether preservedClientData is valid and should be kept up to date bool preservingClientData_ = false; + /// whether there are some input data but no associated request + bool connLeftovers_ = false; + bool tunnelOnError(const err_type); private: diff --git a/src/http/Stream.cc b/src/http/Stream.cc index 1538d69fe2c..d35139fb7cf 100644 --- a/src/http/Stream.cc +++ b/src/http/Stream.cc @@ -540,7 +540,7 @@ Http::Stream::finished() assert(connRegistered_); connRegistered_ = false; - conn->pipeline.popMe(Http::StreamPointer(this)); + conn->remove(Http::StreamPointer(this)); } /// called when we encounter a response-related error From b262c0e5abb70396c7ef35199c67d54f5f6b723b Mon Sep 17 00:00:00 2001 From: Eduard Bagdasaryan Date: Mon, 16 Sep 2024 16:53:22 +0300 Subject: [PATCH 4/9] A couple of fixes * Reset mayUseConnection flag for body-less requests * Use connLeftovers_ only once (i.e., do not reuse it in successive terminateAll() calls) --- src/client_side.cc | 60 ++++++++++++++++++++++++++-------------------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/src/client_side.cc b/src/client_side.cc index 94e41288572..0986a388588 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -1648,37 +1648,41 @@ clientProcessRequest(ConnStateData *conn, const Http1::RequestParserPointer &hp, /* Do we expect a request-body? */ const auto chunked = request->header.chunked(); expectBody = chunked || request->content_length > 0; - if (expectBody && (http->request->method != Http::METHOD_CONNECT)) { - request->body_pipe = conn->expectRequestBody( - chunked ? -1 : request->content_length); - - /* Is it too large? */ - if (!chunked && // if chunked, we will check as we accumulate - clientIsRequestBodyTooLargeForPolicy(request->content_length)) { - clientStreamNode *node = context->getClientReplyContext(); - clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); - assert (repContext); - conn->quitAfterError(request.getRaw()); - repContext->setReplyToError(ERR_TOO_BIG, - Http::scContentTooLarge, nullptr, - conn, http->request, nullptr, nullptr); - assert(context->http->out.offset == 0); - context->pullData(); - clientProcessRequestFinished(conn, request); - return; - } + if (http->request->method != Http::METHOD_CONNECT) { + if (!expectBody) { + conn->pipeline.front()->mayUseConnection(false); + } else { + request->body_pipe = conn->expectRequestBody( + chunked ? -1 : request->content_length); - if (!isFtp) { - // We may stop producing, comm_close, and/or call setReplyToError() - // below, so quit on errors to avoid http->doCallouts() - if (!conn->handleRequestBodyData()) { + /* Is it too large? */ + if (!chunked && // if chunked, we will check as we accumulate + clientIsRequestBodyTooLargeForPolicy(request->content_length)) { + clientStreamNode *node = context->getClientReplyContext(); + clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); + assert (repContext); + conn->quitAfterError(request.getRaw()); + repContext->setReplyToError(ERR_TOO_BIG, + Http::scContentTooLarge, nullptr, + conn, http->request, nullptr, nullptr); + assert(context->http->out.offset == 0); + context->pullData(); clientProcessRequestFinished(conn, request); return; } - if (!request->body_pipe->productionEnded()) { - debugs(33, 5, "need more request body"); - assert(conn->flags.readMore); + if (!isFtp) { + // We may stop producing, comm_close, and/or call setReplyToError() + // below, so quit on errors to avoid http->doCallouts() + if (!conn->handleRequestBodyData()) { + clientProcessRequestFinished(conn, request); + return; + } + + if (!request->body_pipe->productionEnded()) { + debugs(33, 5, "need more request body"); + assert(conn->flags.readMore); + } } } } @@ -3947,6 +3951,10 @@ ConnStateData::terminateAll(const Error &rawError, const LogTagsErrors <e) } } + // use only once to calculate intputToConsume + // may be set during the pipeline cleanup above + connLeftovers_ = false; + if (intputToConsume && !inBuf.isEmpty()) { debugs(83, 5, "forgetting client " << intputToConsume << " bytes: " << inBuf.length()); inBuf.clear(); From b89e8b5f0e852c8f1e746c14e9f4aaab1a2e2024 Mon Sep 17 00:00:00 2001 From: Eduard Bagdasaryan Date: Mon, 16 Sep 2024 17:46:39 +0300 Subject: [PATCH 5/9] Reduced diff --- src/client_side.cc | 58 ++++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/src/client_side.cc b/src/client_side.cc index 0986a388588..25cc9a25287 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -1648,41 +1648,39 @@ clientProcessRequest(ConnStateData *conn, const Http1::RequestParserPointer &hp, /* Do we expect a request-body? */ const auto chunked = request->header.chunked(); expectBody = chunked || request->content_length > 0; - if (http->request->method != Http::METHOD_CONNECT) { - if (!expectBody) { - conn->pipeline.front()->mayUseConnection(false); - } else { - request->body_pipe = conn->expectRequestBody( - chunked ? -1 : request->content_length); + if (http->request->method != Http::METHOD_CONNECT && !expectBody) // TODO: diff reducer: merge conditions + conn->pipeline.front()->mayUseConnection(false); + if (http->request->method != Http::METHOD_CONNECT && expectBody) { + request->body_pipe = conn->expectRequestBody( + chunked ? -1 : request->content_length); + + /* Is it too large? */ + if (!chunked && // if chunked, we will check as we accumulate + clientIsRequestBodyTooLargeForPolicy(request->content_length)) { + clientStreamNode *node = context->getClientReplyContext(); + clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); + assert (repContext); + conn->quitAfterError(request.getRaw()); + repContext->setReplyToError(ERR_TOO_BIG, + Http::scContentTooLarge, nullptr, + conn, http->request, nullptr, nullptr); + assert(context->http->out.offset == 0); + context->pullData(); + clientProcessRequestFinished(conn, request); + return; + } - /* Is it too large? */ - if (!chunked && // if chunked, we will check as we accumulate - clientIsRequestBodyTooLargeForPolicy(request->content_length)) { - clientStreamNode *node = context->getClientReplyContext(); - clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); - assert (repContext); - conn->quitAfterError(request.getRaw()); - repContext->setReplyToError(ERR_TOO_BIG, - Http::scContentTooLarge, nullptr, - conn, http->request, nullptr, nullptr); - assert(context->http->out.offset == 0); - context->pullData(); + if (!isFtp) { + // We may stop producing, comm_close, and/or call setReplyToError() + // below, so quit on errors to avoid http->doCallouts() + if (!conn->handleRequestBodyData()) { clientProcessRequestFinished(conn, request); return; } - if (!isFtp) { - // We may stop producing, comm_close, and/or call setReplyToError() - // below, so quit on errors to avoid http->doCallouts() - if (!conn->handleRequestBodyData()) { - clientProcessRequestFinished(conn, request); - return; - } - - if (!request->body_pipe->productionEnded()) { - debugs(33, 5, "need more request body"); - assert(conn->flags.readMore); - } + if (!request->body_pipe->productionEnded()) { + debugs(33, 5, "need more request body"); + assert(conn->flags.readMore); } } } From dc372e7953c3241d4a283ca19de444889147fae5 Mon Sep 17 00:00:00 2001 From: Eduard Bagdasaryan Date: Thu, 19 Sep 2024 16:54:54 +0300 Subject: [PATCH 6/9] Fixed ConnStateData::add() and ConnStateData::remove() and require that mayUseConnection flag may be true only in a single pipeline Stream object. --- src/Pipeline.cc | 12 ++++++++++++ src/Pipeline.h | 3 +++ src/client_side.cc | 9 ++------- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/Pipeline.cc b/src/Pipeline.cc index 7d1429cec1c..418fd21380e 100644 --- a/src/Pipeline.cc +++ b/src/Pipeline.cc @@ -19,6 +19,8 @@ void Pipeline::add(const Http::StreamPointer &c) { + Assure(!mayUseConnection()); + c->mayUseConnection(true); requests.push_back(c); ++nrequests; debugs(33, 3, "Pipeline " << (void*)this << " add request " << nrequests << ' ' << c); @@ -48,6 +50,16 @@ Pipeline::back() const return requests.back(); } +bool +Pipeline::mayUseConnection() const +{ + for (const auto &t: requests) { + if (t->mayUseConnection()) + return true; + } + return false; +} + void Pipeline::popMe(const Http::StreamPointer &which) { diff --git a/src/Pipeline.h b/src/Pipeline.h index 54a9e157079..e2642d2ab4c 100644 --- a/src/Pipeline.h +++ b/src/Pipeline.h @@ -63,6 +63,9 @@ class Pipeline uint32_t nrequests; private: + /// whether some of the queued transactions use the connection + bool mayUseConnection() const; + /// requests parsed from the connection but not yet completed. std::list requests; }; diff --git a/src/client_side.cc b/src/client_side.cc index 25cc9a25287..d1ece817dae 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -1704,10 +1704,7 @@ ConnStateData::add(const Http::StreamPointer &context) bareError.clear(); } pipeline.add(context); - if (pipeline.count() == 1) { - context->mayUseConnection(true); - connLeftovers_ = false; - } + connLeftovers_ = false; } void @@ -1715,9 +1712,7 @@ ConnStateData::remove(const Http::StreamPointer &context) { debugs(33, 3, context << " from " << pipeline.count() << '/' << pipeline.nrequests); pipeline.popMe(context); - if (!pipeline.empty()) - pipeline.front()->mayUseConnection(true); - else if (!inBuf.isEmpty()) + if (pipeline.empty() && !inBuf.isEmpty()) connLeftovers_ = true; } From 048f2e24db8fed1ea34284c0c723482cb257e18e Mon Sep 17 00:00:00 2001 From: Eduard Bagdasaryan Date: Sun, 29 Sep 2024 02:20:16 +0300 Subject: [PATCH 7/9] Simplified, removing ConnStateData::connLeftovers_. --- src/Pipeline.cc | 11 ++++++----- src/Pipeline.h | 9 ++++++--- src/client_side.cc | 19 ++----------------- src/client_side.h | 3 --- src/http/Stream.cc | 2 +- 5 files changed, 15 insertions(+), 29 deletions(-) diff --git a/src/Pipeline.cc b/src/Pipeline.cc index 418fd21380e..03add1e93ca 100644 --- a/src/Pipeline.cc +++ b/src/Pipeline.cc @@ -20,6 +20,7 @@ void Pipeline::add(const Http::StreamPointer &c) { Assure(!mayUseConnection()); + Assure(!poppedBusy); c->mayUseConnection(true); requests.push_back(c); ++nrequests; @@ -53,11 +54,7 @@ Pipeline::back() const bool Pipeline::mayUseConnection() const { - for (const auto &t: requests) { - if (t->mayUseConnection()) - return true; - } - return false; + return requests.empty() ? false : requests.front()->mayUseConnection(); } void @@ -70,6 +67,10 @@ Pipeline::popMe(const Http::StreamPointer &which) // in reality there may be multiple contexts doing processing in parallel. // XXX: pipeline still assumes HTTP/1 FIFO semantics are obeyed. assert(which == requests.front()); + if (which->mayUseConnection()) { + Assure(requests.size() == 1); + poppedBusy = true; + } requests.pop_front(); } diff --git a/src/Pipeline.h b/src/Pipeline.h index e2642d2ab4c..983f02d47e6 100644 --- a/src/Pipeline.h +++ b/src/Pipeline.h @@ -58,14 +58,17 @@ class Pipeline /// deregister the front request from the pipeline void popMe(const Http::StreamPointer &); + /// whether some of the queued transactions use the connection + bool mayUseConnection() const; + /// Number of requests seen in this pipeline (so far). /// Includes incomplete transactions. uint32_t nrequests; -private: - /// whether some of the queued transactions use the connection - bool mayUseConnection() const; + /// whether popMe() was called on a transaction with mayUseConnection() + bool poppedBusy = false; +private: /// requests parsed from the connection but not yet completed. std::list requests; }; diff --git a/src/client_side.cc b/src/client_side.cc index d1ece817dae..a62b6624bfb 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -1703,17 +1703,6 @@ ConnStateData::add(const Http::StreamPointer &context) context->http->updateError(bareError); bareError.clear(); } - pipeline.add(context); - connLeftovers_ = false; -} - -void -ConnStateData::remove(const Http::StreamPointer &context) -{ - debugs(33, 3, context << " from " << pipeline.count() << '/' << pipeline.nrequests); - pipeline.popMe(context); - if (pipeline.empty() && !inBuf.isEmpty()) - connLeftovers_ = true; } int @@ -3932,8 +3921,8 @@ ConnStateData::terminateAll(const Error &rawError, const LogTagsErrors <e) // error details, but that context may leave unparsed bytes behind. // Consume them to stop checkLogging() from logging them again later. const auto intputToConsume = - connLeftovers_ ? "extra connection bytes" : - (!pipeline.empty() && pipeline.back()->mayUseConnection()) ? "still using connection" : + (pipeline.poppedBusy && !inBuf.isEmpty()) ? "extra connection bytes" : + (!pipeline.empty() && pipeline.mayUseConnection()) ? "still using connection" : nullptr; if (!pipeline.empty()) { @@ -3944,10 +3933,6 @@ ConnStateData::terminateAll(const Error &rawError, const LogTagsErrors <e) } } - // use only once to calculate intputToConsume - // may be set during the pipeline cleanup above - connLeftovers_ = false; - if (intputToConsume && !inBuf.isEmpty()) { debugs(83, 5, "forgetting client " << intputToConsume << " bytes: " << inBuf.length()); inBuf.clear(); diff --git a/src/client_side.h b/src/client_side.h index e0676631d77..c34f098cef8 100644 --- a/src/client_side.h +++ b/src/client_side.h @@ -314,9 +314,6 @@ class ConnStateData: /// registers a newly created stream void add(const Http::StreamPointer &context); - /// unregisters the front stream from the pipeline - void remove(const Http::StreamPointer &context); - /// handle a control message received by context from a peer and call back virtual bool writeControlMsgAndCall(HttpReply *rep, AsyncCall::Pointer &call) = 0; diff --git a/src/http/Stream.cc b/src/http/Stream.cc index d35139fb7cf..1538d69fe2c 100644 --- a/src/http/Stream.cc +++ b/src/http/Stream.cc @@ -540,7 +540,7 @@ Http::Stream::finished() assert(connRegistered_); connRegistered_ = false; - conn->remove(Http::StreamPointer(this)); + conn->pipeline.popMe(Http::StreamPointer(this)); } /// called when we encounter a response-related error From 5e6083c2cf9fe36e77cec1c94d39809a3f117c05 Mon Sep 17 00:00:00 2001 From: Eduard Bagdasaryan Date: Mon, 30 Sep 2024 00:57:42 +0300 Subject: [PATCH 8/9] pipeline.front() may be already nil when handling request body --- src/client_side.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/client_side.cc b/src/client_side.cc index a62b6624bfb..b6f869adcea 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -1648,8 +1648,8 @@ clientProcessRequest(ConnStateData *conn, const Http1::RequestParserPointer &hp, /* Do we expect a request-body? */ const auto chunked = request->header.chunked(); expectBody = chunked || request->content_length > 0; - if (http->request->method != Http::METHOD_CONNECT && !expectBody) // TODO: diff reducer: merge conditions - conn->pipeline.front()->mayUseConnection(false); + if (http->request->method != Http::METHOD_CONNECT && !expectBody) // TODO: diff reducer + context->mayUseConnection(false); if (http->request->method != Http::METHOD_CONNECT && expectBody) { request->body_pipe = conn->expectRequestBody( chunked ? -1 : request->content_length); @@ -1703,6 +1703,7 @@ ConnStateData::add(const Http::StreamPointer &context) context->http->updateError(bareError); bareError.clear(); } + pipeline.add(context); } int @@ -1960,7 +1961,8 @@ ConnStateData::handleRequestBodyData() consumeInput(putSize); if (!bodyPipe->mayNeedMoreData()) { - pipeline.front()->mayUseConnection(false); + if (auto context = pipeline.front()) + context->mayUseConnection(false); // BodyPipe will clear us automagically when we produced everything bodyPipe = nullptr; } From ec953fd6893aea132b14e419b08a90d117507827 Mon Sep 17 00:00:00 2001 From: Eduard Bagdasaryan Date: Mon, 30 Sep 2024 01:02:14 +0300 Subject: [PATCH 9/9] Removed an unused variable --- src/client_side.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/client_side.h b/src/client_side.h index c34f098cef8..4a67203f75a 100644 --- a/src/client_side.h +++ b/src/client_side.h @@ -433,9 +433,6 @@ class ConnStateData: /// whether preservedClientData is valid and should be kept up to date bool preservingClientData_ = false; - /// whether there are some input data but no associated request - bool connLeftovers_ = false; - bool tunnelOnError(const err_type); private: