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

TLSUpgradeProto: don't set multiple results for an event #1117

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

w-miller
Copy link

In the case of a misbehaving server, the client may receive more than one byte in separate data_received() invocations from the server. While we can't do much sane with this, we should handle it gracefully and not crash with asyncio.InvalidStateError when trying to set another result on the event.

Fixes #729

@w-miller w-miller marked this pull request as ready for review January 31, 2024 08:17
@w-miller w-miller force-pushed the will.miller/issue-729 branch from 087f79f to 78d86da Compare February 1, 2024 14:40
In the case of a misbehaving server, the client may receive more than
one byte in separate data_received() invocations from the server. While
we can't do much sane with this, we should handle it gracefully and not
crash with asyncio.InvalidStateError when trying to set another result
on the event.

Fixes MagicStack#729
@eltoder
Copy link
Contributor

eltoder commented Mar 9, 2024

This appears to be a duplicate of #1031, though it's a nicer version and comes with tests. Previously @elprans did not like the approach.

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.

Something is setting the result on a completed task?
3 participants