Skip to content

Improve handling for "no roots in server config" #217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jsha opened this issue Nov 13, 2021 · 1 comment · Fixed by #472
Closed

Improve handling for "no roots in server config" #217

jsha opened this issue Nov 13, 2021 · 1 comment · Fixed by #472

Comments

@jsha
Copy link
Collaborator

jsha commented Nov 13, 2021

Right now, if you call rustls_server_config_builder_build on a config builder where no cert resolver has been set, it will return NULL. This is a little inconsistent with other functions, where returning NULL is reserved for panics or null inputs.

Instead we should do one of two things:

  • Make cert resolver a mandatory argument to rustls_server_config_builder_new and rustls_server_config_builder_new_custom, or
  • Make rustls_server_config_builder_build() take an output parameter to write the built server config, and return a rustls_result, which may return a new error value RUSTLS_RESULT_NO_CERT_RESOLVER.
@cpu
Copy link
Member

cpu commented Oct 4, 2024

Make rustls_server_config_builder_build() take an output parameter to write the built server config, and return a rustls_result, which may return a new error value RUSTLS_RESULT_NO_CERT_RESOLVER.

The 0.14.0 release changed the server config builder to use an out-param, but we're currently returning a RUSTLS_RESULT_GENERAL error for the case described here:

rustls-ffi/src/server.rs

Lines 356 to 360 in eb3ccfa

let mut config = if let Some(r) = builder.cert_resolver {
base.with_cert_resolver(r)
} else {
return rustls_result::General;
};

I will open a fix to use a better error result.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants