Skip to content
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: swallow TLS errors in the accept loop #1990

Merged
merged 2 commits into from
Oct 15, 2024

Conversation

PDXKimani
Copy link
Contributor

Motivation

#1989

A previous commit altered the handling of errors during TLS handshakes. Prior to that change, the various and sundry error IO errors which can arise during a handshake were ignored. The commit in question changed the behavior to instead exit the accept loop (and thus shut down the server) if any IO error which is not explicitly listed occurs. Since that commit, several error kinds have been added to the list piecemeal to address shutdowns, but the tail of errors which might need to be added is likely long.

Solution

This change aims to return the handling of TLS errors to the previous state where all errors from tls.accept would be swallowed.

This "de-unifies" the handling for those two classes of error, but I believe this to be the correct approach to preserve server stability - TLS handshake errors should not generally be fatal to the server.

Copy link
Contributor

@krispraws krispraws left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@djc djc requested a review from tottoto October 14, 2024 08:02
@djc djc added this pull request to the merge queue Oct 15, 2024
Merged via the queue into hyperium:master with commit a00200e Oct 15, 2024
17 checks passed
@PDXKimani PDXKimani deleted the fix/tls-handling branch October 16, 2024 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants