diff --git a/src/core/common/string.cpp b/src/core/common/string.cpp index c7aa7218a1f..fa0dc3e5da7 100644 --- a/src/core/common/string.cpp +++ b/src/core/common/string.cpp @@ -160,6 +160,35 @@ bool StringMatch(const char *aFirstString, const char *aSecondString, StringMatc return Match(aFirstString, aSecondString, aMode) == kFullMatch; } +Error StringCopy(char *aTargetBuffer, uint16_t aTargetSize, const char *aSource, StringEncodingCheck aEncodingCheck) +{ + Error error = kErrorNone; + uint16_t length; + + if (aSource == nullptr) + { + aTargetBuffer[0] = kNullChar; + ExitNow(); + } + + length = StringLength(aSource, aTargetSize); + VerifyOrExit(length < aTargetSize, error = kErrorInvalidArgs); + + switch (aEncodingCheck) + { + case kStringNoEncodingCheck: + break; + case kStringCheckUtf8Encoding: + VerifyOrExit(IsValidUtf8String(aSource), error = kErrorParse); + break; + } + + memcpy(aTargetBuffer, aSource, length + 1); // `+ 1` to also copy null char. + +exit: + return error; +} + Error StringParseUint8(const char *&aString, uint8_t &aUint8) { return StringParseUint8(aString, aUint8, NumericLimits::kMax); diff --git a/src/core/common/string.hpp b/src/core/common/string.hpp index 035bc3f3218..7cb657cc9c9 100644 --- a/src/core/common/string.hpp +++ b/src/core/common/string.hpp @@ -67,6 +67,16 @@ enum StringMatchMode : uint8_t kStringCaseInsensitiveMatch, ///< Case insensitive match (uppercase and lowercase characters are treated as equal). }; +/** + * Represents string encoding check when copying string. + * + */ +enum StringEncodingCheck : uint8_t +{ + kStringNoEncodingCheck, ///< Do not check the string encoding. + kStringCheckUtf8Encoding, ///< Validate that string follows UTF-8 encoding. +}; + static constexpr char kNullChar = '\0'; ///< null character. /** @@ -156,6 +166,43 @@ bool StringEndsWith(const char *aString, const char *aSubString, StringMatchMode */ bool StringMatch(const char *aFirstString, const char *aSecondString, StringMatchMode aMode = kStringExactMatch); +/** + * Copies a string into a given target buffer with a given size if it fits. + * + * @param[out] aTargetBuffer A pointer to the target buffer to copy into. + * @param[out] aTargetSize The size (number of characters) in @p aTargetBuffer array. + * @param[in] aSource A pointer to null-terminated string to copy from. Can be `nullptr` which treated as "". + * @param[in] aEncodingCheck Specifies the encoding format check (e.g., UTF-8) to perform. + * + * @retval kErrorNone The @p aSource fits in the given buffer. @p aTargetBuffer is updated. + * @retval kErrorInvalidArgs The @p aSource does not fit in the given buffer. + * @retval kErrorParse The @p aSource does not follow the encoding format specified by @p aEncodingCheck. + * + */ +Error StringCopy(char *TargetBuffer, uint16_t aTargetSize, const char *aSource, StringEncodingCheck aEncodingCheck); + +/** + * Copies a string into a given target buffer with a given size if it fits. + * + * @tparam kSize The size of buffer. + * + * @param[out] aTargetBuffer A reference to the target buffer array to copy into. + * @param[in] aSource A pointer to null-terminated string to copy from. Can be `nullptr` which treated as "". + * @param[in] aEncodingCheck Specifies the encoding format check (e.g., UTF-8) to perform. + * + * @retval kErrorNone The @p aSource fits in the given buffer. @p aTargetBuffer is updated. + * @retval kErrorInvalidArgs The @p aSource does not fit in the given buffer. + * @retval kErrorParse The @p aSource does not follow the encoding format specified by @p aEncodingCheck. + * + */ +template +Error StringCopy(char (&aTargetBuffer)[kSize], + const char *aSource, + StringEncodingCheck aEncodingCheck = kStringNoEncodingCheck) +{ + return StringCopy(aTargetBuffer, kSize, aSource, aEncodingCheck); +} + /** * Parses a decimal number from a string as `uint8_t` and skips over the parsed characters. * diff --git a/src/core/meshcop/commissioner.cpp b/src/core/meshcop/commissioner.cpp index 8b3aea95c58..858a2c14e61 100644 --- a/src/core/meshcop/commissioner.cpp +++ b/src/core/meshcop/commissioner.cpp @@ -349,21 +349,10 @@ Error Commissioner::Stop(ResignMode aResignMode) Error Commissioner::SetId(const char *aId) { - Error error = kErrorNone; - uint8_t len; + Error error = kErrorNone; VerifyOrExit(IsDisabled(), error = kErrorInvalidState); - VerifyOrExit(aId != nullptr); - VerifyOrExit(IsValidUtf8String(aId), error = kErrorInvalidArgs); - - len = static_cast(StringLength(aId, sizeof(mCommissionerId))); - - // CommissionerIdTlv::SetCommissionerId trims the string to the maximum array size. - // Prevent this from happening returning an error. - VerifyOrExit(len < CommissionerIdTlv::kMaxLength, error = kErrorInvalidArgs); - - memcpy(mCommissionerId, aId, len); - mCommissionerId[len] = '\0'; + error = StringCopy(mCommissionerId, aId, kStringCheckUtf8Encoding); exit: return error; @@ -582,26 +571,7 @@ void Commissioner::RemoveJoiner(Joiner &aJoiner, uint32_t aDelay) Error Commissioner::SetProvisioningUrl(const char *aProvisioningUrl) { - Error error = kErrorNone; - uint8_t len; - - if (aProvisioningUrl == nullptr) - { - mProvisioningUrl[0] = '\0'; - ExitNow(); - } - - VerifyOrExit(IsValidUtf8String(aProvisioningUrl), error = kErrorInvalidArgs); - - len = static_cast(StringLength(aProvisioningUrl, sizeof(mProvisioningUrl))); - - VerifyOrExit(len < sizeof(mProvisioningUrl), error = kErrorInvalidArgs); - - memcpy(mProvisioningUrl, aProvisioningUrl, len); - mProvisioningUrl[len] = '\0'; - -exit: - return error; + return StringCopy(mProvisioningUrl, aProvisioningUrl, kStringCheckUtf8Encoding); } void Commissioner::HandleTimer(void) @@ -779,20 +749,17 @@ void Commissioner::HandleMgmtCommissionerSetResponse(Coap::Message *aMe Error Commissioner::SendPetition(void) { - Error error = kErrorNone; - Coap::Message *message = nullptr; - Tmf::MessageInfo messageInfo(GetInstance()); - CommissionerIdTlv commissionerIdTlv; + Error error = kErrorNone; + Coap::Message *message = nullptr; + Tmf::MessageInfo messageInfo(GetInstance()); mTransmitAttempts++; message = Get().NewPriorityConfirmablePostMessage(kUriLeaderPetition); VerifyOrExit(message != nullptr, error = kErrorNoBufs); - commissionerIdTlv.Init(); - commissionerIdTlv.SetCommissionerId(mCommissionerId); + SuccessOrExit(error = Tlv::Append(*message, mCommissionerId)); - SuccessOrExit(error = commissionerIdTlv.AppendTo(*message)); SuccessOrExit(error = messageInfo.SetSockAddrToRlocPeerAddrToLeaderAloc()); SuccessOrExit( error = Get().SendMessage(*message, messageInfo, Commissioner::HandleLeaderPetitionResponse, this)); diff --git a/src/core/meshcop/commissioner.hpp b/src/core/meshcop/commissioner.hpp index ee436c5338f..e9d24b90cf0 100644 --- a/src/core/meshcop/commissioner.hpp +++ b/src/core/meshcop/commissioner.hpp @@ -616,8 +616,8 @@ class Commissioner : public InstanceLocator, private NonCopyable Ip6::Netif::UnicastAddress mCommissionerAloc; - char mProvisioningUrl[OT_PROVISIONING_URL_MAX_SIZE + 1]; // + 1 is for null char at end of string. - char mCommissionerId[CommissionerIdTlv::kMaxLength + 1]; + ProvisioningUrlTlv::StringType mProvisioningUrl; + CommissionerIdTlv::StringType mCommissionerId; State mState; diff --git a/src/core/meshcop/meshcop_leader.cpp b/src/core/meshcop/meshcop_leader.cpp index c4f0676d6e0..f3622987152 100644 --- a/src/core/meshcop/meshcop_leader.cpp +++ b/src/core/meshcop/meshcop_leader.cpp @@ -67,35 +67,28 @@ template <> void Leader::HandleTmf(Coap::Message &aMessage, { OT_UNUSED_VARIABLE(aMessageInfo); - CommissioningData data; - CommissionerIdTlv commissionerId; - StateTlv::State state = StateTlv::kReject; + CommissioningData data; + CommissionerIdTlv::StringType commissionerId; + StateTlv::State state = StateTlv::kReject; LogInfo("Received %s", UriToString()); VerifyOrExit(Get().IsLeader()); VerifyOrExit(Get().IsRoutingLocator(aMessageInfo.GetPeerAddr())); - SuccessOrExit(Tlv::FindTlv(aMessage, commissionerId)); + + SuccessOrExit(Tlv::Find(aMessage, commissionerId)); if (mTimer.IsRunning()) { - VerifyOrExit((commissionerId.GetCommissionerIdLength() == mCommissionerId.GetCommissionerIdLength()) && - (!strncmp(commissionerId.GetCommissionerId(), mCommissionerId.GetCommissionerId(), - commissionerId.GetCommissionerIdLength()))); - + VerifyOrExit(StringMatch(mCommissionerId, commissionerId)); ResignCommissioner(); } data.Init(aMessageInfo.GetPeerAddr().GetIid().GetLocator(), ++mSessionId); SuccessOrExit(Get().SetCommissioningData(&data, data.GetLength())); - mCommissionerId = commissionerId; - - if (mCommissionerId.GetLength() > CommissionerIdTlv::kMaxLength) - { - mCommissionerId.SetLength(CommissionerIdTlv::kMaxLength); - } + IgnoreError(StringCopy(mCommissionerId, commissionerId)); state = StateTlv::kAccept; mTimer.Start(Time::SecToMsec(kTimeoutLeaderPetition)); @@ -118,7 +111,7 @@ void Leader::SendPetitionResponse(const Coap::Message &aRequest, if (mTimer.IsRunning()) { - SuccessOrExit(error = mCommissionerId.AppendTo(*message)); + SuccessOrExit(error = Tlv::Append(*message, mCommissionerId)); } if (aState == StateTlv::kAccept) diff --git a/src/core/meshcop/meshcop_leader.hpp b/src/core/meshcop/meshcop_leader.hpp index 864771203a8..8ce5eba6795 100644 --- a/src/core/meshcop/meshcop_leader.hpp +++ b/src/core/meshcop/meshcop_leader.hpp @@ -133,8 +133,8 @@ class Leader : public InstanceLocator, private NonCopyable uint32_t mDelayTimerMinimal; - CommissionerIdTlv mCommissionerId; - uint16_t mSessionId; + CommissionerIdTlv::StringType mCommissionerId; + uint16_t mSessionId; }; DeclareTmfHandler(Leader, kUriLeaderPetition); diff --git a/src/core/meshcop/meshcop_tlvs.hpp b/src/core/meshcop/meshcop_tlvs.hpp index d675fbb1c91..4be763a67c7 100644 --- a/src/core/meshcop/meshcop_tlvs.hpp +++ b/src/core/meshcop/meshcop_tlvs.hpp @@ -126,6 +126,7 @@ class Tlv : public ot::Tlv */ static constexpr uint8_t kMaxProvisioningUrlLength = OT_PROVISIONING_URL_MAX_SIZE; + static constexpr uint8_t kMaxCommissionerIdLength = 64; ///< Max length of Commissioner ID TLV. static constexpr uint8_t kMaxVendorNameLength = 32; ///< Max length of Vendor Name TLV. static constexpr uint8_t kMaxVendorModelLength = 32; ///< Max length of Vendor Model TLV. static constexpr uint8_t kMaxVendorSwVersionLength = 16; ///< Max length of Vendor SW Version TLV. @@ -248,6 +249,12 @@ typedef UintTlvInfo PeriodTlv; */ typedef UintTlvInfo ScanDurationTlv; +/** + * Defines Commissioner ID TLV constants and type.s + * + */ +typedef StringTlvInfo CommissionerIdTlv; + /** * Implements Channel TLV generation and parsing. * @@ -756,62 +763,6 @@ class BorderAgentLocatorTlv : public Tlv, public UintTlvInfo -{ -public: - static constexpr uint8_t kMaxLength = 64; ///< maximum length (bytes) - - /** - * Initializes the TLV. - * - */ - void Init(void) - { - SetType(kCommissionerId); - SetLength(sizeof(*this) - sizeof(Tlv)); - } - - /** - * Returns the Commissioner ID length. - * - * @returns The Commissioner ID length. - * - */ - uint8_t GetCommissionerIdLength(void) const - { - return GetLength() <= sizeof(mCommissionerId) ? GetLength() : sizeof(mCommissionerId); - } - - /** - * Returns the Commissioner ID value. - * - * @returns The Commissioner ID value. - * - */ - const char *GetCommissionerId(void) const { return mCommissionerId; } - - /** - * Sets the Commissioner ID value. - * - * @param[in] aCommissionerId A pointer to the Commissioner ID value. - * - */ - void SetCommissionerId(const char *aCommissionerId) - { - uint16_t length = StringLength(aCommissionerId, sizeof(mCommissionerId)); - memcpy(mCommissionerId, aCommissionerId, length); - SetLength(static_cast(length)); - } - -private: - char mCommissionerId[kMaxLength]; -} OT_TOOL_PACKED_END; - /** * Implements Commissioner Session ID TLV generation and parsing. * diff --git a/src/core/thread/network_diagnostic.cpp b/src/core/thread/network_diagnostic.cpp index 97926091d57..7f313a573b2 100644 --- a/src/core/thread/network_diagnostic.cpp +++ b/src/core/thread/network_diagnostic.cpp @@ -82,39 +82,20 @@ Server::Server(Instance &aInstance) Error Server::SetVendorName(const char *aVendorName) { - return SetVendorString(mVendorName, sizeof(mVendorName), aVendorName); + return StringCopy(mVendorName, aVendorName, kStringCheckUtf8Encoding); } Error Server::SetVendorModel(const char *aVendorModel) { - return SetVendorString(mVendorModel, sizeof(mVendorModel), aVendorModel); + return StringCopy(mVendorModel, aVendorModel, kStringCheckUtf8Encoding); } Error Server::SetVendorSwVersion(const char *aVendorSwVersion) { - return SetVendorString(mVendorSwVersion, sizeof(mVendorSwVersion), aVendorSwVersion); + return StringCopy(mVendorSwVersion, aVendorSwVersion, kStringCheckUtf8Encoding); } -Error Server::SetVendorString(char *aDestString, uint16_t kMaxSize, const char *aSrcString) -{ - Error error = kErrorInvalidArgs; - uint16_t length; - - VerifyOrExit(aSrcString != nullptr); - - length = StringLength(aSrcString, kMaxSize); - VerifyOrExit(length < kMaxSize); - - VerifyOrExit(IsValidUtf8String(aSrcString)); - - memcpy(aDestString, aSrcString, length + 1); - error = kErrorNone; - -exit: - return error; -} - -#endif // OPENTHREAD_CONFIG_NET_DIAG_VENDOR_INFO_SET_API_ENABLE +#endif void Server::PrepareMessageInfoForDest(const Ip6::Address &aDestination, Tmf::MessageInfo &aMessageInfo) const { diff --git a/src/core/thread/network_diagnostic.hpp b/src/core/thread/network_diagnostic.hpp index f610f2bb046..de16b7c70be 100644 --- a/src/core/thread/network_diagnostic.hpp +++ b/src/core/thread/network_diagnostic.hpp @@ -208,8 +208,6 @@ class Server : public InstanceLocator, private NonCopyable template void HandleTmf(Coap::Message &aMessage, const Ip6::MessageInfo &aMessageInfo); #if OPENTHREAD_CONFIG_NET_DIAG_VENDOR_INFO_SET_API_ENABLE - Error SetVendorString(char *aDestString, uint16_t kMaxSize, const char *aSrcString); - VendorNameTlv::StringType mVendorName; VendorModelTlv::StringType mVendorModel; VendorSwVersionTlv::StringType mVendorSwVersion; diff --git a/tests/scripts/expect/cli-commissioner.exp b/tests/scripts/expect/cli-commissioner.exp index 2c6737e864a..e5849d3e4e3 100755 --- a/tests/scripts/expect/cli-commissioner.exp +++ b/tests/scripts/expect/cli-commissioner.exp @@ -52,9 +52,9 @@ expect_line "Done" send "commissioner id\n" expect "OpenThread Commissioner" expect_line "Done" -send "commissioner id reallyAndUnnecessaryLongOpenthreadComisionerCustomIdShouldNotFit\n" +send "commissioner id 12345678901234567890123456789012345678901234567890123456789012345\n" expect "Error 7: InvalidArgs" -send "commissioner id reallyAndUnnecessaryLongOpenthreadCommissionerCustomIdShouldFit\n" +send "commissioner id 1234567890123456789012345678901234567890123456789012345678901234\n" expect "Done" send "commissioner id customId\n" expect "Done" diff --git a/tests/unit/test_string.cpp b/tests/unit/test_string.cpp index 80ff3b099d5..7158ca28cd4 100644 --- a/tests/unit/test_string.cpp +++ b/tests/unit/test_string.cpp @@ -366,6 +366,36 @@ void TestStringParseUint8(void) printf("\n\n -- PASS\n"); } +void TestStringCopy(void) +{ + char buffer[10]; + char smallBuffer[1]; + + printf("\nTest 11: StringCopy() function\n"); + + SuccessOrQuit(StringCopy(buffer, "foo", kStringCheckUtf8Encoding)); + VerifyOrQuit(StringMatch(buffer, "foo")); + + SuccessOrQuit(StringCopy(buffer, nullptr, kStringCheckUtf8Encoding)); + VerifyOrQuit(StringMatch(buffer, "")); + + SuccessOrQuit(StringCopy(buffer, "", kStringCheckUtf8Encoding)); + VerifyOrQuit(StringMatch(buffer, "")); + + SuccessOrQuit(StringCopy(buffer, "123456789", kStringCheckUtf8Encoding)); + VerifyOrQuit(StringMatch(buffer, "123456789")); + + VerifyOrQuit(StringCopy(buffer, "1234567890") == kErrorInvalidArgs); + VerifyOrQuit(StringCopy(buffer, "1234567890abcdef") == kErrorInvalidArgs); + + SuccessOrQuit(StringCopy(smallBuffer, "", kStringCheckUtf8Encoding)); + VerifyOrQuit(StringMatch(smallBuffer, "")); + + VerifyOrQuit(StringCopy(smallBuffer, "a") == kErrorInvalidArgs); + + printf(" -- PASS\n"); +} + // gcc-4 does not support constexpr function #if __GNUC__ > 4 static_assert(ot::AreStringsInOrder("a", "b"), "AreStringsInOrder() failed"); @@ -389,6 +419,7 @@ int main(void) ot::TestStringMatch(); ot::TestStringToLowercase(); ot::TestStringParseUint8(); + ot::TestStringCopy(); printf("\nAll tests passed.\n"); return 0; }