[coap-message] remove the payload marker when no payload (#6516)

This commit updates `Coap::Message` to check and remove the payload
marker byte which is added at the end of header options in the case
that the message contains no payload. This check is performed in
`Finish()` method which finalizes the message for transmission. Note
that the presence of a marker followed by a zero-length payload is
processed as a message format error by receivers.

This change helps simplify the preparation of CoAP message by other
modules (i.e., they no longer need to check for empty payload to
remove the marker themselves and can rely on common code in
`Coap::Message`).

src/core/coap/coap_message.cpp

@@ -114,6 +114,16 @@ bool Message::IsNonConfirmablePostRequest(void) const
 
 void Message::Finish(void)
 {
+    // If the payload marker is set but the message contains no
+    // payload, we remove the payload marker from the message. Note
+    // that the presence of a marker followed by a zero-length payload
+    // will be processed as a message format error on the receiver.
+
+    if (GetHelpData().mPayloadMarkerSet && (GetHelpData().mHeaderLength == GetLength()))
+    {
+        IgnoreError(SetLength(GetLength() - 1));
+    }
+
     WriteBytes(0, &GetHelpData().mHeader, GetOptionStart());
 }
 
@@ -330,7 +340,8 @@ Error Message::SetPayloadMarker(void)
 
     VerifyOrExit(GetLength() < kMaxHeaderLength, error = kErrorNoBufs);
     SuccessOrExit(error = Append(marker));
-    GetHelpData().mHeaderLength = GetLength();
+    GetHelpData().mPayloadMarkerSet = true;
+    GetHelpData().mHeaderLength     = GetLength();
 
     // Set offset to the start of payload.
     SetOffset(GetHelpData().mHeaderLength);

src/core/coap/coap_message.hpp

@@ -42,7 +42,9 @@
 #include "common/code_utils.hpp"
 #include "common/encoding.hpp"
 #include "common/message.hpp"
+#include "net/ip6.hpp"
 #include "net/ip6_address.hpp"
+#include "net/udp6.hpp"
 
 namespace ot {
 
@@ -268,6 +270,9 @@ public:
     /**
      * This method writes header to the message. This must be called before sending the message.
      *
+     * This method also checks whether the payload marker is set (`SetPayloadMarker()`) but the message contains no
+     * payload, and if so it removes the payload marker from the message.
+     *
      */
     void Finish(void);
 
@@ -961,11 +966,16 @@ private:
         uint16_t mOptionLast;
         uint16_t mHeaderOffset; ///< The byte offset for the CoAP Header
         uint16_t mHeaderLength;
+        bool     mPayloadMarkerSet;
 #if OPENTHREAD_CONFIG_COAP_BLOCKWISE_TRANSFER_ENABLE
         BlockWiseData mBlockWiseData;
 #endif
     };
 
+    static_assert(sizeof(HelpData) <= sizeof(Ip6::Header) + sizeof(Ip6::HopByHopHeader) + sizeof(Ip6::OptionMpl) +
+                                          sizeof(Ip6::Udp::Header),
+                  "HelpData size exceeds the size of the reserved region in the message");
+
     const HelpData &GetHelpData(void) const
     {
         static_assert(sizeof(mBuffer.mHead.mMetadata) + sizeof(HelpData) + kHelpDataAlignment <= sizeof(mBuffer),

src/core/meshcop/commissioner.cpp

@@ -783,12 +783,6 @@ Error Commissioner::SendMgmtCommissionerSetRequest(const otCommissioningDataset
         SuccessOrExit(error = message->AppendBytes(aTlvs, aLength));
     }
 
-    if (message->GetLength() == message->GetOffset())
-    {
-        // no payload, remove coap payload marker
-        IgnoreError(message->SetLength(message->GetLength() - 1));
-    }
-
     messageInfo.SetSockAddr(Get<Mle::MleRouter>().GetMeshLocal16());
     SuccessOrExit(error = Get<Mle::MleRouter>().GetLeaderAloc(messageInfo.GetPeerAddr()));
     messageInfo.SetPeerPort(Tmf::kUdpPort);

src/core/meshcop/dataset_manager.cpp

@@ -442,12 +442,6 @@ void DatasetManager::SendGetResponse(const Coap::Message &   aRequest,
         }
     }
 
-    if (message->GetLength() == message->GetOffset())
-    {
-        // no payload, remove coap payload marker
-        IgnoreError(message->SetLength(message->GetLength() - 1));
-    }
-
     SuccessOrExit(error = Get<Tmf::Agent>().SendMessage(*message, aMessageInfo));
 
     otLogInfoMeshCoP("sent %s dataset get response to %s", (GetType() == Dataset::kActive ? "active" : "pending"),
@@ -514,12 +508,6 @@ Error DatasetManager::SendSetRequest(const Dataset::Info &aDatasetInfo, const ui
         SuccessOrExit(error = message->AppendBytes(aTlvs, aLength));
     }
 
-    if (message->GetLength() == message->GetOffset())
-    {
-        // no payload, remove coap payload marker
-        IgnoreError(message->SetLength(message->GetLength() - 1));
-    }
-
     messageInfo.SetSockAddr(Get<Mle::MleRouter>().GetMeshLocal16());
     IgnoreError(Get<Mle::MleRouter>().GetLeaderAloc(messageInfo.GetPeerAddr()));
     messageInfo.SetPeerPort(Tmf::kUdpPort);

src/core/thread/network_data_leader_ftd.cpp

@@ -343,12 +343,6 @@ void Leader::SendCommissioningGetResponse(const Coap::Message &   aRequest,
         }
     }
 
-    if (message->GetLength() == message->GetOffset())
-    {
-        // no payload, remove coap payload marker
-        IgnoreError(message->SetLength(message->GetLength() - 1));
-    }
-
     SuccessOrExit(error = Get<Tmf::Agent>().SendMessage(*message, aMessageInfo));
 
     otLogInfoMeshCoP("sent commissioning dataset get response");

src/core/thread/network_diagnostic.cpp

@@ -506,12 +506,6 @@ void NetworkDiagnostic::HandleDiagnosticGetQuery(Coap::Message &aMessage, const
 
     SuccessOrExit(error = FillRequestedTlvs(aMessage, *message, networkDiagnosticTlv));
 
-    if (message->GetLength() == message->GetOffset())
-    {
-        // Remove Payload Marker if payload is actually empty.
-        IgnoreError(message->SetLength(message->GetLength() - 1));
-    }
-
     SuccessOrExit(error = Get<Tmf::Agent>().SendMessage(*message, messageInfo, nullptr, this));
 
     otLogInfoNetDiag("Sent diagnostic get answer");
@@ -550,12 +544,6 @@ void NetworkDiagnostic::HandleDiagnosticGetRequest(Coap::Message &aMessage, cons
 
     SuccessOrExit(error = FillRequestedTlvs(aMessage, *message, networkDiagnosticTlv));
 
-    if (message->GetLength() == message->GetOffset())
-    {
-        // Remove Payload Marker if payload is actually empty.
-        IgnoreError(message->SetLength(message->GetOffset() - 1));
-    }
-
     SuccessOrExit(error = Get<Tmf::Agent>().SendMessage(*message, messageInfo));
 
     otLogInfoNetDiag("Sent diagnostic get response");