Merge pull request #13449 from paul-szczepanek-arm/fix-prep-write

BLE: Fix writing attributes larger than MTU size

connectivity/FEATURE_BLE/libraries/TARGET_CORDIO/include/ble/internal/PalAttClientImpl.h

@@ -381,13 +381,11 @@ private:
     struct PrepareWriteResponseConverter : ResponseConverter<ATTC_PREPARE_WRITE_RSP> {
         static AttPrepareWriteResponse convert(const attEvt_t* event)
         {
-            // WARNING: Not sure if correct, the stack erase the length parameter
             return AttPrepareWriteResponse(
                 event->handle,
-                to_uint16_t(event->pValue + 2),
+                0, /* offset is lost */
-                // FIXME: the stack set the lenght to 0, the data won't be seen ...
                 make_const_Span(
-                    event->pValue + 4,
+                    event->pValue,
                     event->valueLen
                 )
             );

connectivity/FEATURE_BLE/libraries/TARGET_CORDIO/stack/ble-host/sources/stack/att/atts_write.c

@@ -260,7 +260,7 @@ void attsProcPrepWriteReq(attsCcb_t *pCcb, uint16_t len, uint8_t *pPacket)
   }
   /* verify write length, fixed length */
   else if (((pAttr->settings & ATTS_SET_VARIABLE_LEN) == 0) &&
-           (writeLen != pAttr->maxLen))
+           (writeLen > pAttr->maxLen))
   {
     err = ATT_ERR_LENGTH;
   }

connectivity/FEATURE_BLE/libraries/ble-api-implementation/source/GattClientImpl.cpp

