mirage / ocaml-cohttp

An OCaml library for HTTP clients and servers using Lwt or Async
Other
696 stars 172 forks source link

make Cohttp_lwt_unix.default_ctx lazy #1027

Closed akuhlens closed 2 months ago

akuhlens commented 3 months ago

The oss tool semgrep uses cohttp for some of its network requests. We recently got a bug report that showed that even if you didn't do any network requests if you don't have CA certificates installed on the machine you get the following backtrace.

root@1eb4df820b50:/usr/local/src# semgrep --json --skip-unknown-extensions --disable-version-check --metrics=off --no-rewrite-rule-ids -f var-in-href.yaml 
Fatal error: exception Failure("ca-certs: no trust anchor file found, looked into /etc/ssl/certs/ca-certificates.crt, /etc/pki/ca-trust/extracted/pem/tls-ca-bundle.pem, /etc/ssl/ca-bundle.pem.\nPlease report an issue at https://github.com/mirage/ca-certs, including:\n- the output of uname -s\n- the distribution you use\n- the location of default trust anchors (if known)\n")
Raised at Stdlib.failwith in file "stdlib.ml", line 29, characters 17-33
Called from CamlinternalLazy.force_lazy_block in file "camlinternalLazy.ml", line 31, characters 17-27
Re-raised at CamlinternalLazy.force_lazy_block in file "camlinternalLazy.ml", line 36, characters 4-11
Called from Conduit_lwt_unix.default_ctx in file "src/conduit-lwt-unix/conduit_lwt_unix.ml", line 158, characters 26-79
Called from CamlinternalLazy.force_lazy_block in file "camlinternalLazy.ml", line 31, characters 17-27
Re-raised at CamlinternalLazy.force_lazy_block in file "camlinternalLazy.ml", line 36, characters 4-11
Called from Cohttp_lwt_unix__Net.default_ctx in file "cohttp-lwt-unix/src/net.ml", line 33, characters 10-49

This change makes the default_ctx value lazy so that it isn't initialized when the library is loaded. Which allows us to control when it is initialized. This is the same pattern that the conduit library uses for there default_ctx.

Test Plan

I have tested this change by:

mseri commented 3 months ago

There is still a failure

File "cohttp-mirage/src/net.ml", line 1:
Error: The implementation cohttp-mirage/src/net.pp.ml
       does not match the interface cohttp-mirage/src/.cohttp_mirage.objs/byte/cohttp_mirage__Net.cmi:
        ... ... ... ... In module Make:
       Values do not match:
         val default_ctx : ctx
       is not included in
         val default_ctx : ctx lazy_t
       The type ctx is not compatible with the type ctx lazy_t
       File "cohttp-mirage/src/net.mli", line 11, characters 10-47:
         Expected declaration
       File "cohttp-mirage/src/net.ml", line 20, characters 6-17:
         Actual declaration
