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 race when using HTTP proxy. #181

Merged
merged 4 commits into from
Mar 22, 2021
Merged

Fix race when using HTTP proxy. #181

merged 4 commits into from
Mar 22, 2021

Conversation

erikjohnston
Copy link
Member

After start_tls we need to manually call connection_made on the protocol to tell it about the new underlying transport. This gives a brief window where the protocol can receive data without having a transport to write to, causing issues with the APNS connections where it assumes it can write once it starts reading data.

We fix this by wrapping the protocol in a buffer that simply buffers incoming data until connection_made is called.

Fixes #167, hopefully

@erikjohnston erikjohnston changed the title Fix race when using HTTPS proxy. Fix race when using HTTP proxy. Mar 19, 2021
After `start_tls` we need to manually call `connection_made` on the
protocol to tell it about the new underlying transport. This gives a
brief window where the protocol can receive data without having a
transport to write to, causing issues with the APNS connections where it
assumes it can write once it starts reading data.

We fix this by wrapping the protocol in a buffer that simply buffers
incoming data until `connection_made` is called.
Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

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

I don't really see how this is happening (maybe the Twisted reactor is stealing control during the await?) Seems like this should fix the issue though.

tests/asyncio_test_helpers.py Outdated Show resolved Hide resolved
@erikjohnston
Copy link
Member Author

I don't really see how this is happening (maybe the Twisted reactor is stealing control during the await?) Seems like this should fix the issue though.

It's possible there's just a lot of work queued up, or a GC happens, or some TLS session resumption that allows the server to send data immediately, or... something. I don't really know :/

Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>
@erikjohnston erikjohnston merged commit 72ff6c5 into master Mar 22, 2021
@erikjohnston erikjohnston deleted the erikj/fix_https_race branch March 22, 2021 09:51
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.

Using a HTTP proxy sometimes breaks TLS connections
2 participants