Skip to content

Commit

Permalink
Async dns error (#2237)
Browse files Browse the repository at this point in the history
* Fix #2231: raise OSError if DNS lookup found no hosts

* Add tests

* Add changes
  • Loading branch information
asvetlov authored Sep 1, 2017
1 parent d0000cd commit 1cfc612
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 8 deletions.
26 changes: 20 additions & 6 deletions aiohttp/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,38 +60,52 @@ def __init__(self, loop=None, *args, **kwargs):

if not hasattr(self._resolver, 'gethostbyname'):
# aiodns 1.1 is not available, fallback to DNSResolver.query
self.resolve = self.resolve_with_query
self.resolve = self._resolve_with_query

@asyncio.coroutine
def resolve(self, host, port=0, family=socket.AF_INET):
try:
resp = yield from self._resolver.gethostbyname(host, family)
except aiodns.error.DNSError as exc:
msg = exc.args[1] if len(exc.args) >= 1 else "DNS lookup failed"
raise OSError(msg) from exc
hosts = []
resp = yield from self._resolver.gethostbyname(host, family)

for address in resp.addresses:
hosts.append(
{'hostname': host,
'host': address, 'port': port,
'family': family, 'proto': 0,
'flags': socket.AI_NUMERICHOST})

if not hosts:
raise OSError("DNS lookup failed")

return hosts

@asyncio.coroutine
def resolve_with_query(self, host, port=0, family=socket.AF_INET):
def _resolve_with_query(self, host, port=0, family=socket.AF_INET):
if family == socket.AF_INET6:
qtype = 'AAAA'
else:
qtype = 'A'

hosts = []
resp = yield from self._resolver.query(host, qtype)
try:
resp = yield from self._resolver.query(host, qtype)
except aiodns.error.DNSError as exc:
msg = exc.args[1] if len(exc.args) >= 1 else "DNS lookup failed"
raise OSError(msg) from exc

hosts = []
for rr in resp:
hosts.append(
{'hostname': host,
'host': rr.host, 'port': port,
'family': family, 'proto': 0,
'flags': socket.AI_NUMERICHOST})

if not hosts:
raise OSError("DNS lookup failed")

return hosts

@asyncio.coroutine
Expand Down
2 changes: 2 additions & 0 deletions changes/2231.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Raise OSError on async DNS lookup if resolved domain is an alias for
another one, which does not have an A or CNAME record.
25 changes: 23 additions & 2 deletions tests/test_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ def test_async_resolver_negative_lookup(loop):
with patch('aiodns.DNSResolver') as mock:
mock().gethostbyname.side_effect = aiodns.error.DNSError()
resolver = AsyncResolver(loop=loop)
with pytest.raises(aiodns.error.DNSError):
with pytest.raises(OSError):
yield from resolver.resolve('doesnotexist.bla')


Expand All @@ -114,7 +114,28 @@ def test_async_resolver_query_negative_lookup(loop):
del mock().gethostbyname
mock().query.side_effect = aiodns.error.DNSError()
resolver = AsyncResolver(loop=loop)
with pytest.raises(aiodns.error.DNSError):
with pytest.raises(OSError):
yield from resolver.resolve('doesnotexist.bla')


@pytest.mark.skipif(aiodns is None, reason="aiodns required")
@asyncio.coroutine
def test_async_resolver_no_hosts_in_query(loop):
with patch('aiodns.DNSResolver') as mock:
del mock().gethostbyname
mock().query.return_value = fake_query_result([])
resolver = AsyncResolver(loop=loop)
with pytest.raises(OSError):
yield from resolver.resolve('doesnotexist.bla')


@pytest.mark.skipif(not gethostbyname, reason="aiodns 1.1 required")
@asyncio.coroutine
def test_async_resolver_no_hosts_in_gethostbyname(loop):
with patch('aiodns.DNSResolver') as mock:
mock().gethostbyname.return_value = fake_result([])
resolver = AsyncResolver(loop=loop)
with pytest.raises(OSError):
yield from resolver.resolve('doesnotexist.bla')


Expand Down

0 comments on commit 1cfc612

Please sign in to comment.