(cd _build/default && /home/opam/.opam/5.0/bin/ocamlc.opt -w @1..3@5..28@30..39@43@46..47@49..57@61..62-40 -strict-sequence -strict-formats -short-paths -keep-locs -g -bin-annot -I cohttp-mirage/src/.cohttp_mirage.objs/byte -I /home/opam/.opam/5.0/lib/angstrom -I /home/opam/.opam/5.0/lib/asn1-combinators -I /home/opam/.opam/5.0/lib/astring -I /home/opam/.opam/5.0/lib/base64 -I /home/opam/.opam/5.0/lib/bigstringaf -I /home/opam/.opam/5.0/lib/ca-certs-nss -I /home/opam/.opam/5.0/lib/conduit -I /home/opam/.opam/5.0/lib/conduit-lwt -I /home/opam/.opam/5.0/lib/conduit-mirage -I /home/opam/.opam/5.0/lib/cstruct -I /home/opam/.opam/5.0/lib/dns -I /home/opam/.opam/5.0/lib/dns-client -I /home/opam/.opam/5.0/lib/dns-client-mirage -I /home/opam/.opam/5.0/lib/dns/cache -I /home/opam/.opam/5.0/lib/domain-name -I /home/opam/.opam/5.0/lib/duration -I /home/opam/.opam/5.0/lib/eqaf -I /home/opam/.opam/5.0/lib/eqaf/bigstring -I /home/opam/.opam/5.0/lib/eqaf/cstruct -I /home/opam/.opam/5.0/lib/fmt -I /home/opam/.opam/5.0/lib/gmap -I /home/opam/.opam/5.0/lib/happy-eyeballs -I /home/opam/.opam/5.0/lib/hkdf -I /home/opam/.opam/5.0/lib/io-page -I /home/opam/.opam/5.0/lib/ipaddr -I /home/opam/.opam/5.0/lib/ipaddr-sexp -I /home/opam/.opam/5.0/lib/logs -I /home/opam/.opam/5.0/lib/lru -I /home/opam/.opam/5.0/lib/lwt -I /home/opam/.opam/5.0/lib/macaddr -I /home/opam/.opam/5.0/lib/magic-mime -I /home/opam/.opam/5.0/lib/metrics -I /home/opam/.opam/5.0/lib/mirage-channel -I /home/opam/.opam/5.0/lib/mirage-clock -I /home/opam/.opam/5.0/lib/mirage-crypto -I /home/opam/.opam/5.0/lib/mirage-crypto-ec -I /home/opam/.opam/5.0/lib/mirage-crypto-pk -I /home/opam/.opam/5.0/lib/mirage-crypto-rng -I /home/opam/.opam/5.0/lib/mirage-flow -I /home/opam/.opam/5.0/lib/mirage-flow-combinators -I /home/opam/.opam/5.0/lib/mirage-kv -I /home/opam/.opam/5.0/lib/mirage-random -I /home/opam/.opam/5.0/lib/mirage-time -I /home/opam/.opam/5.0/lib/optint -I /home/opam/.opam/5.0/lib/parsexp -I /home/opam/.opam/5.0/lib/pbkdf -I /home/opam/.opam/5.0/lib/ppx_sexp_conv/runtime-lib -I /home/opam/.opam/5.0/lib/psq -I /home/opam/.opam/5.0/lib/ptime -I /home/opam/.opam/5.0/lib/randomconv -I /home/opam/.opam/5.0/lib/re -I /home/opam/.opam/5.0/lib/seq -I /home/opam/.opam/5.0/lib/sexplib -I /home/opam/.opam/5.0/lib/sexplib0 -I /home/opam/.opam/5.0/lib/stringext -I /home/opam/.opam/5.0/lib/tcpip -I /home/opam/.opam/5.0/lib/tls -I /home/opam/.opam/5.0/lib/tls-mirage -I /home/opam/.opam/5.0/lib/uri -I /home/opam/.opam/5.0/lib/uri-sexp -I /home/opam/.opam/5.0/lib/uri/services -I /home/opam/.opam/5.0/lib/vchan -I /home/opam/.opam/5.0/lib/x509 -I /home/opam/.opam/5.0/lib/xenstore -I /home/opam/.opam/5.0/lib/xenstore/client -I /home/opam/.opam/5.0/lib/zarith -I cohttp-lwt/src/.cohttp_lwt.objs/byte -I cohttp/src/.cohttp.objs/byte -I http/src/.http.objs/byte -I http/src/bytebuffer/.http_bytebuffer.objs/byte -intf-suffix .ml -no-alias-deps -opaque -open Cohttp_mirage__ -o cohttp-mirage/src/.cohttp_mirage.objs/byte/cohttp_mirage__Net.cmo -c -impl cohttp-mirage/src/net.pp.ml)
File "cohttp-mirage/src/net.ml", line 1:
Error: The implementation cohttp-mirage/src/net.pp.ml
       does not match the interface cohttp-mirage/src/.cohttp_mirage.objs/byte/cohttp_mirage__Net.cmi:
        ... ... ... ... In module Make:
       Values do not match:
         val default_ctx : ctx
       is not included in
         val default_ctx : ctx lazy_t
       The type ctx is not compatible with the type ctx lazy_t
       File "cohttp-mirage/src/net.mli", line 11, characters 10-47:
         Expected declaration
       File "cohttp-mirage/src/net.ml", line 20, characters 6-17:
         Actual declaration
akuhlens commented 3 months ago

Are the rest of the failures expected? I am not sure how to debug the one that isn't marked experimental.

akuhlens commented 3 months ago

I think this is good to go now!

akuhlens commented 2 months ago

Hi @mseri, is this something that would be accepted? Or should we try to fix this problem internally?

mseri commented 2 months ago

I don't see why not. I wanted to see if anybody had an opinion about it and if we could do it differently, but I don't think I will have time for doing that soon. I'll have a proper look in the weekend, sorry for the delay

akuhlens commented 2 months ago

Hi @mseri, did you get a chance to look at this this last weekend?

mseri commented 2 months ago

Looks good to me

mseri commented 2 months ago

I am going to merge but please @mefyl have a look as well if you have time

mefyl commented 2 months ago

It looks like the problem and the solution here are the Unix backend equivalent of this issue in the jsoo backend, LGTM.