Resolves: #2231827 - Crash with half_closed_client on

This commit is contained in:
Luboš Uhliarik 2023-08-16 15:10:34 +02:00
parent eeccc5fca0
commit 4a1fd2e109
2 changed files with 165 additions and 1 deletions

158
squid-5.5-halfclosed.patch Normal file
View File

@ -0,0 +1,158 @@
diff --git a/src/client_side.cc b/src/client_side.cc
index 4eb6976..63f1b66 100644
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -957,7 +957,7 @@ ConnStateData::kick()
* We are done with the response, and we are either still receiving request
* body (early response!) or have already stopped receiving anything.
*
- * If we are still receiving, then clientParseRequest() below will fail.
+ * If we are still receiving, then parseRequests() below will fail.
* (XXX: but then we will call readNextRequest() which may succeed and
* execute a smuggled request as we are not done with the current request).
*
@@ -977,28 +977,12 @@ ConnStateData::kick()
* Attempt to parse a request from the request buffer.
* If we've been fed a pipelined request it may already
* be in our read buffer.
- *
- \par
- * This needs to fall through - if we're unlucky and parse the _last_ request
- * from our read buffer we may never re-register for another client read.
*/
- if (clientParseRequests()) {
- debugs(33, 3, clientConnection << ": parsed next request from buffer");
- }
+ parseRequests();
- /** \par
- * Either we need to kick-start another read or, if we have
- * a half-closed connection, kill it after the last request.
- * This saves waiting for half-closed connections to finished being
- * half-closed _AND_ then, sometimes, spending "Timeout" time in
- * the keepalive "Waiting for next request" state.
- */
- if (commIsHalfClosed(clientConnection->fd) && pipeline.empty()) {
- debugs(33, 3, "half-closed client with no pending requests, closing");
- clientConnection->close();
+ if (!isOpen())
return;
- }
/** \par
* At this point we either have a parsed request (which we've
@@ -1935,16 +1919,11 @@ ConnStateData::receivedFirstByte()
resetReadTimeout(Config.Timeout.request);
}
-/**
- * Attempt to parse one or more requests from the input buffer.
- * Returns true after completing parsing of at least one request [header]. That
- * includes cases where parsing ended with an error (e.g., a huge request).
- */
-bool
-ConnStateData::clientParseRequests()
+/// Attempt to parse one or more requests from the input buffer.
+/// May close the connection.
+void
+ConnStateData::parseRequests()
{
- bool parsed_req = false;
-
debugs(33, 5, HERE << clientConnection << ": attempting to parse");
// Loop while we have read bytes that are not needed for producing the body
@@ -1989,8 +1968,6 @@ ConnStateData::clientParseRequests()
processParsedRequest(context);
- parsed_req = true; // XXX: do we really need to parse everything right NOW ?
-
if (context->mayUseConnection()) {
debugs(33, 3, HERE << "Not parsing new requests, as this request may need the connection");
break;
@@ -2003,8 +1980,19 @@ ConnStateData::clientParseRequests()
}
}
- /* XXX where to 'finish' the parsing pass? */
- return parsed_req;
+ debugs(33, 7, "buffered leftovers: " << inBuf.length());
+
+ if (isOpen() && commIsHalfClosed(clientConnection->fd)) {
+ if (pipeline.empty()) {
+ // we processed what we could parse, and no more data is coming
+ debugs(33, 5, "closing half-closed without parsed requests: " << clientConnection);
+ clientConnection->close();
+ } else {
+ // we parsed what we could, and no more data is coming
+ debugs(33, 5, "monitoring half-closed while processing parsed requests: " << clientConnection);
+ flags.readMore = false; // may already be false
+ }
+ }
}
void
@@ -2021,18 +2009,7 @@ ConnStateData::afterClientRead()
if (pipeline.empty())
fd_note(clientConnection->fd, "Reading next request");
- if (!clientParseRequests()) {
- if (!isOpen())
- return;
- // We may get here if the client half-closed after sending a partial
- // request. See doClientRead() and shouldCloseOnEof().
- // XXX: This partially duplicates ConnStateData::kick().
- if (pipeline.empty() && commIsHalfClosed(clientConnection->fd)) {
- debugs(33, 5, clientConnection << ": half-closed connection, no completed request parsed, connection closing.");
- clientConnection->close();
- return;
- }
- }
+ parseRequests();
if (!isOpen())
return;
@@ -3789,7 +3766,7 @@ ConnStateData::notePinnedConnectionBecameIdle(PinnedIdleContext pic)
startPinnedConnectionMonitoring();
if (pipeline.empty())
- kick(); // in case clientParseRequests() was blocked by a busy pic.connection
+ kick(); // in case parseRequests() was blocked by a busy pic.connection
}
/// Forward future client requests using the given server connection.
diff --git a/src/client_side.h b/src/client_side.h
index 2793673..7c8d86b 100644
--- a/src/client_side.h
+++ b/src/client_side.h
@@ -93,7 +93,6 @@ public:
virtual void doneWithControlMsg();
/// Traffic parsing
- bool clientParseRequests();
void readNextRequest();
/// try to make progress on a transaction or read more I/O
@@ -422,6 +421,7 @@ private:
void checkLogging();
+ void parseRequests();
void clientAfterReadingRequests();
bool concurrentRequestQueueFilled() const;
diff --git a/src/tests/stub_client_side.cc b/src/tests/stub_client_side.cc
index acf61c4..b1d82bf 100644
--- a/src/tests/stub_client_side.cc
+++ b/src/tests/stub_client_side.cc
@@ -14,7 +14,7 @@
#include "tests/STUB.h"
#include "client_side.h"
-bool ConnStateData::clientParseRequests() STUB_RETVAL(false)
+void ConnStateData::parseRequests() STUB
void ConnStateData::readNextRequest() STUB
bool ConnStateData::isOpen() const STUB_RETVAL(false)
void ConnStateData::kick() STUB

View File

@ -2,7 +2,7 @@
Name: squid
Version: 5.5
Release: 5%{?dist}
Release: 6%{?dist}
Summary: The Squid proxy caching server
Epoch: 7
# See CREDITS for breakdown of non GPLv2+ code
@ -44,6 +44,8 @@ Patch206: squid-5.0.6-openssl3.patch
Patch207: squid-5.0.6-active-ftp.patch
# https://bugzilla.redhat.com/show_bug.cgi?id=1988122
Patch208: squid-5.1-test-store-cppsuite.patch
# https://bugzilla.redhat.com/show_bug.cgi?id=2231827
Patch209: squid-5.5-halfclosed.patch
# Security patches
# https://bugzilla.redhat.com/show_bug.cgi?id=2100721
@ -123,6 +125,7 @@ lookup program (dnsserver), a program for retrieving FTP data
%patch206 -p1 -b .openssl3
%patch207 -p1 -b .active-ftp
%patch208 -p1 -b .test-store-cpp
%patch209 -p1 -b .halfclosed
%patch501 -p1 -b .CVE-2021-46784
%patch502 -p1 -b .CVE-2022-41318
@ -352,6 +355,9 @@ fi
%changelog
* Wed Aug 16 2023 Luboš Uhliarik <luhliari@redhat.com> - 7:5.5-6
- Resolves: #2231827 - Crash with half_closed_client on
* Tue Dec 06 2022 Tomas Korbar <tkorbar@redhat.com> - 7:5.5-5
- Resolves: #2151188 - [RFE] Add the "IP_BIND_ADDRESS_NO_PORT"
flag to sockets created for outgoing connections in the squid source code.