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. 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. 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. > 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. Please add yourself to CONTRIBUTORS in your patch(es) btw if you haven't done so already -- when we merge your patch series remind me to get a short bio from you for the website. Thanks! G -- Gregory Collins <[email protected]> _______________________________________________ Snap mailing list [email protected] http://mailman-mail5.webfaction.com/listinfo/snap
