From 0ffe586ae834d094585826b15741786105eafa35 Mon Sep 17 00:00:00 2001
From: Abtin Keshavarzian <abtink@google.com>
Date: Wed, 29 Apr 2020 11:01:48 -0700
Subject: [PATCH] [dataset-manager] adding DatasetTlv (a generic Dataset TLV to
 read from msg) (#4907)

---
 src/core/meshcop/dataset_manager.hpp     | 29 ++++++++++++
 src/core/meshcop/dataset_manager_ftd.cpp | 56 ++++++++++++------------
 2 files changed, 56 insertions(+), 29 deletions(-)

diff --git a/src/core/meshcop/dataset_manager.hpp b/src/core/meshcop/dataset_manager.hpp
index 88416c9db..0dbfcafd3 100644
--- a/src/core/meshcop/dataset_manager.hpp
+++ b/src/core/meshcop/dataset_manager.hpp
@@ -162,6 +162,35 @@ public:
                            const otIp6Address *                  aAddress) const;
 
 protected:
+    /**
+     * This class defines a generic Dataset TLV to read from a message.
+     *
+     */
+    OT_TOOL_PACKED_BEGIN
+    class DatasetTlv : public Tlv
+    {
+    public:
+        /**
+         * This method reads the Dataset TLV from a given message at a given offset.
+         *
+         * @param[in]  aMessage  A message to read the TLV from.
+         * @param[in]  aOffset   An offset into the message to read from.
+         *
+         * @retval OT_ERROR_NONE    The TLV was read successfully.
+         * @retval OT_ERROR_PARSE   The TLV was not well-formed and could not be parsed.
+         *
+         */
+        otError ReadFromMessage(const Message &aMessage, uint16_t aOffset);
+
+    private:
+        enum
+        {
+            kMaxValueSize = 16, // Maximum size of a Dataset TLV value (bytes).
+        };
+
+        uint8_t mValue[Dataset::kMaxValueSize];
+    } OT_TOOL_PACKED_END;
+
     /**
      * This constructor initializes the object.
      *
diff --git a/src/core/meshcop/dataset_manager_ftd.cpp b/src/core/meshcop/dataset_manager_ftd.cpp
index e0c9e18f3..13cb18fce 100644
--- a/src/core/meshcop/dataset_manager_ftd.cpp
+++ b/src/core/meshcop/dataset_manager_ftd.cpp
@@ -199,20 +199,11 @@ otError DatasetManager::HandleSet(Coap::Message &aMessage, const Ip6::MessageInf
 
         while (offset < aMessage.GetLength())
         {
-            OT_TOOL_PACKED_BEGIN
-            struct
-            {
-                Tlv     tlv;
-                uint8_t value[Dataset::kMaxValueSize];
-            } OT_TOOL_PACKED_END data;
+            DatasetTlv datasetTlv;
 
-            VerifyOrExit(aMessage.Read(offset, sizeof(Tlv), &data.tlv) == sizeof(Tlv), OT_NOOP);
-            VerifyOrExit(data.tlv.GetLength() <= sizeof(data.value), OT_NOOP);
+            SuccessOrExit(datasetTlv.ReadFromMessage(aMessage, offset));
 
-            VerifyOrExit(aMessage.Read(offset + sizeof(Tlv), data.tlv.GetLength(), data.value) == data.tlv.GetLength(),
-                         OT_NOOP);
-
-            switch (data.tlv.GetType())
+            switch (datasetTlv.GetType())
             {
             case Tlv::kCommissionerSessionId:
                 // do not store Commissioner Session ID TLV
@@ -220,26 +211,26 @@ otError DatasetManager::HandleSet(Coap::Message &aMessage, const Ip6::MessageInf
 
             case Tlv::kDelayTimer:
             {
-                DelayTimerTlv *delayTimerTlv = static_cast<DelayTimerTlv *>(&data.tlv);
+                DelayTimerTlv &delayTimerTlv = static_cast<DelayTimerTlv &>(static_cast<Tlv &>(datasetTlv));
 
-                if (doesAffectMasterKey && delayTimerTlv->GetDelayTimer() < DelayTimerTlv::kDelayTimerDefault)
+                if (doesAffectMasterKey && delayTimerTlv.GetDelayTimer() < DelayTimerTlv::kDelayTimerDefault)
                 {
-                    delayTimerTlv->SetDelayTimer(DelayTimerTlv::kDelayTimerDefault);
+                    delayTimerTlv.SetDelayTimer(DelayTimerTlv::kDelayTimerDefault);
                 }
-                else if (delayTimerTlv->GetDelayTimer() < Get<Leader>().GetDelayTimerMinimal())
+                else if (delayTimerTlv.GetDelayTimer() < Get<Leader>().GetDelayTimerMinimal())
                 {
-                    delayTimerTlv->SetDelayTimer(Get<Leader>().GetDelayTimerMinimal());
+                    delayTimerTlv.SetDelayTimer(Get<Leader>().GetDelayTimerMinimal());
                 }
             }
 
                 // fall through
 
             default:
-                SuccessOrExit(dataset.SetTlv(data.tlv));
+                SuccessOrExit(dataset.SetTlv(datasetTlv));
                 break;
             }
 
-            offset += sizeof(Tlv) + data.tlv.GetLength();
+            offset += static_cast<uint16_t>(datasetTlv.GetSize());
         }
 
         SuccessOrExit(Save(dataset));
@@ -304,6 +295,19 @@ exit:
     }
 }
 
+otError DatasetManager::DatasetTlv::ReadFromMessage(const Message &aMessage, uint16_t aOffset)
+{
+    otError error = OT_ERROR_NONE;
+
+    VerifyOrExit(aMessage.Read(aOffset, sizeof(Tlv), this) == sizeof(Tlv), error = OT_ERROR_PARSE);
+    VerifyOrExit(GetLength() <= kMaxValueSize, error = OT_ERROR_PARSE);
+    VerifyOrExit(aMessage.Read(aOffset + sizeof(Tlv), GetLength(), mValue) == GetLength(), error = OT_ERROR_PARSE);
+    VerifyOrExit(Tlv::IsValid(*this), error = OT_ERROR_PARSE);
+
+exit:
+    return error;
+}
+
 otError ActiveDataset::CreateNewNetwork(otOperationalDataset &aDataset)
 {
     otError          error             = OT_ERROR_NONE;
@@ -514,17 +518,11 @@ void PendingDataset::ApplyActiveDataset(const Timestamp &aTimestamp, Coap::Messa
 
     while (offset < aMessage.GetLength())
     {
-        OT_TOOL_PACKED_BEGIN
-        struct
-        {
-            Tlv     tlv;
-            uint8_t value[Dataset::kMaxValueSize];
-        } OT_TOOL_PACKED_END data;
+        DatasetTlv datasetTlv;
 
-        aMessage.Read(offset, sizeof(Tlv), &data.tlv);
-        aMessage.Read(offset + sizeof(Tlv), data.tlv.GetLength(), data.value);
-        dataset.SetTlv(data.tlv);
-        offset += sizeof(Tlv) + data.tlv.GetLength();
+        SuccessOrExit(datasetTlv.ReadFromMessage(aMessage, offset));
+        offset += static_cast<uint16_t>(datasetTlv.GetSize());
+        dataset.SetTlv(datasetTlv);
     }
 
     // add delay timer tlv