Handle duplicate goodbye answers in the same packet (#928)

bdraco · web-flow · commit 97e0b669be60 · 2021-07-28T10:07:59.000-05:00
- Solves an exception being thrown when we tried to remove the known answer from the cache when the second goodbye answer in the same packet was processed - We previously swallowed all exceptions on cache removal so this was not visible until 0.32.x which removed the broad exception catch Fixes #926
diff --git a/tests/test_handlers.py b/tests/test_handlers.py
@@ -1122,3 +1122,35 @@ async def test_guard_against_low_ptr_ttl():
     assert incoming_answer_normal.ttl == const._DNS_OTHER_TTL
     assert zc.cache.async_get_unique(good_bye_answer) is None
     await aiozc.async_close()
+
+
+@pytest.mark.asyncio
+async def test_duplicate_goodbye_answers_in_packet():
+    """Ensure we do not throw an exception when there are duplicate goodbye records in a packet."""
+    aiozc = AsyncZeroconf(interfaces=['127.0.0.1'])
+    zc = aiozc.zeroconf
+    answer_with_normal_ttl = r.DNSPointer(
+        "myservicelow_tcp._tcp.local.",
+        const._TYPE_PTR,
+        const._CLASS_IN | const._CLASS_UNIQUE,
+        const._DNS_OTHER_TTL,
+        'host.local.',
+    )
+    good_bye_answer = r.DNSPointer(
+        "myservicelow_tcp._tcp.local.",
+        const._TYPE_PTR,
+        const._CLASS_IN | const._CLASS_UNIQUE,
+        0,
+        'host.local.',
+    )
+    response = r.DNSOutgoing(const._FLAGS_QR_RESPONSE)
+    response.add_answer_at_time(answer_with_normal_ttl, 0)
+    incoming = r.DNSIncoming(response.packets()[0])
+    zc.record_manager.async_updates_from_response(incoming)
+
+    response = r.DNSOutgoing(const._FLAGS_QR_RESPONSE)
+    response.add_answer_at_time(good_bye_answer, 0)
+    response.add_answer_at_time(good_bye_answer, 0)
+    incoming = r.DNSIncoming(response.packets()[0])
+    zc.record_manager.async_updates_from_response(incoming)
+    await aiozc.async_close()
diff --git a/zeroconf/_handlers.py b/zeroconf/_handlers.py
@@ -331,7 +331,7 @@ def async_updates_from_response(self, msg: DNSIncoming) -> None:
         updates: List[RecordUpdate] = []
         address_adds: List[DNSAddress] = []
         other_adds: List[DNSRecord] = []
-        removes: List[DNSRecord] = []
+        removes: Set[DNSRecord] = set()
         now = msg.now
         unique_types: Set[Tuple[str, int, int]] = set()
 
@@ -355,7 +355,7 @@ def async_updates_from_response(self, msg: DNSIncoming) -> None:
             # expired and exists in the cache
             elif maybe_entry is not None:
                 updates.append(RecordUpdate(record, maybe_entry))
-                removes.append(record)
+                removes.add(record)
 
         if unique_types:
             self._async_mark_unique_cached_records_older_than_1s_to_expire(unique_types, msg.answers, now)
