Skip to content

Make rustls_client_config_builder_build fallible, rm NoneVerifier #422

Closed
@cpu

Description

@cpu

Followup from #421 and #409

The rustls_client_config_builder_new docs say:

/// This starts out with no trusted roots.
/// Caller must add roots with rustls_client_config_builder_load_roots_from_file
/// or provide a custom verifier.

If the caller must set up a verifier, it seems clearer to error at build-time instead of verification time.

The NoneVerifier is only used for the circumstance where we build a client config without the user providing their own verifier with rustls_client_config_builder_dangerous_set_certificate_verifier or rustls_client_config_builder_set_server_verifier. If the ClientConfigBuilder instead changed the verifier field from Arc<dyn ServerCertVerifier> to Option<Arc<dyn ServerCertVerifier>> and rustls_client_config_builder_build returned an error for the case where verifier was None, we could remove NoneVerifier and I think bugs that lead to accidentally failing to set a verifier would be easier to catch/debug. I can't think of a use-case where you would want the NoneVerifier behaviour (and users that do want it can implement it themselves, providing their impl as the explicit verifier).

This is a breaking change, so we should hold it until we're ready to make a few similar changes at once.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions