Httpx: Don't use SNI when host is an IP Address

[sethmlarson]

Per the SNI spec we shouldn't be using SNI when the hostname is an IP address.

[florimondmanca]

Just wondering — what is SNI? 😄 Also, is there any more context on how this bug was discovered?

[sethmlarson]

Server Name Identification (RFC 6066 Section 3) which disallows IP addresses (see the RFC search for "Literal IPv4"). It's set via server_hostname in Trio for example.

Also I found this issue cuz I fixed it for urllib3. :)

[jlaine]

How sure are you that this issue actually exists in httpx?

Unless I'm mistaken connections are established using asyncio.open_connection which AFAICT does the Right Thing.

I had a look with wireshark, and I only see the server_name extension in the ClientHello if I query a host name. If I connect to an IP address, the server_name extension is absent.

[florimondmanca]

@sethmlarson I get the same behavior than @jlaine for the TrioBackend (#276): server_name is absent from the ClientHello if connecting to an IP address:

import trio
import httpx
from httpx.concurrency.trio import TrioBackend


async def main(url):
    async with httpx.AsyncClient(backend=TrioBackend()) as client:
        await client.get(url)


trio.run(main, "https://8.8.8.8")

[sethmlarson]

Nice, then we can probably close this issue.

[jlaine]

This would definitely need to be re-visited if we ever start using server_hostname for asyncio or trio.

[sethmlarson]

So do we not send SNI for any handshakes? Because that's an issue in itself.

[jlaine]

We do send SNI when querying an FQDN, and don't when querying an IP (at least for asyncio). If you don't force asyncio's hand by setting server_hostname it does the right thing on its own.