-
Notifications
You must be signed in to change notification settings - Fork 747
feat: add additional application context into Connection #5637
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
0ec4885 to
1535232
Compare
1535232 to
adfff1a
Compare
maddeleine
reviewed
Dec 2, 2025
* use the generic type to query the TypeId
6661da9 to
e4968bf
Compare
maddeleine
reviewed
Dec 2, 2025
* nit fix for comments
maddeleine
approved these changes
Dec 3, 2025
* Fixing comments and make them more precise * Simplify the match with and_then method * Recover test coverage for context override
WesleyRosenblum
approved these changes
Dec 3, 2025
| self.context() | ||
| .app_context | ||
| .get(&context_type_id) | ||
| .and_then(|app_context| app_context.downcast_ref::<T>()) |
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goal
This PR changes the behavior of the
set_application_contextAPI to add multiple arbitrary application contexts which can be queried during handshakes. The definition of theapp_contextis changed from only accepting one application context to accept a map of application contexts.Why
We are trying to allow s2n-quic to query the remote address information about the client during the handshake. This PR in s2n-tls allows that addressing information to be added with the connection.
How
Change the definition of the Context for the connection to be a
HashMapof connection context. In this way, we can add as many application data as we want. Hence, we can add the remote address on top of context that were originally needed.I also added a
remove_application_contextAPI to help users managing memory usage.Callouts
I need to modify the
memory_test's memory usage in order for this PR to pass the CI. My change involves changing a variable in theContextstruct fromOptionto aHashMapwhich will definitely increase many usage by about 100 bytes per connection.Refers to https://github.com/aws/s2n-tls/pull/5637/files#diff-63ac16ef5c47eee35caf3e2bbf1909d5a3062a8343d888d399eaf960acf7326b for the change.
Testing
This PR has unit tests for all new APIs it introduced. Also, I have implemented relevant changes for s2n-quic which can be found in aws/s2n-quic@main...boquan-fang:s2n-quic:get-ip-from-connection. s2n-quic's
new_server_sessionAPI can now add the client's remote address into the connection.Related
N/A
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.