Skip to content

Backport start_tls() support for Python 3.6 #521

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.

Sign up for GitHub

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

Merged
florimondmanca merged 2 commits into from
Nov 17, 2019
Merged

Backport start_tls() support for Python 3.6 #521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e6f9ec9
Backport start_tls() support for Python 3.6
sethmlarson Nov 11, 2019
9882ab2
Remove version check in start_tls test
florimondmanca Nov 17, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 38 additions & 5 deletions httpx/concurrency/asyncio.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,49 @@ async def start_tls(
self, hostname: str, ssl_context: ssl.SSLContext, timeout: TimeoutConfig
) -> "SocketStream":
loop = asyncio.get_event_loop()
if not hasattr(loop, "start_tls"): # pragma: no cover
raise NotImplementedError(
"asyncio.AbstractEventLoop.start_tls() is only available in Python 3.7+"
)

stream_reader = asyncio.StreamReader()
protocol = asyncio.StreamReaderProtocol(stream_reader)
transport = self.stream_writer.transport

loop_start_tls = loop.start_tls # type: ignore
if hasattr(loop, "start_tls"):
loop_start_tls = loop.start_tls # type: ignore
Comment on lines +66 to +67
Copy link
Contributor

@JayH5 JayH5 Nov 12, 2019

Choose a reason for hiding this comment

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

Removing the type: ignore after changing the if statement to be if sys.version_info >= (3, 7) should pass CI with mypy>=0.730.

Copy link
Contributor Author

@sethmlarson sethmlarson Nov 12, 2019

Choose a reason for hiding this comment

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

Unfortunately we can't assume that loops on Python 3.6 don't have start_tls(). Uvloop supports 3.6 and supports start_tls(). That's interesting that mypy knows about features though! Would an assert work here?

Copy link
Contributor

@lovelydinosaur lovelydinosaur Nov 12, 2019

Choose a reason for hiding this comment

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

getattr would work around any typing issues too.

Copy link
Contributor

@JayH5 JayH5 Nov 12, 2019

Choose a reason for hiding this comment

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

Ah I didn't know about uvloop 😞

That's interesting that mypy knows about features though!

It knows about if/else with sys.version_info comparisons but that's all. It's very limited.

Would an assert work here?

I don't think we can assert much about the function at runtime, unfortunately. Could maybe do something with typing.cast but probably not worth the effort.

else:

async def loop_start_tls(
transport: asyncio.BaseTransport,
protocol: asyncio.BaseProtocol,
sslcontext: ssl.SSLContext = None,
*,
server_side: bool = False,
server_hostname: str = None,
ssl_handshake_timeout: float = None,
) -> asyncio.Transport:
"""Python 3.6 asyncio doesn't have a start_tls() method on the loop
so we use this function in place of the loop's start_tls() method.
Adapted from this comment:
https://github.com/urllib3/urllib3/issues/1323#issuecomment-362494839
"""
import asyncio.sslproto

waiter = loop.create_future()
ssl_protocol = asyncio.sslproto.SSLProtocol(
loop,
protocol,
sslcontext,
waiter,
server_side=False,
server_hostname=server_hostname,
call_connection_made=False,
)

transport.set_protocol(ssl_protocol)
loop.call_soon(ssl_protocol.connection_made, transport)
loop.call_soon(transport.resume_reading) # type: ignore

await waiter
return ssl_protocol._app_transport

transport = await asyncio.wait_for(
loop_start_tls(
transport=transport,
Expand Down
5 changes: 0 additions & 5 deletions tests/test_concurrency.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import sys

import pytest
import trio

Expand Down Expand Up @@ -31,9 +29,6 @@ async def test_start_tls_on_socket_stream(https_server, backend, get_cipher):
See that the concurrency backend can make a connection without TLS then
start TLS on an existing connection.
"""
if isinstance(backend, AsyncioBackend) and sys.version_info < (3, 7):
pytest.xfail(reason="Requires Python 3.7+ for AbstractEventLoop.start_tls()")

ctx = SSLConfig().load_ssl_context_no_verify(HTTPVersionConfig())
timeout = TimeoutConfig(5)

Expand Down