From d8ec76e02b30d6a844efe84a8023e9b5a0c71cf0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 9 Aug 2026 12:57:36 -0500 Subject: [PATCH] Fail on premature enumeration terminators; explicit terminal states - An INVALID_OFFSET/NOT_FOUND before the count from the same cache is a contradiction, not an end-of-range: the walker fails the build and the streamer aborts with the real cause instead of silently truncating (the walker's descriptor loop keeps its terminator - the 64 cap is its only bound) - unconditional_disconnect_'s unset-conn-id path drives the slot to a terminal reported state instead of leaning on the scheduled-teardown timer - find_characteristic/find_cccd log a corrupt range before returning the not-found sentinel (moved out of line - no ESP_LOG in headers) --- .../ble_device_base/ble_gatt_client.cpp | 43 +++++++++++++++++++ .../ble_device_base/ble_gatt_client.h | 26 ++--------- .../bluetooth_connection_bluedroid.cpp | 35 ++++++--------- 3 files changed, 60 insertions(+), 44 deletions(-) create mode 100644 esphome/components/ble_device_base/ble_gatt_client.cpp diff --git a/esphome/components/ble_device_base/ble_gatt_client.cpp b/esphome/components/ble_device_base/ble_gatt_client.cpp new file mode 100644 index 0000000000..66c9f7d0f2 --- /dev/null +++ b/esphome/components/ble_device_base/ble_gatt_client.cpp @@ -0,0 +1,43 @@ +#include "ble_gatt_client.h" + +#ifdef USE_BLE_GATT_CLIENT + +#include "esphome/core/log.h" + +namespace esphome::ble_device_base { + +static const char *const TAG = "ble_gatt_client"; + +const GattCharacteristic *find_characteristic(const GattServiceTable &table, const GattService &service, + const ESPBTUUID &uuid) { + // 32-bit range math: a corrupt first/count pair cannot wrap past the check. + uint32_t end = uint32_t(service.first_characteristic) + service.characteristic_count; + if (end > table.characteristic_count) { + ESP_LOGW(TAG, "characteristic range out of bounds"); + return nullptr; + } + for (uint32_t i = service.first_characteristic; i < end; i++) { + if (table.characteristics[i].uuid == uuid) + return &table.characteristics[i]; + } + return nullptr; +} + +uint16_t find_cccd(const GattServiceTable &table, const GattCharacteristic &characteristic) { + uint32_t end = uint32_t(characteristic.first_descriptor) + characteristic.descriptor_count; + if (end > table.descriptor_count) { + // Corrupt range, not a missing CCCD. + ESP_LOGW(TAG, "descriptor range out of bounds"); + return 0; + } + const ESPBTUUID cccd_uuid = ESPBTUUID::from_uint16(CCCD_UUID); + for (uint32_t i = characteristic.first_descriptor; i < end; i++) { + if (table.descriptors[i].uuid == cccd_uuid) + return table.descriptors[i].handle; + } + return 0; +} + +} // namespace esphome::ble_device_base + +#endif // USE_BLE_GATT_CLIENT diff --git a/esphome/components/ble_device_base/ble_gatt_client.h b/esphome/components/ble_device_base/ble_gatt_client.h index a1f0c4ae71..89c220cbe3 100644 --- a/esphome/components/ble_device_base/ble_gatt_client.h +++ b/esphome/components/ble_device_base/ble_gatt_client.h @@ -164,32 +164,12 @@ inline const GattService *find_service(const GattServiceTable &table, const ESPB return nullptr; } -inline const GattCharacteristic *find_characteristic(const GattServiceTable &table, const GattService &service, - const ESPBTUUID &uuid) { - // 32-bit range math: a corrupt first/count pair cannot wrap past the check. - uint32_t end = uint32_t(service.first_characteristic) + service.characteristic_count; - if (end > table.characteristic_count) - return nullptr; - for (uint32_t i = service.first_characteristic; i < end; i++) { - if (table.characteristics[i].uuid == uuid) - return &table.characteristics[i]; - } - return nullptr; -} +const GattCharacteristic *find_characteristic(const GattServiceTable &table, const GattService &service, + const ESPBTUUID &uuid); /// Handle of the characteristic's Client Characteristic Configuration /// descriptor (0x2902), or 0 when it has none. -inline uint16_t find_cccd(const GattServiceTable &table, const GattCharacteristic &characteristic) { - uint32_t end = uint32_t(characteristic.first_descriptor) + characteristic.descriptor_count; - if (end > table.descriptor_count) - return 0; - const ESPBTUUID cccd_uuid = ESPBTUUID::from_uint16(CCCD_UUID); - for (uint32_t i = characteristic.first_descriptor; i < end; i++) { - if (table.descriptors[i].uuid == cccd_uuid) - return table.descriptors[i].handle; - } - return 0; -} +uint16_t find_cccd(const GattServiceTable &table, const GattCharacteristic &characteristic); } // namespace esphome::ble_device_base diff --git a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp index 0c4990037f..21cffa5063 100644 --- a/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp +++ b/esphome/components/bluetooth_connection/bluetooth_connection_bluedroid.cpp @@ -182,7 +182,11 @@ int BluedroidGattClient::gatt_disconnect() { void BluedroidGattClient::unconditional_disconnect_() { ESP_LOGI(TAG, "[%d] Disconnecting (conn_id: %d)", this->connection_index_, this->conn_id_); if (this->conn_id_ == UNSET_CONN_ID) { + // Terminal state now rather than leaning on the scheduled-teardown timer. ESP_LOGE(TAG, "[%d] conn id unset, cannot disconnect", this->connection_index_); + this->release_services(); + this->set_idle_(); + this->listener_->on_connection_state(false, 0, ble_device_base::GATT_ERR_NOT_CONNECTED); return; } auto err = esp_ble_gattc_close(this->gattc_if_, this->conn_id_); @@ -336,10 +340,9 @@ bool BluedroidGattClient::walk_database_(ServiceFn &&on_service, CharFn &&on_cha uint16_t char_count = 1; auto status = esp_ble_gattc_get_all_char(this->gattc_if_, this->conn_id_, svc.start_handle, svc.end_handle, &chr, &char_count, c); - if (status == ESP_GATT_INVALID_OFFSET || status == ESP_GATT_NOT_FOUND) { - break; - } if (status != ESP_GATT_OK || char_count == 0) { + // An early terminator contradicts svc_chars from the same cache; + // never build a silently truncated table. return false; } if (!on_char(svc, chr)) { @@ -620,16 +623,12 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) { uint16_t cc = 1; auto char_status = esp_ble_gattc_get_all_char(this->gattc_if_, this->conn_id_, service_result.start_handle, service_result.end_handle, &char_result, &cc, char_offset); - if (char_status == ESP_GATT_INVALID_OFFSET || char_status == ESP_GATT_NOT_FOUND) { - break; - } if (char_status != ESP_GATT_OK || cc == 0) { - if (char_status != ESP_GATT_OK) { - this->log_gattc_warning_("esp_ble_gattc_get_all_char", char_status); - conn.abort_service_stream(char_status); - return; - } - break; + // An early terminator contradicts the count from the same cache; + // never stream a silently truncated list. + this->log_gattc_warning_("esp_ble_gattc_get_all_char", char_status); + conn.abort_service_stream(char_status != ESP_GATT_OK ? char_status : ESP_GATT_NOT_FOUND); + return; } service_resp.characteristics.emplace_back(); auto &characteristic_resp = service_resp.characteristics.back(); @@ -656,16 +655,10 @@ void BluedroidGattClient::stream_service_batch(BluetoothConnection &conn) { uint16_t dc = 1; auto desc_status = esp_ble_gattc_get_all_descr(this->gattc_if_, this->conn_id_, char_result.char_handle, &desc_result, &dc, desc_offset); - if (desc_status == ESP_GATT_INVALID_OFFSET || desc_status == ESP_GATT_NOT_FOUND) { - break; - } if (desc_status != ESP_GATT_OK || dc == 0) { - if (desc_status != ESP_GATT_OK) { - this->log_gattc_warning_("esp_ble_gattc_get_all_descr", desc_status); - conn.abort_service_stream(desc_status); - return; - } - break; + this->log_gattc_warning_("esp_ble_gattc_get_all_descr", desc_status); + conn.abort_service_stream(desc_status != ESP_GATT_OK ? desc_status : ESP_GATT_NOT_FOUND); + return; } characteristic_resp.descriptors.emplace_back(); auto &descriptor_resp = characteristic_resp.descriptors.back();