From dea5c4559d5e1730fe1bbcba51d5c8a6ce043b22 Mon Sep 17 00:00:00 2001 From: Abtin Keshavarzian Date: Sun, 12 Apr 2026 19:40:47 -0700 Subject: [PATCH] [meshcop] add `FindIn()` and `AppendTo()` for `SteeringDataTlv` (#12871) This commit introduces static helper methods `SteeringDataTlv::FindIn()` and `SteeringDataTlv::AppendTo()` to simplify the handling of steering data in `Message` objects. `SteeringDataTlv::FindIn()` encapsulates the pattern of searching for a `SteeringDataTlv` in a `Message` and reading its value into a `SteeringData` object. `SteeringDataTlv::AppendTo()` provides a unified way to append steering data to a `Message`, including a validity check. These helpers are adopted across core modules (MeshCoP, MLE, Discovery) and various Nexus tests, replacing manual TLV manipulation with a cleaner and safer helper methods. --- src/core/meshcop/border_agent_admitter.cpp | 12 +++------ src/core/meshcop/commissioner.cpp | 2 +- src/core/meshcop/meshcop_tlvs.cpp | 13 ++++++++++ src/core/meshcop/meshcop_tlvs.hpp | 28 +++++++++++++++++++++ src/core/meshcop/steering_data.hpp | 1 + src/core/thread/discover_scanner.cpp | 17 +++++-------- src/core/thread/mle.cpp | 2 +- tests/nexus/test_1_1_9_2_2.cpp | 2 +- tests/nexus/test_1_1_9_2_6.cpp | 3 +-- tests/nexus/test_border_admitter.cpp | 29 +++++++++------------- 10 files changed, 68 insertions(+), 41 deletions(-) diff --git a/src/core/meshcop/border_agent_admitter.cpp b/src/core/meshcop/border_agent_admitter.cpp index ef9ad6746..3047c2564 100644 --- a/src/core/meshcop/border_agent_admitter.cpp +++ b/src/core/meshcop/border_agent_admitter.cpp @@ -813,7 +813,7 @@ void Admitter::CommissionerPetitioner::SendDataSet(void) VerifyOrExit(message != nullptr, error = kErrorNoBufs); SuccessOrExit(error = Tlv::Append(*message, mSessionId)); - SuccessOrExit(error = Tlv::Append(*message, mSteeringData.GetData(), mSteeringData.GetLength())); + SuccessOrExit(error = SteeringDataTlv::AppendTo(*message, mSteeringData)); if (mJoinerUdpPort != 0) { @@ -1193,10 +1193,9 @@ exit: Error Manager::CoapDtlsSession::ReadSteeringDataTlv(const Message &aMessage, SteeringData &aSteeringData) { - Error error; - OffsetRange offsetRange; + Error error; - SuccessOrExit(error = Tlv::FindTlvValueOffsetRange(aMessage, Tlv::kSteeringData, offsetRange)); + SuccessOrExit(error = SteeringDataTlv::FindIn(aMessage, aSteeringData)); // Ensure the read steering data has a valid length. A length of // one byte is only allowed to indicate `PermitsAllJoiners()`. @@ -1205,7 +1204,7 @@ Error Manager::CoapDtlsSession::ReadSteeringDataTlv(const Message &aMessage, Ste for (uint8_t validLength : Admitter::kEnrollerValidSteeringDataLengths) { - if (offsetRange.GetLength() == validLength) + if (aSteeringData.GetLength() == validLength) { error = kErrorNone; break; @@ -1214,9 +1213,6 @@ Error Manager::CoapDtlsSession::ReadSteeringDataTlv(const Message &aMessage, Ste SuccessOrExit(error); - IgnoreError(aSteeringData.Init(static_cast(offsetRange.GetLength()))); - aMessage.ReadBytes(offsetRange, aSteeringData.GetData()); - if (aSteeringData.GetLength() == 1) { VerifyOrExit(aSteeringData.PermitsAllJoiners() || aSteeringData.IsEmpty(), error = kErrorInvalidArgs); diff --git a/src/core/meshcop/commissioner.cpp b/src/core/meshcop/commissioner.cpp index e021a5247..16d48af5f 100644 --- a/src/core/meshcop/commissioner.cpp +++ b/src/core/meshcop/commissioner.cpp @@ -652,7 +652,7 @@ Error Commissioner::SendMgmtCommissionerSetRequest(const CommissioningDataset &a { const SteeringData &steeringData = aDataset.GetSteeringData(); - SuccessOrExit(error = Tlv::Append(*message, steeringData.GetData(), steeringData.GetLength())); + SuccessOrExit(error = SteeringDataTlv::AppendTo(*message, steeringData)); } if (aDataset.IsJoinerUdpPortSet()) diff --git a/src/core/meshcop/meshcop_tlvs.cpp b/src/core/meshcop/meshcop_tlvs.cpp index 9b207f0e3..267a44e28 100644 --- a/src/core/meshcop/meshcop_tlvs.cpp +++ b/src/core/meshcop/meshcop_tlvs.cpp @@ -65,6 +65,19 @@ Error SteeringDataTlv::CopyTo(SteeringData &aSteeringData) const return aSteeringData.Init(GetSteeringDataLength(), mSteeringData); } +Error SteeringDataTlv::FindIn(const Message &aMessage, SteeringData &aSteeringData) +{ + Error error; + OffsetRange offsetRange; + + SuccessOrExit(error = Tlv::FindTlvValueOffsetRange(aMessage, Tlv::kSteeringData, offsetRange)); + SuccessOrExit(error = aSteeringData.Init(ClampToUint8(offsetRange.GetLength()))); + error = aMessage.Read(offsetRange, aSteeringData.GetData(), aSteeringData.GetLength()); + +exit: + return error; +} + bool SecurityPolicyTlv::IsValid(void) const { return GetLength() >= sizeof(mRotationTime) && GetFlagsLength() >= kThread11FlagsLength; diff --git a/src/core/meshcop/meshcop_tlvs.hpp b/src/core/meshcop/meshcop_tlvs.hpp index 508aee22c..7fea36bab 100644 --- a/src/core/meshcop/meshcop_tlvs.hpp +++ b/src/core/meshcop/meshcop_tlvs.hpp @@ -370,6 +370,34 @@ public: */ Error CopyTo(SteeringData &aSteeringData) const; + /** + * Searches within a given message for Steering Data TLV, parses and validates the TLV value and returns the + * read Steering Data. + * + * @param[in] aMessage The message to search in. + * @param[out] aSteeringData A reference to return the read Steering Data. + * + * @retval kErrorNone Found the TLV, successfully parsed its value, @p aSteeringData is updated. + * @retval kErrorNotFound No Steering Data TLV found in the @p aMessage. + * @retval kErrorParse Found the TLV, but failed to parse it (e.g. not enough bytes in message). + * @retval kErrorInvalidArgs Found the TLV, but TLV length is not valid for Steering Data (e.g., larger than max). + */ + static Error FindIn(const Message &aMessage, SteeringData &aSteeringData); + + /** + * Append a Steering Data TLV to a given message. + * + * @param[in] aMessage The message to append to. + * @param[in] aSteeringData The Steering Data value. + * + * @retval kErrorNone Successfully appended the TLV to @p aMessage. + * @retval kErrorNoBufs Insufficient available buffers to grow the message. + */ + static Error AppendTo(Message &aMessage, const SteeringData &aSteeringData) + { + return Tlv::Append(aMessage, aSteeringData.GetData(), aSteeringData.GetLength()); + } + private: uint8_t mSteeringData[SteeringData::kMaxLength]; } OT_TOOL_PACKED_END; diff --git a/src/core/meshcop/steering_data.hpp b/src/core/meshcop/steering_data.hpp index 7cafba1d8..a857e0c43 100644 --- a/src/core/meshcop/steering_data.hpp +++ b/src/core/meshcop/steering_data.hpp @@ -43,6 +43,7 @@ #include "common/code_utils.hpp" #include "common/equatable.hpp" #include "common/error.hpp" +#include "common/message.hpp" #include "common/string.hpp" #include "mac/mac_types.hpp" diff --git a/src/core/thread/discover_scanner.cpp b/src/core/thread/discover_scanner.cpp index 77f06e12a..5c24bf8bf 100644 --- a/src/core/thread/discover_scanner.cpp +++ b/src/core/thread/discover_scanner.cpp @@ -318,7 +318,7 @@ void DiscoverScanner::HandleDiscoveryResponse(Mle::RxInfo &aRxInfo) const ScanResult result; OffsetRange offsetRange; MeshCoP::DiscoveryResponseTlvValue respTlvValue; - MeshCoP::SteeringDataTlv steeringDataTlv; + MeshCoP::SteeringData steeringData; Mle::Log(Mle::kMessageReceive, Mle::kTypeDiscoveryResponse, aRxInfo.mMessageInfo.GetPeerAddr()); @@ -363,19 +363,14 @@ void DiscoverScanner::HandleDiscoveryResponse(Mle::RxInfo &aRxInfo) const ExitNow(error = kErrorParse); } - switch (Tlv::FindTlv(aRxInfo.mMessage, steeringDataTlv)) + switch (MeshCoP::SteeringDataTlv::FindIn(aRxInfo.mMessage, steeringData)) { case kErrorNone: - if (steeringDataTlv.IsValid()) + AsCoreType(&result.mSteeringData) = steeringData; + + if (mEnableFiltering) { - MeshCoP::SteeringData &steeringData = AsCoreType(&result.mSteeringData); - - IgnoreError(steeringDataTlv.CopyTo(steeringData)); - - if (mEnableFiltering) - { - VerifyOrExit(steeringData.Contains(mFilterIndexes)); - } + VerifyOrExit(steeringData.Contains(mFilterIndexes)); } break; diff --git a/src/core/thread/mle.cpp b/src/core/thread/mle.cpp index 188a67a54..bfc50a5da 100644 --- a/src/core/thread/mle.cpp +++ b/src/core/thread/mle.cpp @@ -4036,7 +4036,7 @@ Error Mle::TxMessage::AppendSteeringDataTlv(void) SuccessOrExit(Get().FindSteeringData(steeringData)); } - error = Tlv::Append(*this, steeringData.GetData(), steeringData.GetLength()); + error = MeshCoP::SteeringDataTlv::AppendTo(*this, steeringData); exit: return error; diff --git a/tests/nexus/test_1_1_9_2_2.cpp b/tests/nexus/test_1_1_9_2_2.cpp index b0770e1c0..97b091ae7 100644 --- a/tests/nexus/test_1_1_9_2_2.cpp +++ b/tests/nexus/test_1_1_9_2_2.cpp @@ -69,7 +69,7 @@ static void AppendSteeringDataTlv(Coap::Message &aMessage) MeshCoP::SteeringData steeringData; steeringData.SetToPermitAllJoiners(); - SuccessOrQuit(Tlv::Append(aMessage, steeringData.GetData(), steeringData.GetLength())); + SuccessOrQuit(MeshCoP::SteeringDataTlv::AppendTo(aMessage, steeringData)); } void Test9_2_2(void) diff --git a/tests/nexus/test_1_1_9_2_6.cpp b/tests/nexus/test_1_1_9_2_6.cpp index e5452dea1..7487f9e25 100644 --- a/tests/nexus/test_1_1_9_2_6.cpp +++ b/tests/nexus/test_1_1_9_2_6.cpp @@ -216,8 +216,7 @@ void Test9_2_6(void) { MeshCoP::SteeringData steeringData; steeringData.SetToPermitAllJoiners(); - SuccessOrQuit( - Tlv::Append(*message, steeringData.GetData(), steeringData.GetLength())); + SuccessOrQuit(MeshCoP::SteeringDataTlv::AppendTo(*message, steeringData)); } SuccessOrQuit(agent.SendMessageToLeaderAloc(*message)); diff --git a/tests/nexus/test_border_admitter.cpp b/tests/nexus/test_border_admitter.cpp index 45dccfb5f..03bf74af9 100644 --- a/tests/nexus/test_border_admitter.cpp +++ b/tests/nexus/test_border_admitter.cpp @@ -475,7 +475,7 @@ void TestBorderAdmitterEnrollerInteraction(void) SuccessOrQuit(Tlv::Append(*message, kEnrollerId)); SuccessOrQuit(Tlv::Append(*message, mode)); - SuccessOrQuit(Tlv::Append(*message, steeringData.GetData(), steeringData.GetLength())); + SuccessOrQuit(MeshCoP::SteeringDataTlv::AppendTo(*message, steeringData)); responseContext.Clear(); SuccessOrQuit(enroller.Get().SendMessage(*message, HandleResponse, &responseContext)); @@ -652,7 +652,7 @@ void TestBorderAdmitterEnrollerInteraction(void) SuccessOrQuit(steeringData.UpdateBloomFilter(admitter.Get().GetExtAddress())); SuccessOrQuit(Tlv::Append(*message, MeshCoP::StateTlv::kAccept)); - SuccessOrQuit(Tlv::Append(*message, steeringData.GetData(), steeringData.GetLength())); + SuccessOrQuit(MeshCoP::SteeringDataTlv::AppendTo(*message, steeringData)); responseContext.Clear(); SuccessOrQuit(enroller.Get().SendMessage(*message, HandleResponse, &responseContext)); @@ -742,7 +742,7 @@ void TestBorderAdmitterEnrollerInteraction(void) SuccessOrQuit(Tlv::Append(*message, kEnrollerId)); SuccessOrQuit(Tlv::Append(*message, mode)); - SuccessOrQuit(Tlv::Append(*message, steeringData.GetData(), steeringData.GetLength())); + SuccessOrQuit(MeshCoP::SteeringDataTlv::AppendTo(*message, steeringData)); responseContext.Clear(); SuccessOrQuit(enroller.Get().SendMessage(*message, HandleResponse, &responseContext)); @@ -816,7 +816,7 @@ void TestBorderAdmitterEnrollerInteraction(void) SuccessOrQuit(Tlv::Append(*message, kEnrollerId)); SuccessOrQuit(Tlv::Append(*message, mode)); - SuccessOrQuit(Tlv::Append(*message, steeringData.GetData(), steeringData.GetLength())); + SuccessOrQuit(MeshCoP::SteeringDataTlv::AppendTo(*message, steeringData)); responseContext.Clear(); SuccessOrQuit(enroller.Get().SendMessage(*message, HandleResponse, &responseContext)); @@ -851,7 +851,7 @@ void TestBorderAdmitterEnrollerInteraction(void) SuccessOrQuit(Tlv::Append(*message, kEnrollerIdAlt)); SuccessOrQuit(Tlv::Append(*message, mode)); - SuccessOrQuit(Tlv::Append(*message, steeringData.GetData(), steeringData.GetLength())); + SuccessOrQuit(MeshCoP::SteeringDataTlv::AppendTo(*message, steeringData)); responseContext.Clear(); SuccessOrQuit(enroller.Get().SendMessage(*message, HandleResponse, &responseContext)); @@ -964,8 +964,7 @@ void TestBorderAdmitterEnrollerInteraction(void) if (testIter != 2) { - SuccessOrQuit( - Tlv::Append(*message, steeringData.GetData(), steeringData.GetLength())); + SuccessOrQuit(MeshCoP::SteeringDataTlv::AppendTo(*message, steeringData)); } responseContext.Clear(); @@ -1005,8 +1004,7 @@ void TestBorderAdmitterEnrollerInteraction(void) SuccessOrQuit(Tlv::Append(*message, kEnrollerId)); SuccessOrQuit(Tlv::Append(*message, mode)); - SuccessOrQuit( - Tlv::Append(*message, steeringData.GetData(), steeringData.GetLength())); + SuccessOrQuit(MeshCoP::SteeringDataTlv::AppendTo(*message, steeringData)); responseContext.Clear(); SuccessOrQuit(enroller.Get().SendMessage(*message, HandleResponse, &responseContext)); @@ -1031,7 +1029,7 @@ void TestBorderAdmitterEnrollerInteraction(void) SuccessOrQuit(Tlv::Append(*message, kEnrollerId)); SuccessOrQuit(Tlv::Append(*message, mode)); - SuccessOrQuit(Tlv::Append(*message, steeringData.GetData(), steeringData.GetLength())); + SuccessOrQuit(MeshCoP::SteeringDataTlv::AppendTo(*message, steeringData)); responseContext.Clear(); SuccessOrQuit(enroller.Get().SendMessage(*message, HandleResponse, &responseContext)); @@ -1154,7 +1152,7 @@ void TestBorderAdmitterCommissionerConflictAndPetitionerRetry(void) SuccessOrQuit(Tlv::Append(*message, kEnrollerId)); SuccessOrQuit(Tlv::Append(*message, mode)); - SuccessOrQuit(Tlv::Append(*message, steeringData.GetData(), steeringData.GetLength())); + SuccessOrQuit(MeshCoP::SteeringDataTlv::AppendTo(*message, steeringData)); SuccessOrQuit(enroller.Get().SendMessage(*message)); @@ -1465,8 +1463,7 @@ void TestBorderAdmitterMultipleEnrollers(void) SuccessOrQuit(Tlv::Append(*message, kEnrollerIds[i])); SuccessOrQuit(Tlv::Append(*message, mode)); - SuccessOrQuit( - Tlv::Append(*message, steeringData[i].GetData(), steeringData[i].GetLength())); + SuccessOrQuit(MeshCoP::SteeringDataTlv::AppendTo(*message, steeringData[i])); responseContexts[i].Clear(); SuccessOrQuit( @@ -1790,8 +1787,7 @@ void TestBorderAdmitterJoinerEnrollerInteraction(void) SuccessOrQuit(Tlv::Append(*message, kEnrollerIds[i])); SuccessOrQuit(Tlv::Append(*message, modes[i])); - SuccessOrQuit( - Tlv::Append(*message, steeringData.GetData(), steeringData.GetLength())); + SuccessOrQuit(MeshCoP::SteeringDataTlv::AppendTo(*message, steeringData)); responseContexts[i].Clear(); SuccessOrQuit( @@ -3288,8 +3284,7 @@ void TestBorderAdmitterForwardingUdpProxy(void) SuccessOrQuit(Tlv::Append(*message, kEnrollerIds[i])); SuccessOrQuit(Tlv::Append(*message, modes[i])); - SuccessOrQuit( - Tlv::Append(*message, steeringData.GetData(), steeringData.GetLength())); + SuccessOrQuit(MeshCoP::SteeringDataTlv::AppendTo(*message, steeringData)); responseContexts[i].Clear(); SuccessOrQuit(