diff --git a/src/core/common/heap_data.cpp b/src/core/common/heap_data.cpp index 88b4ef905..d12686e2c 100644 --- a/src/core/common/heap_data.cpp +++ b/src/core/common/heap_data.cpp @@ -54,13 +54,19 @@ exit: Error Data::SetFrom(const Message &aMessage) { - Error error; - uint16_t length = aMessage.GetLength() - aMessage.GetOffset(); + return SetFrom(aMessage, aMessage.GetOffset(), aMessage.GetLength() - aMessage.GetOffset()); +} - SuccessOrExit(error = UpdateBuffer(length)); - VerifyOrExit(length != 0); +Error Data::SetFrom(const Message &aMessage, uint16_t aOffset, uint16_t aLength) +{ + Error error; - SuccessOrAssert(aMessage.Read(aMessage.GetOffset(), mData.GetBytes(), mData.GetLength())); + VerifyOrExit(aOffset + aLength <= aMessage.GetLength(), error = kErrorParse); + + SuccessOrExit(error = UpdateBuffer(aLength)); + VerifyOrExit(aLength != 0); + + SuccessOrAssert(aMessage.Read(aOffset, mData.GetBytes(), aLength)); exit: return error; diff --git a/src/core/common/heap_data.hpp b/src/core/common/heap_data.hpp index 2b41ab226..041f5397a 100644 --- a/src/core/common/heap_data.hpp +++ b/src/core/common/heap_data.hpp @@ -112,12 +112,11 @@ public: Error SetFrom(const uint8_t *aBuffer, uint16_t aLength); /** - * This method sets the `HeapData` from the content of a given message. + * This method sets the `Heap::Data` from the content of a given message. * * The bytes are copied from current offset in @p aMessage till the end of the message. * * @param[in] aMessage The message to copy bytes from (starting from offset till the end of message). - * @param[in] aLength The buffer length (number of bytes). * * @retval kErrorNone Successfully set the `Heap::Data`. * @retval kErrorNoBufs Failed to allocate buffer. @@ -125,6 +124,22 @@ public: */ Error SetFrom(const Message &aMessage); + /** + * This method sets the `Heap::Data` from the content of a given message read at a given offset up to a given + * length. + * + * @param[in] aMessage The message to read and copy bytes from. + * @param[in] aOffset The offset into the message to start reading the bytes from. + * @param[in] aLength The number of bytes to read from message. If @p aMessage contains fewer bytes than + * requested, then `Heap::Data` is cleared and this method returns `kErrorParse`. + * + * @retval kErrorNone Successfully set the `Heap::Data`. + * @retval kErrorNoBufs Failed to allocate buffer. + * @retval kErrorParse Not enough bytes in @p aMessage to read the requested @p aLength bytes. + * + */ + Error SetFrom(const Message &aMessage, uint16_t aOffset, uint16_t aLength); + /** * This method sets the `Heap::Data` from another one (move semantics). * diff --git a/src/core/net/srp_server.cpp b/src/core/net/srp_server.cpp index 190fb579c..c8a372017 100644 --- a/src/core/net/srp_server.cpp +++ b/src/core/net/srp_server.cpp @@ -979,7 +979,7 @@ Error Server::ProcessServiceDescriptionInstructions(Host & aHost, // (i.e., we saw both SRV and TXT record) or both are default // (cleared) value (i.e., we saw neither of them). - VerifyOrExit((desc.mPort == 0) == (desc.mTxtData == nullptr), error = kErrorFailed); + VerifyOrExit((desc.mPort == 0) == desc.mTxtData.IsNull(), error = kErrorFailed); } aMetadata.mOffset = offset; @@ -1620,7 +1620,7 @@ exit: void Server::Service::Description::Free(void) { mInstanceName.Free(); - Heap::Free(mTxtData); + mTxtData.Free(); Heap::Free(this); } @@ -1630,8 +1630,6 @@ Server::Service::Description::Description(Host &aHost) , mPriority(0) , mWeight(0) , mPort(0) - , mTxtLength(0) - , mTxtData(nullptr) , mLease(0) , mKeyLease(0) , mUpdateTime(TimerMilli::GetNow().GetDistantPast()) @@ -1641,20 +1639,12 @@ Server::Service::Description::Description(Host &aHost) void Server::Service::Description::ClearResources(void) { mPort = 0; - Heap::Free(mTxtData); - mTxtData = nullptr; - mTxtLength = 0; + mTxtData.Free(); } void Server::Service::Description::TakeResourcesFrom(Description &aDescription) { - // Take ownership and move the heap allocated `mTxtData` buffer - // from `aDescription - Heap::Free(mTxtData); - mTxtData = aDescription.mTxtData; - mTxtLength = aDescription.mTxtLength; - aDescription.mTxtData = nullptr; - aDescription.mTxtLength = 0; + mTxtData.SetFrom(static_cast(aDescription.mTxtData)); mPriority = aDescription.mPriority; mWeight = aDescription.mWeight; @@ -1667,23 +1657,16 @@ void Server::Service::Description::TakeResourcesFrom(Description &aDescription) Error Server::Service::Description::SetTxtDataFromMessage(const Message &aMessage, uint16_t aOffset, uint16_t aLength) { - Error error = kErrorNone; - uint8_t *txtData; + Error error; - txtData = static_cast(Heap::CAlloc(1, aLength)); - VerifyOrExit(txtData != nullptr, error = kErrorNoBufs); - - VerifyOrExit(aMessage.ReadBytes(aOffset, txtData, aLength) == aLength, error = kErrorParse); - VerifyOrExit(Dns::TxtRecord::VerifyTxtData(txtData, aLength, /* aAllowEmpty */ false), error = kErrorParse); - - Heap::Free(mTxtData); - mTxtData = txtData; - mTxtLength = aLength; + SuccessOrExit(error = mTxtData.SetFrom(aMessage, aOffset, aLength)); + VerifyOrExit(Dns::TxtRecord::VerifyTxtData(mTxtData.GetBytes(), mTxtData.GetLength(), /* aAllowEmpty */ false), + error = kErrorParse); exit: if (error != kErrorNone) { - Heap::Free(txtData); + mTxtData.Free(); } return error; diff --git a/src/core/net/srp_server.hpp b/src/core/net/srp_server.hpp index 296fa97b8..ae7bb76b6 100644 --- a/src/core/net/srp_server.hpp +++ b/src/core/net/srp_server.hpp @@ -57,6 +57,7 @@ #include "common/as_core_type.hpp" #include "common/clearable.hpp" #include "common/heap.hpp" +#include "common/heap_data.hpp" #include "common/heap_string.hpp" #include "common/linked_list.hpp" #include "common/locator.hpp" @@ -276,7 +277,7 @@ public: * @returns A pointer to the buffer containing the TXT record data. * */ - const uint8_t *GetTxtData(void) const { return mDescription.mTxtData; } + const uint8_t *GetTxtData(void) const { return mDescription.mTxtData.GetBytes(); } /** * This method returns the TXT record data length of the service instance. @@ -284,7 +285,7 @@ public: * @return The TXT record data length (number of bytes in buffer returned from `GetTxtData()`). * */ - uint16_t GetTxtDataLength(void) const { return mDescription.mTxtLength; } + uint16_t GetTxtDataLength(void) const { return mDescription.mTxtData.GetLength(); } /** * This method returns the host which the service instance reside on. @@ -351,11 +352,10 @@ public: Description *mNext; Heap::String mInstanceName; Host & mHost; + Heap::Data mTxtData; uint16_t mPriority; uint16_t mWeight; uint16_t mPort; - uint16_t mTxtLength; - uint8_t * mTxtData; uint32_t mLease; // The LEASE time in seconds. uint32_t mKeyLease; // The KEY-LEASE time in seconds. TimeMilli mUpdateTime; diff --git a/tests/unit/test_heap_string.cpp b/tests/unit/test_heap_string.cpp index 011cb2842..962adce4d 100644 --- a/tests/unit/test_heap_string.cpp +++ b/tests/unit/test_heap_string.cpp @@ -203,6 +203,7 @@ void TestHeapData(void) MessagePool * messagePool; Message * message; Heap::Data data; + uint16_t offset; const uint8_t *oldBuffer; static const uint8_t kData1[] = {10, 20, 3, 15, 100, 0, 60, 16}; @@ -261,6 +262,24 @@ void TestHeapData(void) SuccessOrQuit(data.SetFrom(*message)); VerifyData(data, kData3); + SuccessOrQuit(message->Append(kData4)); + + offset = 0; + SuccessOrQuit(data.SetFrom(*message, offset, sizeof(kData2))); + VerifyData(data, kData2); + + offset = sizeof(kData2); + SuccessOrQuit(data.SetFrom(*message, offset, sizeof(kData3))); + VerifyData(data, kData3); + + offset += sizeof(kData3); + SuccessOrQuit(data.SetFrom(*message, offset, sizeof(kData4))); + VerifyData(data, kData4); + + VerifyOrQuit(data.SetFrom(*message, offset, sizeof(kData4) + 1) == kErrorParse); + VerifyOrQuit(data.SetFrom(*message, 0, message->GetLength() + 1) == kErrorParse); + VerifyOrQuit(data.SetFrom(*message, 1, message->GetLength()) == kErrorParse); + printf("------------------------------------------------------------------------------------\n"); printf("Free()\n");