Skip to content

Implement accidental synchronization protection (RFC2762 section 5.2) #773

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bdraco merged 3 commits into from
Jun 20, 2021
Merged

Implement accidental synchronization protection (RFC2762 section 5.2) #773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
523438b
Implement accidental synchronization protection (RFC2762 section 5.2)
bdraco Jun 20, 2021
15d824a
cover
bdraco Jun 20, 2021
afe7530
typing
bdraco Jun 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 44 additions & 2 deletions tests/services/test_browser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
import pytest

import zeroconf as r
from zeroconf import DNSPointer, DNSQuestion, const, current_time_millis
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._services import ServiceStateChange
Expand Down Expand Up @@ -453,7 +453,11 @@ def send(out, addr=const._MDNS_ADDR, port=const._MDNS_PORT):
# patch the backoff limit to prevent test running forever
with unittest.mock.patch.object(zeroconf_browser, "async_send", send), unittest.mock.patch.object(
_services_browser, "current_time_millis", current_time_millis
), unittest.mock.patch.object(_services_browser, "_BROWSER_BACKOFF_LIMIT", 10):
), unittest.mock.patch.object(
_services_browser, "_BROWSER_BACKOFF_LIMIT", 10
), unittest.mock.patch.object(
_services_browser, "_FIRST_QUERY_DELAY_RANDOM_INTERVAL", (0, 0)
):
# dummy service callback
def on_service_state_change(zeroconf, service_type, state_change, name):
pass
Expand Down Expand Up @@ -498,6 +502,44 @@ def on_service_state_change(zeroconf, service_type, state_change, name):
zeroconf_browser.close()


def test_first_query_delay():
"""Verify the first query is delayed.

https://datatracker.ietf.org/doc/html/rfc6762#section-5.2
"""
type_ = "_http._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_query_time = None

def send(out, addr=const._MDNS_ADDR, port=const._MDNS_PORT):
"""Sends an outgoing packet."""
nonlocal first_query_time
if first_query_time is None:
first_query_time = current_time_millis()
old_send(out, addr=addr, port=port)

# patch the zeroconf send
with unittest.mock.patch.object(zeroconf_browser, "async_send", send):
# dummy service callback
def on_service_state_change(zeroconf, service_type, state_change, name):
pass

start_time = current_time_millis()
browser = ServiceBrowser(zeroconf_browser, type_, [on_service_state_change])
time.sleep(millis_to_seconds(_services_browser._FIRST_QUERY_DELAY_RANDOM_INTERVAL[1] + 5))
try:
assert (
current_time_millis() - start_time > _services_browser._FIRST_QUERY_DELAY_RANDOM_INTERVAL[0]
)
finally:
browser.cancel()
zeroconf_browser.close()


def test_integration():
service_added = Event()
service_removed = Event()
Expand Down
26 changes: 22 additions & 4 deletions zeroconf/_services/browser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import concurrent.futures
import contextlib
import queue
import random
import threading
import warnings
from collections import OrderedDict
Expand All @@ -41,7 +42,7 @@
)
from .._utils.aio import get_best_available_queue, get_running_loop
from .._utils.name import service_type_name
from .._utils.time import current_time_millis
from .._utils.time import current_time_millis, millis_to_seconds
from ..const import (
_BROWSER_BACKOFF_LIMIT,
_BROWSER_TIME,
Expand All @@ -56,6 +57,8 @@
_TYPE_PTR,
)

# https://datatracker.ietf.org/doc/html/rfc6762#section-5.2
_FIRST_QUERY_DELAY_RANDOM_INTERVAL = (20, 120) # ms

if TYPE_CHECKING:
# https://github.com/PyCQA/pylint/issues/3525
Expand Down Expand Up @@ -190,14 +193,15 @@ def __init__(
self.addr = addr
self.port = port
self.multicast = self.addr in (None, _MDNS_ADDR, _MDNS_ADDR6)
current_time = current_time_millis()
self._next_time = {check_type_: current_time for check_type_ in self.types}
self._delay = {check_type_: delay for check_type_ in self.types}
self._next_time: Dict[str, float] = {}
self._delay: Dict[str, float] = {check_type_: delay for check_type_ in self.types}
self._pending_handlers: OrderedDict[Tuple[str, str], ServiceStateChange] = OrderedDict()
self._service_state_changed = Signal()
self.queue: Optional[queue.Queue] = None
self.done = False

self._generate_first_next_time()

if hasattr(handlers, 'add_service'):
listener = cast('ServiceListener', handlers)
handlers = None
Expand All @@ -212,6 +216,20 @@ def __init__(

self.zc.add_listener(self, [DNSQuestion(type_, _TYPE_PTR, _CLASS_IN) for type_ in self.types])

def _generate_first_next_time(self) -> None:
"""Generate the initial next query times.

https://datatracker.ietf.org/doc/html/rfc6762#section-5.2
To avoid accidental synchronization when, for some reason, multiple
clients begin querying at exactly the same moment (e.g., because of
some common external trigger event), a Multicast DNS querier SHOULD
also delay the first query of the series by a randomly chosen amount
in the range 20-120 ms.
"""
delay = millis_to_seconds(random.randint(*_FIRST_QUERY_DELAY_RANDOM_INTERVAL))
next_time = current_time_millis() + delay
self._next_time = {check_type_: next_time for check_type_ in self.types}

@property
def service_state_changed(self) -> SignalRegistrationInterface:
return self._service_state_changed.registration_interface
Expand Down