BTGattHandler/AttPDU: Use safe MAX_ATT_MTU = 512 + 1 for opcode, not for other ATTPDU parameter; Better serverMTU max(min(..)) safety, API-doc clarity

4 files changed, 18 insertions, 7 deletions

diff --git a/api/direct_bt/ATTPDUTypes.hpp b/api/direct_bt/ATTPDUTypes.hpp
index ce6ae2e3..582a7437 100644
--- a/api/direct_bt/ATTPDUTypes.hpp
+++ b/api/direct_bt/ATTPDUTypes.hpp
@@ -603,7 +603,7 @@ namespace direct_bt {
             }
 
             /**
-             * Returns the net octet size of the value attributes in this PDI (without opcode),
+             * Returns the net octet size of this PDU's attributes value,
              * i.e.
              * - `pdu.size - getAuthSigSize() - value-offset` or
              * - `getPDUParamSize() - getPDUValueOffset() + 1`
@@ -622,7 +622,7 @@ namespace direct_bt {
             constexpr_cxx20 jau::nsize_t getPDUValueSize() const noexcept { return getPDUParamSize() - getPDUValueOffset() + 1; }
 
             /**
-             * Returns the theoretical maximum value size of a PDU.
+             * Returns the theoretical maximum value size of a PDU's attribute value.
              * <pre>
              *  ATT_MTU - getAuthSigSize() - value-offset
              * </pre>
diff --git a/api/direct_bt/BTGattHandler.hpp b/api/direct_bt/BTGattHandler.hpp
index cf9fdf3b..254cbd35 100644
--- a/api/direct_bt/BTGattHandler.hpp
+++ b/api/direct_bt/BTGattHandler.hpp
@@ -153,8 +153,13 @@ namespace direct_bt {
     class BTGattHandler {
         public:
             enum class Defaults : uint16_t {
-                /* BT Core Spec v5.2: Vol 3, Part F 3.2.8: Maximum length of an attribute value. */
-                MAX_ATT_MTU = 512 + 5,
+                /**
+                 * BT Core Spec v5.2: Vol 3, Part F 3.2.8: Maximum length of an attribute value.
+                 *
+                 * We add +1 for opcode, but don't add for different PDU type's parameter
+                 * upfront the attribute value.
+                 */
+                MAX_ATT_MTU = 512 + 1,
 
                 /* BT Core Spec v5.2: Vol 3, Part G GATT: 5.2.1 ATT_MTU */
                 MIN_ATT_MTU = 23
diff --git a/api/direct_bt/DBGattServer.hpp b/api/direct_bt/DBGattServer.hpp
index 7547372e..d2776bc5 100644
--- a/api/direct_bt/DBGattServer.hpp
+++ b/api/direct_bt/DBGattServer.hpp
@@ -343,6 +343,8 @@ namespace direct_bt {
         public:
             /**
              * Listener to remote master device's operations on the local GATT-Server.
+             *
+             * All methods shall return as soon as possible to not block GATT event processing.
              */
             class Listener {
                 public:
@@ -408,7 +410,7 @@ namespace direct_bt {
 
         public:
             /** Used maximum server Rx ATT_MTU */
-            uint16_t att_mtu = 512 + 5; // BTGattHandler::Defaults::MAX_ATT_MTU;
+            uint16_t att_mtu = 512 + 1; // BTGattHandler::Defaults::MAX_ATT_MTU;
 
             /** List of Services */
             jau::darray<DBGattService> services;
diff --git a/src/direct_bt/BTGattHandler.cpp b/src/direct_bt/BTGattHandler.cpp
index aca2ac29..ed0b560f 100644
--- a/src/direct_bt/BTGattHandler.cpp
+++ b/src/direct_bt/BTGattHandler.cpp
@@ -576,7 +576,7 @@ void BTGattHandler::replyAttPDUReq(std::unique_ptr<const AttPDUMsg> && pdu) {
         case AttPDUMsg::Opcode::EXCHANGE_MTU_REQ: {
             const AttExchangeMTU * p = static_cast<const AttExchangeMTU*>(pdu.get());
             const uint16_t clientMTU = p->getMTUSize();
-            usedMTU = std::min((int)serverMTU, (int)clientMTU);
+            usedMTU = std::min(serverMTU, clientMTU);
             const AttExchangeMTU rsp(AttPDUMsg::ReqRespType::RESPONSE, usedMTU);
             COND_PRINT(env.DEBUG_DATA, "GATT-Req: MTU recv: %u, %s  -> %u %s from %s",
                     clientMTU, pdu->toString().c_str(),
@@ -805,7 +805,11 @@ BTGattHandler::BTGattHandler(const std::shared_ptr<BTDevice> &device, L2CAPComm&
             usedMTU = std::min(number(Defaults::MAX_ATT_MTU), serverMTU);
         }
     } else {
-        serverMTU = nullptr != gattServerData ? gattServerData->att_mtu : number(Defaults::MAX_ATT_MTU);
+        if( nullptr != gattServerData ) {
+            serverMTU = std::max( std::min( gattServerData->att_mtu, number(Defaults::MAX_ATT_MTU) ), number(Defaults::MIN_ATT_MTU) );
+        } else {
+            serverMTU = number(Defaults::MAX_ATT_MTU);
+        }
         usedMTU = number(Defaults::MIN_ATT_MTU); // until negotiated!
     }
 }