On 11/02/2010 06:43 PM, Gregory Collins wrote:
> John Lenz<[email protected]>  writes:
>
>> Hi,
>>
>> I have implemented SSL support for snap-server, you can see the result
>> in this commit
>
> This is awesome, thanks! I don't have time to read your changeset right
> now but I do have a couple of comments to your questions later, as well
> as a couple of ideas about how I would like the work to be done.
>
>
>> http://github.com/wuzzeb/snap-server/commit/33170f1198234126b5809b25f25953632d2f91b2
>>
>> I just directly bind to gnutls because the existing hackage package uses
>> lazy bytestrings and the API of the hackage package wasn't really what
>> we would want.
>>
>> It isn't quite ready to be pulled because I would like your feedback on
>> a few points:
>>
>>    - The way I think the ev backend will work is that for each cpu we
>> create two mkEvIO, one for the normal socket and one for the ssl
>> socket.  Both will feed the same Channel.  Then in withConnection we
>> test and see if we have an ssl session.  Thus for each cpu we have two
>> accept threads feeding one thread calling withConnection.
>
> This isn't really what I had in mind re: SSL support -- instead of
> dual-purposing one of the other backends to add SSL support (thereby
> putting nasty conditionals in places they shouldn't be), we should
> instead add an additional SSL backend.
>
> Note that each of the backends obeys the same basic interface; what we
> need to do first and foremost is to abstract out the backend interface
> into a typeclass, then provide instances for libev and for the simple
> backend, and use the preprocessor to switch individual backends on and
> off. The config stuff will need to be extended to allow you to open N
> simultaneous backends on N ports.

Ok, I didn't want to make major changes without at least some feedback. 
  One thing is that I don't think we want to duplicate the entire 
backend for SSL support.  If you look at my patch there are only very 
minimal changes to simpleBackend and I expect the ev backend to be 
similar.  The SSL backend will have all the same code for accepting, 
creating the event loops, monitoring timeouts, etc.  The only thing that 
changes is how we read and write the data plus a little setup code.

Perhaps we could split the backend into two apis, one class which takes 
care of the event loop

perhaps something like

class NetworkPort a where
     bind :: a -> IO (Socket, CInt)
     recv :: a -> Int -> IO b -> IO (Maybe ByteString)
     send :: ....
     whatever else is needed

class EventLoop a where
     attachListener :: (NetworkPort b) => a -> b -> IO ()
     waitForEvents  :: a -> IO ()
     withConnection :: ...
     other functions...

Have two implementations of NetworkPort, one for normal sockets and one 
for SSL and have two implementations of EventLoop, one the simple 
backend and one the ev backend.  Then we can create an event loop for 
each cpu and attach ports depending on the config.

Of course, I will need to try and implement this to see if it will work :)

>
> Re: "isSecure", Snap.Internal.Http.Server will need to set this based on
> the value of the backend config, we probably need a typeclass function
> for this, i.e.
>
>      class Backend b where
>          isSecure :: b ->  Bool
>
> For each individual accept loop we should put the Backend inside the
> ServerState, that way we don't really need to change any of the
> ServerMonad functions. Probably the value should be wrapped in an
> existential type, i.e.
>
>      data SomeBackend = forall b . Backend b =>  SomeBackend b
>
> if you give SomeBackend a Backend instance you can treat them uniformly
> in the code. I'm not 100% sure of the existential though, we have to
> maybe be careful so that we can still inline/specialize the code rather
> than doing dictionary lookups in there. Another alternative would be to
> parameterize ServerState and ServerMonad based on the backend (i.e. add
> a type argument) and then do some {-# SPECIALIZE #-}ing -- we'll need to
> experiment. Perf testing will be needed one way or the other, we should
> be able to do this without slowdowns.
>
> This is medium-to-major surgery so I've been putting it off, but I think
> this should be done first.

Yes, I agree a backend interface makes the most sense.

>
> Re: SSL, I think that HOpenSSL is the safest bet here since you can
> reuse 99% of the SimpleBackend (although some common stuff will have to
> be lifted) but if you think an ssl-libev backend is easy also we should
> have that too.

I think a direct binding to the ssl library makes the most sense because 
of how low level we are working.  I am not sure if the ev backend can be 
made to work with HOpenSSL since HOpenSSL has a high level interface and 
wants you to pass a socket and then just call the API functions to read 
and write.

Note that the direct binding isn't all that much code because the C api 
is pretty much exactly what we want.

>
>
>> Do you think instead we should create 2n backends?  We could use mkEvIO
>> to create two different accept callbacks in Backend.new and stick them
>> in the same evLoop, but have the two accept callbacks create their own
>> private channels to pass on two two separate threads both bound to the
>> same cpu calling withConnection.
>
> Yes, I'm thinking 2n backends.
>
>
>> - I probably need to do some cabal hacking allow the server to be built
>> without SSL support.
>
> Yes definitely, I would even say that SSL should have to be explicitly
> enabled with a compile-time flag.
>
> Let me know what you think about the matter. Since you're in there
> already, would you be willing to do the abstraction work? I just started
> a new job and my time is severly limited right now. Please let me know
> what you think.


Sure, I will work on it this week and see what kind of API makes the 
most sense.

John
_______________________________________________
Snap mailing list
[email protected]
http://mailman-mail5.webfaction.com/listinfo/snap

Reply via email to