On Jan 27, 2008 3:03 PM, Daniel Stenberg <daniel_at_haxx.se> wrote:
> On Sat, 26 Jan 2008, Gautam Kachroo wrote:
>
> >> It seems that for a unpatched version the connection is re-used fine in the
> >> test (which means the connection is re-used when the connection is not in
> >> ssl_connection_complete yet), while the patched version refuses to reuse that
> >> connection then.
> >
> > That makes sense since test 509 changes the url in the middle of the
> > ssl connection process. The connection for the first url is never
> > fully initialized. With the check for ssl_connection_complete, that
> > connection won't be re-used. The problem is why using a new connection
> > would cause problems.
>
> Well, as far as I remember this, the whole idea behind this particular test
> case is taken from a real-life use case. The actual URL to get is then
> received in the certificate somehow, so the redirect is really meant and
> supposed to be done on the same connection. I guess the reason then it has
> reached the complete SSL connection state is that it gets the certificate and
> thus the new URL etc before the entire negotiation is complete!
>
> The question is of course what to do about this situation. Do we stop allowing
> the method used in test 509 or do we somehow allow it and make sure that the
> re-used connection's SSL connection is continued to the complete state? Or is
> there any other alternatives?
>
> Either way, this gives me the feeling we won't have a solid and tested
> approach completed by tuesday so I think we can safely say that this fix will
> go in after 7.18.0 is released.
>

I got the test to work by turning on forking in sws. The problem when
sws doesn't fork is that the server is blocked waiting for a request
on the first connection. Of course, there is never a request on that
connection since the client has initiated a second connection. When
connection reuse is forbidden, the first connection is closed which
frees up the server for the second request. Otherwise, before this
patch, the connection is reused which doesn't cause the server to be
hung. With this patch, the first connection lingers keeping the
(non-forking) server blocked.

I think that means the patch is fine. There may be a concern about
keeping a connection to the server lingering, but that's not the worst
thing in the world since test 509 seems like a rare case.
Of course, Curl_done could close the first connection when it sees
that the ssl connection state is not complete. There's no point in
having the connection in the connection cache if it willl never be
re-used.

hope this helps,
-gk
Received on 2008-01-28