On 18.01.13 18:37, Alex Rousskov wrote:

Further to this...

* Use expectNoForwarding() instead of adding forwardToBitbucket().

* In ConnStateData::noteBodyConsumerAborted() do not stop reading if
flags.readMore is set. Just keep auto-consuming. This may prevent
authenticating connections closures even though expectNoForwarding() is
used. Needs to be tested: Will we correctly stop and switch to the new
request when the body finally ends?

I have changed to using expectNoForwarding() and removed the stopReceiving() call from noteBodyConsumerAborted() and this appears to work correctly:

1. Making a request which results in an error, with "Connection: close" produces a response to the client, and then once the request body has been fully received, Squid closes the connection.

2. Making a request which results in an error, with "Connection: keep-alive" produces a response to the client, and then once the request body has been fully received Squid reads the next request.

3. Making a request which does *NOT* result in an error, with "Connection: keep-alive" works as expected and then waits for the next request.

4. Making a request which does *NOT* result in an error, with "Connection: close" works as expected and then closes the connection.

5. Making a request which results in an ICAP error (I shut down a non-bypassable ICAP service), with "Connection: keep-alive" produces a 500 error response and then waits for the next request.

6. Making a request which results in an ICAP error, with "Connection: close" produces a 500 error response and then Squid closes the connection.

I don't really understand the purpose of the readMore flag, can you explain it?

* Discuss and possibly remove readNextRequest setting from
ClientHttpRequest::handleAdaptationFailure(), to remove inconsistency
discussed in #2 above.

As mentioned above, I don't understand what readMore is actually for. However, as I mentioned in my previous email, the only time I can see us wanting to blindly close the connection for adaptation failures is when we've already started sending a response to the client and the adaptation service blows up in the middle of the response body. It doesn't _look_ like handleAdaptationFailure() handles that event though, unless I'm mistaken?

* Discuss whether we are OK with clients tying up Squid after an
[authentication] error. Perhaps we need another timeout there, perhaps
there are already mechanisms to address that, or perhaps we do not
really care.

There are 4 possible ways I can see of a client tieing up Squid:

1. The client connects but doesn't send anything.
2. The client connects, makes a keep-alive request that results in an error, then doesn't send any new request. 3. The client makes a request that generates an error, but doesn't send the complete request body. 4. The client makes a request that generates an error, uses chunked encoding and sends an infinitely long body.

(1) and (2) are basically the same thing. I've tested this and Squid times out the connections and closes them after a few minutes.

(3) similarly gets timed out after a while.

I haven't tested (4), but I have no reason to believe there's anything that currently stops it from happening. I'm not sure whether or not we should do something to prevent it - that's one for discussion.

--

 - 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]

Reply via email to