rthalley / dnspython

a powerful DNS toolkit for python
http://www.dnspython.org
Other
2.46k stars 518 forks source link

Timout in async and sync DoH query #978

Open fleurysun opened 1 year ago

fleurysun commented 1 year ago

Sometimes timeout in dns.asyncquery.https and dns.query.https doesn't work as expected, when querying server 75.119.137.220, which in fact isn't a DoH server, it will never timeout, it will hang at calling the_client.get or the_client.post method.

To Reproduce:

import asyncio
import dns.message
import dns.asyncquery
import dns.rdatatype

async def check_doh(ip, query_name, timeout):
    query = dns.message.make_query(query_name, dns.rdatatype.A)
    response = await dns.asyncquery.https(query, ip, verify=False, timeout=timeout)
    print(response)

if __name__ == "__main__":
    asyncio.run(check_doh("75.119.137.220", "dnspython.org", 6))

System info:

rthalley commented 1 year ago

This one seems to be a limitation in the way timeouts work in httpx/httpcore. I'm seeing the correct timeout value get passed into httpcore, but the API only has the concept of individual operation timeouts. I see it trying to get the body of the response, and constantly reading 16KiB chunks of data, and this seems to go on for some time, certainly longer than the 6 second lifetime timeout we would like.

I am not sure how to fix this for the sync API, but for async I can fix it by limiting how long we wait for the future. E.g. if I do this:

async def check_doh(ip, query_name, timeout):
    query = dns.message.make_query(query_name, dns.rdatatype.A)
    response = await asyncio.wait_for(dns.asyncquery.https(query, ip, verify=False), timeout)
    print(response)

it returns after 6 seconds. I can move that kind of logic into dns.asyncquery.https().

fleurysun commented 1 year ago

Thank you! Yep, the httpx API timeouts aren't fully supported, as mentioned in encode/httpx#2658

rthalley commented 1 year ago

I'm leaving this ticket open so we remember to fix this for sync if httpx gives us a way to do so in the future. I made fixes for asyncio and trio, and they were included in the 2.4.2 release.

rthalley commented 2 months ago

httpcore has a PR (936) for this; it adds an extension to the request called "timeout" with an overall timeout value. We can use this if it gets merged.