ocaml-tls icon indicating copy to clipboard operation
ocaml-tls copied to clipboard

Expose async tls_handler

Open mbacarella opened this issue 3 years ago • 3 comments

I proprose this PR to expose the async tls_handler to users of the library. This is needed if you want to do your own Tcp service management, such as if you're plugging async tls support into other frameworks.

mbacarella avatar Jul 30 '22 03:07 mbacarella

that's a pretty huge diff for the description. any chance you could remove any code formatting changes from this PR?

also, maybe @torinnd could take a look at the changes?

hannesm avatar Aug 05 '22 08:08 hannesm

I'd probably recommend collapsing the new test_server_low_level example into the test_server. They don't look different enough to me to justify persisting both.

I also think I'd refactor the MLI as follows:

type 'a io_handler = Reader.t -> Writer.t -> 'a Deferred.t
type 'a tls_handler = Session.t -> 'a io_handler

val upgrade_server_handler
  : config:Tls.Config.server
  -> 'a tls_handler
  -> 'a io_handler

Notably this excludes any mention of the socket address from the type signature as that isn't relevant to the TLS upgrade being performed here. Callers would probably do something like upgrade_server_handler ~config (my_cool_handler socket) to satisfy the Async.Tcp.Server.create interface.

torinnd avatar Aug 05 '22 09:08 torinnd

Thanks for the review. Collapsing the test servers into one and the .mli refactor done.

I believe all of the extraneous code formatting has been removed (mostly in the dune file).

mbacarella avatar Aug 05 '22 18:08 mbacarella

Thanks for your contribution. I squashed and merged, this will be part of the next release.

hannesm avatar Sep 13 '22 22:09 hannesm