From ca8f23dca5efcddbef2de6ecab9f080ea2b532e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Ani=C4=87=20Bani=C4=87?= Date: Wed, 1 Jul 2026 14:39:47 +0200 Subject: [PATCH 1/2] [Breaking] Allow application to reject reads/writes with an ATT error The GATT access handler discarded the outcome of the characteristic write callback and always returned 0 (success) to the peer, so an application had no way to reject a write with an ATT error even under write-with-response. This replaces the the non-return value callbacks for characteristic and descriptor read/write callbacks with callbacks that return an enum value from the added `NimBLEGattStatus` enum. This will allow applications to accept or deny value access with a standard BLE response code. A rejection reaches the peer only for write-with-response; a write command is unacknowledged so the code is dropped by the stack, by spec. * Removed deprecated `NimBLECharacteristic::onStatus` callback. --- examples/NimBLE_Server/main/main.cpp | 14 ++++--- src/NimBLECharacteristic.cpp | 37 +++++++++-------- src/NimBLECharacteristic.h | 21 +++++----- src/NimBLEDescriptor.cpp | 24 ++++++++--- src/NimBLEDescriptor.h | 16 ++++---- src/NimBLEDevice.h | 1 + src/NimBLELocalValueAttribute.h | 59 ++++++++++++++++++++++++---- src/NimBLEServer.cpp | 9 +++-- src/NimBLEStream.cpp | 15 ++++--- src/NimBLEStream.h | 8 +--- 10 files changed, 134 insertions(+), 70 deletions(-) diff --git a/examples/NimBLE_Server/main/main.cpp b/examples/NimBLE_Server/main/main.cpp index 499232e7..4e502f35 100644 --- a/examples/NimBLE_Server/main/main.cpp +++ b/examples/NimBLE_Server/main/main.cpp @@ -68,22 +68,24 @@ class ServerCallbacks : public NimBLEServerCallbacks { /** Handler class for characteristic actions */ class CharacteristicCallbacks : public NimBLECharacteristicCallbacks { - void onRead(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo) override { + NimBLEGattStatus onRead(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo) override { printf("%s : onRead(), value: %s\n", pCharacteristic->getUUID().toString().c_str(), pCharacteristic->getValue().c_str()); + return NimBLEGattStatus::Success; } - void onWrite(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo) override { + NimBLEGattStatus onWrite(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo) override { printf("%s : onWrite(), value: %s\n", pCharacteristic->getUUID().toString().c_str(), pCharacteristic->getValue().c_str()); + return NimBLEGattStatus::Success; } /** * The value returned in code is the NimBLE host return code. */ - void onStatus(NimBLECharacteristic* pCharacteristic, int code) override { + void onStatus(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo&, int code) override { printf("Notification/Indication return code: %d, %s\n", code, NimBLEUtils::returnCodeToString(code)); } @@ -110,13 +112,15 @@ class CharacteristicCallbacks : public NimBLECharacteristicCallbacks { /** Handler class for descriptor actions */ class DescriptorCallbacks : public NimBLEDescriptorCallbacks { - void onWrite(NimBLEDescriptor* pDescriptor, NimBLEConnInfo& connInfo) override { + NimBLEGattStatus onWrite(NimBLEDescriptor* pDescriptor, NimBLEConnInfo& connInfo) override { std::string dscVal = pDescriptor->getValue(); printf("Descriptor written value: %s\n", dscVal.c_str()); + return NimBLEGattStatus::Success; } - void onRead(NimBLEDescriptor* pDescriptor, NimBLEConnInfo& connInfo) override { + NimBLEGattStatus onRead(NimBLEDescriptor* pDescriptor, NimBLEConnInfo& connInfo) override { printf("%s Descriptor read\n", pDescriptor->getUUID().toString().c_str()); + return NimBLEGattStatus::Success; } } dscCallbacks; diff --git a/src/NimBLECharacteristic.cpp b/src/NimBLECharacteristic.cpp index d4cf3b75..949cf069 100644 --- a/src/NimBLECharacteristic.cpp +++ b/src/NimBLECharacteristic.cpp @@ -421,9 +421,10 @@ void NimBLECharacteristic::updatePeerStatus(const NimBLEConnInfo& peerInfo) cons /** * @brief Handle a read event from a client. * @param [in] connInfo A reference to a NimBLEConnInfo instance containing the peer info. + * @return The status of the read operation. */ -void NimBLECharacteristic::readEvent(NimBLEConnInfo& connInfo) { - m_pCallbacks->onRead(this, connInfo); +NimBLEGattStatus NimBLECharacteristic::readEvent(NimBLEConnInfo& connInfo) { + return m_pCallbacks->onRead(this, connInfo); } // readEvent /** @@ -431,10 +432,19 @@ void NimBLECharacteristic::readEvent(NimBLEConnInfo& connInfo) { * @param [in] val A pointer to the data written by the client. * @param [in] len The length of the data written by the client. * @param [in] connInfo A reference to a NimBLEConnInfo instance containing the peer info. + * @return The status of the write operation. */ -void NimBLECharacteristic::writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& connInfo) { +NimBLEGattStatus NimBLECharacteristic::writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& connInfo) { + // Commit before the callback so getValue() reflects the new value inside it + // (onWriteStatus defaults to calling the legacy onWrite), but snapshot first + // so a rejected write can be rolled back and never becomes observable state. + NimBLEAttValue previous = m_value; setValue(val, len); - m_pCallbacks->onWrite(this, connInfo); + NimBLEGattStatus rc = m_pCallbacks->onWrite(this, connInfo); + if (rc != NimBLEGattStatus::Success) { + m_value = previous; + } + return rc; } // writeEvent /** @@ -483,31 +493,24 @@ std::string NimBLECharacteristic::toString() const { * @brief Callback function to support a read request. * @param [in] pCharacteristic The characteristic that is the source of the event. * @param [in] connInfo A reference to a NimBLEConnInfo instance containing the peer info. + * @return The status of the read operation. */ -void NimBLECharacteristicCallbacks::onRead(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo) { +NimBLEGattStatus NimBLECharacteristicCallbacks::onRead(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo) { NIMBLE_LOGD("NimBLECharacteristicCallbacks", "onRead: default"); + return NimBLEGattStatus::Success; } // onRead /** * @brief Callback function to support a write request. * @param [in] pCharacteristic The characteristic that is the source of the event. * @param [in] connInfo A reference to a NimBLEConnInfo instance containing the peer info. + * @return The status of the write operation. */ -void NimBLECharacteristicCallbacks::onWrite(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo) { +NimBLEGattStatus NimBLECharacteristicCallbacks::onWrite(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo) { NIMBLE_LOGD("NimBLECharacteristicCallbacks", "onWrite: default"); + return NimBLEGattStatus::Success; } // onWrite -/** - * @brief Callback function to support a Notify/Indicate Status report. - * @param [in] pCharacteristic The characteristic that is the source of the event. - * @param [in] code Status return code from the NimBLE stack. - * @details The status code for success is 0 for notifications and BLE_HS_EDONE for indications, - * any other value is an error. - */ -void NimBLECharacteristicCallbacks::onStatus(NimBLECharacteristic* pCharacteristic, int code) { - NIMBLE_LOGD("NimBLECharacteristicCallbacks", "onStatus: default"); -} // onStatus - /** * @brief Callback function to support a Notify/Indicate Status report. * @param [in] pCharacteristic The characteristic that is the source of the event. diff --git a/src/NimBLECharacteristic.h b/src/NimBLECharacteristic.h index 4149bc12..8bcbc220 100644 --- a/src/NimBLECharacteristic.h +++ b/src/NimBLECharacteristic.h @@ -272,13 +272,13 @@ class NimBLECharacteristic : public NimBLELocalValueAttribute { friend class NimBLEServer; friend class NimBLEService; - void setService(NimBLEService* pService); - void readEvent(NimBLEConnInfo& connInfo) override; - void writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& connInfo) override; - bool sendValue(const uint8_t* value, - size_t length, - bool is_notification = true, - uint16_t connHandle = BLE_HS_CONN_HANDLE_NONE) const; + void setService(NimBLEService* pService); + NimBLEGattStatus readEvent(NimBLEConnInfo& connInfo) override; + NimBLEGattStatus writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& connInfo) override; + bool sendValue(const uint8_t* value, + size_t length, + bool is_notification = true, + uint16_t connHandle = BLE_HS_CONN_HANDLE_NONE) const; struct SubPeerEntry { enum : uint8_t { AWAITING_SECURE = 1 << 0, SECURE = 1 << 1, SUB_NOTIFY = 1 << 2, SUB_INDICATE = 1 << 3 }; @@ -319,10 +319,9 @@ class NimBLECharacteristic : public NimBLELocalValueAttribute { class NimBLECharacteristicCallbacks { public: virtual ~NimBLECharacteristicCallbacks() {} - virtual void onRead(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo); - virtual void onWrite(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo); - virtual void onStatus(NimBLECharacteristic* pCharacteristic, int code); // deprecated - virtual void onStatus(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo, int code); + virtual NimBLEGattStatus onRead(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo); + virtual NimBLEGattStatus onWrite(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo); + virtual void onStatus(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo, int code); virtual void onSubscribe(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo, uint16_t subValue); }; diff --git a/src/NimBLEDescriptor.cpp b/src/NimBLEDescriptor.cpp index fc02281c..e11d9d65 100644 --- a/src/NimBLEDescriptor.cpp +++ b/src/NimBLEDescriptor.cpp @@ -123,31 +123,43 @@ std::string NimBLEDescriptor::toString() const { return res; } // toString -void NimBLEDescriptor::readEvent(NimBLEConnInfo& connInfo) { - m_pCallbacks->onRead(this, connInfo); +NimBLEGattStatus NimBLEDescriptor::readEvent(NimBLEConnInfo& connInfo) { + return m_pCallbacks->onRead(this, connInfo); } // readEvent -void NimBLEDescriptor::writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& connInfo) { +NimBLEGattStatus NimBLEDescriptor::writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& connInfo) { + // Commit before the callback so getValue() reflects the new value inside it + // (onWriteStatus defaults to calling the legacy onWrite), but snapshot first + // so a rejected write can be rolled back and never becomes observable state. + NimBLEAttValue previous = m_value; setValue(val, len); - m_pCallbacks->onWrite(this, connInfo); + NimBLEGattStatus rc = m_pCallbacks->onWrite(this, connInfo); + if (rc != NimBLEGattStatus::Success) { + m_value = previous; + } + return rc; } // writeEvent /** * @brief Callback function to support a read request. * @param [in] pDescriptor The descriptor that is the source of the event. * @param [in] connInfo A reference to a NimBLEConnInfo instance containing the peer info. + * @return The status of the read operation. */ -void NimBLEDescriptorCallbacks::onRead(NimBLEDescriptor* pDescriptor, NimBLEConnInfo& connInfo) { +NimBLEGattStatus NimBLEDescriptorCallbacks::onRead(NimBLEDescriptor* pDescriptor, NimBLEConnInfo& connInfo) { NIMBLE_LOGD("NimBLEDescriptorCallbacks", "onRead: default"); + return NimBLEGattStatus::Success; } // onRead /** * @brief Callback function to support a write request. * @param [in] pDescriptor The descriptor that is the source of the event. * @param [in] connInfo A reference to a NimBLEConnInfo instance containing the peer info. + * @return The status of the write operation. */ -void NimBLEDescriptorCallbacks::onWrite(NimBLEDescriptor* pDescriptor, NimBLEConnInfo& connInfo) { +NimBLEGattStatus NimBLEDescriptorCallbacks::onWrite(NimBLEDescriptor* pDescriptor, NimBLEConnInfo& connInfo) { NIMBLE_LOGD("NimBLEDescriptorCallbacks", "onWrite: default"); + return NimBLEGattStatus::Success; } // onWrite #endif // CONFIG_BT_NIMBLE_ENABLED && MYNEWT_VAL(BLE_ROLE_PERIPHERAL) diff --git a/src/NimBLEDescriptor.h b/src/NimBLEDescriptor.h index 1a7e38f1..c4f2e920 100644 --- a/src/NimBLEDescriptor.h +++ b/src/NimBLEDescriptor.h @@ -50,26 +50,26 @@ class NimBLEDescriptor : public NimBLELocalValueAttribute { friend class NimBLECharacteristic; friend class NimBLEService; - void setCharacteristic(NimBLECharacteristic* pChar); - void readEvent(NimBLEConnInfo& connInfo) override; - void writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& connInfo) override; + void setCharacteristic(NimBLECharacteristic* pChar); + NimBLEGattStatus readEvent(NimBLEConnInfo& connInfo) override; + NimBLEGattStatus writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& connInfo) override; NimBLEDescriptorCallbacks* m_pCallbacks{nullptr}; NimBLECharacteristic* m_pCharacteristic{nullptr}; }; // NimBLEDescriptor /** - * @brief Callbacks that can be associated with a %BLE descriptors to inform of events. + * @brief Callbacks that can be associated with a BLE descriptors to inform of events. * - * When a server application creates a %BLE descriptor, we may wish to be informed when there is either - * a read or write request to the descriptors value. An application can register a + * When a server application creates a BLE descriptor, we may wish to be informed when there is either + * a read or write request to the descriptor's value. An application can register a * sub-classed instance of this class and will be notified when such an event happens. */ class NimBLEDescriptorCallbacks { public: virtual ~NimBLEDescriptorCallbacks() = default; - virtual void onRead(NimBLEDescriptor* pDescriptor, NimBLEConnInfo& connInfo); - virtual void onWrite(NimBLEDescriptor* pDescriptor, NimBLEConnInfo& connInfo); + virtual NimBLEGattStatus onRead(NimBLEDescriptor* pDescriptor, NimBLEConnInfo& connInfo); + virtual NimBLEGattStatus onWrite(NimBLEDescriptor* pDescriptor, NimBLEConnInfo& connInfo); }; # include "NimBLE2904.h" diff --git a/src/NimBLEDevice.h b/src/NimBLEDevice.h index a64c678e..b4fefe22 100644 --- a/src/NimBLEDevice.h +++ b/src/NimBLEDevice.h @@ -109,6 +109,7 @@ class NimBLEDeviceCallbacks; # define BLEL2CAPClientCallbacks NimBLEL2CAPClientCallbacks # define BLEL2CAPChannel NimBLEL2CAPChannel # define BLEL2CAPChannelCallbacks NimBLEL2CAPChannelCallbacks +# define BLEGattStatus NimBLEGattStatus enum class NimBLETxPowerType { All = 0, Advertise = 1, Scan = 2, Connection = 3 }; diff --git a/src/NimBLELocalValueAttribute.h b/src/NimBLELocalValueAttribute.h index 0c4f3909..f0673910 100644 --- a/src/NimBLELocalValueAttribute.h +++ b/src/NimBLELocalValueAttribute.h @@ -21,7 +21,7 @@ #include "syscfg/syscfg.h" #if CONFIG_BT_NIMBLE_ENABLED && MYNEWT_VAL(BLE_ROLE_PERIPHERAL) -#ifdef USING_NIMBLE_ARDUINO_HEADERS +# ifdef USING_NIMBLE_ARDUINO_HEADERS # include "nimble/nimble/host/include/host/ble_hs.h" # else # include "host/ble_hs.h" @@ -32,6 +32,11 @@ # undef max /**************************/ +# include "NimBLELocalAttribute.h" +# include "NimBLEValueAttribute.h" +# include "NimBLEAttValue.h" +# include + typedef enum { READ = BLE_GATT_CHR_F_READ, READ_ENC = BLE_GATT_CHR_F_READ_ENC, @@ -47,10 +52,48 @@ typedef enum { INDICATE = BLE_GATT_CHR_F_INDICATE } NIMBLE_PROPERTY; -# include "NimBLELocalAttribute.h" -# include "NimBLEValueAttribute.h" -# include "NimBLEAttValue.h" -# include +enum class NimBLEGattStatus : uint8_t { + // Standard GATT Success State + Success = 0x00, + + // Standard ATT Protocol Errors (0x01 - 0x11) + InvalidHandle = 0x01, ///< The attribute handle given was not valid on this server. + ReadNotPermitted = 0x02, ///< The attribute cannot be read. + WriteNotPermitted = 0x03, ///< The attribute cannot be written. + InvalidPdu = 0x04, ///< The Attribute PDU was invalid or malformed. + Authentication = 0x05, ///< The link requires authentication/bonding before access. + RequestNotSupported = 0x06, ///< The ATT request opcode is not supported by this server. + InvalidOffset = 0x07, ///< The offset specified was past the end of the attribute value. + Authorization = 0x08, ///< The link requires authorization before access. + PrepareQueueFull = 0x09, ///< Too many prepare write requests have been queued. + AttributeNotFound = 0x0A, ///< No attribute found within the given handle range. + AttributeNotLong = 0x0B, ///< The attribute cannot be read/written using Read/Write Blob requests. + InsufficientKeySize = 0x0C, ///< The encryption key size is too short to access this data. + InvalidAttributeLength = 0x0D, ///< The length of the attribute value written is invalid for this profile. + UnlikelyError = 0x0E, ///< A highly unusual internal error occurred; the operation failed. + InsufficientEncryption = 0x0F, ///< The link must be encrypted before accessing this attribute. + UnsupportedGroupType = 0x10, ///< The attribute type is not a supported grouping attribute. + InsufficientResources = 0x11, ///< Temporary failure due to lack of device resources (RAM/buffers). + DatabaseOutOfSync = 0x12, ///< The database is out of sync. + ValueNotAllowed = 0x13, ///< The parameter value written is outside the valid ranges of that application profile. + + // ------------------------------------------------------------------------- + // User / Application Defined Errors (0x80 - 0x9F) + // The Bluetooth Specification explicitly reserves this range for developers + // to map their own custom domain logic failures back to the client. + // Examples: + // ------------------------------------------------------------------------- + AppErrorMin = 0x80, ///< Minimum boundary for custom application-specific errors. + + /* Example extensions: + AppInvalidPin = 0x81, + AppDeviceBusy = 0x82, + AppCrcCheckFailed = 0x83, + */ + + AppErrorMax = 0x9F ///< Maximum boundary for custom application-specific errors. +}; + class NimBLEConnInfo; class NimBLELocalValueAttribute : public NimBLELocalAttribute, public NimBLEValueAttribute { @@ -112,8 +155,9 @@ class NimBLELocalValueAttribute : public NimBLELocalAttribute, public NimBLEValu * @brief Callback function to support a read request. * @param [in] connInfo A reference to a NimBLEConnInfo instance containing the peer info. * @details This function is called by NimBLEServer when a read request is received. + * @return NimBLEGattStatus::Success or other error code to reject the read. */ - virtual void readEvent(NimBLEConnInfo& connInfo) = 0; + virtual NimBLEGattStatus readEvent(NimBLEConnInfo& connInfo) = 0; /** * @brief Callback function to support a write request. @@ -121,8 +165,9 @@ class NimBLELocalValueAttribute : public NimBLELocalAttribute, public NimBLEValu * @param [in] len The length of the value. * @param [in] connInfo A reference to a NimBLEConnInfo instance containing the peer info. * @details This function is called by NimBLEServer when a write request is received. + * @return NimBLEGattStatus::Success or other error code to reject the write. */ - virtual void writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& connInfo) = 0; + virtual NimBLEGattStatus writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& connInfo) = 0; /** * @brief Get a reference to value of the attribute. diff --git a/src/NimBLEServer.cpp b/src/NimBLEServer.cpp index 19c454dd..21672341 100644 --- a/src/NimBLEServer.cpp +++ b/src/NimBLEServer.cpp @@ -583,7 +583,6 @@ int NimBLEServer::handleGapEvent(ble_gap_event* event, void* arg) { } } - pChar->m_pCallbacks->onStatus(pChar, event->notify_tx.status); pChar->m_pCallbacks->onStatus(pChar, peerInfo, event->notify_tx.status); break; } // BLE_GAP_EVENT_NOTIFY_TX @@ -752,7 +751,10 @@ int NimBLEServer::handleGattEvent(uint16_t connHandle, uint16_t attrHandle, ble_ // Don't call readEvent if the buffer len is 0 (this is a follow up to a previous read), // or if this is an internal read (handle is NONE) if (ctxt->om->om_len > 0 && connHandle != BLE_HS_CONN_HANDLE_NONE) { - pAtt->readEvent(peerInfo); + NimBLEGattStatus status = pAtt->readEvent(peerInfo); + if (status != NimBLEGattStatus::Success) { + return static_cast(status); + } } NimBLEAttValue::Lock lock; @@ -782,8 +784,7 @@ int NimBLEServer::handleGattEvent(uint16_t connHandle, uint16_t attrHandle, ble_ next = SLIST_NEXT(next, om_next); } - pAtt->writeEvent(buf, len, peerInfo); - return 0; + return static_cast(pAtt->writeEvent(buf, len, peerInfo)); } default: diff --git a/src/NimBLEStream.cpp b/src/NimBLEStream.cpp index bc2cbbba..b47d3b7e 100644 --- a/src/NimBLEStream.cpp +++ b/src/NimBLEStream.cpp @@ -813,16 +813,21 @@ bool NimBLEStreamServer::ready() const { * @brief Callback for when the characteristic is written to by a client. * @param pChr Pointer to the characteristic that was written to. * @param connInfo Information about the connection that performed the write. - * @details This will push the received data into the RX buffer and call any user-defined callbacks. + * @return The user callback status, or success if no user callback is set. + * @details Received data is pushed into the RX buffer only if the user callback succeeds. */ -void NimBLEStreamServer::ChrCallbacks::onWrite(NimBLECharacteristic* pChr, NimBLEConnInfo& connInfo) { - // Push received data into RX buffer +NimBLEGattStatus NimBLEStreamServer::ChrCallbacks::onWrite(NimBLECharacteristic* pChr, NimBLEConnInfo& connInfo) { auto val = pChr->getValue(); - m_parent->pushRx(val.data(), val.size()); if (m_userCallbacks) { - m_userCallbacks->onWrite(pChr, connInfo); + NimBLEGattStatus status = m_userCallbacks->onWrite(pChr, connInfo); + if (status != NimBLEGattStatus::Success) { + return status; + } } + + m_parent->pushRx(val.data(), val.size()); + return NimBLEGattStatus::Success; } /** diff --git a/src/NimBLEStream.h b/src/NimBLEStream.h index 5799d11a..d82dec4e 100644 --- a/src/NimBLEStream.h +++ b/src/NimBLEStream.h @@ -172,15 +172,9 @@ class NimBLEStreamServer : public NimBLEStream { struct ChrCallbacks : public NimBLECharacteristicCallbacks { ChrCallbacks(NimBLEStreamServer* parent) : m_parent(parent), m_userCallbacks(nullptr), m_peerHandle(BLE_HS_CONN_HANDLE_NONE) {} - void onWrite(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo) override; + NimBLEGattStatus onWrite(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo) override; void onSubscribe(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo, uint16_t subValue) override; void onStatus(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo, int code) override; - // override this to avoid recursion when debug logs are enabled - void onStatus(NimBLECharacteristic* pCharacteristic, int code) override { - if (m_userCallbacks != nullptr) { - m_userCallbacks->onStatus(pCharacteristic, code); - } - } NimBLEStreamServer* m_parent; NimBLECharacteristicCallbacks* m_userCallbacks; From d4cc906a666c9e5360f1169fce1608866cc6b5c0 Mon Sep 17 00:00:00 2001 From: h2zero Date: Tue, 27 Oct 2026 09:35:37 -0600 Subject: [PATCH 2/2] [Breaking] Add value param to Characteristic and Descriptor write callback This adds a value parameter to the `onWrite` callbacks for `NimBLECharacteristic` and `NimBLEDescriptor` classes. This allows the application to reject the write and not have it's internal value changed. --- examples/NimBLE_Server/main/main.cpp | 9 ++++---- src/NimBLEAttValue.cpp | 7 +++++-- src/NimBLECharacteristic.cpp | 31 +++++++++++++++------------- src/NimBLECharacteristic.h | 4 ++-- src/NimBLEDescriptor.cpp | 26 ++++++++++++----------- src/NimBLEDescriptor.h | 4 ++-- src/NimBLELocalValueAttribute.h | 3 +-- src/NimBLEServer.cpp | 18 ++++++++++------ src/NimBLEStream.cpp | 9 ++++---- src/NimBLEStream.h | 2 +- 10 files changed, 62 insertions(+), 51 deletions(-) diff --git a/examples/NimBLE_Server/main/main.cpp b/examples/NimBLE_Server/main/main.cpp index 4e502f35..01a95517 100644 --- a/examples/NimBLE_Server/main/main.cpp +++ b/examples/NimBLE_Server/main/main.cpp @@ -75,10 +75,10 @@ class CharacteristicCallbacks : public NimBLECharacteristicCallbacks { return NimBLEGattStatus::Success; } - NimBLEGattStatus onWrite(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo) override { + NimBLEGattStatus onWrite(NimBLECharacteristic* pCharacteristic, NimBLEAttValue& value, NimBLEConnInfo& connInfo) override { printf("%s : onWrite(), value: %s\n", pCharacteristic->getUUID().toString().c_str(), - pCharacteristic->getValue().c_str()); + value.c_str()); return NimBLEGattStatus::Success; } @@ -112,9 +112,8 @@ class CharacteristicCallbacks : public NimBLECharacteristicCallbacks { /** Handler class for descriptor actions */ class DescriptorCallbacks : public NimBLEDescriptorCallbacks { - NimBLEGattStatus onWrite(NimBLEDescriptor* pDescriptor, NimBLEConnInfo& connInfo) override { - std::string dscVal = pDescriptor->getValue(); - printf("Descriptor written value: %s\n", dscVal.c_str()); + NimBLEGattStatus onWrite(NimBLEDescriptor* pDescriptor, NimBLEAttValue& value, NimBLEConnInfo& connInfo) override { + printf("Descriptor written value: %s\n", value.c_str()); return NimBLEGattStatus::Success; } diff --git a/src/NimBLEAttValue.cpp b/src/NimBLEAttValue.cpp index 86055f30..9ccf7c21 100644 --- a/src/NimBLEAttValue.cpp +++ b/src/NimBLEAttValue.cpp @@ -75,13 +75,16 @@ NimBLEAttValue::NimBLEAttValue(uint16_t init_len, uint16_t max_len) { NIMBLE_CPP_DEBUG_ASSERT(m_attr_value); if (m_attr_value == nullptr) { - NIMBLE_LOGE(LOG_TAG, "Failed to calloc ctx"); + NIMBLE_LOGE(LOG_TAG, "Failed to allocate value buffer"); + m_attr_len = 0; + m_capacity = 0; + m_attr_max_len = 0; } } // Value constructor implementation. NimBLEAttValue::NimBLEAttValue(const uint8_t* value, uint16_t len, uint16_t max_len) : NimBLEAttValue(len, max_len) { - if (m_attr_value != nullptr) { + if (m_attr_value != nullptr && len > 0) { memcpy(m_attr_value, value, len); m_attr_len = len; } diff --git a/src/NimBLECharacteristic.cpp b/src/NimBLECharacteristic.cpp index 949cf069..995e9b16 100644 --- a/src/NimBLECharacteristic.cpp +++ b/src/NimBLECharacteristic.cpp @@ -29,6 +29,8 @@ # include "NimBLEDevice.h" # include "NimBLELog.h" +# include + static NimBLECharacteristicCallbacks defaultCallback; static const char* LOG_TAG = "NimBLECharacteristic"; @@ -429,21 +431,16 @@ NimBLEGattStatus NimBLECharacteristic::readEvent(NimBLEConnInfo& connInfo) { /** * @brief Handle a write event from a client. - * @param [in] val A pointer to the data written by the client. - * @param [in] len The length of the data written by the client. + * @param [in] val A reference to the value written by the client. * @param [in] connInfo A reference to a NimBLEConnInfo instance containing the peer info. * @return The status of the write operation. */ -NimBLEGattStatus NimBLECharacteristic::writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& connInfo) { - // Commit before the callback so getValue() reflects the new value inside it - // (onWriteStatus defaults to calling the legacy onWrite), but snapshot first - // so a rejected write can be rolled back and never becomes observable state. - NimBLEAttValue previous = m_value; - setValue(val, len); - NimBLEGattStatus rc = m_pCallbacks->onWrite(this, connInfo); - if (rc != NimBLEGattStatus::Success) { - m_value = previous; +NimBLEGattStatus NimBLECharacteristic::writeEvent(NimBLEAttValue& val, NimBLEConnInfo& connInfo) { + NimBLEGattStatus rc = m_pCallbacks->onWrite(this, val, connInfo); + if (rc == NimBLEGattStatus::Success) { + m_value = std::move(val); } + return rc; } // writeEvent @@ -493,7 +490,7 @@ std::string NimBLECharacteristic::toString() const { * @brief Callback function to support a read request. * @param [in] pCharacteristic The characteristic that is the source of the event. * @param [in] connInfo A reference to a NimBLEConnInfo instance containing the peer info. - * @return The status of the read operation. + * @return The status of the read operation, NimBLEGattStatus::Success if the read is accepted, or an error code otherwise. */ NimBLEGattStatus NimBLECharacteristicCallbacks::onRead(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo) { NIMBLE_LOGD("NimBLECharacteristicCallbacks", "onRead: default"); @@ -503,10 +500,16 @@ NimBLEGattStatus NimBLECharacteristicCallbacks::onRead(NimBLECharacteristic* pCh /** * @brief Callback function to support a write request. * @param [in] pCharacteristic The characteristic that is the source of the event. + * @param [in] value The value being written to the characteristic. * @param [in] connInfo A reference to a NimBLEConnInfo instance containing the peer info. - * @return The status of the write operation. + * @return The status of the write operation, NimBLEGattStatus::Success if the write is accepted, or an error code otherwise. + * @details If the write is rejected, the characteristic's value will not be updated, otherwise it will be updated with the new value. + * The value parameter contains the new value being written to the characteristic, it can be modified by the callback if needed. + * WARNING: Modifying the value parameter directly affects the value being written to the characteristic, + * if the max length of the value is modified, such as by assigning another value with a different max length, i.e. value = NimBLEAttValue(); + * it will change the max length used for the characteristic's value. */ -NimBLEGattStatus NimBLECharacteristicCallbacks::onWrite(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo) { +NimBLEGattStatus NimBLECharacteristicCallbacks::onWrite(NimBLECharacteristic* pCharacteristic, NimBLEAttValue& value, NimBLEConnInfo& connInfo) { NIMBLE_LOGD("NimBLECharacteristicCallbacks", "onWrite: default"); return NimBLEGattStatus::Success; } // onWrite diff --git a/src/NimBLECharacteristic.h b/src/NimBLECharacteristic.h index 8bcbc220..0045064a 100644 --- a/src/NimBLECharacteristic.h +++ b/src/NimBLECharacteristic.h @@ -274,7 +274,7 @@ class NimBLECharacteristic : public NimBLELocalValueAttribute { void setService(NimBLEService* pService); NimBLEGattStatus readEvent(NimBLEConnInfo& connInfo) override; - NimBLEGattStatus writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& connInfo) override; + NimBLEGattStatus writeEvent(NimBLEAttValue& val, NimBLEConnInfo& connInfo) override; bool sendValue(const uint8_t* value, size_t length, bool is_notification = true, @@ -320,7 +320,7 @@ class NimBLECharacteristicCallbacks { public: virtual ~NimBLECharacteristicCallbacks() {} virtual NimBLEGattStatus onRead(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo); - virtual NimBLEGattStatus onWrite(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo); + virtual NimBLEGattStatus onWrite(NimBLECharacteristic* pCharacteristic, NimBLEAttValue& value, NimBLEConnInfo& connInfo); virtual void onStatus(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo, int code); virtual void onSubscribe(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo, uint16_t subValue); }; diff --git a/src/NimBLEDescriptor.cpp b/src/NimBLEDescriptor.cpp index e11d9d65..ef10e889 100644 --- a/src/NimBLEDescriptor.cpp +++ b/src/NimBLEDescriptor.cpp @@ -22,6 +22,7 @@ # include "NimBLELog.h" # include +# include static const char* LOG_TAG = "NimBLEDescriptor"; static NimBLEDescriptorCallbacks defaultCallbacks; @@ -127,15 +128,10 @@ NimBLEGattStatus NimBLEDescriptor::readEvent(NimBLEConnInfo& connInfo) { return m_pCallbacks->onRead(this, connInfo); } // readEvent -NimBLEGattStatus NimBLEDescriptor::writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& connInfo) { - // Commit before the callback so getValue() reflects the new value inside it - // (onWriteStatus defaults to calling the legacy onWrite), but snapshot first - // so a rejected write can be rolled back and never becomes observable state. - NimBLEAttValue previous = m_value; - setValue(val, len); - NimBLEGattStatus rc = m_pCallbacks->onWrite(this, connInfo); - if (rc != NimBLEGattStatus::Success) { - m_value = previous; +NimBLEGattStatus NimBLEDescriptor::writeEvent(NimBLEAttValue& val, NimBLEConnInfo& connInfo) { + NimBLEGattStatus rc = m_pCallbacks->onWrite(this, val, connInfo); + if (rc == NimBLEGattStatus::Success) { + m_value = std::move(val); } return rc; } // writeEvent @@ -144,7 +140,7 @@ NimBLEGattStatus NimBLEDescriptor::writeEvent(const uint8_t* val, uint16_t len, * @brief Callback function to support a read request. * @param [in] pDescriptor The descriptor that is the source of the event. * @param [in] connInfo A reference to a NimBLEConnInfo instance containing the peer info. - * @return The status of the read operation. + * @return The status of the read operation, NimBLEGattStatus::Success if the read is accepted, or an error code otherwise. */ NimBLEGattStatus NimBLEDescriptorCallbacks::onRead(NimBLEDescriptor* pDescriptor, NimBLEConnInfo& connInfo) { NIMBLE_LOGD("NimBLEDescriptorCallbacks", "onRead: default"); @@ -154,10 +150,16 @@ NimBLEGattStatus NimBLEDescriptorCallbacks::onRead(NimBLEDescriptor* pDescriptor /** * @brief Callback function to support a write request. * @param [in] pDescriptor The descriptor that is the source of the event. + * @param [in] value The value being written to the descriptor. * @param [in] connInfo A reference to a NimBLEConnInfo instance containing the peer info. - * @return The status of the write operation. + * @return The status of the write operation, NimBLEGattStatus::Success if the write is accepted, or an error code otherwise. + * @details If the write is rejected, the descriptor's value will not be updated, otherwise it will be updated with the new value. + * The value parameter contains the new value being written to the descriptor, it can be modified by the callback if needed. + * WARNING: Modifying the value parameter directly affects the value being written to the descriptor, + * if the max length of the value is modified, such as by assigning another value with a different max length, i.e. value = NimBLEAttValue(); + * it will change the max length used for the descriptor's value. */ -NimBLEGattStatus NimBLEDescriptorCallbacks::onWrite(NimBLEDescriptor* pDescriptor, NimBLEConnInfo& connInfo) { +NimBLEGattStatus NimBLEDescriptorCallbacks::onWrite(NimBLEDescriptor* pDescriptor, NimBLEAttValue& value, NimBLEConnInfo& connInfo) { NIMBLE_LOGD("NimBLEDescriptorCallbacks", "onWrite: default"); return NimBLEGattStatus::Success; } // onWrite diff --git a/src/NimBLEDescriptor.h b/src/NimBLEDescriptor.h index c4f2e920..67de9a1d 100644 --- a/src/NimBLEDescriptor.h +++ b/src/NimBLEDescriptor.h @@ -52,7 +52,7 @@ class NimBLEDescriptor : public NimBLELocalValueAttribute { void setCharacteristic(NimBLECharacteristic* pChar); NimBLEGattStatus readEvent(NimBLEConnInfo& connInfo) override; - NimBLEGattStatus writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& connInfo) override; + NimBLEGattStatus writeEvent(NimBLEAttValue& val, NimBLEConnInfo& connInfo) override; NimBLEDescriptorCallbacks* m_pCallbacks{nullptr}; NimBLECharacteristic* m_pCharacteristic{nullptr}; @@ -69,7 +69,7 @@ class NimBLEDescriptorCallbacks { public: virtual ~NimBLEDescriptorCallbacks() = default; virtual NimBLEGattStatus onRead(NimBLEDescriptor* pDescriptor, NimBLEConnInfo& connInfo); - virtual NimBLEGattStatus onWrite(NimBLEDescriptor* pDescriptor, NimBLEConnInfo& connInfo); + virtual NimBLEGattStatus onWrite(NimBLEDescriptor* pDescriptor, NimBLEAttValue& value, NimBLEConnInfo& connInfo); }; # include "NimBLE2904.h" diff --git a/src/NimBLELocalValueAttribute.h b/src/NimBLELocalValueAttribute.h index f0673910..e8712c24 100644 --- a/src/NimBLELocalValueAttribute.h +++ b/src/NimBLELocalValueAttribute.h @@ -162,12 +162,11 @@ class NimBLELocalValueAttribute : public NimBLELocalAttribute, public NimBLEValu /** * @brief Callback function to support a write request. * @param [in] val The value to write. - * @param [in] len The length of the value. * @param [in] connInfo A reference to a NimBLEConnInfo instance containing the peer info. * @details This function is called by NimBLEServer when a write request is received. * @return NimBLEGattStatus::Success or other error code to reject the write. */ - virtual NimBLEGattStatus writeEvent(const uint8_t* val, uint16_t len, NimBLEConnInfo& connInfo) = 0; + virtual NimBLEGattStatus writeEvent(NimBLEAttValue& val, NimBLEConnInfo& connInfo) = 0; /** * @brief Get a reference to value of the attribute. diff --git a/src/NimBLEServer.cpp b/src/NimBLEServer.cpp index 21672341..26cebfc3 100644 --- a/src/NimBLEServer.cpp +++ b/src/NimBLEServer.cpp @@ -770,21 +770,27 @@ int NimBLEServer::handleGattEvent(uint16_t connHandle, uint16_t attrHandle, ble_ return BLE_ATT_ERR_INVALID_ATTR_VALUE_LEN; } - uint8_t buf[maxLen]; - memcpy(buf, ctxt->om->om_data, len); + NimBLEAttValue buf(ctxt->om->om_data, len, maxLen); + if (buf.data() == nullptr || buf.size() != len) { + return BLE_ATT_ERR_INSUFFICIENT_RES; + } + + buf.setTimeStamp(); - os_mbuf* next; - next = SLIST_NEXT(ctxt->om, om_next); + os_mbuf* next = SLIST_NEXT(ctxt->om, om_next); while (next != NULL) { if ((len + next->om_len) > maxLen) { return BLE_ATT_ERR_INVALID_ATTR_VALUE_LEN; } - memcpy(&buf[len], next->om_data, next->om_len); + buf.append(next->om_data, next->om_len); len += next->om_len; + if (buf.size() != len) { + return BLE_ATT_ERR_INSUFFICIENT_RES; + } next = SLIST_NEXT(next, om_next); } - return static_cast(pAtt->writeEvent(buf, len, peerInfo)); + return static_cast(pAtt->writeEvent(buf, peerInfo)); } default: diff --git a/src/NimBLEStream.cpp b/src/NimBLEStream.cpp index b47d3b7e..ac3e06eb 100644 --- a/src/NimBLEStream.cpp +++ b/src/NimBLEStream.cpp @@ -812,21 +812,20 @@ bool NimBLEStreamServer::ready() const { /** * @brief Callback for when the characteristic is written to by a client. * @param pChr Pointer to the characteristic that was written to. + * @param value The value that was written to the characteristic. * @param connInfo Information about the connection that performed the write. * @return The user callback status, or success if no user callback is set. * @details Received data is pushed into the RX buffer only if the user callback succeeds. */ -NimBLEGattStatus NimBLEStreamServer::ChrCallbacks::onWrite(NimBLECharacteristic* pChr, NimBLEConnInfo& connInfo) { - auto val = pChr->getValue(); - +NimBLEGattStatus NimBLEStreamServer::ChrCallbacks::onWrite(NimBLECharacteristic* pChr, NimBLEAttValue& value, NimBLEConnInfo& connInfo) { if (m_userCallbacks) { - NimBLEGattStatus status = m_userCallbacks->onWrite(pChr, connInfo); + NimBLEGattStatus status = m_userCallbacks->onWrite(pChr, value, connInfo); if (status != NimBLEGattStatus::Success) { return status; } } - m_parent->pushRx(val.data(), val.size()); + m_parent->pushRx(value.data(), value.size()); return NimBLEGattStatus::Success; } diff --git a/src/NimBLEStream.h b/src/NimBLEStream.h index d82dec4e..bf40e45e 100644 --- a/src/NimBLEStream.h +++ b/src/NimBLEStream.h @@ -172,7 +172,7 @@ class NimBLEStreamServer : public NimBLEStream { struct ChrCallbacks : public NimBLECharacteristicCallbacks { ChrCallbacks(NimBLEStreamServer* parent) : m_parent(parent), m_userCallbacks(nullptr), m_peerHandle(BLE_HS_CONN_HANDLE_NONE) {} - NimBLEGattStatus onWrite(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo) override; + NimBLEGattStatus onWrite(NimBLECharacteristic* pCharacteristic, NimBLEAttValue& value, NimBLEConnInfo& connInfo) override; void onSubscribe(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo, uint16_t subValue) override; void onStatus(NimBLECharacteristic* pCharacteristic, NimBLEConnInfo& connInfo, int code) override;