Skip to content

Commit 90add95

Browse files
authored
Bring async receive_udp ignore_errors in line with the sync one (#1297)
ignore_errors means "discard the datagram and keep listening for a valid response", but dns.asyncquery.receive_udp() passed continue_on_error to from_wire(), so a datagram with a valid header and an unparsable body was returned with its errors recorded instead of being skipped. Also removes the ignore_errors parameter added to dns.asyncquery.receive_tcp() at the same time; dns.query.receive_tcp() has no such parameter and nothing passes it. Co-authored-by: Dylan Pulver <dylanpulver@users.noreply.github.com>
1 parent 3ba78ab commit 90add95

4 files changed

Lines changed: 56 additions & 3 deletions

File tree

dns/asyncquery.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,6 @@ async def receive_udp(
160160
one_rr_per_rrset=one_rr_per_rrset,
161161
ignore_trailing=ignore_trailing,
162162
raise_on_truncation=raise_on_truncation,
163-
continue_on_error=ignore_errors,
164163
)
165164
except dns.message.Truncated as e:
166165
# See the comment in query.py for details.
@@ -363,7 +362,6 @@ async def receive_tcp(
363362
keyring: dict[dns.name.Name, dns.tsig.Key] | None = None,
364363
request_mac: bytes | None = b"",
365364
ignore_trailing: bool = False,
366-
ignore_errors: bool = False,
367365
) -> tuple[dns.message.Message, float]:
368366
"""Read a DNS message from a TCP socket.
369367
@@ -384,7 +382,6 @@ async def receive_tcp(
384382
request_mac=request_mac,
385383
one_rr_per_rrset=one_rr_per_rrset,
386384
ignore_trailing=ignore_trailing,
387-
continue_on_error=ignore_errors,
388385
)
389386
return (r, received_time)
390387

doc/whatsnew.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,13 @@ TBD
6060

6161
* The async `zone_from_name()` function now behaves the same as the sync version.
6262

63+
* `dns.asyncquery.receive_udp()` with ``ignore_errors`` set again discards a datagram
64+
that fails to parse and keeps listening for a valid response, matching
65+
`dns.query.receive_udp()`. It had been returning the unparsable datagram with its
66+
errors recorded on the message. The unused ``ignore_errors`` parameter added to
67+
`dns.asyncquery.receive_tcp()` at the same time has been removed, as the sync
68+
version has no such parameter.
69+
6370
* Documentation has been augmented and modernized.
6471

6572
* The HHIT and BRID rdata types are now supported, and the NXNAME metatype is defined.

tests/test_async.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -972,6 +972,31 @@ async def run():
972972

973973
self.async_run(run)
974974

975+
def test_unparsable_wire_is_skipped_not_salvaged(self):
976+
# ignore_errors means "keep listening for a valid response", so a
977+
# datagram that fails to parse must be discarded rather than returned
978+
# with its parse errors recorded. The first datagram below has a
979+
# valid header and a different rcode, so returning it instead of the
980+
# second one is detectable.
981+
async def run():
982+
bad_r = dns.message.make_response(self.q)
983+
bad_r.set_rcode(dns.rcode.SERVFAIL)
984+
bad_r_wire = bad_r.to_wire() + b"abcd"
985+
s = MockSock(
986+
bad_r_wire, ("127.0.0.1", 53), self.good_r_wire, ("127.0.0.1", 53)
987+
)
988+
r, _, _ = await dns.asyncquery.receive_udp(
989+
s,
990+
("127.0.0.1", 53),
991+
time.time() + 2,
992+
ignore_errors=True,
993+
query=self.q,
994+
)
995+
self.assertEqual(r, self.good_r)
996+
self.assertEqual(r.errors, [])
997+
998+
self.async_run(run)
999+
9751000
def test_trailing_wire_not_ignored(self):
9761001
wire = self.good_r_wire + b"abcd"
9771002

tests/test_query.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -864,6 +864,30 @@ def test_trailing_wire(self):
864864
wire = self.good_r_wire + b"abcd"
865865
self.mock_receive(wire, ("127.0.0.1", 53), self.good_r_wire, ("127.0.0.1", 53))
866866

867+
def test_unparsable_wire_is_skipped_not_salvaged(self):
868+
# The twin of the dns.asyncquery test of the same name. The first
869+
# datagram has a valid header and a different rcode, so returning it
870+
# instead of the second one is detectable.
871+
bad_r = dns.message.make_response(self.q)
872+
bad_r.set_rcode(dns.rcode.SERVFAIL)
873+
bad_r_wire = bad_r.to_wire() + b"abcd"
874+
s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
875+
try:
876+
with mock_udp_recv(
877+
bad_r_wire, ("127.0.0.1", 53), self.good_r_wire, ("127.0.0.1", 53)
878+
):
879+
r, _ = dns.query.receive_udp(
880+
s,
881+
("127.0.0.1", 53),
882+
time.time() + 2,
883+
ignore_errors=True,
884+
query=self.q,
885+
)
886+
self.assertEqual(r, self.good_r)
887+
self.assertEqual(r.errors, [])
888+
finally:
889+
s.close()
890+
867891
def test_trailing_wire_not_ignored(self):
868892
wire = self.good_r_wire + b"abcd"
869893

0 commit comments

Comments
 (0)