@@ -637,11 +637,13 @@ struct GattClient::WriteControlBlock : public ProcedureControlBlock {
     using ProcedureControlBlock::connection_handle;
 
     WriteControlBlock(
-		connection_handle_t connection_handle, uint16_t attribute_handle,
+        connection_handle_t connection_handle,
-		uint8_t* data, uint16_t len
+        uint16_t attribute_handle,
+        uint8_t* data,
+        uint16_t write_length
     ) : ProcedureControlBlock(WRITE_PROCEDURE, connection_handle),
-		attribute_handle(attribute_handle), len(len), offset(0), data(data),
+        attribute_handle(attribute_handle), write_length(write_length), offset(0), data(data),
-		prepare_success(false), status(BLE_ERROR_UNSPECIFIED), error_code(0xFF) {
+        prepare_success(false), status(BLE_ERROR_INITIALIZATION_INCOMPLETE), error_code(0xFF) {
     }
 
     virtual ~WriteControlBlock() {
@@ -722,19 +724,21 @@ struct GattClient::WriteControlBlock : public ProcedureControlBlock {
 
     void handle_prepare_write_response(GattClient* client, const AttPrepareWriteResponse& write_response) {
         ble_error_t err = BLE_ERROR_UNSPECIFIED;
-
+        offset += write_response.partial_value.size();
-		uint16_t mtu_size = client->get_mtu(connection_handle);
+        uint16_t data_left = write_length - offset; /* offset is guaranteed to be less of equal to write_length */
-		offset = write_response.offset + write_response.partial_value.size();
+        if (data_left) {
-		if (offset < len) {
+            uint16_t chunk_size = client->get_mtu(connection_handle) - PREPARE_WRITE_HEADER_LENGTH;
+            if (chunk_size > data_left) {
+                chunk_size = data_left;
+            }
             err = client->_pal_client.queue_prepare_write(
-				connection_handle, attribute_handle,
+                connection_handle,
-				make_const_Span(
+                attribute_handle,
-					data + offset,
+                make_const_Span(data + offset, chunk_size),
-					std::min((len - offset), (mtu_size - 5))
-				),
                 offset
             );
         } else {
+            prepare_success = true;
             err = client->_pal_client.execute_write_queue(
                 connection_handle, true
             );
@@ -829,7 +833,7 @@ struct GattClient::WriteControlBlock : public ProcedureControlBlock {
     }
 
     uint16_t attribute_handle;
-	uint16_t len;
+    uint16_t write_length;
     uint16_t offset;
     uint8_t* data;
     bool prepare_success;
@@ -1199,7 +1203,7 @@ ble_error_t GattClient::write(
             err = _pal_client.queue_prepare_write(
                 connection_handle,
                 attribute_handle,
-                make_const_Span(value, mtu - PREPARE_WRITE_HEADER_LENGTH),
+                make_const_Span(data, mtu - PREPARE_WRITE_HEADER_LENGTH),
                 /* offset */0
             );
         } else {

connectivity/FEATURE_BLE/libraries/ble-api-implementation/source/GattServerImpl.cpp

@@ -329,6 +329,7 @@ ble_error_t GattServer::insert_characteristic_value_attribute(
     }
     if (properties & WRITABLE_PROPERTIES) {
         attribute_it->settings |= ATTS_SET_WRITE_CBACK;
+        attribute_it->settings |= ATTS_SET_ALLOW_OFFSET;
     }
     if (value_attribute.getUUID().shortOrLong() == UUID::UUID_TYPE_LONG) {
         attribute_it->settings |= ATTS_SET_UUID_128;
@@ -990,9 +991,39 @@ uint8_t GattServer::atts_write_cb(
 {
     uint8_t err;
 
-    /* TODO: offset is not handled properly */
+    GattCharacteristic* auth_char = getInstance().get_auth_char(handle);
-    if ((err = AttsSetAttr(handle, len, pValue)) != ATT_SUCCESS) {
+    if (auth_char && auth_char->isWriteAuthorizationEnabled()) {
-        return err;
+        GattWriteAuthCallbackParams write_auth_params = {
+            connId,
+            handle,
+            offset,
+            len,
+            pValue,
+            AUTH_CALLBACK_REPLY_SUCCESS
+        };
+
+        GattAuthCallbackReply_t ret = auth_char->authorizeWrite(&write_auth_params);
+        if (ret!= AUTH_CALLBACK_REPLY_SUCCESS) {
+            return ret & 0xFF;
+        }
+    }
+
+    /* we don't write anything during the prepare phase */
+    bool write_happened = (operation != ATT_PDU_PREP_WRITE_REQ);
+
+    MBED_ASSERT(len + offset <= pAttr->maxLen);
+
+    if (write_happened) {
+        WsfTaskLock();
+
+        memcpy((pAttr->pValue + offset), pValue, len);
+
+        /* write the length if variable length attribute */
+        if ((pAttr->settings & ATTS_SET_VARIABLE_LEN) != 0) {
+            *(pAttr->pLen) = offset + len;
+        }
+
+        WsfTaskUnlock();
     }
 
     GattWriteCallbackParams::WriteOp_t writeOp;
@@ -1025,23 +1056,7 @@ uint8_t GattServer::atts_write_cb(
             break;
     }
 
-    GattCharacteristic* auth_char = getInstance().get_auth_char(handle);
+    if (write_happened) {
-    if (auth_char && auth_char->isWriteAuthorizationEnabled()) {
-        GattWriteAuthCallbackParams write_auth_params = {
-            connId,
-            handle,
-            offset,
-            len,
-            pValue,
-            AUTH_CALLBACK_REPLY_SUCCESS
-        };
-
-        GattAuthCallbackReply_t ret = auth_char->authorizeWrite(&write_auth_params);
-        if (ret!= AUTH_CALLBACK_REPLY_SUCCESS) {
-            return ret & 0xFF;
-        }
-    }
-
         GattWriteCallbackParams write_params = {
             connId,
             handle,
@@ -1050,7 +1065,9 @@ uint8_t GattServer::atts_write_cb(
             len,
             pValue
         };
+
         getInstance().handleDataWrittenEvent(&write_params);
+    }
 
     return ATT_SUCCESS;
 }