mirror of
https://github.com/esphome/esphome.git
synced 2026-05-30 23:54:04 +08:00
[api] Fix truncation of Home Assistant attributes longer than 255 characters (#13348)
This commit is contained in:
@@ -1712,17 +1712,16 @@ void APIConnection::on_home_assistant_state_response(const HomeAssistantStateRes
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Create null-terminated state for callback (parse_number needs null-termination)
|
// Create null-terminated state for callback (parse_number needs null-termination)
|
||||||
// HA state max length is 255, so 256 byte buffer covers all cases
|
// HA state max length is 255 characters, but attributes can be much longer
|
||||||
char state_buf[256];
|
// Use stack buffer for common case (states), heap fallback for large attributes
|
||||||
size_t copy_len = msg.state.size();
|
size_t state_len = msg.state.size();
|
||||||
if (copy_len >= sizeof(state_buf)) {
|
SmallBufferWithHeapFallback<256> state_buf_alloc(state_len + 1);
|
||||||
copy_len = sizeof(state_buf) - 1; // Truncate to leave space for null terminator
|
char *state_buf = reinterpret_cast<char *>(state_buf_alloc.get());
|
||||||
|
if (state_len > 0) {
|
||||||
|
memcpy(state_buf, msg.state.c_str(), state_len);
|
||||||
}
|
}
|
||||||
if (copy_len > 0) {
|
state_buf[state_len] = '\0';
|
||||||
memcpy(state_buf, msg.state.c_str(), copy_len);
|
it.callback(StringRef(state_buf, state_len));
|
||||||
}
|
|
||||||
state_buf[copy_len] = '\0';
|
|
||||||
it.callback(StringRef(state_buf, copy_len));
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
#endif
|
#endif
|
||||||
|
|||||||
@@ -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 {
|
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
|
SmallBufferWithHeapFallback<17> buffer_alloc(len + 1); // Most I2C writes are <= 16 bytes
|
||||||
uint8_t *buffer = buffer_alloc.get(len + 1);
|
uint8_t *buffer = buffer_alloc.get();
|
||||||
|
|
||||||
buffer[0] = a_register;
|
buffer[0] = a_register;
|
||||||
std::copy(data, data + len, buffer + 1);
|
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 {
|
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
|
SmallBufferWithHeapFallback<18> buffer_alloc(len + 2); // Most I2C writes are <= 16 bytes + 2 for register
|
||||||
uint8_t *buffer = buffer_alloc.get(len + 2);
|
uint8_t *buffer = buffer_alloc.get();
|
||||||
|
|
||||||
buffer[0] = a_register >> 8;
|
buffer[0] = a_register >> 8;
|
||||||
buffer[1] = a_register;
|
buffer[1] = a_register;
|
||||||
|
|||||||
@@ -11,22 +11,6 @@
|
|||||||
namespace esphome {
|
namespace esphome {
|
||||||
namespace i2c {
|
namespace i2c {
|
||||||
|
|
||||||
/// @brief Helper class for efficient buffer allocation - uses stack for small sizes, heap for large
|
|
||||||
template<size_t STACK_SIZE> class SmallBufferWithHeapFallback {
|
|
||||||
public:
|
|
||||||
uint8_t *get(size_t size) {
|
|
||||||
if (size <= STACK_SIZE) {
|
|
||||||
return this->stack_buffer_;
|
|
||||||
}
|
|
||||||
this->heap_buffer_ = std::unique_ptr<uint8_t[]>(new uint8_t[size]);
|
|
||||||
return this->heap_buffer_.get();
|
|
||||||
}
|
|
||||||
|
|
||||||
private:
|
|
||||||
uint8_t stack_buffer_[STACK_SIZE];
|
|
||||||
std::unique_ptr<uint8_t[]> heap_buffer_;
|
|
||||||
};
|
|
||||||
|
|
||||||
/// @brief Error codes returned by I2CBus and I2CDevice methods
|
/// @brief Error codes returned by I2CBus and I2CDevice methods
|
||||||
enum ErrorCode {
|
enum ErrorCode {
|
||||||
NO_ERROR = 0, ///< No error found during execution of method
|
NO_ERROR = 0, ///< No error found during execution of method
|
||||||
@@ -92,8 +76,8 @@ class I2CBus {
|
|||||||
total_len += read_buffers[i].len;
|
total_len += read_buffers[i].len;
|
||||||
}
|
}
|
||||||
|
|
||||||
SmallBufferWithHeapFallback<128> buffer_alloc; // Most I2C reads are small
|
SmallBufferWithHeapFallback<128> buffer_alloc(total_len); // Most I2C reads are small
|
||||||
uint8_t *buffer = buffer_alloc.get(total_len);
|
uint8_t *buffer = buffer_alloc.get();
|
||||||
|
|
||||||
auto err = this->write_readv(address, nullptr, 0, buffer, total_len);
|
auto err = this->write_readv(address, nullptr, 0, buffer, total_len);
|
||||||
if (err != ERROR_OK)
|
if (err != ERROR_OK)
|
||||||
@@ -116,8 +100,8 @@ class I2CBus {
|
|||||||
total_len += write_buffers[i].len;
|
total_len += write_buffers[i].len;
|
||||||
}
|
}
|
||||||
|
|
||||||
SmallBufferWithHeapFallback<128> buffer_alloc; // Most I2C writes are small
|
SmallBufferWithHeapFallback<128> buffer_alloc(total_len); // Most I2C writes are small
|
||||||
uint8_t *buffer = buffer_alloc.get(total_len);
|
uint8_t *buffer = buffer_alloc.get();
|
||||||
|
|
||||||
size_t pos = 0;
|
size_t pos = 0;
|
||||||
for (size_t i = 0; i != count; i++) {
|
for (size_t i = 0; i != count; i++) {
|
||||||
|
|||||||
@@ -366,6 +366,35 @@ template<typename T> class FixedVector {
|
|||||||
const T *end() const { return data_ + size_; }
|
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<size_t STACK_SIZE> 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
|
/// @name Mathematics
|
||||||
|
|||||||
@@ -108,6 +108,25 @@ text_sensor:
|
|||||||
format: "HA Empty state updated: %s"
|
format: "HA Empty state updated: %s"
|
||||||
args: ['x.c_str()']
|
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 component for testing HA number control
|
||||||
number:
|
number:
|
||||||
- platform: template
|
- platform: template
|
||||||
|
|||||||
@@ -40,6 +40,7 @@ async def test_api_homeassistant(
|
|||||||
humidity_update_future = loop.create_future()
|
humidity_update_future = loop.create_future()
|
||||||
motion_update_future = loop.create_future()
|
motion_update_future = loop.create_future()
|
||||||
weather_update_future = loop.create_future()
|
weather_update_future = loop.create_future()
|
||||||
|
long_attr_future = loop.create_future()
|
||||||
|
|
||||||
# Number future
|
# Number future
|
||||||
ha_number_future = loop.create_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.]+)")
|
humidity_update_pattern = re.compile(r"HA Humidity state updated: ([\d.]+)")
|
||||||
motion_update_pattern = re.compile(r"HA Motion state changed: (ON|OFF)")
|
motion_update_pattern = re.compile(r"HA Motion state changed: (ON|OFF)")
|
||||||
weather_update_pattern = re.compile(r"HA Weather condition updated: (\w+)")
|
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
|
# Number pattern
|
||||||
ha_number_pattern = re.compile(r"Setting HA number to: ([\d.]+)")
|
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):
|
elif not weather_update_future.done() and weather_update_pattern.search(line):
|
||||||
weather_update_future.set_result(line)
|
weather_update_future.set_result(line)
|
||||||
|
|
||||||
# Check number pattern
|
# Check long attribute pattern - separate if since it can come at different times
|
||||||
elif not ha_number_future.done() and ha_number_pattern.search(line):
|
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)
|
match = ha_number_pattern.search(line)
|
||||||
if match:
|
if match:
|
||||||
ha_number_future.set_result(match.group(1))
|
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("binary_sensor.external_motion", "", "ON")
|
||||||
client.send_home_assistant_state("weather.home", "condition", "sunny")
|
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
|
# Test edge cases for zero-copy implementation safety
|
||||||
# Empty entity_id should be silently ignored (no crash)
|
# Empty entity_id should be silently ignored (no crash)
|
||||||
client.send_home_assistant_state("", "", "should_be_ignored")
|
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)
|
number_value = await asyncio.wait_for(ha_number_future, timeout=5.0)
|
||||||
assert number_value == "42.5", f"Unexpected number value: {number_value}"
|
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
|
# Wait for completion
|
||||||
await asyncio.wait_for(tests_complete_future, timeout=5.0)
|
await asyncio.wait_for(tests_complete_future, timeout=5.0)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user