Re: [PATCH] Do not send unretriable requests on reused pinned connections

Hello,

    It just occurred to me that there is nothing wrong, from theoretical
protocol point of view, with reseting the client connection when a
server-side race condition is detected _and_ we know that the client was
the one that caused Squid to open the server connection. I personally do
not know whether there are use cases that would be broken by that.

To summarize, our choices for a pinned connection created for the
current client are:

Before sending the request:

1. Reopen and repin used pconns to send unretriable requests,
   just like non-pinned connection code does. This eliminates
   HTTP pconn race conditions for unretriable requests. This is
   what the proposed patch does for SslBump.

2. Send all requests without reopening the pinned connection.
   If we encounter an HTTP pconn race condition, see below.

After the request, if an HTTP pconn race happens:

0. Send an error message to the client.
   This is what we do today and it breaks things.

1. Reset the client connection.
   Pretend to be a dumb tunnel.

2. Retry, just like the current non-pinned connection code does.
   This is what the proposed patch implements.

Current code does 2+0. That does not work.

The proposed patch does 1+2. That works in my limited tests.

Henrik suggested 2+1. I agree that it is also an option worth
considering. Will it break actual clients? I do not know, but:

a) If we are talking about unretriable requests, the client already
violated HTTP by sending such a request over the persistent client
connection. Some of these clients may fail to handle resets properly.

b) To implement 2+1 correctly, Squid will need to close the _client_
connection when the origin server sends "Connection: close". We do not
do that now IIRC.

Thank you,

Alex.
Received on Sun Dec 02 2012 - 01:02:32 MST