fix(bindings): Apply with_system_certs to Config builder() API #4456
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes:
Currently the Config
builder()
API returns a default Builder:s2n-tls/bindings/rust/s2n-tls/src/config.rs
Lines 44 to 46 in d916942
Due to the
#[derive(Default]]
statement on the Builder struct, this means that the Config set in the Builder contains a fully built default Config:s2n-tls/bindings/rust/s2n-tls/src/config.rs
Lines 98 to 102 in d916942
We currently load default system certificates into the Config by default, so the Config created with the Config
builder()
API will always contain default certificates, even if the with_system_certs API is used.This PR uses
Builder::new()
to create the Builder's default Config, giving users the opportunity to callwith_system_certs
before the Config is built.Testing:
I updated the existing system cert loading test to also create a Config with
Config::builder()
in addition toBuilder::new()
. I confirmed that this test failed without the fix applied.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.