diff --git a/src/core/coap/coap_message.cpp b/src/core/coap/coap_message.cpp index 41a001dab..3f34b5d71 100644 --- a/src/core/coap/coap_message.cpp +++ b/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); diff --git a/src/core/coap/coap_message.hpp b/src/core/coap/coap_message.hpp index 2fd59da42..305c265ec 100644 --- a/src/core/coap/coap_message.hpp +++ b/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), diff --git a/src/core/meshcop/commissioner.cpp b/src/core/meshcop/commissioner.cpp index fd2ff640d..87f3c8397 100644 --- a/src/core/meshcop/commissioner.cpp +++ b/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().GetMeshLocal16()); SuccessOrExit(error = Get().GetLeaderAloc(messageInfo.GetPeerAddr())); messageInfo.SetPeerPort(Tmf::kUdpPort); diff --git a/src/core/meshcop/dataset_manager.cpp b/src/core/meshcop/dataset_manager.cpp index 4b0ef40f8..de4adf973 100644 --- a/src/core/meshcop/dataset_manager.cpp +++ b/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().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().GetMeshLocal16()); IgnoreError(Get().GetLeaderAloc(messageInfo.GetPeerAddr())); messageInfo.SetPeerPort(Tmf::kUdpPort); diff --git a/src/core/thread/network_data_leader_ftd.cpp b/src/core/thread/network_data_leader_ftd.cpp index 8e5d82f29..60f653536 100644 --- a/src/core/thread/network_data_leader_ftd.cpp +++ b/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().SendMessage(*message, aMessageInfo)); otLogInfoMeshCoP("sent commissioning dataset get response"); diff --git a/src/core/thread/network_diagnostic.cpp b/src/core/thread/network_diagnostic.cpp index df98a378a..74f7e0c3a 100644 --- a/src/core/thread/network_diagnostic.cpp +++ b/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().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().SendMessage(*message, messageInfo)); otLogInfoNetDiag("Sent diagnostic get response");