New ServiceBrowsers now request QU in the first outgoing when unspecified (#812)

bdraco · web-flow · commit e32bb5d98be0 · 2021-06-22T13:24:56.000-10:00
diff --git a/tests/services/test_browser.py b/tests/services/test_browser.py
@@ -544,19 +544,23 @@ def on_service_state_change(zeroconf, service_type, state_change, name):
             zeroconf_browser.close()
 
 
-def test_asking_default_is_asking_qm_questions():
-    """Verify the service browser can ask QU questions."""
+def test_asking_default_is_asking_qm_questions_after_the_first_qu():
+    """Verify the service browser's first question is QU and subsequent ones are QM questions."""
     type_ = "_quservice._tcp.local."
     zeroconf_browser = Zeroconf(interfaces=['127.0.0.1'])
 
     # we are going to patch the zeroconf send to check query transmission
     old_send = zeroconf_browser.async_send
 
     first_outgoing = None
+    second_outgoing = None
 
     def send(out, addr=const._MDNS_ADDR, port=const._MDNS_PORT):
         """Sends an outgoing packet."""
         nonlocal first_outgoing
+        nonlocal second_outgoing
+        if first_outgoing is not None and second_outgoing is None:
+            second_outgoing = out
         if first_outgoing is None:
             first_outgoing = out
         old_send(out, addr=addr, port=port)
@@ -567,10 +571,11 @@ def send(out, addr=const._MDNS_ADDR, port=const._MDNS_PORT):
         def on_service_state_change(zeroconf, service_type, state_change, name):
             pass
 
-        browser = ServiceBrowser(zeroconf_browser, type_, [on_service_state_change])
-        time.sleep(millis_to_seconds(_services_browser._FIRST_QUERY_DELAY_RANDOM_INTERVAL[1] + 5))
+        browser = ServiceBrowser(zeroconf_browser, type_, [on_service_state_change], delay=5)
+        time.sleep(millis_to_seconds(_services_browser._FIRST_QUERY_DELAY_RANDOM_INTERVAL[1] + 120 + 5))
         try:
-            assert first_outgoing.questions[0].unicast == False
+            assert first_outgoing.questions[0].unicast == True
+            assert second_outgoing.questions[0].unicast == False
         finally:
             browser.cancel()
             zeroconf_browser.close()
@@ -1016,7 +1021,7 @@ async def test_generate_service_query_suppress_duplicate_questions():
     aiozc = AsyncZeroconf(interfaces=['127.0.0.1'])
     zc = aiozc.zeroconf
     now = current_time_millis()
-    name = "_hap._tcp.local."
+    name = "_suppresstest._tcp.local."
     question = r.DNSQuestion(name, const._TYPE_PTR, const._CLASS_IN)
     answer = r.DNSPointer(
         name,
@@ -1048,7 +1053,7 @@ async def test_generate_service_query_suppress_duplicate_questions():
     outs = _services_browser.generate_service_query(zc, now, [name], multicast=False)
     assert outs
 
-    zc.question_history.async_expire(now + 1000)
+    zc.question_history.async_expire(now + 2000)
     # No suppression after clearing the history
     outs = _services_browser.generate_service_query(zc, now, [name], multicast=True)
     assert outs
diff --git a/zeroconf/_services/browser.py b/zeroconf/_services/browser.py
@@ -146,11 +146,14 @@ def generate_service_query(
             for record in zc.cache.get_all_by_details(type_, _TYPE_PTR, _CLASS_IN)
             if not record.is_stale(now)
         )
-        if multicast and zc.question_history.suppresses(question, now, cast(Set[DNSRecord], known_answers)):
+        if not qu_question and zc.question_history.suppresses(
+            question, now, cast(Set[DNSRecord], known_answers)
+        ):
             log.debug("Asking %s was suppressed by the question history", question)
             continue
         questions_with_known_answers[question] = known_answers
-        zc.question_history.add_question_at_time(question, now, cast(Set[DNSRecord], known_answers))
+        if not qu_question:
+            zc.question_history.add_question_at_time(question, now, cast(Set[DNSRecord], known_answers))
 
     return _group_ptr_queries_with_known_answers(now, multicast, questions_with_known_answers)
 
@@ -379,7 +382,7 @@ def _async_cancel(self) -> None:
         self.done = True
         self.zc.async_remove_listener(self)
 
-    def generate_ready_queries(self) -> List[DNSOutgoing]:
+    def _generate_ready_queries(self, first_request: bool) -> List[DNSOutgoing]:
         """Generate the service browser query for any type that is due."""
         now = current_time_millis()
         if self._millis_to_wait(current_time_millis()):
@@ -395,7 +398,13 @@ def generate_ready_queries(self) -> List[DNSOutgoing]:
             self._next_time[type_] = now + self._delay[type_]
             self._delay[type_] = min(_BROWSER_BACKOFF_LIMIT * 1000, self._delay[type_] * 2)
 
-        return generate_service_query(self.zc, now, ready_types, self.multicast, self.question_type)
+        # If they did not specify and this is the first request, ask QU questions
+        # https://datatracker.ietf.org/doc/html/rfc6762#section-5.4 since we are
+        # just starting up and we know our cache is likely empty. This ensures
+        # the next outgoing will be sent with the known answers list.
+        question_type = DNSQuestionType.QU if not self.question_type and first_request else self.question_type
+
+        return generate_service_query(self.zc, now, ready_types, self.multicast, question_type)
 
     def _millis_to_wait(self, now: float) -> Optional[float]:
         """Returns the number of milliseconds to wait for the next event."""
@@ -406,14 +415,17 @@ def _millis_to_wait(self, now: float) -> Optional[float]:
     async def async_browser_task(self) -> None:
         """Run the browser task."""
         await self.zc.async_wait_for_start()
+        first_request = True
         while True:
             timeout = self._millis_to_wait(current_time_millis())
             if timeout:
                 await self.zc.async_wait(timeout)
 
-            outs = self.generate_ready_queries()
-            for out in outs:
-                self.zc.async_send(out, addr=self.addr, port=self.port)
+            outs = self._generate_ready_queries(first_request)
+            if outs:
+                first_request = False
+                for out in outs:
+                    self.zc.async_send(out, addr=self.addr, port=self.port)
 
     async def _async_cancel_browser(self) -> None:
         """Cancel the browser."""
