feat: ensure ServiceInfo.properties always returns bytes (#1333)

bdraco · web-flow · commit d29553ab7de6 · 2023-12-13T13:05:57.000-10:00
diff --git a/src/zeroconf/_services/info.pxd b/src/zeroconf/_services/info.pxd
@@ -72,6 +72,7 @@ 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(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
@@ -191,7 +191,7 @@ 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
         if isinstance(properties, bytes):
             self._set_text(properties)
         else:
@@ -260,14 +260,8 @@ 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:
@@ -356,21 +350,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:
@@ -392,7 +396,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
@@ -133,6 +133,7 @@ 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.addresses == addresses[:1]  # no V6 by default
             assert set(info.addresses_by_version(r.IPVersion.All)) == set(addresses)
 
