Client configuration API #57
Conversation
Ralith
force-pushed
the
dangerous-configuration
branch
3 times, most recently
from
66e2ccf
to
6622dd7
Compare
quinn-proto/src/connection.rs
Outdated
| @@ -323,6 +329,7 @@ impl Connection { | |||
| data_recvd: 0, | |||
| local_max_data: config.receive_window as u64, | |||
| server_name: None, | |||
| tls_config: None, | |||
There was a problem hiding this comment.
I don't like this very much. We're putting a Config state in here, optionally only for the initiators, which is only used in the very short time between the instantiation of the Connection and the connect(). Seems to me like we might want to create a new_client() constructor instead that just does the connect() bits straightaway, and then we don't have to store the ClientConfig.
There was a problem hiding this comment.
The ClientConfig is stored for use in resetting state on retry packet receipt. That said, I'm not actually sure resetting TLS state is necessary; tests pass and draft 11 doesn't seem to specifically require it. It won't be needed in draft 15 either, so I'll just strip it out.
quinn-proto/src/connection.rs
Outdated
| config: &Arc<ClientConfig>, | ||
| server_name: &str, | ||
| ) -> Result<(), ConnectError> { | ||
| assert_eq!( |
There was a problem hiding this comment.
If we're asserting this here, should we have a similar assert for the server side?
There was a problem hiding this comment.
In fact, rustls contains a similar assert already, so perhaps this should be removed.
Ralith
force-pushed
the
dangerous-configuration
branch
from
a670a50
to
c239cb1
Compare
|
Yeah, got a bit carried away with the big blob o' intermingled changes, apologies. |
Ralith
force-pushed
the
dangerous-configuration
branch
2 times, most recently
from
bc2469e
to
cb6fead
Compare
There was a problem hiding this comment.
Thanks for addressing the comments.
I noticed you had a commit to remove the unit test socket things. I was thinking we might make them optional, disabled by default; would that work for you? I understand that they can be a bit of a pain, but debugging with Wireshark can be really helpful.
|
That commit disappeared when I realized the nondeterminism was just that I'd left a test server running in the background. Disabled-by-default (perhaps mediated by an env var?) would make sense, but so long as it's easy to run them in practice I don't think it's urgent. |
- Allow configuration of individual clients within an endpoint - Implement feature-gated config helper for untrusted connections - Don't enable keylogging by default - Test rejection of invalid certs
Ralith
force-pushed
the
dangerous-configuration
branch
from
cb6fead
to
3761d07
Compare
Candidate to address #35.