Async runtime: HTTPS support #92
Conversation
This PR adds HTTPS support to the Async runtime, based on `Async_ssl`. Once `ocaml-tls` provides an Async runtime, we can add that one here.
| async core angstrom-async faraday-async httpaf | ||
| (select ssl_io.ml from | ||
| (async_ssl -> ssl_io_real.ml) | ||
| (!async_ssl -> ssl_io_dummy.ml))) |
There was a problem hiding this comment.
I'm not too familiar with dune conventions, but this feels like a bit of a poor user experience to me. If I understand correctly, it means that I can install httpaf-async, write a file that uses SSL that compiles fine, but at runtime I get an error about SSL not being available. What are our other options here?
There was a problem hiding this comment.
If I understand correctly, it means that I can install httpaf-async, write a file that uses SSL that compiles fine, but at runtime I get an error about SSL not being available
That's correct. I agree this is not desirable.
I copied this almost line by line from conduit-async, but I think there are other options.
- I think it's valuable to let SSL be optionally installed, do you agree with this?
- We could probably make this fail at compile time if a user calls the function, like how
Async.Printfdoesn't compile and produces a type error ofProbably_should_not_use_blocking_Core_Printf_functions_with_Async. If you agree this is a good enough solution I can implement it. Same for HTTPS support for the Lwt bindings #88.
There was a problem hiding this comment.
- I think it's valuable to let SSL be optionally installed, do you agree with this?
Not entirely, but if there are competing SSL implementations, it might be hard to avoid. I don't see a compelling argument for SSL as a feature being optional, at least.
- We could probably make this fail at compile time if a user calls the function
I'm thinking about a similar alternative where httpaf_async.mli exposes a module alias module Ssl = Ssl, and then if no ssl library is installed, that module is simply blank (or has some polymorphic variant stubs for documentation like Async.Printf); otherwise, it has Server and Client modules to mirror the ssl-less implementation. It also means you can simply open Httpaf_async.Ssl and get the interface you want.
There was a problem hiding this comment.
To be more explicit, the thing I wish we could express is "the user must have either library A or B installed, but if neither exists then install A as a dependency". I would be surprised if that was possible, though.
There was a problem hiding this comment.
To be more explicit, the thing I wish we could express is "the user must have either library A or B installed, but if neither exists then install A as a dependency". I would be surprised if that was possible, though.
I don't think this is possible to express. Not that I have seen, at least.
Not entirely, but if there are competing SSL implementations, it might be hard to avoid. I don't see a compelling argument for SSL as a feature being optional, at least.
My point is that there are systems where people have trouble building either SSL or TLS (I've heard reports from Windows). My goal with the current state of this PR was to still have the library build (e.g. if people are just using the Server module and deploying behind a reverse proxy).
I'm thinking about a similar alternative where httpaf_async.mli exposes a module alias module Ssl = Ssl, and then if no ssl library is installed, that module is simply blank
I don't follow 100%, but I can put together a proposal soon for you too look at.
There was a problem hiding this comment.
We're currently porting the OCaml TLS stack to be fully Dune'd up, and it also has Async bindings. The original decision in Conduit-async was because we weren't sure of the maturity of OCaml TLS vs OpenSSL then, but it's worked out pretty well in OCaml TLS' favour since.
Once the deps are all Duned up, Windows support should be much easier to guarantee...
There was a problem hiding this comment.
One reason why I currently use ocaml-ssl over ocaml-tls is because of lacking support for elliptic curve ciphersuites in the latter. That was my motivation for including both implementations in this PR.
|
See #131. Async's encryption support should either use the approach described in that issue, or just solely use ocaml-tls. |
This is a version of #90 that's ready for review. I only needed to fix the build errors related to "inconsistent assumptions about module Buffer".