Fix ServiceBrowsers not getting ServiceStateChange.Removed callbacks on PTR record expire (#1064)

bdraco · web-flow · commit 10ee2053a80f · 2022-05-01T11:43:32.000-05:00
diff --git a/tests/services/test_browser.py b/tests/services/test_browser.py
@@ -3,7 +3,6 @@
 
 """ Unit tests for zeroconf._services.browser. """
 
-import asyncio
 import logging
 import socket
 import time
@@ -17,7 +16,7 @@
 import zeroconf as r
 from zeroconf import DNSPointer, DNSQuestion, const, current_time_millis, millis_to_seconds
 import zeroconf._services.browser as _services_browser
-from zeroconf import Zeroconf
+from zeroconf import _core, _handlers, Zeroconf
 from zeroconf._services import ServiceStateChange
 from zeroconf._services.browser import ServiceBrowser
 from zeroconf._services.info import ServiceInfo
@@ -1100,3 +1099,73 @@ def mock_incoming_msg(records) -> r.DNSIncoming:
     browser.cancel()
 
     zc.close()
+
+
+@patch.object(_handlers, '_DNS_PTR_MIN_TTL', 1)
+@patch.object(_core, "_CACHE_CLEANUP_INTERVAL", 10)
+def test_service_browser_expire_callbacks():
+    """Test that the ServiceBrowser matching does not match partial names."""
+    # instantiate a zeroconf instance
+    zc = Zeroconf(interfaces=['127.0.0.1'])
+    # start a browser
+    type_ = "_http._tcp.local."
+    registration_name = "xxxyyy.%s" % type_
+    callbacks = []
+
+    class MyServiceListener(r.ServiceListener):
+        def add_service(self, zc, type_, name) -> None:
+            nonlocal callbacks
+            if name == registration_name:
+                callbacks.append(("add", type_, name))
+
+        def remove_service(self, zc, type_, name) -> None:
+            nonlocal callbacks
+            if name == registration_name:
+                callbacks.append(("remove", type_, name))
+
+        def update_service(self, zc, type_, name) -> None:
+            nonlocal callbacks
+            if name == registration_name:
+                callbacks.append(("update", type_, name))
+
+    listener = MyServiceListener()
+
+    browser = r.ServiceBrowser(zc, type_, None, listener)
+
+    desc = {'path': '/~paulsm/'}
+    address_parsed = "10.0.1.2"
+    address = socket.inet_aton(address_parsed)
+    info = ServiceInfo(
+        type_, registration_name, 80, 0, 0, desc, "ash-2.local.", host_ttl=1, other_ttl=1, addresses=[address]
+    )
+
+    def mock_incoming_msg(records) -> r.DNSIncoming:
+        generated = r.DNSOutgoing(const._FLAGS_QR_RESPONSE)
+        for record in records:
+            generated.add_answer_at_time(record, 0)
+        return r.DNSIncoming(generated.packets()[0])
+
+    _inject_response(
+        zc,
+        mock_incoming_msg([info.dns_pointer(), info.dns_service(), info.dns_text(), *info.dns_addresses()]),
+    )
+    time.sleep(0.2)
+    info.port = 400
+    _inject_response(
+        zc,
+        mock_incoming_msg([info.dns_service()]),
+    )
+
+    assert callbacks == [
+        ('add', type_, registration_name),
+        ('update', type_, registration_name),
+    ]
+    time.sleep(1.2)
+    assert callbacks == [
+        ('add', type_, registration_name),
+        ('update', type_, registration_name),
+        ('remove', type_, registration_name),
+    ]
+    browser.cancel()
+
+    zc.close()
diff --git a/zeroconf/_core.py b/zeroconf/_core.py
@@ -167,7 +167,7 @@ def _async_cache_cleanup(self) -> None:
         now = current_time_millis()
         self.zc.question_history.async_expire(now)
         self.zc.record_manager.async_updates(
-            now, [RecordUpdate(record, None) for record in self.zc.cache.async_expire(now)]
+            now, [RecordUpdate(record, record) for record in self.zc.cache.async_expire(now)]
         )
         self.zc.record_manager.async_updates_complete()
         assert self.loop is not None

Original file line number	Diff line number	Diff line change
`@@ -167,7 +167,7 @@ def _async_cache_cleanup(self) -> None:`
`167`	`167`	`now = current_time_millis()`
`168`	`168`	`self.zc.question_history.async_expire(now)`
`169`	`169`	`self.zc.record_manager.async_updates(`
`170`		`- now, [RecordUpdate(record, None) for record in self.zc.cache.async_expire(now)]`
	`170`	`+ now, [RecordUpdate(record, record) for record in self.zc.cache.async_expire(now)]`
`171`	`171`	`)`
`172`	`172`	`self.zc.record_manager.async_updates_complete()`
`173`	`173`	`assert self.loop is not None`