-
Notifications
You must be signed in to change notification settings - Fork 612
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
server: allow acceptor to send alerts after error #1811
Conversation
Benchmark resultsInstruction countsSignificant differencesThere are no significant instruction count differences Other differencesClick to expand
Wall-timeSignificant differencesThere are no significant wall-time differences Other differencesClick to expand
Additional informationCheckout details:
|
96679d2
to
665ddbe
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1811 +/- ##
==========================================
- Coverage 95.98% 95.95% -0.04%
==========================================
Files 84 84
Lines 18791 18819 +28
==========================================
+ Hits 18036 18057 +21
- Misses 755 762 +7 ☔ View full report in Codecov by Sentry. |
Yes would be good to have at least two test cases for this: one where the clienthello is just junk (so They other thing we should do in this PR is make |
I probably won't be able contribute the test/example work this week. |
I'm looking at this now (but won't push to this branch myself since it seems like you're also rebasing ATM)
I think 2a60b63 would meet your requirement
Working on this one now.
Here's an example update commit: d01215a
Here's that fix: 0c8c0c3 |
f560584
to
d9ce3b0
Compare
Rebased this. Feel free to take the baton for this branch! |
d9ebef9
to
bb2704b
Compare
🤝 I completed the requested test coverage and rolled it and the one new I've updated the PR description and I think this is now ready for review. |
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.
LGTM, thanks for taking care of the tests/examples.
Fixes #1792.
Acceptor
'saccept
andinto_connection
fns now return both anError
and anAcceptedAlert
in the error case. TheAcceptedAlert
can be used to write any TLS alerts produced during processing.accept
with an alert producedinto_connection
with an alert producedserver_acceptor.rs
example program is updated to demonstrate using theAcceptedAlert
return to write alerts.