diff --git a/esphome/components/api/api.proto b/esphome/components/api/api.proto index b8dfb71a6a8..18dac6a2d17 100644 --- a/esphome/components/api/api.proto +++ b/esphome/components/api/api.proto @@ -824,7 +824,7 @@ message HomeAssistantStateResponse { option (ifdef) = "USE_API_HOMEASSISTANT_STATES"; string entity_id = 1; - string state = 2 [(null_terminate) = true]; + string state = 2; string attribute = 3; } @@ -882,7 +882,7 @@ message ExecuteServiceArgument { bool bool_ = 1; int32 legacy_int = 2; float float_ = 3; - string string_ = 4 [(null_terminate) = true]; + string string_ = 4; // ESPHome 1.14 (api v1.3) make int a signed value sint32 int_ = 5; repeated bool bool_array = 6 [packed=false, (fixed_vector) = true]; diff --git a/esphome/components/api/api_connection.cpp b/esphome/components/api/api_connection.cpp index 58917af968d..5ee64c44f1b 100644 --- a/esphome/components/api/api_connection.cpp +++ b/esphome/components/api/api_connection.cpp @@ -1702,6 +1702,13 @@ void APIConnection::on_home_assistant_state_response(const HomeAssistantStateRes return; } + // Null-terminate state in-place for safe c_str() usage (e.g., parse_number in callbacks). + // Safe: decode is complete, byte after string data was already consumed during parse, + // and frame helpers reserve RX_BUF_NULL_TERMINATOR extra byte in rx_buf_. + if (!msg.state.empty()) { + const_cast(msg.state.c_str())[msg.state.size()] = '\0'; + } + for (auto &it : this->parent_->get_state_subs()) { if (msg.entity_id != it.entity_id) { continue; @@ -1713,13 +1720,20 @@ void APIConnection::on_home_assistant_state_response(const HomeAssistantStateRes continue; } - // msg.state is already null-terminated in-place after protobuf decode it.callback(msg.state); } } #endif #ifdef USE_API_USER_DEFINED_ACTIONS void APIConnection::on_execute_service_request(const ExecuteServiceRequest &msg) { + // Null-terminate string args in-place for safe c_str() usage in YAML service triggers. + // Safe: full ExecuteServiceRequest decode is complete, all bytes in rx_buf_ consumed, + // and frame helpers reserve RX_BUF_NULL_TERMINATOR extra byte for the last field. + for (auto &arg : msg.args) { + if (!arg.string_.empty()) { + const_cast(arg.string_.c_str())[arg.string_.size()] = '\0'; + } + } bool found = false; #ifdef USE_API_USER_DEFINED_ACTION_RESPONSES // Register the call and get a unique server-generated action_call_id diff --git a/esphome/components/api/api_options.proto b/esphome/components/api/api_options.proto index 163a170fb93..a863f2c7a84 100644 --- a/esphome/components/api/api_options.proto +++ b/esphome/components/api/api_options.proto @@ -90,13 +90,4 @@ extend google.protobuf.FieldOptions { // - uint16_t _length_{0}; // - uint16_t _count_{0}; optional bool packed_buffer = 50015 [default=false]; - - // null_terminate: Write a null byte after string data in the decode buffer. - // When set on a string field in a SOURCE_CLIENT (decodable) message, the - // generated decode() override writes '\0' at data[length] after decoding. - // This makes the StringRef safe for c_str() usage without copying. - // Safe because: (1) frame helpers reserve +1 byte in rx_buf_, and - // (2) the overwritten byte was already consumed during decode. - // Only mark fields that actually need null-terminated access. - optional bool null_terminate = 50016 [default=false]; } diff --git a/esphome/components/api/api_pb2.cpp b/esphome/components/api/api_pb2.cpp index ac72b20c72a..5c50a8aa5b9 100644 --- a/esphome/components/api/api_pb2.cpp +++ b/esphome/components/api/api_pb2.cpp @@ -953,12 +953,6 @@ bool HomeAssistantStateResponse::decode_length(uint32_t field_id, ProtoLengthDel } return true; } -void HomeAssistantStateResponse::decode(const uint8_t *buffer, size_t length) { - ProtoDecodableMessage::decode(buffer, length); - if (!this->state.empty()) { - const_cast(this->state.c_str())[this->state.size()] = '\0'; - } -} #endif bool GetTimeResponse::decode_length(uint32_t field_id, ProtoLengthDelimited value) { switch (field_id) { @@ -1063,9 +1057,6 @@ void ExecuteServiceArgument::decode(const uint8_t *buffer, size_t length) { uint32_t count_string_array = ProtoDecodableMessage::count_repeated_field(buffer, length, 9); this->string_array.init(count_string_array); ProtoDecodableMessage::decode(buffer, length); - if (!this->string_.empty()) { - const_cast(this->string_.c_str())[this->string_.size()] = '\0'; - } } bool ExecuteServiceRequest::decode_varint(uint32_t field_id, ProtoVarInt value) { switch (field_id) { diff --git a/esphome/components/api/api_pb2.h b/esphome/components/api/api_pb2.h index b27d25ac2d2..c90873d9931 100644 --- a/esphome/components/api/api_pb2.h +++ b/esphome/components/api/api_pb2.h @@ -1095,7 +1095,6 @@ class HomeAssistantStateResponse final : public ProtoDecodableMessage { StringRef entity_id{}; StringRef state{}; StringRef attribute{}; - void decode(const uint8_t *buffer, size_t length) override; #ifdef HAS_PROTO_MESSAGE_DUMP const char *dump_to(DumpBuffer &out) const override; #endif diff --git a/script/api_protobuf/api_protobuf.py b/script/api_protobuf/api_protobuf.py index 6a338e1559d..cc881caa5cf 100755 --- a/script/api_protobuf/api_protobuf.py +++ b/script/api_protobuf/api_protobuf.py @@ -2028,8 +2028,6 @@ def build_message_type( # Collect fixed_vector fields for custom decode generation fixed_vector_fields = [] - # Collect fields with (null_terminate) = true option - null_terminate_fields = [] for field in desc.field: # Skip deprecated fields completely @@ -2072,10 +2070,6 @@ def build_message_type( ti = create_field_type_info(field, needs_decode, needs_encode) - # Collect fields with (null_terminate) = true for post-decode null-termination - if needs_decode and get_field_opt(field, pb.null_terminate, False): - null_terminate_fields.append(ti.field_name) - # Skip field declarations for fields that are in the base class # but include their encode/decode logic if field.name not in common_field_names: @@ -2182,8 +2176,8 @@ def build_message_type( prot = "bool decode_64bit(uint32_t field_id, Proto64Bit value) override;" protected_content.insert(0, prot) - # Generate custom decode() override for messages with FixedVector or null_terminate fields - if fixed_vector_fields or null_terminate_fields: + # Generate custom decode() override for messages with FixedVector fields + if fixed_vector_fields: # Generate the decode() implementation in cpp o = f"void {desc.name}::decode(const uint8_t *buffer, size_t length) {{\n" # Count and init each FixedVector field @@ -2192,13 +2186,6 @@ def build_message_type( o += f" this->{field_name}.init(count_{field_name});\n" # Call parent decode to populate the fields o += " ProtoDecodableMessage::decode(buffer, length);\n" - # Null-terminate fields marked with (null_terminate) = true in-place. - # Safe: decode is complete, byte after string was already parsed (next field tag) - # or is the +1 reserved byte at end of rx_buf_. - for field_name in null_terminate_fields: - o += f" if (!this->{field_name}.empty()) {{\n" - o += f" const_cast(this->{field_name}.c_str())[this->{field_name}.size()] = '\\0';\n" - o += " }\n" o += "}\n" cpp += o # Generate the decode() declaration in header (public method)