From be58cdda3ba00a8b0943271d0935294e209ba1c3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 15 Jun 2025 16:19:04 -0500 Subject: [PATCH] Fix protobuf encoding size mismatch by passing force parameter in encode_string (#9074) --- esphome/components/api/proto.h | 2 +- .../host_mode_empty_string_options.yaml | 58 +++++++++ .../test_host_mode_empty_string_options.py | 110 ++++++++++++++++++ 3 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 tests/integration/fixtures/host_mode_empty_string_options.yaml create mode 100644 tests/integration/test_host_mode_empty_string_options.py diff --git a/esphome/components/api/proto.h b/esphome/components/api/proto.h index 5265c4520d..eb0dbc151b 100644 --- a/esphome/components/api/proto.h +++ b/esphome/components/api/proto.h @@ -216,7 +216,7 @@ class ProtoWriteBuffer { this->buffer_->insert(this->buffer_->end(), data, data + len); } void encode_string(uint32_t field_id, const std::string &value, bool force = false) { - this->encode_string(field_id, value.data(), value.size()); + this->encode_string(field_id, value.data(), value.size(), force); } void encode_bytes(uint32_t field_id, const uint8_t *data, size_t len, bool force = false) { this->encode_string(field_id, reinterpret_cast(data), len, force); diff --git a/tests/integration/fixtures/host_mode_empty_string_options.yaml b/tests/integration/fixtures/host_mode_empty_string_options.yaml new file mode 100644 index 0000000000..ab8e6cd005 --- /dev/null +++ b/tests/integration/fixtures/host_mode_empty_string_options.yaml @@ -0,0 +1,58 @@ +esphome: + name: host-empty-string-test + +host: + +api: + batch_delay: 50ms + +select: + - platform: template + name: "Select Empty First" + id: select_empty_first + optimistic: true + options: + - "" # Empty string at the beginning + - "Option A" + - "Option B" + - "Option C" + initial_option: "Option A" + + - platform: template + name: "Select Empty Middle" + id: select_empty_middle + optimistic: true + options: + - "Option 1" + - "Option 2" + - "" # Empty string in the middle + - "Option 3" + - "Option 4" + initial_option: "Option 1" + + - platform: template + name: "Select Empty Last" + id: select_empty_last + optimistic: true + options: + - "Choice X" + - "Choice Y" + - "Choice Z" + - "" # Empty string at the end + initial_option: "Choice X" + +# Add a sensor to ensure we have other entities in the list +sensor: + - platform: template + name: "Test Sensor" + id: test_sensor + lambda: |- + return 42.0; + update_interval: 60s + +binary_sensor: + - platform: template + name: "Test Binary Sensor" + id: test_binary_sensor + lambda: |- + return true; diff --git a/tests/integration/test_host_mode_empty_string_options.py b/tests/integration/test_host_mode_empty_string_options.py new file mode 100644 index 0000000000..d2df839a75 --- /dev/null +++ b/tests/integration/test_host_mode_empty_string_options.py @@ -0,0 +1,110 @@ +"""Integration test for protobuf encoding of empty string options in select entities.""" + +from __future__ import annotations + +import asyncio + +from aioesphomeapi import EntityState, SelectInfo +import pytest + +from .types import APIClientConnectedFactory, RunCompiledFunction + + +@pytest.mark.asyncio +async def test_host_mode_empty_string_options( + yaml_config: str, + run_compiled: RunCompiledFunction, + api_client_connected: APIClientConnectedFactory, +) -> None: + """Test that select entities with empty string options are correctly encoded in protobuf messages. + + This tests the fix for the bug where the force parameter was not passed in encode_string, + causing empty strings in repeated fields to be skipped during encoding but included in + size calculation, leading to protobuf decoding errors. + """ + # Write, compile and run the ESPHome device, then connect to API + loop = asyncio.get_running_loop() + async with run_compiled(yaml_config), api_client_connected() as client: + # Verify we can get device info + device_info = await client.device_info() + assert device_info is not None + assert device_info.name == "host-empty-string-test" + + # Get list of entities - this will encode ListEntitiesSelectResponse messages + # with empty string options that would trigger the bug + entity_info, services = await client.list_entities_services() + + # Find our select entities + select_entities = [e for e in entity_info if isinstance(e, SelectInfo)] + assert len(select_entities) == 3, ( + f"Expected 3 select entities, got {len(select_entities)}" + ) + + # Verify each select entity by name and check their options + selects_by_name = {e.name: e for e in select_entities} + + # Check "Select Empty First" - empty string at beginning + assert "Select Empty First" in selects_by_name + empty_first = selects_by_name["Select Empty First"] + assert len(empty_first.options) == 4 + assert empty_first.options[0] == "" # Empty string at beginning + assert empty_first.options[1] == "Option A" + assert empty_first.options[2] == "Option B" + assert empty_first.options[3] == "Option C" + + # Check "Select Empty Middle" - empty string in middle + assert "Select Empty Middle" in selects_by_name + empty_middle = selects_by_name["Select Empty Middle"] + assert len(empty_middle.options) == 5 + assert empty_middle.options[0] == "Option 1" + assert empty_middle.options[1] == "Option 2" + assert empty_middle.options[2] == "" # Empty string in middle + assert empty_middle.options[3] == "Option 3" + assert empty_middle.options[4] == "Option 4" + + # Check "Select Empty Last" - empty string at end + assert "Select Empty Last" in selects_by_name + empty_last = selects_by_name["Select Empty Last"] + assert len(empty_last.options) == 4 + assert empty_last.options[0] == "Choice X" + assert empty_last.options[1] == "Choice Y" + assert empty_last.options[2] == "Choice Z" + assert empty_last.options[3] == "" # Empty string at end + + # If we got here without protobuf decoding errors, the fix is working + # The bug would have caused "Invalid protobuf message" errors with trailing bytes + + # Also verify we can interact with the select entities + # Subscribe to state changes + states: dict[int, EntityState] = {} + state_change_future: asyncio.Future[None] = loop.create_future() + + def on_state(state: EntityState) -> None: + """Track state changes.""" + states[state.key] = state + # When we receive the state change for our select, resolve the future + if state.key == empty_first.key and not state_change_future.done(): + state_change_future.set_result(None) + + client.subscribe_states(on_state) + + # Try setting a select to an empty string option + # This further tests that empty strings are handled correctly + client.select_command(empty_first.key, "") + + # Wait for state update with timeout + try: + await asyncio.wait_for(state_change_future, timeout=5.0) + except asyncio.TimeoutError: + pytest.fail( + "Did not receive state update after setting select to empty string" + ) + + # Verify the state was set to empty string + assert empty_first.key in states + select_state = states[empty_first.key] + assert hasattr(select_state, "state") + assert select_state.state == "" + + # The test passes if no protobuf decoding errors occurred + # With the bug, we would have gotten "Invalid protobuf message" errors