From 787a52518b1c1f99b5ffc80bc9170f2da645baa9 Mon Sep 17 00:00:00 2001 From: Raphael Hehl <7577984+RaHehl@users.noreply.github.com> Date: Sun, 8 Nov 2026 19:25:31 +0100 Subject: [PATCH 1/9] Return -70405 when reading a characteristic without paired read GET /characteristics returned the stored value of every characteristic, including write-only ones such as Identify or a camera key. The HAP spec only allows reads of characteristics with the paired read (pr) permission and answers everything else with -70405. HAP-NodeJS does the same. The getter is not called for such characteristics. The /accessories database already omitted their value; this adds tests for it. --- pyhap/accessory_driver.py | 17 ++++-- tests/test_accessory_driver.py | 105 +++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 5 deletions(-) diff --git a/pyhap/accessory_driver.py b/pyhap/accessory_driver.py index a68eae6c..bdbccad0 100644 --- a/pyhap/accessory_driver.py +++ b/pyhap/accessory_driver.py @@ -36,9 +36,10 @@ from pyhap import util from pyhap.accessory import Accessory, Bridge, get_topic -from pyhap.characteristic import Characteristic, CharacteristicError +from pyhap.characteristic import PROP_PERMISSIONS, Characteristic, CharacteristicError from pyhap.const import ( HAP_PERMISSION_NOTIFY, + HAP_PERMISSION_READ, HAP_PROTOCOL_SHORT_VERSION, HAP_REPR_ACCS, HAP_REPR_AID, @@ -955,10 +956,16 @@ def get_characteristics(self, char_ids: Any) -> dict[str, Any]: available = False # A characteristic IID may resolve to a Service or be unknown; - # only a Characteristic can report a value. - if available and isinstance(char, Characteristic): - rep[HAP_REPR_VALUE] = char.get_value() - rep[HAP_REPR_STATUS] = HAP_SERVER_STATUS.SUCCESS + # only a Characteristic can report a value, and only when it + # has the paired read permission. + if isinstance(char, Characteristic): + if HAP_PERMISSION_READ not in char.properties[PROP_PERMISSIONS]: + rep[HAP_REPR_STATUS] = ( + HAP_SERVER_STATUS.WRITE_ONLY_CHARACTERISTIC + ) + elif available: + rep[HAP_REPR_VALUE] = char.get_value() + rep[HAP_REPR_STATUS] = HAP_SERVER_STATUS.SUCCESS except CharacteristicError: logger.error( "%s: Error getting value for characteristic %s.", diff --git a/tests/test_accessory_driver.py b/tests/test_accessory_driver.py index 97627156..d4ebbe8f 100644 --- a/tests/test_accessory_driver.py +++ b/tests/test_accessory_driver.py @@ -32,10 +32,12 @@ ) from pyhap.const import ( HAP_PERMISSIONS, + HAP_REPR_ACCS, HAP_REPR_AID, HAP_REPR_CHARS, HAP_REPR_IID, HAP_REPR_PID, + HAP_REPR_SERVICES, HAP_REPR_STATUS, HAP_REPR_TTL, HAP_REPR_VALUE, @@ -140,6 +142,109 @@ def test_get_characteristics_honors_availability( getter.assert_called_once_with() +def _add_bridged_or_standalone( + driver: AccessoryDriver, permissions: list[str], bridged: bool +) -> tuple[Accessory, Characteristic, MagicMock]: + """Add an accessory with one int characteristic that has a getter.""" + acc = Accessory(driver, "Test Accessory", aid=2 if bridged else STANDALONE_AID) + service = Service(uuid1(), "Test Service") + char = Characteristic( + "Value", + uuid1(), + {PROP_FORMAT: HAP_FORMAT_INT, PROP_PERMISSIONS: permissions}, + ) + getter = MagicMock(return_value=42) + char.getter_callback = getter + service.add_characteristic(char) + acc.add_service(service) + if bridged: + bridge = Bridge(driver, "Test Bridge") + bridge.add_accessory(acc) + driver.add_accessory(bridge) + else: + driver.add_accessory(acc) + return acc, char, getter + + +READ_PERMISSION_CASES = [ + pytest.param( + ["pr"], + {HAP_REPR_STATUS: HAP_SERVER_STATUS.SUCCESS, HAP_REPR_VALUE: 42}, + 1, + id="read", + ), + pytest.param( + ["pr", "pw", "ev"], + {HAP_REPR_STATUS: HAP_SERVER_STATUS.SUCCESS, HAP_REPR_VALUE: 42}, + 1, + id="read-write-notify", + ), + pytest.param( + ["pw"], + {HAP_REPR_STATUS: HAP_SERVER_STATUS.WRITE_ONLY_CHARACTERISTIC}, + 0, + id="write-only", + ), + pytest.param( + ["pw", "ev"], + {HAP_REPR_STATUS: HAP_SERVER_STATUS.WRITE_ONLY_CHARACTERISTIC}, + 0, + id="write-notify", + ), + pytest.param( + ["pw", "tw"], + {HAP_REPR_STATUS: HAP_SERVER_STATUS.WRITE_ONLY_CHARACTERISTIC}, + 0, + id="timed-write-only", + ), +] + + +@pytest.mark.parametrize("bridged", [False, True], ids=["standalone", "bridged"]) +@pytest.mark.parametrize(("permissions", "expected", "reads"), READ_PERMISSION_CASES) +def test_get_characteristics_requires_read_permission( + driver: AccessoryDriver, + bridged: bool, + permissions: list[str], + expected: dict[str, Any], + reads: int, +) -> None: + """A read of a characteristic without paired read returns no value.""" + acc, char, getter = _add_bridged_or_standalone(driver, permissions, bridged) + iid = acc.iid_manager.get_iid(char) + + result = driver.get_characteristics([f"{acc.aid}.{iid}"]) + + assert result == { + HAP_REPR_CHARS: [{HAP_REPR_AID: acc.aid, HAP_REPR_IID: iid, **expected}] + } + assert getter.call_count == reads + + +@pytest.mark.parametrize( + ("permissions", "values", "reads"), + [ + (["pr"], {HAP_REPR_VALUE}, 1), + (["pr", "pw", "ev"], {HAP_REPR_VALUE}, 1), + (["pw"], set(), 0), + (["pw", "ev"], set(), 0), + (["pw", "tw"], set(), 0), + ], +) +def test_get_accessories_value_requires_read_permission( + driver: AccessoryDriver, permissions: list[str], values: set[str], reads: int +) -> None: + """The accessory database only includes values the controller may read.""" + acc, char, getter = _add_bridged_or_standalone(driver, permissions, False) + + (acc_rep,) = driver.get_accessories()[HAP_REPR_ACCS] + (char_rep,) = acc_rep[HAP_REPR_SERVICES][-1][HAP_REPR_CHARS] + + assert char_rep[HAP_REPR_IID] == acc.iid_manager.get_iid(char) + assert char_rep.keys() & {HAP_REPR_VALUE} == values + assert getter.call_count == reads + + def test_auto_add_aid_mac(driver: AccessoryDriver) -> None: acc = Accessory(driver, "Test Accessory") driver.add_accessory(acc) From 071f96d22df7ad5ee098fc14ffdc9ef9cc3fab5a Mon Sep 17 00:00:00 2001 From: Raphael Hehl <7577984+RaHehl@users.noreply.github.com> Date: Sun, 8 Nov 2026 19:27:00 +0100 Subject: [PATCH 2/9] Answer -70406 for event subscriptions without notify permission Any characteristic could be subscribed to, whether or not it has the notify (ev) permission. A controller could then receive the values that another controller wrote to a write-only characteristic. Per the HAP spec, (un)subscribing a characteristic without ev now answers -70406 and leaves the subscriptions unchanged, the same as HAP-NodeJS. A value in the same request entry is not written. Some tests subscribed to Version, which has no ev permission. They now use CurrentDoorState. --- pyhap/accessory_driver.py | 38 +++++++-- tests/test_accessory_driver.py | 146 +++++++++++++++++++++++++++++++++ tests/test_hap_handler.py | 10 +-- tests/test_hap_protocol.py | 4 +- 4 files changed, 183 insertions(+), 15 deletions(-) diff --git a/pyhap/accessory_driver.py b/pyhap/accessory_driver.py index bdbccad0..bfe143cf 100644 --- a/pyhap/accessory_driver.py +++ b/pyhap/accessory_driver.py @@ -1021,11 +1021,9 @@ def set_characteristics( :type chars_query: dict """ - # TODO: Add support for chars that do no support notifications. - queries: list[dict[str, Any]] = chars_query[HAP_REPR_CHARS] - self._notify(queries, client_addr) + not_notifiable = self._notify(queries, client_addr) updates_by_accessories_services: dict[ Accessory, dict[Service, dict[Characteristic, Any]] @@ -1045,12 +1043,20 @@ def set_characteristics( assert primary_accessory is not None # noqa: S101 primary_aid = primary_accessory.aid - for query in queries: - if HAP_REPR_VALUE not in query and not expired: - continue + for aid, iid in not_notifiable: + results[aid][iid] = { + HAP_REPR_STATUS: HAP_SERVER_STATUS.NOTIFICATION_NOT_SUPPORTED + } + for query in queries: aid = query[HAP_REPR_AID] iid = query[HAP_REPR_IID] + if (aid, iid) in not_notifiable: + continue + if HAP_REPR_VALUE not in query and not expired: + # Listed with status 0 if the response has other results. + results[aid][iid] = {HAP_REPR_STATUS: HAP_SERVER_STATUS.SUCCESS} + continue value = query.get(HAP_REPR_VALUE) write_response_requested = query.get(HAP_REPR_WRITE_RESPONSE, False) @@ -1184,8 +1190,15 @@ def signal_handler(self, _signal: Any, _frame: Any) -> None: def _notify( self, queries: list[dict[str, Any]], client_addr: tuple[str, int] - ) -> None: - """Notify the driver that the client has subscribed or unsubscribed.""" + ) -> set[tuple[int, int]]: + """Notify the driver that the client has subscribed or unsubscribed. + + Characteristics without the notify permission are left alone; their + ``(aid, iid)`` pairs are returned so the request can report + -70406 for them. + """ + assert self.accessory is not None # noqa: S101 + not_notifiable: set[tuple[int, int]] = set() for query in queries: if HAP_PERMISSION_NOTIFY not in query: continue @@ -1193,7 +1206,16 @@ def _notify( iid = query[HAP_REPR_IID] ev = query[HAP_PERMISSION_NOTIFY] + char = self.accessory.get_characteristic(aid, iid) + if ( + char is not None + and HAP_PERMISSION_NOTIFY not in char.properties[PROP_PERMISSIONS] + ): + not_notifiable.add((aid, iid)) + continue + char_topic = get_topic(aid, iid) action = "Subscribed" if ev else "Unsubscribed" logger.debug("%s client %s to topic %s", action, client_addr, char_topic) self.async_subscribe_client_topic(client_addr, char_topic, ev) + return not_notifiable diff --git a/tests/test_accessory_driver.py b/tests/test_accessory_driver.py index d4ebbe8f..eddff96b 100644 --- a/tests/test_accessory_driver.py +++ b/tests/test_accessory_driver.py @@ -31,6 +31,7 @@ Characteristic, ) from pyhap.const import ( + HAP_PERMISSION_NOTIFY, HAP_PERMISSIONS, HAP_REPR_ACCS, HAP_REPR_AID, @@ -245,6 +246,151 @@ def test_get_accessories_value_requires_read_permission( assert getter.call_count == reads +@pytest.mark.parametrize("bridged", [False, True], ids=["standalone", "bridged"]) +@pytest.mark.parametrize( + "permissions", [["pr", "ev"], ["pr", "pw", "ev"], ["pw", "ev"]] +) +def test_subscribe_with_notify_permission( + driver: AccessoryDriver, bridged: bool, permissions: list[str] +) -> None: + """Characteristics with the notify permission can be (un)subscribed.""" + acc, char, _ = _add_bridged_or_standalone(driver, permissions, bridged) + iid = acc.iid_manager.get_iid(char) + client = ("1.2.3.4", 5) + + subscribe = {HAP_REPR_AID: acc.aid, HAP_REPR_IID: iid, HAP_PERMISSION_NOTIFY: True} + assert driver.set_characteristics({HAP_REPR_CHARS: [subscribe]}, client) is None + assert driver.topics == {f"{acc.aid}.{iid}": {client}} + + unsubscribe = {**subscribe, HAP_PERMISSION_NOTIFY: False} + assert driver.set_characteristics({HAP_REPR_CHARS: [unsubscribe]}, client) is None + assert driver.topics == {} + + +@pytest.mark.parametrize("bridged", [False, True], ids=["standalone", "bridged"]) +@pytest.mark.parametrize("ev", [True, False], ids=["subscribe", "unsubscribe"]) +@pytest.mark.parametrize("permissions", [["pr"], ["pr", "pw"], ["pw"], ["pw", "tw"]]) +def test_subscribe_without_notify_permission( + driver: AccessoryDriver, bridged: bool, ev: bool, permissions: list[str] +) -> None: + """(Un)subscribing a characteristic without notify answers -70406.""" + acc, char, _ = _add_bridged_or_standalone(driver, permissions, bridged) + iid = acc.iid_manager.get_iid(char) + client = ("1.2.3.4", 5) + + result = driver.set_characteristics( + { + HAP_REPR_CHARS: [ + {HAP_REPR_AID: acc.aid, HAP_REPR_IID: iid, HAP_PERMISSION_NOTIFY: ev} + ] + }, + client, + ) + + assert result == { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: iid, + HAP_REPR_STATUS: HAP_SERVER_STATUS.NOTIFICATION_NOT_SUPPORTED, + } + ] + } + assert driver.topics == {} + + +def test_refused_subscription_lists_the_others(driver: AccessoryDriver) -> None: + """A 207 for a refused subscription also lists the accepted ones.""" + acc = Accessory(driver, "Test Accessory", aid=STANDALONE_AID) + service = Service(uuid1(), "Test Service") + notifiable = Characteristic( + "Notifiable", + uuid1(), + {PROP_FORMAT: HAP_FORMAT_INT, PROP_PERMISSIONS: ["pr", "ev"]}, + ) + silent = Characteristic( + "Silent", uuid1(), {PROP_FORMAT: HAP_FORMAT_INT, PROP_PERMISSIONS: ["pr"]} + ) + service.add_characteristic(notifiable) + service.add_characteristic(silent) + acc.add_service(service) + driver.add_accessory(acc) + ok_iid = acc.iid_manager.get_iid(notifiable) + refused_iid = acc.iid_manager.get_iid(silent) + client = ("1.2.3.4", 5) + + result = driver.set_characteristics( + { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: ok_iid, + HAP_PERMISSION_NOTIFY: True, + }, + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: refused_iid, + HAP_PERMISSION_NOTIFY: True, + }, + ] + }, + client, + ) + + assert result == { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: refused_iid, + HAP_REPR_STATUS: HAP_SERVER_STATUS.NOTIFICATION_NOT_SUPPORTED, + }, + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: ok_iid, + HAP_REPR_STATUS: HAP_SERVER_STATUS.SUCCESS, + }, + ] + } + assert driver.topics == {f"{acc.aid}.{ok_iid}": {client}} + + +@pytest.mark.parametrize("ev", [True, False], ids=["subscribe", "unsubscribe"]) +def test_write_with_subscribe_without_notify_permission( + driver: AccessoryDriver, ev: bool +) -> None: + """A write combined with a refused subscription is not applied.""" + acc, char, _ = _add_bridged_or_standalone(driver, ["pr", "pw"], False) + iid = acc.iid_manager.get_iid(char) + setter = MagicMock() + char.setter_callback = setter + + result = driver.set_characteristics( + { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: iid, + HAP_PERMISSION_NOTIFY: ev, + HAP_REPR_VALUE: 5, + } + ] + }, + ("1.2.3.4", 5), + ) + + assert result == { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: iid, + HAP_REPR_STATUS: HAP_SERVER_STATUS.NOTIFICATION_NOT_SUPPORTED, + } + ] + } + setter.assert_not_called() + assert driver.topics == {} + + def test_auto_add_aid_mac(driver: AccessoryDriver) -> None: acc = Accessory(driver, "Test Accessory") driver.add_accessory(acc) diff --git a/tests/test_hap_handler.py b/tests/test_hap_handler.py index 98590bea..4ae0378e 100644 --- a/tests/test_hap_handler.py +++ b/tests/test_hap_handler.py @@ -727,7 +727,7 @@ def test_handle_set_handle_set_characteristics_encrypted_pid_missing_prepare( response = hap_handler.HAPResponse() handler.response = response handler.request_body = ( - b'{"pid":123,"characteristics":[{"aid":1,"iid":9,"ev":true}]}' + b'{"pid":123,"characteristics":[{"aid":1,"iid":11,"ev":true}]}' ) handler.handle_set_characteristics() @@ -759,7 +759,7 @@ def test_handle_set_handle_set_characteristics_encrypted_with_prepare( response = hap_handler.HAPResponse() handler.response = response handler.request_body = ( - b'{"pid":123,"characteristics":[{"aid":1,"iid":9,"ev":true}]}' + b'{"pid":123,"characteristics":[{"aid":1,"iid":11,"ev":true}]}' ) handler.handle_set_characteristics() @@ -797,7 +797,7 @@ def test_handle_set_handle_set_characteristics_encrypted_with_multiple_prepare( response = hap_handler.HAPResponse() handler.response = response handler.request_body = ( - b'{"pid":123,"characteristics":[{"aid":1,"iid":9,"ev":true}]}' + b'{"pid":123,"characteristics":[{"aid":1,"iid":11,"ev":true}]}' ) handler.handle_set_characteristics() @@ -851,7 +851,7 @@ def test_handle_set_handle_set_characteristics_encrypted_with_expired_ttl( response = hap_handler.HAPResponse() handler.response = response handler.request_body = ( - b'{"pid":123,"characteristics":[{"aid":1,"iid":9,"ev":true}]}' + b'{"pid":123,"characteristics":[{"aid":1,"iid":11,"ev":true}]}' ) handler.handle_set_characteristics() @@ -883,7 +883,7 @@ def test_handle_set_handle_set_characteristics_encrypted_with_wrong_pid( response = hap_handler.HAPResponse() handler.response = response handler.request_body = ( - b'{"pid":456,"characteristics":[{"aid":1,"iid":9,"ev":true}]}' + b'{"pid":456,"characteristics":[{"aid":1,"iid":11,"ev":true}]}' ) handler.handle_set_characteristics() diff --git a/tests/test_hap_protocol.py b/tests/test_hap_protocol.py index c5b0ad37..749e5b00 100644 --- a/tests/test_hap_protocol.py +++ b/tests/test_hap_protocol.py @@ -433,10 +433,10 @@ def test_set_characteristics_with_crypto(driver: AccessoryDriver) -> None: hap_proto.data_received( b"PUT /characteristics HTTP/1.1\r\n" b"Host: HASS12\\032AD1C22._hap._tcp.local\r\n" - b"Content-Length: 49\r\n" + b"Content-Length: 50\r\n" b"Content-Type: application/hap+json\r\n" b"\r\n" - b'{"characteristics":[{"aid":1,"iid":9,"ev":true}]}' + b'{"characteristics":[{"aid":1,"iid":11,"ev":true}]}' ) hap_proto.close() From 3fd5a1b451b085c44180a7b698ce6e5d583dc007 Mon Sep 17 00:00:00 2001 From: Raphael Hehl <7577984+RaHehl@users.noreply.github.com> Date: Sun, 8 Nov 2026 19:45:10 +0100 Subject: [PATCH 3/9] Return -70404 when writing a characteristic without paired write A value written to a characteristic without the pw permission still reached its setter. Answer it with -70404 like HAP-NodeJS instead, without calling the characteristic, service or accessory setters. A subscription in the same entry and the other characteristics of the request are still handled. --- pyhap/accessory_driver.py | 10 +++ tests/test_accessory_driver.py | 132 ++++++++++++++++++++++++++++++++- tests/test_hap_handler.py | 2 +- 3 files changed, 142 insertions(+), 2 deletions(-) diff --git a/pyhap/accessory_driver.py b/pyhap/accessory_driver.py index bfe143cf..c9f97599 100644 --- a/pyhap/accessory_driver.py +++ b/pyhap/accessory_driver.py @@ -40,6 +40,7 @@ from pyhap.const import ( HAP_PERMISSION_NOTIFY, HAP_PERMISSION_READ, + HAP_PERMISSION_WRITE, HAP_PROTOCOL_SHORT_VERSION, HAP_REPR_ACCS, HAP_REPR_AID, @@ -1075,6 +1076,15 @@ def set_characteristics( if char is None: continue + if ( + value is not None + and HAP_PERMISSION_WRITE not in char.properties[PROP_PERMISSIONS] + ): + results[aid][iid] = { + HAP_REPR_STATUS: HAP_SERVER_STATUS.READ_ONLY_CHARACTERISTIC + } + continue + set_result = HAP_SERVER_STATUS.INVALID_VALUE_IN_REQUEST set_result_value = None diff --git a/tests/test_accessory_driver.py b/tests/test_accessory_driver.py index eddff96b..92c582ac 100644 --- a/tests/test_accessory_driver.py +++ b/tests/test_accessory_driver.py @@ -32,6 +32,7 @@ ) from pyhap.const import ( HAP_PERMISSION_NOTIFY, + HAP_PERMISSION_WRITE, HAP_PERMISSIONS, HAP_REPR_ACCS, HAP_REPR_AID, @@ -52,7 +53,7 @@ CHAR_PROPS = { PROP_FORMAT: HAP_FORMAT_INT, - PROP_PERMISSIONS: HAP_PERMISSION_READ, + PROP_PERMISSIONS: [HAP_PERMISSION_READ, HAP_PERMISSION_WRITE], } @@ -391,6 +392,135 @@ def test_write_with_subscribe_without_notify_permission( assert driver.topics == {} +def _write_value( + driver: AccessoryDriver, acc: Accessory, char: Characteristic, **query: Any +) -> tuple[dict[str, Any] | None, MagicMock, MagicMock]: + """Write 5 to ``char`` and return the result and both setter mocks.""" + setter = MagicMock() + char.setter_callback = setter + service_setter = MagicMock() + assert char.service is not None + char.service.setter_callback = service_setter + iid = acc.iid_manager.get_iid(char) + result = driver.set_characteristics( + { + HAP_REPR_CHARS: [ + {HAP_REPR_AID: acc.aid, HAP_REPR_IID: iid, HAP_REPR_VALUE: 5, **query} + ] + }, + ("1.2.3.4", 5), + ) + return result, setter, service_setter + + +@pytest.mark.parametrize("bridged", [False, True], ids=["standalone", "bridged"]) +@pytest.mark.parametrize("permissions", [["pr"], ["pr", "ev"]]) +def test_write_without_write_permission( + driver: AccessoryDriver, bridged: bool, permissions: list[str] +) -> None: + """A write to a characteristic without paired write is refused.""" + acc, char, _ = _add_bridged_or_standalone(driver, permissions, bridged) + + result, setter, service_setter = _write_value(driver, acc, char) + + assert result == { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: acc.iid_manager.get_iid(char), + HAP_REPR_STATUS: HAP_SERVER_STATUS.READ_ONLY_CHARACTERISTIC, + } + ] + } + setter.assert_not_called() + service_setter.assert_not_called() + assert char.value == 0 + + +@pytest.mark.parametrize("bridged", [False, True], ids=["standalone", "bridged"]) +@pytest.mark.parametrize("permissions", [["pw"], ["pr", "pw"], ["pr", "pw", "ev"]]) +def test_write_with_write_permission( + driver: AccessoryDriver, bridged: bool, permissions: list[str] +) -> None: + """A write to a characteristic with paired write is applied.""" + acc, char, _ = _add_bridged_or_standalone(driver, permissions, bridged) + + result, setter, service_setter = _write_value(driver, acc, char) + + assert result is None + setter.assert_called_once_with(5) + service_setter.assert_called_once_with({char.display_name: 5}) + assert char.value == 5 + + +def test_write_with_subscribe_without_write_permission( + driver: AccessoryDriver, +) -> None: + """A subscription is applied even when the write in the same entry is not.""" + acc, char, _ = _add_bridged_or_standalone(driver, ["pr", "ev"], False) + client = ("1.2.3.4", 5) + iid = acc.iid_manager.get_iid(char) + + result, setter, _ = _write_value(driver, acc, char, **{HAP_PERMISSION_NOTIFY: True}) + + assert result == { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: iid, + HAP_REPR_STATUS: HAP_SERVER_STATUS.READ_ONLY_CHARACTERISTIC, + } + ] + } + setter.assert_not_called() + assert driver.topics == {f"{acc.aid}.{iid}": {client}} + + +def test_refused_write_does_not_block_others(driver: AccessoryDriver) -> None: + """A refused write leaves the other characteristics of the request alone.""" + acc = Accessory(driver, "Test Accessory", aid=STANDALONE_AID) + service = Service(uuid1(), "Test Service") + read_only = Characteristic( + "ReadOnly", uuid1(), {PROP_FORMAT: HAP_FORMAT_INT, PROP_PERMISSIONS: ["pr"]} + ) + writable = Characteristic("Writable", uuid1(), CHAR_PROPS) + service.add_characteristic(read_only, writable) + acc.add_service(service) + driver.add_accessory(acc) + setter = MagicMock() + writable.setter_callback = setter + read_only_iid = acc.iid_manager.get_iid(read_only) + writable_iid = acc.iid_manager.get_iid(writable) + + result = driver.set_characteristics( + { + HAP_REPR_CHARS: [ + {HAP_REPR_AID: acc.aid, HAP_REPR_IID: read_only_iid, HAP_REPR_VALUE: 5}, + {HAP_REPR_AID: acc.aid, HAP_REPR_IID: writable_iid, HAP_REPR_VALUE: 5}, + ] + }, + ("1.2.3.4", 5), + ) + + assert result == { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: read_only_iid, + HAP_REPR_STATUS: HAP_SERVER_STATUS.READ_ONLY_CHARACTERISTIC, + }, + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: writable_iid, + HAP_REPR_STATUS: HAP_SERVER_STATUS.SUCCESS, + }, + ] + } + setter.assert_called_once_with(5) + assert read_only.value == 0 + assert writable.value == 5 + + def test_auto_add_aid_mac(driver: AccessoryDriver) -> None: acc = Accessory(driver, "Test Accessory") driver.add_accessory(acc) diff --git a/tests/test_hap_handler.py b/tests/test_hap_handler.py index 4ae0378e..9dc91435 100644 --- a/tests/test_hap_handler.py +++ b/tests/test_hap_handler.py @@ -930,7 +930,7 @@ def _mock_failure(*_: Any) -> NoReturn: response = hap_handler.HAPResponse() handler.response = response - handler.request_body = b'{"characteristics":[{"aid":1,"iid":11,"value":1}]}' + handler.request_body = b'{"characteristics":[{"aid":1,"iid":12,"value":1}]}' handler.handle_set_characteristics() assert response.status_code == 207 From 1ef0721abc04d43ec4367203fbd97151e439394f Mon Sep 17 00:00:00 2001 From: Raphael Hehl <7577984+RaHehl@users.noreply.github.com> Date: Sun, 8 Nov 2026 19:28:30 +0100 Subject: [PATCH 4/9] Only send write responses for characteristics with wr permission A write with "r": true got the setter's return value back for any characteristic. The HAP spec ties write responses to the write response (wr) permission, and HAP-NodeJS ignores the request for other characteristics. Do the same. When a write response is sent, it becomes the characteristic's value instead of the written one, which HAP-NodeJS does as well. The sent value is the stored one, after the usual format checks. A response the characteristic can't hold is still sent and the written value is kept. The write response tests are now parametrized and also cover a wr characteristic without paired read: neither the written value nor the response can be read back. --- pyhap/accessory_driver.py | 33 ++++++- pyhap/characteristic.py | 4 +- tests/test_accessory_driver.py | 169 +++++++++++++++++---------------- 3 files changed, 122 insertions(+), 84 deletions(-) diff --git a/pyhap/accessory_driver.py b/pyhap/accessory_driver.py index c9f97599..bd05a8c2 100644 --- a/pyhap/accessory_driver.py +++ b/pyhap/accessory_driver.py @@ -41,6 +41,7 @@ HAP_PERMISSION_NOTIFY, HAP_PERMISSION_READ, HAP_PERMISSION_WRITE, + HAP_PERMISSION_WRITE_RESPONSE, HAP_PROTOCOL_SHORT_VERSION, HAP_REPR_ACCS, HAP_REPR_AID, @@ -87,6 +88,24 @@ MAX_PREPARED_WRITE_TTL_SECONDS = 30.0 +def _store_write_response(char: Characteristic, response: Any) -> Any: + """Keep the write response as the value and return what to send. + + The written value of a write response characteristic, such as an SDP + offer, is a request. What the accessory answered is its state. + """ + try: + char.value = char.to_valid_value(response) + except ValueError: + logger.debug( + "%s: keeping the written value, the write response is invalid: %s", + char.display_name, + response, + ) + return response + return char.value + + def _wrap_char_setter(char: Any, value: Any, client_addr: Any) -> tuple[int, Any]: """Process an characteristic setter callback trapping and logging all exceptions.""" try: @@ -1059,7 +1078,6 @@ def set_characteristics( results[aid][iid] = {HAP_REPR_STATUS: HAP_SERVER_STATUS.SUCCESS} continue value = query.get(HAP_REPR_VALUE) - write_response_requested = query.get(HAP_REPR_WRITE_RESPONSE, False) if aid == primary_aid: acc = primary_accessory @@ -1093,8 +1111,17 @@ def set_characteristics( char, value, client_addr ) - if set_result_value is not None and write_response_requested: - result = {HAP_REPR_STATUS: set_result, HAP_REPR_VALUE: set_result_value} + # Only characteristics with the write response permission answer + # a write with a value, and only when the controller asks for it. + if ( + set_result_value is not None + and query.get(HAP_REPR_WRITE_RESPONSE, False) + and HAP_PERMISSION_WRITE_RESPONSE in char.properties[PROP_PERMISSIONS] + ): + result = { + HAP_REPR_STATUS: set_result, + HAP_REPR_VALUE: _store_write_response(char, set_result_value), + } else: result = {HAP_REPR_STATUS: set_result} diff --git a/pyhap/characteristic.py b/pyhap/characteristic.py index bcbdd3e2..a0b09a69 100644 --- a/pyhap/characteristic.py +++ b/pyhap/characteristic.py @@ -201,7 +201,9 @@ def __init__( self._value = self._get_default_value() self.getter_callback: Callable[[], Any] | None = None # Setters may take just the value or (value, sender_client_addr); see - # ``_setter_wants_addr``. The return value is used for write responses. + # ``_setter_wants_addr``. The return value is sent back as the write + # response when the characteristic has the "wr" permission and the + # controller asked for one. self.setter_callback: Callable[..., Any] | None = None # Cache of whether ``setter_callback`` opts into the client address, # recomputed only when the setter object itself changes. diff --git a/tests/test_accessory_driver.py b/tests/test_accessory_driver.py index 92c582ac..475a3c7f 100644 --- a/tests/test_accessory_driver.py +++ b/tests/test_accessory_driver.py @@ -609,109 +609,118 @@ def test_listen_address(kwargs: dict[str, Any], expected: Any) -> None: assert server.call_args.args[0] == (expected, 51234) -def test_write_response_returned_when_not_requested(driver: AccessoryDriver) -> None: - bridge = Bridge(driver, "mybridge") - acc = Accessory(driver, "TestAcc", aid=2) - service = Service(uuid1(), "NFCAccess") - char_nfc_access_control_point = Characteristic( - "NFCAccessControlPoint", uuid1(), CHAR_PROPS - ) - service.add_characteristic(char_nfc_access_control_point) - - mock_callback = MagicMock() - service.setter_callback = mock_callback - - def setter_with_write_response(value: int = 0) -> int: - return 1 - - char_nfc_access_control_point.setter_callback = setter_with_write_response - acc.add_service(service) - - char_nfc_access_control_point_iid = char_nfc_access_control_point.to_HAP()[ - HAP_REPR_IID - ] - - bridge.add_accessory(acc) - driver.add_accessory(bridge) - - response = driver.set_characteristics( +def _write( + driver: AccessoryDriver, acc: Accessory, char: Characteristic, **query: Any +) -> dict[str, Any] | None: + """Write 0 to ``char`` from a mock client with extra query fields.""" + return driver.set_characteristics( { HAP_REPR_CHARS: [ { HAP_REPR_AID: acc.aid, - HAP_REPR_IID: char_nfc_access_control_point_iid, + HAP_REPR_IID: acc.iid_manager.get_iid(char), HAP_REPR_VALUE: 0, - HAP_REPR_WRITE_RESPONSE: False, + **query, } ] }, - "mock_addr", # ty: ignore[invalid-argument-type] + ("1.2.3.4", 5), ) - assert response is None - response = driver.set_characteristics( - { - HAP_REPR_CHARS: [ - { - HAP_REPR_AID: acc.aid, - HAP_REPR_IID: char_nfc_access_control_point_iid, - HAP_REPR_VALUE: 0, - } - ] - }, - "mock_addr", # ty: ignore[invalid-argument-type] - ) - assert response is None +@pytest.mark.parametrize("bridged", [False, True], ids=["standalone", "bridged"]) +@pytest.mark.parametrize("permissions", [["pr", "pw", "wr"], ["pw", "wr"]]) +@pytest.mark.parametrize(("returned", "sent"), [(1, 1), (1.7, 1)]) +def test_write_response_returned_when_requested( + driver: AccessoryDriver, + bridged: bool, + permissions: list[str], + returned: Any, + sent: int, +) -> None: + """A write response carries the setter's value as the char stores it.""" + acc, char, _ = _add_bridged_or_standalone(driver, permissions, bridged) + service_setter = MagicMock() + assert char.service is not None + char.service.setter_callback = service_setter + char.setter_callback = MagicMock(return_value=returned) -def test_write_response_returned_when_requested(driver: AccessoryDriver) -> None: - bridge = Bridge(driver, "mybridge") - acc = Accessory(driver, "TestAcc", aid=2) - service = Service(uuid1(), "NFCAccess") - char_nfc_access_control_point = Characteristic( - "NFCAccessControlPoint", uuid1(), CHAR_PROPS - ) - service.add_characteristic(char_nfc_access_control_point) + response = _write(driver, acc, char, **{HAP_REPR_WRITE_RESPONSE: True}) - mock_callback = MagicMock() - service.setter_callback = mock_callback + assert response == { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: acc.iid_manager.get_iid(char), + HAP_REPR_STATUS: HAP_SERVER_STATUS.SUCCESS, + HAP_REPR_VALUE: sent, + } + ] + } + char.setter_callback.assert_called_once_with(0) + service_setter.assert_called_once_with({"Value": 0}) + # Like HAP-NodeJS, the response is the value from now on. + assert char.value == sent - def setter_with_write_response(value: int = 0) -> int: - return 1 - char_nfc_access_control_point.setter_callback = setter_with_write_response - acc.add_service(service) +@pytest.mark.parametrize( + ("permissions", "query"), + [ + pytest.param(["pr", "pw", "wr"], {}, id="not-requested"), + pytest.param( + ["pr", "pw", "wr"], {HAP_REPR_WRITE_RESPONSE: False}, id="declined" + ), + pytest.param( + ["pr", "pw"], {HAP_REPR_WRITE_RESPONSE: True}, id="no-wr-permission" + ), + pytest.param(["pw"], {HAP_REPR_WRITE_RESPONSE: True}, id="write-only"), + ], +) +def test_write_response_not_returned( + driver: AccessoryDriver, permissions: list[str], query: dict[str, Any] +) -> None: + """Without a request or the wr permission the write has no response body.""" + acc, char, _ = _add_bridged_or_standalone(driver, permissions, False) + char.setter_callback = MagicMock(return_value=1) - char_nfc_access_control_point_iid = char_nfc_access_control_point.to_HAP()[ - HAP_REPR_IID - ] + assert _write(driver, acc, char, **query) is None + char.setter_callback.assert_called_once_with(0) + assert char.value == 0 - bridge.add_accessory(acc) - driver.add_accessory(bridge) - response = driver.set_characteristics( - { - HAP_REPR_CHARS: [ - { - HAP_REPR_AID: acc.aid, - HAP_REPR_IID: char_nfc_access_control_point_iid, - HAP_REPR_VALUE: 0, - HAP_REPR_WRITE_RESPONSE: True, - } - ] - }, - "mock_addr", # ty: ignore[invalid-argument-type] - ) - assert response == { +def test_invalid_write_response_keeps_written_value(driver: AccessoryDriver) -> None: + """A response the char can't hold is still sent, the written value stays.""" + acc, char, _ = _add_bridged_or_standalone(driver, ["pr", "pw", "wr"], False) + char.setter_callback = MagicMock(return_value="not a number") + + response = _write(driver, acc, char, **{HAP_REPR_WRITE_RESPONSE: True}) + + assert response is not None + assert response[HAP_REPR_CHARS][0][HAP_REPR_VALUE] == "not a number" + assert char.value == 0 + + +def test_write_response_value_of_write_only_char_not_readable( + driver: AccessoryDriver, +) -> None: + """Neither the written nor the response value of a wr char without pr leaks.""" + acc, char, _ = _add_bridged_or_standalone(driver, ["pw", "wr"], False) + char.setter_callback = MagicMock(return_value=1) + iid = acc.iid_manager.get_iid(char) + + _write(driver, acc, char, **{HAP_REPR_WRITE_RESPONSE: True}) + + assert driver.get_characteristics([f"{acc.aid}.{iid}"]) == { HAP_REPR_CHARS: [ { HAP_REPR_AID: acc.aid, - HAP_REPR_IID: char_nfc_access_control_point_iid, - HAP_REPR_STATUS: 0, - HAP_REPR_VALUE: 1, - }, + HAP_REPR_IID: iid, + HAP_REPR_STATUS: HAP_SERVER_STATUS.WRITE_ONLY_CHARACTERISTIC, + } ] } + (acc_rep,) = driver.get_accessories()[HAP_REPR_ACCS] + assert HAP_REPR_VALUE not in acc_rep[HAP_REPR_SERVICES][-1][HAP_REPR_CHARS][0] def test_service_callbacks(driver: AccessoryDriver) -> None: From f946d1f3fa32750bb4857450aac4278ce63d48a7 Mon Sep 17 00:00:00 2001 From: Raphael Hehl <7577984+RaHehl@users.noreply.github.com> Date: Sun, 8 Nov 2026 21:58:12 +0100 Subject: [PATCH 5/9] Only let the writing connection read back a write response A write response of a characteristic that also has pr was stored as its value, so any paired controller could read it with GET /characteristics or /accessories. Keep it per connection instead: the connection that wrote reads its last response until it closes. Other connections read the characteristic's own value, which the write leaves as it was, also when the setter raises or returns nothing. Any new write of a connection drops its former response. Characteristics without pr keep the response as the value. --- pyhap/accessory_driver.py | 106 +++++++++++--- pyhap/hap_handler.py | 2 +- tests/test_accessory_driver.py | 257 +++++++++++++++++++++++++++++++-- tests/test_hap_handler.py | 56 ++++++- 4 files changed, 385 insertions(+), 36 deletions(-) diff --git a/pyhap/accessory_driver.py b/pyhap/accessory_driver.py index bd05a8c2..88379cbe 100644 --- a/pyhap/accessory_driver.py +++ b/pyhap/accessory_driver.py @@ -88,22 +88,12 @@ MAX_PREPARED_WRITE_TTL_SECONDS = 30.0 -def _store_write_response(char: Characteristic, response: Any) -> Any: - """Keep the write response as the value and return what to send. - - The written value of a write response characteristic, such as an SDP - offer, is a request. What the accessory answered is its state. - """ +def _stored_value(char: Any, value: Any) -> Any: + """Return the value ``client_update_value`` stores for ``value``.""" try: - char.value = char.to_valid_value(response) + return char.to_valid_value(value) except ValueError: - logger.debug( - "%s: keeping the written value, the write response is invalid: %s", - char.display_name, - response, - ) - return response - return char.value + return value def _wrap_char_setter(char: Any, value: Any, client_addr: Any) -> tuple[int, Any]: @@ -356,6 +346,8 @@ def __init__( network_tuple = (self._listen_address, self.state.port) self.http_server = HAPServer(network_tuple, self) self.prepared_writes = {} + # Write responses of readable characteristics, per client. + self._write_responses: dict[tuple[str, int], dict[Characteristic, Any]] = {} def listen_address(self) -> str | list[str]: """Return the address, or list of addresses, the HAP server listens on.""" @@ -602,6 +594,8 @@ def connection_lost(self, client: Any) -> None: for topic in client_topics: self.async_subscribe_client_topic(client, topic, subscribe=False) self.prepared_writes.pop(client, None) + # Deferred, so a write response that is already on its way is dropped too. + self.loop.call_soon(self._write_responses.pop, client, None) def publish( self, @@ -926,12 +920,18 @@ def get_accessories(self, include_value: bool = True) -> dict[str, Any]: logger.debug("Get accessories response: %s", hap_rep) return {HAP_REPR_ACCS: hap_rep} - def get_characteristics(self, char_ids: Any) -> dict[str, Any]: + def get_characteristics( + self, char_ids: Any, client_addr: tuple[str, int] | None = None + ) -> dict[str, Any]: """Returns values for the required characteristics. :param char_ids: A list of characteristic "paths", e.g. "1.2" is aid 1, iid 2. :type char_ids: list + :param client_addr: The client that reads. It reads back its own write + responses. + :type client_addr: tuple + :return: Status success for each required characteristic. For example: .. code-block:: python @@ -947,6 +947,9 @@ def get_characteristics(self, char_ids: Any) -> dict[str, Any]: :rtype: dict """ assert self.accessory is not None # noqa: S101 + write_responses = ( + self._write_responses.get(client_addr, {}) if client_addr else {} + ) chars = [] for aid_iid in char_ids: aid, iid = (int(i) for i in aid_iid.split(".")) @@ -984,7 +987,11 @@ def get_characteristics(self, char_ids: Any) -> dict[str, Any]: HAP_SERVER_STATUS.WRITE_ONLY_CHARACTERISTIC ) elif available: - rep[HAP_REPR_VALUE] = char.get_value() + rep[HAP_REPR_VALUE] = ( + write_responses[char] + if char in write_responses + else char.get_value() + ) rep[HAP_REPR_STATUS] = HAP_SERVER_STATUS.SUCCESS except CharacteristicError: logger.error( @@ -1103,6 +1110,24 @@ def set_characteristics( } continue + # Only characteristics with the write response permission answer + # a write with a value, and only when the controller asks for it. + write_response = bool( + query.get(HAP_REPR_WRITE_RESPONSE, False) + and HAP_PERMISSION_WRITE_RESPONSE in char.properties[PROP_PERMISSIONS] + ) + # A new write replaces a former response of this connection. The + # request of a readable write response characteristic is not + # shared, whatever the setter does, except to ev subscribers. + if value is not None: + self._write_responses.get(client_addr, {}).pop(char, None) + private_write = ( + write_response + and value is not None + and HAP_PERMISSION_READ in char.properties[PROP_PERMISSIONS] + ) + previous_value = char.value + set_result = HAP_SERVER_STATUS.INVALID_VALUE_IN_REQUEST set_result_value = None @@ -1110,17 +1135,16 @@ def set_characteristics( set_result, set_result_value = _wrap_char_setter( char, value, client_addr ) + # Put back the former value unless the setter set its own. + if private_write and char.value == _stored_value(char, value): + char.value = previous_value - # Only characteristics with the write response permission answer - # a write with a value, and only when the controller asks for it. - if ( - set_result_value is not None - and query.get(HAP_REPR_WRITE_RESPONSE, False) - and HAP_PERMISSION_WRITE_RESPONSE in char.properties[PROP_PERMISSIONS] - ): + if set_result_value is not None and write_response: result = { HAP_REPR_STATUS: set_result, - HAP_REPR_VALUE: _store_write_response(char, set_result_value), + HAP_REPR_VALUE: self._store_write_response( + char, set_result_value, client_addr + ), } else: result = {HAP_REPR_STATUS: set_result} @@ -1170,6 +1194,40 @@ def set_characteristics( return {HAP_REPR_CHARS: characteristics} if nonempty_results_exist else None + def _store_write_response( + self, + char: Characteristic, + response: Any, + client_addr: tuple[str, int] | None, + ) -> Any: + """Keep the write response and return what to send. + + The written value of a write response characteristic, such as an SDP + offer, is a request. What the accessory answered is its state. If the + characteristic is readable, only the connection that wrote reads the + response back, until it closes, and it wins over the getter and a later + ``set_value``. Other connections read the characteristic's own value, + which the write leaves as it was. A write without ``r`` stays readable + for everyone, and a characteristic that also has ``ev`` notifies the + written value as any write does. + """ + readable = HAP_PERMISSION_READ in char.properties[PROP_PERMISSIONS] + try: + response = char.to_valid_value(response) + except ValueError: + logger.debug( + "%s: not keeping the write response, it is invalid: %s", + char.display_name, + response, + ) + return response + if readable: + if client_addr is not None: + self._write_responses.setdefault(client_addr, {})[char] = response + else: + char.value = response + return response + def prepare( self, prepare_query: dict[str, Any], client_addr: tuple[str, int] ) -> dict[str, int]: diff --git a/pyhap/hap_handler.py b/pyhap/hap_handler.py index 83a47380..e027811a 100644 --- a/pyhap/hap_handler.py +++ b/pyhap/hap_handler.py @@ -679,7 +679,7 @@ def handle_get_characteristics(self) -> None: assert self.parsed_url is not None # noqa: S101 params = parse_qs(self.parsed_url.query) response = self.accessory_handler.get_characteristics( - params["id"][0].split(",") + params["id"][0].split(","), cast("tuple[str, int]", self.client_address) ) chars = response[HAP_REPR_CHARS] diff --git a/tests/test_accessory_driver.py b/tests/test_accessory_driver.py index 475a3c7f..bc85ddb2 100644 --- a/tests/test_accessory_driver.py +++ b/tests/test_accessory_driver.py @@ -2,6 +2,7 @@ import asyncio from concurrent.futures import ThreadPoolExecutor +from functools import partial import os import tempfile import time @@ -609,6 +610,10 @@ def test_listen_address(kwargs: dict[str, Any], expected: Any) -> None: assert server.call_args.args[0] == (expected, 51234) +WRITER = ("1.2.3.4", 5) +OTHER_CONTROLLER = ("5.6.7.8", 9) + + def _write( driver: AccessoryDriver, acc: Accessory, char: Characteristic, **query: Any ) -> dict[str, Any] | None: @@ -624,17 +629,25 @@ def _write( } ] }, - ("1.2.3.4", 5), + WRITER, ) @pytest.mark.parametrize("bridged", [False, True], ids=["standalone", "bridged"]) -@pytest.mark.parametrize("permissions", [["pr", "pw", "wr"], ["pw", "wr"]]) +@pytest.mark.parametrize( + ("permissions", "stored"), + [ + # A readable char keeps its default value, see the read tests below. + pytest.param(["pr", "pw", "wr"], 0, id="readable"), + pytest.param(["pw", "wr"], 1, id="write-only"), + ], +) @pytest.mark.parametrize(("returned", "sent"), [(1, 1), (1.7, 1)]) def test_write_response_returned_when_requested( driver: AccessoryDriver, bridged: bool, permissions: list[str], + stored: int, returned: Any, sent: int, ) -> None: @@ -659,8 +672,7 @@ def test_write_response_returned_when_requested( } char.setter_callback.assert_called_once_with(0) service_setter.assert_called_once_with({"Value": 0}) - # Like HAP-NodeJS, the response is the value from now on. - assert char.value == sent + assert char.value == stored @pytest.mark.parametrize( @@ -688,16 +700,27 @@ def test_write_response_not_returned( assert char.value == 0 -def test_invalid_write_response_keeps_written_value(driver: AccessoryDriver) -> None: - """A response the char can't hold is still sent, the written value stays.""" - acc, char, _ = _add_bridged_or_standalone(driver, ["pr", "pw", "wr"], False) +@pytest.mark.parametrize( + ("permissions", "stored"), + [ + pytest.param(["pr", "pw", "wr"], 0, id="readable"), + pytest.param(["pw", "wr"], 5, id="write-only"), + ], +) +def test_invalid_write_response_is_sent_but_not_kept( + driver: AccessoryDriver, permissions: list[str], stored: int +) -> None: + """A response the char can't hold is still sent, but not kept as its value.""" + acc, char, _ = _add_bridged_or_standalone(driver, permissions, False) char.setter_callback = MagicMock(return_value="not a number") - response = _write(driver, acc, char, **{HAP_REPR_WRITE_RESPONSE: True}) + response = _write( + driver, acc, char, **{HAP_REPR_VALUE: 5, HAP_REPR_WRITE_RESPONSE: True} + ) assert response is not None assert response[HAP_REPR_CHARS][0][HAP_REPR_VALUE] == "not a number" - assert char.value == 0 + assert char.value == stored def test_write_response_value_of_write_only_char_not_readable( @@ -723,6 +746,222 @@ def test_write_response_value_of_write_only_char_not_readable( assert HAP_REPR_VALUE not in acc_rep[HAP_REPR_SERVICES][-1][HAP_REPR_CHARS][0] +def _add_write_response_char( + driver: AccessoryDriver, bridged: bool +) -> tuple[Accessory, Characteristic, str]: + """Add a pr, pw and wr char that answers a write with 7.""" + acc, char, _ = _add_bridged_or_standalone(driver, ["pr", "pw", "wr"], bridged) + char.getter_callback = None + char.setter_callback = MagicMock(return_value=7) + return acc, char, f"{acc.aid}.{acc.iid_manager.get_iid(char)}" + + +def _read(driver: AccessoryDriver, char_id: str, client_addr: Any) -> Any: + """Return the value ``client_addr`` reads from ``char_id``.""" + (rep,) = driver.get_characteristics([char_id], client_addr)[HAP_REPR_CHARS] + assert rep[HAP_REPR_STATUS] == HAP_SERVER_STATUS.SUCCESS + return rep[HAP_REPR_VALUE] + + +@pytest.mark.parametrize("bridged", [False, True], ids=["standalone", "bridged"]) +@pytest.mark.parametrize( + ("reader", "expected"), + [ + pytest.param(WRITER, 7, id="writer"), + pytest.param(OTHER_CONTROLLER, 0, id="other-controller"), + pytest.param(None, 0, id="no-client"), + ], +) +def test_write_response_readable_only_by_writer( + driver: AccessoryDriver, bridged: bool, reader: Any, expected: int +) -> None: + """Only the controller that wrote reads its write response back.""" + acc, char, char_id = _add_write_response_char(driver, bridged) + + _write(driver, acc, char, **{HAP_REPR_VALUE: 5, HAP_REPR_WRITE_RESPONSE: True}) + + assert _read(driver, char_id, reader) == expected + + +def test_write_responses_kept_per_controller(driver: AccessoryDriver) -> None: + """Each controller reads its own last write response.""" + acc, char, char_id = _add_write_response_char(driver, False) + char.setter_callback = MagicMock(side_effect=[7, 8]) + request = { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: acc.iid_manager.get_iid(char), + HAP_REPR_VALUE: 5, + HAP_REPR_WRITE_RESPONSE: True, + } + ] + } + + driver.set_characteristics(request, WRITER) + driver.set_characteristics(request, OTHER_CONTROLLER) + + assert _read(driver, char_id, WRITER) == 7 + assert _read(driver, char_id, OTHER_CONTROLLER) == 8 + + +@pytest.mark.parametrize("bridged", [False, True], ids=["standalone", "bridged"]) +def test_write_response_not_in_accessories( + driver: AccessoryDriver, bridged: bool +) -> None: + """The accessory database has the default value, not the write response.""" + acc, char, _ = _add_write_response_char(driver, bridged) + + _write(driver, acc, char, **{HAP_REPR_VALUE: 5, HAP_REPR_WRITE_RESPONSE: True}) + + acc_rep = driver.get_accessories()[HAP_REPR_ACCS][-1] + assert acc_rep[HAP_REPR_SERVICES][-1][HAP_REPR_CHARS][0][HAP_REPR_VALUE] == 0 + + +async def test_write_response_dropped_on_disconnect(driver: AccessoryDriver) -> None: + """A write response is not read back after the connection closed.""" + driver.loop = asyncio.get_running_loop() + acc, char, char_id = _add_write_response_char(driver, False) + _write(driver, acc, char, **{HAP_REPR_VALUE: 5, HAP_REPR_WRITE_RESPONSE: True}) + + driver.connection_lost(WRITER) + assert _read(driver, char_id, WRITER) == 7 + # The response is dropped once the loop runs. + await asyncio.sleep(0) + + assert _read(driver, char_id, WRITER) == 0 + + +def test_plain_write_replaces_write_response(driver: AccessoryDriver) -> None: + """A write without ``r`` drops the writer's former response.""" + acc, char, char_id = _add_write_response_char(driver, False) + _write(driver, acc, char, **{HAP_REPR_VALUE: 5, HAP_REPR_WRITE_RESPONSE: True}) + + _write(driver, acc, char, **{HAP_REPR_VALUE: 9}) + + assert _read(driver, char_id, WRITER) == 9 + assert _read(driver, char_id, OTHER_CONTROLLER) == 9 + + +@pytest.mark.parametrize( + ("setter_kwargs", "writer_reads"), + [ + pytest.param({"return_value": 7}, 7, id="answered"), + pytest.param({"side_effect": RuntimeError}, 3, id="raises"), + ], +) +def test_write_response_keeps_accessory_value( + driver: AccessoryDriver, setter_kwargs: dict[str, Any], writer_reads: int +) -> None: + """Other connections keep reading the value the accessory set.""" + acc, char, char_id = _add_write_response_char(driver, False) + char.setter_callback = MagicMock(**setter_kwargs) + char.set_value(3) + + _write(driver, acc, char, **{HAP_REPR_VALUE: 5, HAP_REPR_WRITE_RESPONSE: True}) + + assert _read(driver, char_id, WRITER) == writer_reads + assert _read(driver, char_id, OTHER_CONTROLLER) == 3 + + +def test_write_response_keeps_value_set_by_setter(driver: AccessoryDriver) -> None: + """A value the setter sets on its own characteristic is kept.""" + acc, char, char_id = _add_write_response_char(driver, False) + + def _setter(value: int) -> int: + char.set_value(42) + return 7 + + char.setter_callback = _setter + + _write(driver, acc, char, **{HAP_REPR_VALUE: 5, HAP_REPR_WRITE_RESPONSE: True}) + + assert _read(driver, char_id, WRITER) == 7 + assert _read(driver, char_id, OTHER_CONTROLLER) == 42 + + +@pytest.mark.parametrize("reader", [WRITER, OTHER_CONTROLLER], ids=["writer", "other"]) +@pytest.mark.parametrize( + "make_setter", + [ + pytest.param(partial(MagicMock, side_effect=RuntimeError), id="raises"), + pytest.param(partial(MagicMock, return_value=None), id="no-response"), + pytest.param(lambda: None, id="no-setter"), + ], +) +def test_write_response_request_not_shared_without_response( + driver: AccessoryDriver, reader: Any, make_setter: Any +) -> None: + """The written request stays private when no response is kept.""" + acc, char, char_id = _add_write_response_char(driver, False) + char.setter_callback = make_setter() + + _write(driver, acc, char, **{HAP_REPR_VALUE: 5, HAP_REPR_WRITE_RESPONSE: True}) + + assert _read(driver, char_id, reader) == 0 + + +@pytest.mark.parametrize( + "second", + [ + pytest.param(RuntimeError("boom"), id="raises"), + pytest.param("not a number", id="invalid"), + pytest.param(None, id="no-response"), + ], +) +def test_stale_write_response_dropped(driver: AccessoryDriver, second: Any) -> None: + """A later write that keeps no response drops the former one.""" + acc, char, char_id = _add_write_response_char(driver, False) + char.setter_callback = MagicMock(side_effect=[7, second]) + query = {HAP_REPR_VALUE: 5, HAP_REPR_WRITE_RESPONSE: True} + + _write(driver, acc, char, **query) + _write(driver, acc, char, **query) + + assert _read(driver, char_id, WRITER) == 0 + + +def test_write_response_without_client_not_kept(driver: AccessoryDriver) -> None: + """A write response without a known connection is sent but not kept.""" + acc, char, _ = _add_write_response_char(driver, False) + request = { + HAP_REPR_CHARS: [ + { + HAP_REPR_AID: acc.aid, + HAP_REPR_IID: acc.iid_manager.get_iid(char), + HAP_REPR_VALUE: 5, + HAP_REPR_WRITE_RESPONSE: True, + } + ] + } + + no_client: Any = None + response = driver.set_characteristics(request, no_client) + + assert response is not None + assert response[HAP_REPR_CHARS][0][HAP_REPR_VALUE] == 7 + assert driver._write_responses == {} + + +@pytest.mark.parametrize("reader", [WRITER, OTHER_CONTROLLER], ids=["writer", "other"]) +@pytest.mark.parametrize( + "query", + [ + pytest.param({}, id="not-requested"), + pytest.param({HAP_REPR_WRITE_RESPONSE: False}, id="declined"), + ], +) +def test_written_value_readable_without_write_response( + driver: AccessoryDriver, reader: Any, query: dict[str, Any] +) -> None: + """Without a write response the written value is read by every controller.""" + acc, char, char_id = _add_write_response_char(driver, False) + + _write(driver, acc, char, **{HAP_REPR_VALUE: 5, **query}) + + assert _read(driver, char_id, reader) == 5 + + def test_service_callbacks(driver: AccessoryDriver) -> None: bridge = Bridge(driver, "mybridge") acc = Accessory(driver, "TestAcc", aid=2) diff --git a/tests/test_hap_handler.py b/tests/test_hap_handler.py index 9dc91435..fc390c95 100644 --- a/tests/test_hap_handler.py +++ b/tests/test_hap_handler.py @@ -4,7 +4,7 @@ from typing import Any, NoReturn from unittest.mock import patch from urllib.parse import urlparse -from uuid import UUID +from uuid import UUID, uuid1 from chacha20poly1305_reuseable import ChaCha20Poly1305Reusable as ChaCha20Poly1305 from cryptography.hazmat.primitives import serialization @@ -18,8 +18,15 @@ MAX_FAILED_PAIRING_ATTEMPTS, AccessoryDriver, ) -from pyhap.characteristic import CharacteristicError +from pyhap.characteristic import ( + HAP_FORMAT_INT, + PROP_FORMAT, + PROP_PERMISSIONS, + Characteristic, + CharacteristicError, +) from pyhap.const import HAP_PERMISSIONS, HAP_SERVER_STATUS +from pyhap.service import Service CLIENT_UUID = UUID("7d0d1ee9-46fe-4a56-a115-69df3f6860c1") CLIENT_UUID_BYTES = str(CLIENT_UUID).upper().encode("utf-8") @@ -1018,6 +1025,51 @@ def test_handle_get_characteristics_encrypted(driver: AccessoryDriver) -> None: assert decoded_response["characteristics"][0]["status"] == -70402 +@pytest.mark.parametrize( + ("reader", "expected"), + [ + pytest.param(("1.2.3.4", 5), b'"value":7', id="writer"), + pytest.param(("5.6.7.8", 9), b'"value":0', id="other-controller"), + ], +) +def test_handle_get_characteristics_write_response_per_client( + driver: AccessoryDriver, reader: tuple[str, int], expected: bytes +) -> None: + """Only the client that wrote reads the write response back.""" + acc = Accessory(driver, "TestAcc", aid=1) + service = Service(uuid1(), "Test Service") + char = Characteristic( + "Value", + uuid1(), + {PROP_FORMAT: HAP_FORMAT_INT, PROP_PERMISSIONS: ["pr", "pw", "wr"]}, + ) + char.setter_callback = lambda _value: 7 + service.add_characteristic(char) + acc.add_service(service) + driver.add_accessory(acc) + iid = acc.iid_manager.get_iid(char) + + writer = hap_handler.HAPServerHandler(driver, ("1.2.3.4", 5)) + writer.is_encrypted = True + writer.response = hap_handler.HAPResponse() + writer.request_body = ( + b'{"characteristics":[{"aid":1,"iid":%d,"value":5,"r":true}]}' % iid + ) + writer.handle_set_characteristics() + assert b'"value":7' in writer.response.body + + handler = hap_handler.HAPServerHandler(driver, reader) + handler.is_encrypted = True + response = hap_handler.HAPResponse() + handler.response = response + handler.path = f"/characteristics?id=1.{iid}" + handler.parsed_url = urlparse(handler.path) + handler.handle_get_characteristics() + + assert response.status_code == 200 + assert expected in response.body + + def test_handle_resource_unencrypted_rejected(driver: AccessoryDriver) -> None: """Verify an unencrypted POST /resource is rejected with 401. From 8f298c6b8b7149e7d7a4a15baad7c7f8fcaf18ec Mon Sep 17 00:00:00 2001 From: Raphael Hehl <7577984+RaHehl@users.noreply.github.com> Date: Tue, 10 Nov 2026 13:31:11 +0100 Subject: [PATCH 6/9] Drop the redundant bool() around the write response check --- pyhap/accessory_driver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyhap/accessory_driver.py b/pyhap/accessory_driver.py index 88379cbe..bef8e269 100644 --- a/pyhap/accessory_driver.py +++ b/pyhap/accessory_driver.py @@ -1112,7 +1112,7 @@ def set_characteristics( # Only characteristics with the write response permission answer # a write with a value, and only when the controller asks for it. - write_response = bool( + write_response = ( query.get(HAP_REPR_WRITE_RESPONSE, False) and HAP_PERMISSION_WRITE_RESPONSE in char.properties[PROP_PERMISSIONS] ) From ff12fb69d8669d2f7e901f93161e1c5808147699 Mon Sep 17 00:00:00 2001 From: Raphael Hehl <7577984+RaHehl@users.noreply.github.com> Date: Tue, 10 Nov 2026 13:32:46 +0100 Subject: [PATCH 7/9] Type the write response helpers with the HAP JSON value types --- pyhap/accessory_driver.py | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/pyhap/accessory_driver.py b/pyhap/accessory_driver.py index bef8e269..04f57d7b 100644 --- a/pyhap/accessory_driver.py +++ b/pyhap/accessory_driver.py @@ -87,13 +87,17 @@ MAX_PREPARED_WRITES_PER_CLIENT = 16 MAX_PREPARED_WRITE_TTL_SECONDS = 30.0 +# A characteristic value as HAP JSON carries it. +HAPValue = bool | int | float | str | None -def _stored_value(char: Any, value: Any) -> Any: + +def _stored_value(char: Characteristic, value: HAPValue) -> HAPValue: """Return the value ``client_update_value`` stores for ``value``.""" try: - return char.to_valid_value(value) + stored: HAPValue = char.to_valid_value(value) except ValueError: return value + return stored def _wrap_char_setter(char: Any, value: Any, client_addr: Any) -> tuple[int, Any]: @@ -347,7 +351,9 @@ def __init__( self.http_server = HAPServer(network_tuple, self) self.prepared_writes = {} # Write responses of readable characteristics, per client. - self._write_responses: dict[tuple[str, int], dict[Characteristic, Any]] = {} + self._write_responses: dict[ + tuple[str, int], dict[Characteristic, HAPValue] + ] = {} def listen_address(self) -> str | list[str]: """Return the address, or list of addresses, the HAP server listens on.""" @@ -1197,9 +1203,9 @@ def set_characteristics( def _store_write_response( self, char: Characteristic, - response: Any, + response: HAPValue, client_addr: tuple[str, int] | None, - ) -> Any: + ) -> HAPValue: """Keep the write response and return what to send. The written value of a write response characteristic, such as an SDP @@ -1213,7 +1219,7 @@ def _store_write_response( """ readable = HAP_PERMISSION_READ in char.properties[PROP_PERMISSIONS] try: - response = char.to_valid_value(response) + valid: HAPValue = char.to_valid_value(response) except ValueError: logger.debug( "%s: not keeping the write response, it is invalid: %s", @@ -1223,10 +1229,10 @@ def _store_write_response( return response if readable: if client_addr is not None: - self._write_responses.setdefault(client_addr, {})[char] = response + self._write_responses.setdefault(client_addr, {})[char] = valid else: - char.value = response - return response + char.value = valid + return valid def prepare( self, prepare_query: dict[str, Any], client_addr: tuple[str, int] From c9ed715b687bea4ed9e1f527f1098bdf739603d4 Mon Sep 17 00:00:00 2001 From: Raphael Hehl <7577984+RaHehl@users.noreply.github.com> Date: Tue, 10 Nov 2026 13:32:51 +0100 Subject: [PATCH 8/9] Explain why a private write puts the former value back --- pyhap/accessory_driver.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pyhap/accessory_driver.py b/pyhap/accessory_driver.py index 04f57d7b..387ba419 100644 --- a/pyhap/accessory_driver.py +++ b/pyhap/accessory_driver.py @@ -1141,7 +1141,11 @@ def set_characteristics( set_result, set_result_value = _wrap_char_setter( char, value, client_addr ) - # Put back the former value unless the setter set its own. + # A write with ``r`` to a readable characteristic is a request, + # e.g. a Setup Endpoints offer with the writer's SRTP keys. Only + # the writer reads the answer back, see _store_write_response. + # client_update_value stored the request as the value, so put + # back the former one unless the setter set a value of its own. if private_write and char.value == _stored_value(char, value): char.value = previous_value From 2e0018828bdfef72cbcf18621566e61b54de2af3 Mon Sep 17 00:00:00 2001 From: Raphael Hehl <7577984+RaHehl@users.noreply.github.com> Date: Tue, 10 Nov 2026 14:32:57 +0100 Subject: [PATCH 9/9] Don't tell subscribers the request of a write with r client_update_value takes notify=False for a write response request on a readable characteristic, so ev subscribers don't get a request that only the writer may read, e.g. a WiFi configuration with a key. HAPValue moves to pyhap.characteristic next to the formats. --- pyhap/accessory_driver.py | 20 ++++++++++++-------- pyhap/characteristic.py | 15 ++++++++++++--- tests/test_accessory_driver.py | 28 ++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 11 deletions(-) diff --git a/pyhap/accessory_driver.py b/pyhap/accessory_driver.py index 387ba419..08e3e817 100644 --- a/pyhap/accessory_driver.py +++ b/pyhap/accessory_driver.py @@ -36,7 +36,12 @@ from pyhap import util from pyhap.accessory import Accessory, Bridge, get_topic -from pyhap.characteristic import PROP_PERMISSIONS, Characteristic, CharacteristicError +from pyhap.characteristic import ( + PROP_PERMISSIONS, + Characteristic, + CharacteristicError, + HAPValue, +) from pyhap.const import ( HAP_PERMISSION_NOTIFY, HAP_PERMISSION_READ, @@ -87,9 +92,6 @@ MAX_PREPARED_WRITES_PER_CLIENT = 16 MAX_PREPARED_WRITE_TTL_SECONDS = 30.0 -# A characteristic value as HAP JSON carries it. -HAPValue = bool | int | float | str | None - def _stored_value(char: Characteristic, value: HAPValue) -> HAPValue: """Return the value ``client_update_value`` stores for ``value``.""" @@ -100,10 +102,12 @@ def _stored_value(char: Characteristic, value: HAPValue) -> HAPValue: return stored -def _wrap_char_setter(char: Any, value: Any, client_addr: Any) -> tuple[int, Any]: +def _wrap_char_setter( + char: Any, value: Any, client_addr: Any, notify: bool = True +) -> tuple[int, Any]: """Process an characteristic setter callback trapping and logging all exceptions.""" try: - response = char.client_update_value(value, client_addr) + response = char.client_update_value(value, client_addr, notify=notify) except Exception: logger.exception( "%s: Error while setting characteristic %s to %s", @@ -1124,7 +1128,7 @@ def set_characteristics( ) # A new write replaces a former response of this connection. The # request of a readable write response characteristic is not - # shared, whatever the setter does, except to ev subscribers. + # shared, whatever the setter does, also not with ev subscribers. if value is not None: self._write_responses.get(client_addr, {}).pop(char, None) private_write = ( @@ -1139,7 +1143,7 @@ def set_characteristics( if value is not None: set_result, set_result_value = _wrap_char_setter( - char, value, client_addr + char, value, client_addr, notify=not private_write ) # A write with ``r`` to a readable characteristic is a request, # e.g. a Setup Endpoints offer with the writer's SRTP keys. Only diff --git a/pyhap/characteristic.py b/pyhap/characteristic.py index a0b09a69..773aa57f 100644 --- a/pyhap/characteristic.py +++ b/pyhap/characteristic.py @@ -47,6 +47,9 @@ def _setter_wants_client_addr(setter: Callable[..., Any]) -> bool: # ### HAP Format ### +# A characteristic value as HAP JSON carries it. +HAPValue = bool | int | float | str | None + HAP_FORMAT_BOOL = "bool" HAP_FORMAT_INT = "int" HAP_FORMAT_FLOAT = "float" @@ -394,11 +397,17 @@ def set_value(self, value: Any, should_notify: bool = True) -> None: self.value = None def client_update_value( - self, value: Any, sender_client_addr: tuple[str, int] | None = None + self, + value: Any, + sender_client_addr: tuple[str, int] | None = None, + *, + notify: bool = True, ) -> Any: """Called from broker for value change in Home app. - Change self.value to value and call callback. + Change self.value to value and call callback. With ``notify`` False, + subscribers are not told about the written value, e.g. for a request + that only the writing connection may read back. """ original_value = value if not self._always_null or original_value is not None: @@ -421,7 +430,7 @@ def client_update_value( else: response = self.setter_callback(value) changed = self._value != previous_value - if changed: + if changed and notify: self.notify(sender_client_addr) if self._always_null: self.value = None diff --git a/tests/test_accessory_driver.py b/tests/test_accessory_driver.py index f7442732..13a5d471 100644 --- a/tests/test_accessory_driver.py +++ b/tests/test_accessory_driver.py @@ -880,6 +880,34 @@ def _setter(value: int) -> int: assert _read(driver, char_id, OTHER_CONTROLLER) == 42 +@pytest.mark.parametrize( + ("write_response", "notified"), + [ + pytest.param(True, [], id="private"), + pytest.param(False, [5], id="plain"), + ], +) +def test_private_write_not_notified( + driver: AccessoryDriver, write_response: bool, notified: list[int] +) -> None: + """Subscribers are not told the request of a write with ``r``.""" + acc, char, _ = _add_bridged_or_standalone(driver, ["pr", "pw", "ev", "wr"], False) + char.getter_callback = None + char.setter_callback = MagicMock(return_value=7) + published = [] + with patch.object( + acc, "publish", side_effect=lambda value, *_: published.append(value) + ): + _write( + driver, + acc, + char, + **{HAP_REPR_VALUE: 5, HAP_REPR_WRITE_RESPONSE: write_response}, + ) + + assert published == notified + + @pytest.mark.parametrize("reader", [WRITER, OTHER_CONTROLLER], ids=["writer", "other"]) @pytest.mark.parametrize( "make_setter",