fix(bindings): make Context borrow immutable #5071
Conversation
jmayclin
changed the title
| { | ||
| let raw = NonNull::new(conn_ptr).expect("connection should not be null"); | ||
| let mut conn = Connection::from_raw(raw); | ||
| let mut config = conn.config().expect("config should not be null"); | ||
| let context = config.context_mut(); |
There was a problem hiding this comment.
Can we remove the context_mut method entirely? It should really only be used during the builder. If it's on the config, it's too easy to call. Or at the very least mark it unsafe.
* make context_mut unsafe
|
I didn't expect that there was a copy-pasted |
| @@ -334,7 +338,12 @@ impl Builder { | |||
| ) | |||
| .into_result() | |||
| }; | |||
| self.context_mut().application_owned_certs.push(chain); | |||
| let context = unsafe { | |||
There was a problem hiding this comment.
The copy pastes are a bit tedious, but I did it this way to try and prevent anyone from passing a mut_context in as the context parameter of the s2n callback.
Practically it doesn't make a difference because the pointer isn't actually treated as mut, but it would be very dangerous if it were, so the extra friction seemed warranted? Let me know if folks prefer it to just be a method on the Builder.
Release Summary:
A breaking change was made to the renegotiation callback interface. This only affects Rust customers using the unstable-renegotiate feature.
Description of changes:
with_contextis invoked as part of callbacks. Callbacks are set on configs, and as such may be invoked concurrently. Therefore it is not valid to hold a&mutto the Context. This PR updateswith_contextto use an immutable ref.We weren't actually using the context as mutable anywhere, so for most callbacks this is not a breaking change.
For the renegotiate callback, this is a breaking change. Although this is not expected to significantly change the feature. From the PR, it can be observed that the feature still works largely the same even with the type signature change.
Testing:
All tests should continue to pass.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.