search

rustls / rustls-ffi

Use Rustls from any language
Other
124 stars 31 forks source link

Zero callbacks #381

Closed billywhizz closed 1 month ago

billywhizz commented 6 months ago

I have been working on JavaScript bindings for rustls-ffi and found these "improvements" useful while doing so. Maybe you would be willing to accept them as a PR? If so, please let me know what needs changing in order for the PR to be accepted.

In JavaScript (on V8) calling back into JS from C++/Rust has quite some overhead (~70 nanoseconds) and adds some complexity to ffi/bindings, so I have added read_tls_from_fd and write_tls_to_fd methods which allows us to bypass the callbacks altogether. I think this will only work on linux/macos, so have marked the new methods and imports as unix only. I'm not expert in Rust so if there is a cleaner way to do this please let me know.

I also added a root_cert_store_builder_load_roots_from_bytes method to load the cert store from an array of bytes rather than being forced to read from an external file. This is useful for me as I want to embed the certs in the executable or download them and load them on the fly without having to read from disk. I imagine others would find this useful too.

This PR also includes 2 small changes to Cargo.toml to add read_buf feature by default and to the Makefile to generate a C++ compatible header file using cbindgen - i cannot compile using C++ compiler with the current src/rustls.h.

cpu commented 6 months ago

Thanks for the PR! Exciting to hear you're working on using this for JS bindings :+1:

I'm not sure about the other maintainers but it may take me a few days to get to reviewing this. I just wanted to leave a comment about that up-front.

cpu commented 5 months ago

rustls-ffi / Clippy nightly (optional) (pull_request) Failing after 18s

Merging https://github.com/rustls/rustls-ffi/pull/382 and rebasing will fix this nightly clippy finding. It can be ignored in this branch :-)

billywhizz commented 1 month ago

i'm happy to take a look at this again and prepare a PR if anyone can advise on how best to proceed? or, if it's not a supported idea, that's fine too. it seems that v8 has methods now to make callbacks from native into JS much faster, but maybe others will find this PR useful regardless?

cpu commented 1 month ago

Speaking only for myself I'm not likely to have the time required to help get this branch into shape. It might help to try and pull out some of the smaller unrelated changes (e.g. the C++ bindgen support, root store init w/o backing file, etc) into their own PRs. Those improvements are likely easier to land. New unsafe code and fd abstractions are a tougher sell.

billywhizz commented 1 month ago

Speaking only for myself I'm not likely to have the time required to help get this branch into shape. It might help to try and pull out some of the smaller unrelated changes (e.g. the C++ bindgen support, root store init w/o backing file, etc) into their own PRs. Those improvements are likely easier to land. New unsafe code and fd abstractions are a tougher sell.

makes sense. i will do a separate PR for the small changes. can leave this open in case anyone does want to take a look, but if you want to go ahead and close it feel free. thanks.

cpu commented 1 month ago

i will do a separate PR for the small changes.

Thanks!

can leave this open in case anyone does want to take a look, but if you want to go ahead and close it feel free.

Since none of the other maintainers have commented to indicate they can support the work I'm going to close it for now.