V4.9.2 (#482)

* Late-import base36 and QR code libraries; remove SUPPORT_QR_CODE flag * Increase idle connection check interval to 300s (#475) This check was creating a lot of TimerHandles when the user had multiple bridges. We do not need to check very often as connections usually stay around for 24+hours * Implement zerocopy writes for the encrypted protocol (#476) * Implement zerocopy writes for the encrypted protocol With Python 3.12+ and later `transport.writelines` is implemented as [`sendmsg(..., IOV_MAX)`](python/cpython#91166) which allows us to avoid joining the bytes and sending them in one go. Older Python will effectively do the same thing we do now `b"".join(...)` * update tests * Revert "Late-import base36 and QR code libraries; remove SUPPORT_QR_CODE flag" (#477) * Avoid os.chmod failing on Windows if file non-existant (#471) * Avoid os.chmod failing on Windows if file non-existant * Update accessory_driver.py --------- Co-authored-by: Ivan Kalchev <25887324+ikalchev@users.noreply.github.com> * Fix mdns tests (#478) * Fix pylint complaints (#480) * Address remaining pylint complaints (#481) * Address remaining pylint complaints * Address remaining pylint complaints * v4.9.2 --------- Co-authored-by: Aarni Koskela <akx@iki.fi> Co-authored-by: J. Nick Koston <nick@koston.org> Co-authored-by: Perry Kundert <perry@kundert.ca> Co-authored-by: Ivan Kalchev <ii.kalchev@gmail.com>
ikalchev · Nov 3, 2024 · 34c9659 · 34c9659
1 parent 5265b54
commit 34c9659
Show file tree

Hide file tree

Showing 12 changed files with 65 additions and 53 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,11 @@ Sections
 ### Developers
 -->
 
+## [4.9.2] - 2024-11-03
+
+- Implement zerocopy writes for the encrypted protocol. [#476](https://github.com/ikalchev/HAP-python/pull/476)
+- Linter and test fixe.
+
 ## [4.9.1] - 2023-10-25
 
 - Fix handling of explict close. [#467](https://github.com/ikalchev/HAP-python/pull/467)

diff --git a/pyhap/accessory.py b/pyhap/accessory.py
@@ -207,7 +207,7 @@ def xhm_uri(self) -> str:
             int(self.driver.state.pincode.replace(b"-", b""), 10) & 0x7FFFFFFF
         )  # pincode
 
-        encoded_payload = base36.dumps(payload).upper()
+        encoded_payload = base36.dumps(payload).upper()  # pylint: disable=possibly-used-before-assignment
         encoded_payload = encoded_payload.rjust(9, "0")
 
         return "X-HM://" + encoded_payload + self.driver.state.setup_id

diff --git a/pyhap/accessory_driver.py b/pyhap/accessory_driver.py
@@ -323,7 +323,7 @@ def start(self):
                 and os.name != "nt"
             ):
                 logger.debug("Setting child watcher")
-                watcher = asyncio.SafeChildWatcher()
+                watcher = asyncio.SafeChildWatcher() # pylint: disable=deprecated-class
                 watcher.attach_loop(self.loop)
                 asyncio.set_child_watcher(watcher)
             else:
@@ -642,16 +642,19 @@ def persist(self):
         tmp_filename = None
         try:
             temp_dir = os.path.dirname(self.persist_file)
+            logger.debug("Creating temp persist file in '%s'", temp_dir)
             with tempfile.NamedTemporaryFile(
                 mode="w", dir=temp_dir, delete=False
             ) as file_handle:
                 tmp_filename = file_handle.name
+                logger.debug("Created temp persist file '%s' named '%s'", file_handle, tmp_filename)
                 self.encoder.persist(file_handle, self.state)
             if (
                 os.name == "nt"
             ):  # Or `[WinError 5] Access Denied` will be raised on Windows
                 os.chmod(tmp_filename, 0o644)
-                os.chmod(self.persist_file, 0o644)
+                if os.path.exists(self.persist_file):
+                    os.chmod(self.persist_file, 0o644)
             os.replace(tmp_filename, self.persist_file)
         except Exception:  # pylint: disable=broad-except
             logger.exception("Failed to persist accessory state")

diff --git a/pyhap/const.py b/pyhap/const.py
@@ -1,7 +1,7 @@
 """This module contains constants used by other modules."""
 MAJOR_VERSION = 4
 MINOR_VERSION = 9
-PATCH_VERSION = 1
+PATCH_VERSION = 2
 __short_version__ = f"{MAJOR_VERSION}.{MINOR_VERSION}"
 __version__ = f"{__short_version__}.{PATCH_VERSION}"
 REQUIRED_PYTHON_VER = (3, 7)

diff --git a/pyhap/hap_crypto.py b/pyhap/hap_crypto.py
@@ -3,7 +3,7 @@
 import logging
 import struct
 from struct import Struct
-from typing import List
+from typing import Iterable, List
 
 from chacha20poly1305_reuseable import ChaCha20Poly1305Reusable as ChaCha20Poly1305
 from cryptography.hazmat.backends import default_backend
@@ -112,7 +112,7 @@ def decrypt(self) -> bytes:
 
         return result
 
-    def encrypt(self, data: bytes) -> bytes:
+    def encrypt(self, data: bytes) -> Iterable[bytes]:
         """Encrypt and send the return bytes."""
         result: List[bytes] = []
         offset = 0
@@ -127,7 +127,4 @@ def encrypt(self, data: bytes) -> bytes:
             offset += length
             self._out_count += 1
 
-        # Join the result once instead of concatenating each time
-        # as this is much faster than generating an new immutable
-        # byte string each time.
-        return b"".join(result)
+        return result
diff --git a/pyhap/hap_protocol.py b/pyhap/hap_protocol.py
@@ -104,7 +104,7 @@ def write(self, data: bytes) -> None:
                 self.handler.client_uuid,
                 data,
             )
-            self.transport.write(result)
+            self.transport.writelines(result)
         else:
             logger.debug(
                 "%s (%s): Send unencrypted: %s",

diff --git a/pyhap/hap_server.py b/pyhap/hap_server.py
@@ -16,7 +16,7 @@
 
 logger = logging.getLogger(__name__)
 
-IDLE_CONNECTION_CHECK_INTERVAL_SECONDS = 120
+IDLE_CONNECTION_CHECK_INTERVAL_SECONDS = 300
 
 
 class HAPServer:

diff --git a/pyproject.toml b/pyproject.toml
@@ -77,6 +77,7 @@ disable = [
     "too-many-return-statements",
     "too-many-statements",
     "too-many-boolean-expressions",
+    "too-many-positional-arguments",
     "unused-argument",
     "wrong-import-order",
     "unused-argument",

diff --git a/tests/test_accessory_driver.py b/tests/test_accessory_driver.py
@@ -961,7 +961,7 @@ def test_mdns_service_info(driver: AccessoryDriver):
     assert mdns_info.server == "Test-Accessory-000000.local."
     assert mdns_info.port == port
     assert mdns_info.addresses == [b"\xac\x00\x00\x01"]
-    assert mdns_info.properties == {
+    assert mdns_info.decoded_properties == {
         "md": "Test Accessory",
         "pv": "1.1",
         "id": "00:00:00:00:00:00",
@@ -990,7 +990,7 @@ def test_mdns_service_info_with_specified_server(driver: AccessoryDriver):
     assert mdns_info.server == "hap1.local."
     assert mdns_info.port == port
     assert mdns_info.addresses == [b"\xac\x00\x00\x01"]
-    assert mdns_info.properties == {
+    assert mdns_info.decoded_properties == {
         "md": "Test Accessory",
         "pv": "1.1",
         "id": "00:00:00:00:00:00",

diff --git a/tests/test_hap_crypto.py b/tests/test_hap_crypto.py
@@ -15,7 +15,7 @@ def test_round_trip():
     crypto.OUT_CIPHER_INFO = crypto.IN_CIPHER_INFO
     crypto.reset(key)
 
-    encrypted = bytearray(crypto.encrypt(plaintext))
+    encrypted = bytearray(b"".join(crypto.encrypt(plaintext)))
 
     # Receive no data
     assert crypto.decrypt() == b""

diff --git a/tests/test_hap_protocol.py b/tests/test_hap_protocol.py
@@ -246,13 +246,13 @@ def test_get_accessories_with_crypto(driver):
     hap_proto.hap_crypto = MockHAPCrypto()
     hap_proto.handler.is_encrypted = True
 
-    with patch.object(hap_proto.transport, "write") as writer:
+    with patch.object(hap_proto.transport, "writelines") as writelines:
         hap_proto.data_received(
             b"GET /accessories HTTP/1.1\r\nHost: Bridge\\032C77C47._hap._tcp.local\r\n\r\n"  # pylint: disable=line-too-long
         )
 
     hap_proto.close()
-    assert b"accessories" in writer.call_args_list[0][0][0]
+    assert b"accessories" in b"".join(writelines.call_args_list[0][0])
 
 
 def test_get_characteristics_with_crypto(driver):
@@ -273,7 +273,7 @@ def test_get_characteristics_with_crypto(driver):
     hap_proto.hap_crypto = MockHAPCrypto()
     hap_proto.handler.is_encrypted = True
 
-    with patch.object(hap_proto.transport, "write") as writer:
+    with patch.object(hap_proto.transport, "writelines") as writelines:
         hap_proto.data_received(
             b"GET /characteristics?id=3762173001.7 HTTP/1.1\r\nHost: HASS\\032Bridge\\032YPHW\\032B223AD._hap._tcp.local\r\n\r\n"  # pylint: disable=line-too-long
         )
@@ -282,13 +282,15 @@ def test_get_characteristics_with_crypto(driver):
         )
 
     hap_proto.close()
-    assert b"Content-Length:" in writer.call_args_list[0][0][0]
-    assert b"Transfer-Encoding: chunked\r\n\r\n" not in writer.call_args_list[0][0][0]
-    assert b"-70402" in writer.call_args_list[0][0][0]
+    joined0 = b"".join(writelines.call_args_list[0][0])
+    assert b"Content-Length:" in joined0
+    assert b"Transfer-Encoding: chunked\r\n\r\n" not in joined0
+    assert b"-70402" in joined0
 
-    assert b"Content-Length:" in writer.call_args_list[1][0][0]
-    assert b"Transfer-Encoding: chunked\r\n\r\n" not in writer.call_args_list[1][0][0]
-    assert b"TestAcc" in writer.call_args_list[1][0][0]
+    joined1 = b"".join(writelines.call_args_list[1][0])
+    assert b"Content-Length:" in joined1
+    assert b"Transfer-Encoding: chunked\r\n\r\n" not in joined1
+    assert b"TestAcc" in joined1
 
 
 def test_set_characteristics_with_crypto(driver):
@@ -309,13 +311,15 @@ def test_set_characteristics_with_crypto(driver):
     hap_proto.hap_crypto = MockHAPCrypto()
     hap_proto.handler.is_encrypted = True
 
-    with patch.object(hap_proto.transport, "write") as writer:
+    with patch.object(hap_proto.transport, "writelines") as writelines:
         hap_proto.data_received(
             b'PUT /characteristics HTTP/1.1\r\nHost: HASS12\\032AD1C22._hap._tcp.local\r\nContent-Length: 49\r\nContent-Type: application/hap+json\r\n\r\n{"characteristics":[{"aid":1,"iid":9,"ev":true}]}'  # pylint: disable=line-too-long
         )
 
     hap_proto.close()
-    assert writer.call_args_list[0][0][0] == b"HTTP/1.1 204 No Content\r\n\r\n"
+    assert (
+        b"".join(writelines.call_args_list[0][0]) == b"HTTP/1.1 204 No Content\r\n\r\n"
+    )
 
 
 def test_crypto_failure_closes_connection(driver):
@@ -352,14 +356,14 @@ def test_empty_encrypted_data(driver):
 
     hap_proto.hap_crypto = MockHAPCrypto()
     hap_proto.handler.is_encrypted = True
-    with patch.object(hap_proto.transport, "write") as writer:
+    with patch.object(hap_proto.transport, "writelines") as writelines:
         hap_proto.data_received(b"")
         hap_proto.data_received(
             b"GET /accessories HTTP/1.1\r\nHost: Bridge\\032C77C47._hap._tcp.local\r\n\r\n"  # pylint: disable=line-too-long
         )
 
     hap_proto.close()
-    assert b"accessories" in writer.call_args_list[0][0][0]
+    assert b"accessories" in b"".join(writelines.call_args_list[0][0])
 
 
 def test_http_11_keep_alive(driver):
@@ -434,13 +438,13 @@ def test_camera_snapshot_without_snapshot_support(driver):
     hap_proto.hap_crypto = MockHAPCrypto()
     hap_proto.handler.is_encrypted = True
 
-    with patch.object(hap_proto.transport, "write") as writer:
+    with patch.object(hap_proto.transport, "writelines") as writelines:
         hap_proto.data_received(
             b'POST /resource HTTP/1.1\r\nHost: HASS\\032Bridge\\032BROZ\\0323BF435._hap._tcp.local\r\nContent-Length: 79\r\nContent-Type: application/hap+json\r\n\r\n{"image-height":360,"resource-type":"image","image-width":640,"aid":1411620844}'  # pylint: disable=line-too-long
         )
 
     hap_proto.close()
-    assert b"-70402" in writer.call_args_list[0][0][0]
+    assert b"-70402" in b"".join(writelines.call_args_list[0][0])
 
 
 @pytest.mark.asyncio
@@ -464,14 +468,14 @@ def _get_snapshot(*_):
     hap_proto.hap_crypto = MockHAPCrypto()
     hap_proto.handler.is_encrypted = True
 
-    with patch.object(hap_proto.transport, "write") as writer:
+    with patch.object(hap_proto.transport, "writelines") as writelines:
         hap_proto.data_received(
             b'POST /resource HTTP/1.1\r\nHost: HASS\\032Bridge\\032BROZ\\0323BF435._hap._tcp.local\r\nContent-Length: 79\r\nContent-Type: application/hap+json\r\n\r\n{"image-height":360,"resource-type":"image","image-width":640,"aid":1411620844}'  # pylint: disable=line-too-long
         )
         await hap_proto.response.task
         await asyncio.sleep(0)
 
-    assert b"fakesnap" in writer.call_args_list[0][0][0]
+    assert b"fakesnap" in b"".join(writelines.call_args_list[0][0])
 
     hap_proto.close()
 
@@ -497,14 +501,14 @@ async def _async_get_snapshot(*_):
     hap_proto.hap_crypto = MockHAPCrypto()
     hap_proto.handler.is_encrypted = True
 
-    with patch.object(hap_proto.transport, "write") as writer:
+    with patch.object(hap_proto.transport, "writelines") as writelines:
         hap_proto.data_received(
             b'POST /resource HTTP/1.1\r\nHost: HASS\\032Bridge\\032BROZ\\0323BF435._hap._tcp.local\r\nContent-Length: 79\r\nContent-Type: application/hap+json\r\n\r\n{"image-height":360,"resource-type":"image","image-width":640,"aid":1411620844}'  # pylint: disable=line-too-long
         )
         await hap_proto.response.task
         await asyncio.sleep(0)
 
-    assert b"fakesnap" in writer.call_args_list[0][0][0]
+    assert b"fakesnap" in b"".join(writelines.call_args_list[0][0])
 
     hap_proto.close()
 
@@ -532,14 +536,14 @@ async def _async_get_snapshot(*_):
     hap_proto.handler.is_encrypted = True
 
     with patch.object(hap_handler, "RESPONSE_TIMEOUT", 0.1), patch.object(
-        hap_proto.transport, "write"
-    ) as writer:
+        hap_proto.transport, "writelines"
+    ) as writelines:
         hap_proto.data_received(
             b'POST /resource HTTP/1.1\r\nHost: HASS\\032Bridge\\032BROZ\\0323BF435._hap._tcp.local\r\nContent-Length: 79\r\nContent-Type: application/hap+json\r\n\r\n{"image-height":360,"resource-type":"image","image-width":640,"aid":1411620844}'  # pylint: disable=line-too-long
         )
         await asyncio.sleep(0.3)
 
-    assert b"-70402" in writer.call_args_list[0][0][0]
+    assert b"-70402" in b"".join(writelines.call_args_list[0][0])
 
     hap_proto.close()
 
@@ -564,7 +568,7 @@ def _make_response(*_):
         response.shared_key = b"newkey"
         return response
 
-    with patch.object(hap_proto.transport, "write"), patch.object(
+    with patch.object(hap_proto.transport, "writelines"), patch.object(
         hap_proto.handler, "dispatch", _make_response
     ):
         hap_proto.data_received(
@@ -635,7 +639,7 @@ async def _async_get_snapshot(*_):
     hap_proto.hap_crypto = MockHAPCrypto()
     hap_proto.handler.is_encrypted = True
 
-    with patch.object(hap_proto.transport, "write") as writer:
+    with patch.object(hap_proto.transport, "writelines") as writelines:
         hap_proto.data_received(
             b'POST /resource HTTP/1.1\r\nHost: HASS\\032Bridge\\032BROZ\\0323BF435._hap._tcp.local\r\nContent-Length: 79\r\nContent-Type: application/hap+json\r\n\r\n{"image-height":360,"resource-type":"image","image-width":640,"aid":1411620844}'  # pylint: disable=line-too-long
         )
@@ -645,7 +649,7 @@ async def _async_get_snapshot(*_):
             pass
         await asyncio.sleep(0)
 
-    assert b"-70402" in writer.call_args_list[0][0][0]
+    assert b"-70402" in b"".join(writelines.call_args_list[0][0])
 
     hap_proto.close()
 
@@ -671,7 +675,7 @@ def _get_snapshot(*_):
     hap_proto.hap_crypto = MockHAPCrypto()
     hap_proto.handler.is_encrypted = True
 
-    with patch.object(hap_proto.transport, "write") as writer:
+    with patch.object(hap_proto.transport, "writelines") as writelines:
         hap_proto.data_received(
             b'POST /resource HTTP/1.1\r\nHost: HASS\\032Bridge\\032BROZ\\0323BF435._hap._tcp.local\r\nContent-Length: 79\r\nContent-Type: application/hap+json\r\n\r\n{"image-height":360,"resource-type":"image","image-width":640,"aid":1411620844}'  # pylint: disable=line-too-long
         )
@@ -681,7 +685,7 @@ def _get_snapshot(*_):
             pass
         await asyncio.sleep(0)
 
-    assert b"-70402" in writer.call_args_list[0][0][0]
+    assert b"-70402" in b"".join(writelines.call_args_list[0][0])
 
     hap_proto.close()
 
@@ -702,14 +706,14 @@ async def test_camera_snapshot_missing_accessory(driver):
     hap_proto.hap_crypto = MockHAPCrypto()
     hap_proto.handler.is_encrypted = True
 
-    with patch.object(hap_proto.transport, "write") as writer:
+    with patch.object(hap_proto.transport, "writelines") as writelines:
         hap_proto.data_received(
             b'POST /resource HTTP/1.1\r\nHost: HASS\\032Bridge\\032BROZ\\0323BF435._hap._tcp.local\r\nContent-Length: 79\r\nContent-Type: application/hap+json\r\n\r\n{"image-height":360,"resource-type":"image","image-width":640,"aid":1411620844}'  # pylint: disable=line-too-long
         )
         await asyncio.sleep(0)
 
     assert hap_proto.response is None
-    assert b"-70402" in writer.call_args_list[0][0][0]
+    assert b"-70402" in b"".join(writelines.call_args_list[0][0])
     hap_proto.close()
 
 
@@ -777,20 +781,22 @@ def test_explicit_close(driver: AccessoryDriver):
     hap_proto.handler.is_encrypted = True
     assert hap_proto.transport.is_closing() is False
 
-    with patch.object(hap_proto.transport, "write") as writer:
+    with patch.object(hap_proto.transport, "writelines") as writelines:
         hap_proto.data_received(
             b"GET /characteristics?id=3762173001.7 HTTP/1.1\r\nHost: HASS\\032Bridge\\032YPHW\\032B223AD._hap._tcp.local\r\n\r\n"  # pylint: disable=line-too-long
         )
         hap_proto.data_received(
             b"GET /characteristics?id=1.5 HTTP/1.1\r\nConnection: close\r\nHost: HASS\\032Bridge\\032YPHW\\032B223AD._hap._tcp.local\r\n\r\n"  # pylint: disable=line-too-long
         )
 
-    assert b"Content-Length:" in writer.call_args_list[0][0][0]
-    assert b"Transfer-Encoding: chunked\r\n\r\n" not in writer.call_args_list[0][0][0]
-    assert b"-70402" in writer.call_args_list[0][0][0]
+    join0 = b"".join(writelines.call_args_list[0][0])
+    assert b"Content-Length:" in join0
+    assert b"Transfer-Encoding: chunked\r\n\r\n" not in join0
+    assert b"-70402" in join0
 
-    assert b"Content-Length:" in writer.call_args_list[1][0][0]
-    assert b"Transfer-Encoding: chunked\r\n\r\n" not in writer.call_args_list[1][0][0]
-    assert b"TestAcc" in writer.call_args_list[1][0][0]
+    join1 = b"".join(writelines.call_args_list[1][0])
+    assert b"Content-Length:" in join1
+    assert b"Transfer-Encoding: chunked\r\n\r\n" not in join1
+    assert b"TestAcc" in join1
 
     assert hap_proto.transport.is_closing() is True
diff --git a/tox.ini b/tox.ini
@@ -61,8 +61,8 @@ deps =
     -r{toxinidir}/requirements_all.txt
     -r{toxinidir}/requirements_test.txt
 commands =
-    pylint pyhap --disable=missing-docstring,empty-docstring,invalid-name,fixme --max-line-length=120
-    pylint tests --disable=duplicate-code,missing-docstring,empty-docstring,invalid-name,fixme --max-line-length=120
+    pylint pyhap --disable=missing-docstring,empty-docstring,invalid-name,fixme,too-many-positional-arguments --max-line-length=120
+    pylint tests --disable=duplicate-code,missing-docstring,empty-docstring,invalid-name,fixme,too-many-positional-arguments --max-line-length=120
 
 
 [testenv:bandit]