From 049fdb8fa3a91891c53ccefdb96f08b93781de7f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 13 Dec 2023 12:46:59 -1000 Subject: [PATCH 1/5] feat: add decoded_properties method to ServiceInfo Clean up the typing on ServiceInfo.properties to always return bytes. The typing on properties was very confusing because it could have a mix of bytes and str types, but only bytes would ever come back from the backend and cache. The only way str could happen is if someone manually passed it in. We now ensure all passed in data is converted to bytes --- src/zeroconf/_services/info.pxd | 5 ++++ src/zeroconf/_services/info.py | 45 +++++++++++++++++++++++++-------- tests/test_services.py | 8 ++++++ 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/src/zeroconf/_services/info.pxd b/src/zeroconf/_services/info.pxd index ec19fcc6e..c53342cbc 100644 --- a/src/zeroconf/_services/info.pxd +++ b/src/zeroconf/_services/info.pxd @@ -53,6 +53,7 @@ cdef class ServiceInfo(RecordUpdateListener): cdef public str server cdef public str server_key cdef public cython.dict _properties + cdef public cython.dict _decoded_properties cdef public object host_ttl cdef public object other_ttl cdef public object interface_index @@ -72,6 +73,10 @@ cdef class ServiceInfo(RecordUpdateListener): @cython.locals(length="unsigned char", index="unsigned int", key_value=bytes, key_sep_value=tuple) cdef void _unpack_text_into_properties(self) + @cython.locals(k=bytes, v=bytes) + cdef void _generate_decoded_properties(self) + + @cython.locals(properties_contain_str=bint) cpdef _set_properties(self, cython.dict properties) cdef _set_text(self, cython.bytes text) diff --git a/src/zeroconf/_services/info.py b/src/zeroconf/_services/info.py index 704c46b64..c998e76fe 100644 --- a/src/zeroconf/_services/info.py +++ b/src/zeroconf/_services/info.py @@ -143,6 +143,7 @@ class ServiceInfo(RecordUpdateListener): "server", "server_key", "_properties", + "_decoded_properties", "host_ttl", "other_ttl", "interface_index", @@ -191,7 +192,8 @@ def __init__( self.priority = priority self.server = server if server else None self.server_key = server.lower() if server else None - self._properties: Optional[Dict[Union[str, bytes], Optional[Union[str, bytes]]]] = None + self._properties: Optional[Dict[bytes, Optional[bytes]]] = None + self._decoded_properties: Optional[Dict[str, Optional[str]]] = None if isinstance(properties, bytes): self._set_text(properties) else: @@ -260,20 +262,23 @@ def addresses(self, value: List[bytes]) -> None: self._ipv6_addresses.append(addr) @property - def properties(self) -> Dict[Union[str, bytes], Optional[Union[str, bytes]]]: - """If properties were set in the constructor this property returns the original dictionary - of type `Dict[Union[bytes, str], Any]`. - - If properties are coming from the network, after decoding a TXT record, the keys are always - bytes and the values are either bytes, if there was a value, even empty, or `None`, if there - was none. No further decoding is attempted. The type returned is `Dict[bytes, Optional[bytes]]`. - """ + def properties(self) -> Dict[bytes, Optional[bytes]]: + """Return properties as bytes.""" if self._properties is None: self._unpack_text_into_properties() if TYPE_CHECKING: assert self._properties is not None return self._properties + @property + def decoded_properties(self) -> Dict[str, Optional[str]]: + """Return properties as strings.""" + if self._decoded_properties is None: + self._generate_decoded_properties() + if TYPE_CHECKING: + assert self._decoded_properties is not None + return self._decoded_properties + def async_clear_cache(self) -> None: """Clear the cache for this service info.""" self._dns_address_cache = None @@ -356,21 +361,31 @@ def parsed_scoped_addresses(self, version: IPVersion = IPVersion.All) -> List[st def _set_properties(self, properties: Dict[Union[str, bytes], Optional[Union[str, bytes]]]) -> None: """Sets properties and text of this info from a dictionary""" - self._properties = properties list_: List[bytes] = [] + properties_contain_str = False result = b'' for key, value in properties.items(): if isinstance(key, str): key = key.encode('utf-8') + properties_contain_str = True record = key if value is not None: if not isinstance(value, bytes): value = str(value).encode('utf-8') + properties_contain_str = True record += b'=' + value list_.append(record) for item in list_: result = b''.join((result, bytes((len(item),)), item)) + if not properties_contain_str: + # If there are no str keys or values, we can use the properties + # as-is, without decoding them, otherwise calling + # self.properties will lazy decode them, which is expensive. + if TYPE_CHECKING: + self._properties = cast("Dict[bytes, Optional[bytes]]", properties) + else: + self._properties = properties self.text = result def _set_text(self, text: bytes) -> None: @@ -381,6 +396,14 @@ def _set_text(self, text: bytes) -> None: # Clear the properties cache self._properties = None + def _generate_decoded_properties(self) -> None: + """Generates decoded properties from the properties""" + decoded_properties: dict[str, Optional[str]] = { + k.decode("ascii", "replace"): None if v is None else v.decode("utf-8", "replace") + for k, v in self.properties.items() + } + self._decoded_properties = decoded_properties + def _unpack_text_into_properties(self) -> None: """Unpacks the text field into properties""" text = self.text @@ -392,7 +415,7 @@ def _unpack_text_into_properties(self) -> None: return index = 0 - properties: Dict[Union[str, bytes], Optional[Union[str, bytes]]] = {} + properties: Dict[bytes, Optional[bytes]] = {} while index < end: length = text[index] index += 1 diff --git a/tests/test_services.py b/tests/test_services.py index e21c23d94..2a8601b00 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -133,6 +133,14 @@ def update_service(self, zeroconf, type, name): assert info.properties[b'prop_blank'] == properties['prop_blank'] assert info.properties[b'prop_true'] == b'1' assert info.properties[b'prop_false'] == b'0' + + assert info.decoded_properties['prop_none'] is None + assert info.decoded_properties['prop_string'] == b'a_prop'.decode('utf-8') + assert info.decoded_properties['prop_float'] == '1.0' + assert info.decoded_properties['prop_blank'] == b'a blanked string'.decode('utf-8') + assert info.decoded_properties['prop_true'] == '1' + assert info.decoded_properties['prop_false'] == '0' + assert info.addresses == addresses[:1] # no V6 by default assert set(info.addresses_by_version(r.IPVersion.All)) == set(addresses) From ff5f519e79ac9657d8b09bb78a4dafdc3c4392e0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 13 Dec 2023 13:05:29 -1000 Subject: [PATCH 2/5] fix: ensure cache is cleared when text is reset --- src/zeroconf/_services/info.py | 1 + tests/test_services.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/zeroconf/_services/info.py b/src/zeroconf/_services/info.py index c998e76fe..7fa4ca0d8 100644 --- a/src/zeroconf/_services/info.py +++ b/src/zeroconf/_services/info.py @@ -395,6 +395,7 @@ def _set_text(self, text: bytes) -> None: self.text = text # Clear the properties cache self._properties = None + self._decoded_properties = None def _generate_decoded_properties(self) -> None: """Generates decoded properties from the properties""" diff --git a/tests/test_services.py b/tests/test_services.py index 2a8601b00..0c9af90d4 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -201,11 +201,13 @@ def update_service(self, zeroconf, type, name): info = zeroconf_browser.get_service_info(type_, registration_name) assert info is not None assert info.properties[b'prop_blank'] == properties['prop_blank'] + assert info.decoded_properties['prop_blank'] == b'a blanked string'.decode('utf-8') cached_info = ServiceInfo(subtype, registration_name) cached_info.load_from_cache(zeroconf_browser) assert cached_info.properties is not None assert cached_info.properties[b'prop_blank'] == properties['prop_blank'] + assert cached_info.decoded_properties['prop_blank'] == b'a blanked string'.decode('utf-8') zeroconf_registrar.unregister_service(info_service) service_removed.wait(1) From ad184c89d63ba346d1eaa519dfbb948a40fd4c61 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 13 Dec 2023 13:12:47 -1000 Subject: [PATCH 3/5] fix: cleanup --- src/zeroconf/_services/info.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/zeroconf/_services/info.py b/src/zeroconf/_services/info.py index 7fa4ca0d8..962e76bff 100644 --- a/src/zeroconf/_services/info.py +++ b/src/zeroconf/_services/info.py @@ -399,11 +399,10 @@ def _set_text(self, text: bytes) -> None: def _generate_decoded_properties(self) -> None: """Generates decoded properties from the properties""" - decoded_properties: dict[str, Optional[str]] = { + self._decoded_properties = { k.decode("ascii", "replace"): None if v is None else v.decode("utf-8", "replace") for k, v in self.properties.items() } - self._decoded_properties = decoded_properties def _unpack_text_into_properties(self) -> None: """Unpacks the text field into properties""" From 96ac9639c10331eab3b32d143f2ab8d64175e30a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 13 Dec 2023 13:17:22 -1000 Subject: [PATCH 4/5] fix: test --- tests/test_services.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_services.py b/tests/test_services.py index 0c9af90d4..19f79e74d 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -207,7 +207,7 @@ def update_service(self, zeroconf, type, name): cached_info.load_from_cache(zeroconf_browser) assert cached_info.properties is not None assert cached_info.properties[b'prop_blank'] == properties['prop_blank'] - assert cached_info.decoded_properties['prop_blank'] == b'a blanked string'.decode('utf-8') + assert cached_info.decoded_properties['prop_blank'] == b'an updated string'.decode('utf-8') zeroconf_registrar.unregister_service(info_service) service_removed.wait(1) From 3e6b2d410532127756994d6bd32567ca51060da1 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 13 Dec 2023 13:21:17 -1000 Subject: [PATCH 5/5] fix: test --- tests/test_services.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_services.py b/tests/test_services.py index 19f79e74d..b7bebfa9b 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -201,7 +201,7 @@ def update_service(self, zeroconf, type, name): info = zeroconf_browser.get_service_info(type_, registration_name) assert info is not None assert info.properties[b'prop_blank'] == properties['prop_blank'] - assert info.decoded_properties['prop_blank'] == b'a blanked string'.decode('utf-8') + assert info.decoded_properties['prop_blank'] == b'an updated string'.decode('utf-8') cached_info = ServiceInfo(subtype, registration_name) cached_info.load_from_cache(zeroconf_browser)