Re: [RFC] CONNECT and use_ssl cache_peer

On 09/09/2012 08:12 PM, Amos Jeffries wrote:
>> sön 2012-09-09 klockan 21:34 +1200 skrev Amos Jeffries:
>>
>>> Henrik and I seem to disagree on this being a good idea being plugged
>>> into BodyPipe. But as I see it BodyPipe is where Upgrade, WebSockets and
>>> SSL-Bump should be happening in a node which internally "receives" the
>>> tunnel connection and acts like a ConnStateData reading out of the
>>> tunnel.

> I'm imagining a setup of:
>
> client TCP -> ConnStateData(BodyPipeProducer) -> Bump(BodyPipeReader,
> 'fake' ConnStateData(BodyPipeProducer) ) -> BodyPipe -> server-side
>
> so that:
> * the bumped CONNECT tunnel can be the source of a whole pipeline of
> requests (HTTPS, etc)
> * the original ConnStateData->Bump section of code retains all the
> CONNECT semantics down to and including terminate 'server' connection
> (aka Bump Node destructs) on end of CONNECT independent of what Bump ->
> server-side does with the server-side connection.
> * when receiving a CONNECT we detect the right type of streams reader
> to handle the data and allocate it:
> ** the Bump node could be Upgrade:TLS, Upgrade:WebSockets,
> Upgrade:HTTP/2, SSL-bump, pass-thru tunnel or some future protocol

FWIW, a "processing chain" design like that was one of the two primary
alternatives discussed when Squid3 was born. I agree that this design is
attractive for many reasons. I am guessing that ClientStreams were an
attempt to implement it (but we need to do much better than that!).

With ClientStream and adaptation chains lessons in mind, I would be
strongly opposed to making BodyPipe a _central_ piece of a processing
chain coordination. BodyPipe should be used for low-level shoveling of
opaque bytes within the chain, but there is more high-level semantics
that a message processing chain must handle and BodyPipe should not. New
"message processing stream" and "message processing stream node" classes
are needed if we want to build such chains, with the existing
ClientStream, StoreClient, and Adaptation code upgraded to use that new
API. This is Squid v4.0 stuff IMO...

Personally, I would welcome efforts in that direction, and I am glad you
have a very interested client pushing you there. However, this
monumental work should not become a precondition for fixing this
isolated bug, IMO.

Isolating [secure] peer connection establishment code into a class
shared by tunnel.cc and forward.cc code will already make any future
chaining improvements easier. Let's view this as a single step in the
right direction. I hope you do not insist on us running the whole
marathon within this bug scope :-).

Cheers,

Alex.
Received on Mon Sep 10 2012 - 15:35:52 MDT