diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 0804985cc5..25512de4c7 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -1712,17 +1712,16 @@ void APIConnection::on_home_assistant_state_response(const HomeAssistantStateRes } // Create null-terminated state for callback (parse_number needs null-termination) - // HA state max length is 255, so 256 byte buffer covers all cases - char state_buf[256]; - size_t copy_len = msg.state.size(); - if (copy_len >= sizeof(state_buf)) { - copy_len = sizeof(state_buf) - 1; // Truncate to leave space for null terminator + // HA state max length is 255 characters, but attributes can be much longer + // Use stack buffer for common case (states), heap fallback for large attributes + size_t state_len = msg.state.size(); + SmallBufferWithHeapFallback<256> state_buf_alloc(state_len + 1); + char *state_buf = reinterpret_cast(state_buf_alloc.get()); + if (state_len > 0) { + memcpy(state_buf, msg.state.c_str(), state_len); } - if (copy_len > 0) { - memcpy(state_buf, msg.state.c_str(), copy_len); - } - state_buf[copy_len] = '\0'; - it.callback(StringRef(state_buf, copy_len)); + state_buf[state_len] = '\0'; + it.callback(StringRef(state_buf, state_len)); } } #endif diff --git a/esphome/components/i2c/i2c.cpp b/esphome/components/i2c/i2c.cpp index f8c7a1b40b..c1e7336ce4 100644 --- a/esphome/components/i2c/i2c.cpp +++ b/esphome/components/i2c/i2c.cpp @@ -42,8 +42,8 @@ ErrorCode I2CDevice::read_register16(uint16_t a_register, uint8_t *data, size_t } ErrorCode I2CDevice::write_register(uint8_t a_register, const uint8_t *data, size_t len) const { - SmallBufferWithHeapFallback<17> buffer_alloc; // Most I2C writes are <= 16 bytes - uint8_t *buffer = buffer_alloc.get(len + 1); + SmallBufferWithHeapFallback<17> buffer_alloc(len + 1); // Most I2C writes are <= 16 bytes + uint8_t *buffer = buffer_alloc.get(); buffer[0] = a_register; std::copy(data, data + len, buffer + 1); @@ -51,8 +51,8 @@ ErrorCode I2CDevice::write_register(uint8_t a_register, const uint8_t *data, siz } ErrorCode I2CDevice::write_register16(uint16_t a_register, const uint8_t *data, size_t len) const { - SmallBufferWithHeapFallback<18> buffer_alloc; // Most I2C writes are <= 16 bytes + 2 for register - uint8_t *buffer = buffer_alloc.get(len + 2); + SmallBufferWithHeapFallback<18> buffer_alloc(len + 2); // Most I2C writes are <= 16 bytes + 2 for register + uint8_t *buffer = buffer_alloc.get(); buffer[0] = a_register >> 8; buffer[1] = a_register; diff --git a/esphome/components/i2c/i2c_bus.h b/esphome/components/i2c/i2c_bus.h index 1acbe506a3..3de5d5ca7b 100644 --- a/esphome/components/i2c/i2c_bus.h +++ b/esphome/components/i2c/i2c_bus.h @@ -11,22 +11,6 @@ namespace esphome { namespace i2c { -/// @brief Helper class for efficient buffer allocation - uses stack for small sizes, heap for large -template class SmallBufferWithHeapFallback { - public: - uint8_t *get(size_t size) { - if (size <= STACK_SIZE) { - return this->stack_buffer_; - } - this->heap_buffer_ = std::unique_ptr(new uint8_t[size]); - return this->heap_buffer_.get(); - } - - private: - uint8_t stack_buffer_[STACK_SIZE]; - std::unique_ptr heap_buffer_; -}; - /// @brief Error codes returned by I2CBus and I2CDevice methods enum ErrorCode { NO_ERROR = 0, ///< No error found during execution of method @@ -92,8 +76,8 @@ class I2CBus { total_len += read_buffers[i].len; } - SmallBufferWithHeapFallback<128> buffer_alloc; // Most I2C reads are small - uint8_t *buffer = buffer_alloc.get(total_len); + SmallBufferWithHeapFallback<128> buffer_alloc(total_len); // Most I2C reads are small + uint8_t *buffer = buffer_alloc.get(); auto err = this->write_readv(address, nullptr, 0, buffer, total_len); if (err != ERROR_OK) @@ -116,8 +100,8 @@ class I2CBus { total_len += write_buffers[i].len; } - SmallBufferWithHeapFallback<128> buffer_alloc; // Most I2C writes are small - uint8_t *buffer = buffer_alloc.get(total_len); + SmallBufferWithHeapFallback<128> buffer_alloc(total_len); // Most I2C writes are small + uint8_t *buffer = buffer_alloc.get(); size_t pos = 0; for (size_t i = 0; i != count; i++) { diff --git a/esphome/core/helpers.h b/esphome/core/helpers.h index 8b581eb97a..4185596c7b 100644 --- a/esphome/core/helpers.h +++ b/esphome/core/helpers.h @@ -366,6 +366,35 @@ template class FixedVector { const T *end() const { return data_ + size_; } }; +/// @brief Helper class for efficient buffer allocation - uses stack for small sizes, heap for large +/// This is useful when most operations need a small buffer but occasionally need larger ones. +/// The stack buffer avoids heap allocation in the common case, while heap fallback handles edge cases. +template class SmallBufferWithHeapFallback { + public: + explicit SmallBufferWithHeapFallback(size_t size) { + if (size <= STACK_SIZE) { + this->buffer_ = this->stack_buffer_; + } else { + this->heap_buffer_ = new uint8_t[size]; + this->buffer_ = this->heap_buffer_; + } + } + ~SmallBufferWithHeapFallback() { delete[] this->heap_buffer_; } + + // Delete copy and move operations to prevent double-delete + SmallBufferWithHeapFallback(const SmallBufferWithHeapFallback &) = delete; + SmallBufferWithHeapFallback &operator=(const SmallBufferWithHeapFallback &) = delete; + SmallBufferWithHeapFallback(SmallBufferWithHeapFallback &&) = delete; + SmallBufferWithHeapFallback &operator=(SmallBufferWithHeapFallback &&) = delete; + + uint8_t *get() { return this->buffer_; } + + private: + uint8_t stack_buffer_[STACK_SIZE]; + uint8_t *heap_buffer_{nullptr}; + uint8_t *buffer_; +}; + ///@} /// @name Mathematics diff --git a/tests/integration/fixtures/api_homeassistant.yaml b/tests/integration/fixtures/api_homeassistant.yaml index 8fe23b9a19..2d77821ff3 100644 --- a/tests/integration/fixtures/api_homeassistant.yaml +++ b/tests/integration/fixtures/api_homeassistant.yaml @@ -108,6 +108,25 @@ text_sensor: format: "HA Empty state updated: %s" args: ['x.c_str()'] + # Test long attribute handling (>255 characters) + # HA states are limited to 255 chars, but attributes are not + - platform: homeassistant + name: "HA Long Attribute" + entity_id: sensor.long_data + attribute: long_value + id: ha_long_attribute + on_value: + then: + - logger.log: + format: "HA Long attribute received, length: %d" + args: ['x.size()'] + # Log the first 50 and last 50 chars to verify no truncation + - lambda: |- + if (x.size() >= 100) { + ESP_LOGI("test", "Long attribute first 50 chars: %.50s", x.c_str()); + ESP_LOGI("test", "Long attribute last 50 chars: %s", x.c_str() + x.size() - 50); + } + # Number component for testing HA number control number: - platform: template diff --git a/tests/integration/test_api_homeassistant.py b/tests/integration/test_api_homeassistant.py index 3fe0dfe045..b4adedf873 100644 --- a/tests/integration/test_api_homeassistant.py +++ b/tests/integration/test_api_homeassistant.py @@ -40,6 +40,7 @@ async def test_api_homeassistant( humidity_update_future = loop.create_future() motion_update_future = loop.create_future() weather_update_future = loop.create_future() + long_attr_future = loop.create_future() # Number future ha_number_future = loop.create_future() @@ -58,6 +59,7 @@ async def test_api_homeassistant( humidity_update_pattern = re.compile(r"HA Humidity state updated: ([\d.]+)") motion_update_pattern = re.compile(r"HA Motion state changed: (ON|OFF)") weather_update_pattern = re.compile(r"HA Weather condition updated: (\w+)") + long_attr_pattern = re.compile(r"HA Long attribute received, length: (\d+)") # Number pattern ha_number_pattern = re.compile(r"Setting HA number to: ([\d.]+)") @@ -143,8 +145,14 @@ async def test_api_homeassistant( elif not weather_update_future.done() and weather_update_pattern.search(line): weather_update_future.set_result(line) - # Check number pattern - elif not ha_number_future.done() and ha_number_pattern.search(line): + # Check long attribute pattern - separate if since it can come at different times + if not long_attr_future.done(): + match = long_attr_pattern.search(line) + if match: + long_attr_future.set_result(int(match.group(1))) + + # Check number pattern - separate if since it can come at different times + if not ha_number_future.done(): match = ha_number_pattern.search(line) if match: ha_number_future.set_result(match.group(1)) @@ -179,6 +187,14 @@ async def test_api_homeassistant( client.send_home_assistant_state("binary_sensor.external_motion", "", "ON") client.send_home_assistant_state("weather.home", "condition", "sunny") + # Send a long attribute (300 characters) to test that attributes aren't truncated + # HA states are limited to 255 chars, but attributes are NOT limited + # This tests the fix for the 256-byte buffer truncation bug + long_attr_value = "X" * 300 # 300 chars - enough to expose truncation bug + client.send_home_assistant_state( + "sensor.long_data", "long_value", long_attr_value + ) + # Test edge cases for zero-copy implementation safety # Empty entity_id should be silently ignored (no crash) client.send_home_assistant_state("", "", "should_be_ignored") @@ -225,6 +241,13 @@ async def test_api_homeassistant( number_value = await asyncio.wait_for(ha_number_future, timeout=5.0) assert number_value == "42.5", f"Unexpected number value: {number_value}" + # Long attribute test - verify 300 chars weren't truncated to 255 + long_attr_len = await asyncio.wait_for(long_attr_future, timeout=5.0) + assert long_attr_len == 300, ( + f"Long attribute was truncated! Expected 300 chars, got {long_attr_len}. " + "This indicates the 256-byte truncation bug." + ) + # Wait for completion await asyncio.wait_for(tests_complete_future, timeout=5.0)