On 11.09.2012 03:35, Alex Rousskov wrote:
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 :-).
Fine by me.
Amos
