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

Fix: Check for ErrorKind::WouldBlock in LazyConfigAcceptor #48

Merged

Conversation

jbr
Copy link
Contributor

@jbr jbr commented Mar 5, 2024

Similar to #47

@djc
Copy link
Member

djc commented Mar 6, 2024

Since this seems a little finicky, could you add a test?

@jbr jbr force-pushed the send-alert-would-block-in-lazy-config-acceptor branch from 2143357 to 263fd4b Compare March 6, 2024 19:17
@jbr
Copy link
Contributor Author

jbr commented Mar 6, 2024

Good call with the test, the original implementation didn't handle the possibility that the write buffer was too small to fit the whole alert

@jbr jbr requested a review from djc March 6, 2024 19:17
@jbr jbr force-pushed the send-alert-would-block-in-lazy-config-acceptor branch from 263fd4b to f8fd742 Compare March 6, 2024 19:28
@djc djc requested review from ctz and quininer March 7, 2024 08:57
Copy link
Member

@ctz ctz left a comment

Choose a reason for hiding this comment

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

Thanks for catching these!

@djc
Copy link
Member

djc commented Mar 7, 2024

(Needs a rebase now.)

Copy link
Member

@quininer quininer left a comment

Choose a reason for hiding this comment

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

LGTM

@jbr jbr force-pushed the send-alert-would-block-in-lazy-config-acceptor branch from f8fd742 to a981580 Compare March 7, 2024 18:07
@jbr
Copy link
Contributor Author

jbr commented Mar 7, 2024

Rebased

@quininer quininer merged commit d26502c into rustls:main Mar 8, 2024
6 checks passed
@quininer
Copy link
Member

quininer commented Mar 8, 2024

Thank you!

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