[bitreq] Bitreq client builder#502
Conversation
APonce911
changed the title
|
Holla if you want/need any review mate. Otherwise, for me at least, while there are WIP commits I'll assume you are happily working on it still. |
|
Holla at me also if you want me to kick CI into action for you. Once you have a patch merged CI will run automatically. You could just do a basic fix somewhere else in the repo if this is going to be long running work. |
|
Thanks @tcharding. My idea with this draft was to give a bit of visibility of where we're at and to guarantee that the solution is in the right path. No formal code review is needed yet. Thanks for the tip about the CI, so far it's ok not to run. |
| } | ||
|
|
||
| #[cfg(all(feature = "rustls", feature = "tokio-rustls"))] | ||
| pub(super) async fn wrap_async_stream_with_configs( |
There was a problem hiding this comment.
same as wrap_async_stream but with additional parameter custom_client_config
APonce911
requested review from
TheBlueMatt,
oleonardolima and
tnull
as code owners
|
@tcharding hey mate, can you please run the CI once? It's ready for review. |
TheBlueMatt
left a comment
TheBlueMatt
left a comment
There was a problem hiding this comment.
Obv needs docs, but at a high level looks good to me.
| Self { capacity: 1, client_config: None } | ||
| } | ||
|
|
||
| pub fn with_root_certificate<T: Into<Vec<u8>>>(mut self, certificate: T) -> Self { |
There was a problem hiding this comment.
Its pretty awkward that we don't error here but then just ignore certs silently. IMO we kinda need to actually make this fallible, store a root cert store in ClientConfig, and pass that around instead. We'll have to have an abstract root cert store in rustls_stream (or somewhere), but that should be pretty straightforward.
There was a problem hiding this comment.
@TheBlueMatt makes sense. Will work on that tomorrow! Thanks
There was a problem hiding this comment.
@TheBlueMatt updated the PR with your suggestion. I moved the new certificates logic to a new module. We still have lots of stuff in the rustls_stream module, but I thought refactoring that would make this PR too large.
3 participants
Summary
This PR addresses issue #473 by creating a Client Builder able to receive custom root certificates at runtime.
The ClientBuilder requires the same features as Client (async-https-rustls).
Changes