-
Notifications
You must be signed in to change notification settings - Fork 746
Description
In #1365 (comment), @cpu asked for more of my thoughts on the ergonomics of ConfigBuilder's typestate-based API. I figured I'd hoist them into their own issue.
Docs
rustdoc doesn't do a good job of rendering types that have multiple impl blocks. For instance, in ConfigBuilder's docs, there's are two entries in the sidebar named with_client_auth_cert
: 1 2. When you click on them, there's no indication that they are on different types (ConfigBuilder<ClientConfig, WantsTransparencyPolicyOrClientCert>
vs ConfigBuilder<ClientConfig, WantsClientCert>
), unless you scroll up or collapse the doc sections. This is a problem for rustdoc to fix, but speaking as a rustdoc maintainer it's a bit challenging - I haven't thought of a great way to do it yet.
All or Nothing
As a user I think of a config as having various defaults, any of which I can change independently. The default ClientConfig is built like:
ClientConfig::builder()
.with_safe_defaults()
...
But if I want to set "only use TLS 1.3," I have to not only call the with_protocol_versions
accessor, I have to expand the list to include two settings I don't want to change:
ClientConfig::builder()
.with_safe_default_cipher_suites()
.with_safe_default_kx_groups()
.with_protocol_versions(&[rustls::version::TLS13])
...
And the selection of those settings feels arbitrary. Why do those two settings become mandatory when I choose to override the protocol versions? Why not alpn_protocols
or resumption
? I happen to know that it's because they control code paths that the linker may optimize away if the user doesn't want them, but it feels like a lot of implementation details being exposed.
Order dependency
The above example is valid, but this is not:
ClientConfig::builder()
.with_protocol_versions(&[rustls::version::TLS13])
.with_safe_default_cipher_suites()
.with_safe_default_kx_groups()
...
That feels arbitrary, and it's hard to remember what order things go in. This could maybe be solved with better verbiage in the documentation. For instance we could explain up top "If you wish to customize the protocol versions, cipher suites, or key exchange groups, you must set them in this order: 1. cipher suites 2. key exchange groups 3. protocol versions."
Error messages
With the above invalid example is compiled, this is the error:
error[E0599]: no method named `with_protocol_versions` found for struct `ConfigBuilder<ClientConfig, WantsCipherSuites>` in the current scope
--> src/main.rs:3:10
|
2 | / rustls::ClientConfig::builder()
3 | | .with_protocol_versions(&[rustls::version::TLS13])
| | -^^^^^^^^^^^^^^^^^^^^^^ method not found in `ConfigBuilder<ClientConfig, WantsCipherSuites>`
| |_________|
|
|
= note: the method was found for
- `ConfigBuilder<S, WantsVersions>`
This is... remarkably good on rustc's part, actually. But it still doesn't precisely explain the nature of the problem: these methods must be called in a certain order, and the order is wrong.
Compile time validation vs runtime validation
The typestate system allows us to lean on compile-time checking for a lot of properties, which is nice. But we can't cover them all at compile time; there are still Result
returns in ConfigBuilder
. If we relied more on runtime checking, we could give more useful error messages. And since configs are usually constructed the same way during each run of a program, I think this is not much of a sacrifice in ability to catch problems ahead of time.
Multiple unwraps
In the ServerConfig example:
ServerConfig::builder()
.with_safe_default_cipher_suites()
.with_safe_default_kx_groups()
.with_safe_default_protocol_versions()
.unwrap()
.with_no_client_auth()
.with_single_cert(certs, private_key)
.expect("bad certificate/key");
This looks like a single chain of settings on a builder; it's not clear to me why there is an unwrap in the middle and at the end, rather than a single unwrap at the end that checks all the relevant settings.
State machines
State machines are a powerful tool for reducing complexity in thinking about protocols. They allow clearly ruling out invalid states and invalid transitions. But I think introducing a state machine to people's mental model of a config builder actually increases complexity. Most of the states in the machine don't have inherent meaning. And our correctness requirements are a property of the final bundle of settings, not of the transitions between intermediate states.