As I posted on the squid-users list yesterday, it seems that when a
client makes a forbidden request (tested with 403 Denied and 407 Auth
Required), Squid reads the entire request body into the BodyPipe with no
consumer. If the request body is too large (e.g. a large file being
POSTed), the BodyPipe and the receive buffer fill up and Squid logs:
2013/01/17 17:50:50.780 kid1| client_side.cc(2322)
maybeMakeSpaceAvailable: request buffer full:
client_request_buffer_max_size=524288
Squid stops reading from the client and since the client is still
sending, the socket's rx queue grows. Eventually the rx queue is full
and the TCP stack shrinks the TCP window to 0. The client and Squid
both sit there doing nothing with an open socket until the client drops
the connection (which may be some time).
This is reproducible by setting an ACL:
http_access deny all
Then make a large post request via the proxy:
curl --proxy http://yourproxy:3128 -v -F foo=@somefile -H "Expect:"
http://someurl
Where "somefile" is a file of a few megabytes in size.
The attached patch enables autoconsumption of the BodyPipe if access
isn't allowed in order to discard the request body rather than letting
it fill the buffers.
I'm not entirely sure this is the best fix - calling
expectNoForwarding() would seem to be neater, but this also causes the
client-side socket to close, which would break authentication mechanisms
that require the connection to be kept alive. Hopefully someone with a
better understanding of the Squid internals can review the patch and
tell me if it is the right approach?
--
- Steve Hill
Technical Director
Opendium Limited http://www.opendium.com
Direct contacts:
Instant messager: xmpp:[email protected]
Email: [email protected]
Phone: sip:[email protected]
Sales / enquiries contacts:
Email: [email protected]
Phone: +44-844-9791439 / sip:[email protected]
Support contacts:
Email: [email protected]
Phone: +44-844-4844916 / sip:[email protected]
Index: src/client_side.h
===================================================================
--- src/client_side.h (revision 167)
+++ src/client_side.h (working copy)
@@ -273,6 +273,7 @@
BodyPipe::Pointer expectRequestBody(int64_t size);
virtual void noteMoreBodySpaceAvailable(BodyPipe::Pointer);
+ virtual void forwardToBitbucket(void);
virtual void noteBodyConsumerAborted(BodyPipe::Pointer);
bool handleReadData(char *buf, size_t size);
Index: src/client_side.cc
===================================================================
--- src/client_side.cc (revision 167)
+++ src/client_side.cc (working copy)
@@ -3055,6 +3055,14 @@
}
void
+ConnStateData::forwardToBitbucket(void)
+{
+ // request reader may get stuck waiting for space if nobody consumes body
+ if (bodyPipe != NULL)
+ bodyPipe->enableAutoConsumption();
+}
+
+void
ConnStateData::noteBodyConsumerAborted(BodyPipe::Pointer )
{
// request reader may get stuck waiting for space if nobody consumes body
Index: src/client_side_request.cc
===================================================================
--- src/client_side_request.cc (revision 167)
+++ src/client_side_request.cc (working copy)
@@ -829,6 +829,7 @@
NULL);
#endif
http->getConn()->flags.readMore = true; // resume any pipeline reads.
+ http->getConn()->forwardToBitbucket();
node = (clientStreamNode *)http->client_stream.tail->data;
clientStreamRead(node, http, node->readBuffer);
return;
