[spinel] add spinel property codec (openthread#10934)

This commit adds spinel_prop_codec module to lib/spinel to provide
encoding & decoding functions for complex spinel properties.

This commit moves the encoding of `SPINEL_PROP_DNSSD_HOST`,
`SPINEL_PROP_DNSSD_SERVICE` and `SPINEL_PROP_DNSSD_KEY_RECORD` from
NcpBase to the lib and adds the decoding functions. The background is
that I found the encoding & decoding of complex spinel properties are
error-prone. However the encoding and decoding of one property are
usually put in different places. For example, encoding is in
openthread ncp while decoding is in ot-br-posix ncp spinel. It's
difficult to debug the decoding in ot-br-posix and there is no unit
tests for the encoding & decoding.

This commit puts the encoding & decoding together and adds unit tests
to ensure they are correct.

src/lib/spinel/CMakeLists.txt

@@ -73,6 +73,7 @@ set(COMMON_SOURCES
     spinel_decoder.cpp
     spinel_encoder.cpp
     spinel_helper.cpp
+    spinel_prop_codec.cpp
 )
 
 set(OT_SPINEL_VENDOR_HOOK_SOURCE "" CACHE STRING "set vendor hook source file for Spinel")

src/lib/spinel/spinel.h

@@ -4859,7 +4859,7 @@ enum
      * `S`       : The service port number.
      * `S`       : The service priority.
      * `S`       : The service weight.
-     * `S`       : The service TTL in seconds.
+     * `L`       : The service TTL in seconds.
      * `L`       : The Dnssd Request ID.
      * `D`       : The context of the request. (A pointer to the callback for the request)
      *
@@ -4875,7 +4875,7 @@ enum
      * `t(U)` : The service type if key is for a service (does not include domain name).
      * `d`    : Byte array containing the key record data.
      * `S`    : The resource record class.
-     * `S`    : The TTL in seconds.
+     * `L`    : The TTL in seconds.
      * `L`    : The Dnssd Request ID.
      * `D`    : The context of the request. (A pointer to the callback for the request)
      *

src/lib/spinel/spinel_prop_codec.cpp

@@ -0,0 +1,200 @@
+/*
+ *  Copyright (c) 2024, The OpenThread Authors.
+ *  All rights reserved.
+ *
+ *  Redistribution and use in source and binary forms, with or without
+ *  modification, are permitted provided that the following conditions are met:
+ *  1. Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ *  2. Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in the
+ *     documentation and/or other materials provided with the distribution.
+ *  3. Neither the name of the copyright holder nor the
+ *     names of its contributors may be used to endorse or promote products
+ *     derived from this software without specific prior written permission.
+ *
+ *  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ *  AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ *  IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ *  ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ *  LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ *  CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ *  SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ *  INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ *  CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ *  ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ *  POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/**
+ * @file This file implements spinel property encoding and decoding functions.
+ */
+
+#include "lib/spinel/spinel_prop_codec.hpp"
+#include "lib/utils/utils.hpp"
+
+namespace ot {
+namespace Spinel {
+
+template <>
+otError EncodeDnssd<otPlatDnssdHost>(Encoder                    &aEncoder,
+                                     const otPlatDnssdHost      &aObj,
+                                     otPlatDnssdRequestId        aRequestId,
+                                     otPlatDnssdRegisterCallback aCallback)
+{
+    otError error = OT_ERROR_NONE;
+
+    EXPECT_NO_ERROR(error = aEncoder.WriteUintPacked(SPINEL_PROP_DNSSD_HOST));
+    EXPECT_NO_ERROR(error = aEncoder.WriteUtf8(aObj.mHostName == nullptr ? "" : aObj.mHostName));
+    EXPECT_NO_ERROR(error = aEncoder.WriteUint16(aObj.mAddressesLength));
+    for (uint16_t i = 0; i < aObj.mAddressesLength; i++)
+    {
+        EXPECT_NO_ERROR(error = aEncoder.WriteIp6Address(aObj.mAddresses[i]));
+    }
+    EXPECT_NO_ERROR(error = aEncoder.WriteUint32(aRequestId));
+    EXPECT_NO_ERROR(error = aEncoder.WriteData(reinterpret_cast<const uint8_t *>(&aCallback), sizeof(aCallback)));
+
+exit:
+    return error;
+}
+
+template <>
+otError EncodeDnssd<otPlatDnssdService>(Encoder                    &aEncoder,
+                                        const otPlatDnssdService   &aObj,
+                                        otPlatDnssdRequestId        aRequestId,
+                                        otPlatDnssdRegisterCallback aCallback)
+{
+    otError error = OT_ERROR_NONE;
+
+    EXPECT_NO_ERROR(error = aEncoder.WriteUintPacked(SPINEL_PROP_DNSSD_SERVICE));
+    EXPECT_NO_ERROR(error = aEncoder.WriteUtf8(aObj.mHostName == nullptr ? "" : aObj.mHostName));
+    EXPECT_NO_ERROR(error = aEncoder.WriteUtf8(aObj.mServiceInstance == nullptr ? "" : aObj.mServiceInstance));
+    EXPECT_NO_ERROR(error = aEncoder.WriteUtf8(aObj.mServiceType == nullptr ? "" : aObj.mServiceType));
+    EXPECT_NO_ERROR(error = aEncoder.OpenStruct());
+    for (uint16_t i = 0; i < aObj.mSubTypeLabelsLength; i++)
+    {
+        EXPECT_NO_ERROR(error = aEncoder.WriteUtf8(aObj.mSubTypeLabels[i]));
+    }
+    EXPECT_NO_ERROR(error = aEncoder.CloseStruct());
+    EXPECT_NO_ERROR(error = aEncoder.WriteDataWithLen(aObj.mTxtData, aObj.mTxtDataLength));
+    EXPECT_NO_ERROR(error = aEncoder.WriteUint16(aObj.mPort));
+    EXPECT_NO_ERROR(error = aEncoder.WriteUint16(aObj.mPriority));
+    EXPECT_NO_ERROR(error = aEncoder.WriteUint16(aObj.mWeight));
+    EXPECT_NO_ERROR(error = aEncoder.WriteUint32(aObj.mTtl));
+    EXPECT_NO_ERROR(error = aEncoder.WriteUint32(aRequestId));
+    EXPECT_NO_ERROR(error = aEncoder.WriteData(reinterpret_cast<const uint8_t *>(&aCallback), sizeof(aCallback)));
+
+exit:
+    return error;
+}
+
+template <>
+otError EncodeDnssd<otPlatDnssdKey>(Encoder                    &aEncoder,
+                                    const otPlatDnssdKey       &aObj,
+                                    otPlatDnssdRequestId        aRequestId,
+                                    otPlatDnssdRegisterCallback aCallback)
+{
+    otError error = OT_ERROR_NONE;
+
+    EXPECT_NO_ERROR(error = aEncoder.WriteUintPacked(SPINEL_PROP_DNSSD_KEY_RECORD));
+    EXPECT_NO_ERROR(error = aEncoder.WriteUtf8(aObj.mName == nullptr ? "" : aObj.mName));
+    EXPECT_NO_ERROR(error = aEncoder.OpenStruct());
+    if (aObj.mServiceType != nullptr)
+    {
+        EXPECT_NO_ERROR(error = aEncoder.WriteUtf8(aObj.mServiceType));
+    }
+    EXPECT_NO_ERROR(error = aEncoder.CloseStruct());
+    EXPECT_NO_ERROR(error = aEncoder.WriteDataWithLen(aObj.mKeyData, aObj.mKeyDataLength));
+    EXPECT_NO_ERROR(error = aEncoder.WriteUint16(aObj.mClass));
+    EXPECT_NO_ERROR(error = aEncoder.WriteUint32(aObj.mTtl));
+    EXPECT_NO_ERROR(error = aEncoder.WriteUint32(aRequestId));
+    EXPECT_NO_ERROR(error = aEncoder.WriteData(reinterpret_cast<const uint8_t *>(&aCallback), sizeof(aCallback)));
+
+exit:
+    return error;
+}
+
+otError DecodeDnssdHost(Decoder              &aDecoder,
+                        otPlatDnssdHost      &aHost,
+                        otPlatDnssdRequestId &aRequestId,
+                        const uint8_t       *&aCallbackData,
+                        uint16_t             &aCallbackDataLen)
+{
+    otError error = OT_ERROR_NONE;
+
+    EXPECT_NO_ERROR(error = aDecoder.ReadUtf8(aHost.mHostName));
+    EXPECT_NO_ERROR(error = aDecoder.ReadUint16(aHost.mAddressesLength));
+    EXPECT_NO_ERROR(error = aDecoder.ReadIp6Address(aHost.mAddresses));
+    EXPECT_NO_ERROR(error = aDecoder.ReadUint32(aRequestId));
+    EXPECT_NO_ERROR(error = aDecoder.ReadData(aCallbackData, aCallbackDataLen));
+
+exit:
+    return error;
+}
+
+otError DecodeDnssdService(Decoder              &aDecoder,
+                           otPlatDnssdService   &aService,
+                           const char          **aSubTypeLabels,
+                           uint16_t             &aSubTypeLabelsCount,
+                           otPlatDnssdRequestId &aRequestId,
+                           const uint8_t       *&aCallbackData,
+                           uint16_t             &aCallbackDataLen)
+{
+    otError error = OT_ERROR_NONE;
+    uint8_t index = 0;
+
+    EXPECT_NO_ERROR(error = aDecoder.ReadUtf8(aService.mHostName));
+    EXPECT_NO_ERROR(error = aDecoder.ReadUtf8(aService.mServiceInstance));
+    EXPECT_NO_ERROR(error = aDecoder.ReadUtf8(aService.mServiceType));
+    EXPECT_NO_ERROR(error = aDecoder.OpenStruct());
+    while (!aDecoder.IsAllReadInStruct())
+    {
+        EXPECT(index < aSubTypeLabelsCount, error = OT_ERROR_NO_BUFS);
+        EXPECT_NO_ERROR(error = aDecoder.ReadUtf8(aSubTypeLabels[index]));
+        index++;
+    }
+    aSubTypeLabelsCount = index;
+    EXPECT_NO_ERROR(error = aDecoder.CloseStruct());
+    EXPECT_NO_ERROR(error = aDecoder.ReadDataWithLen(aService.mTxtData, aService.mTxtDataLength));
+    EXPECT_NO_ERROR(error = aDecoder.ReadUint16(aService.mPort));
+    EXPECT_NO_ERROR(error = aDecoder.ReadUint16(aService.mPriority));
+    EXPECT_NO_ERROR(error = aDecoder.ReadUint16(aService.mWeight));
+    EXPECT_NO_ERROR(error = aDecoder.ReadUint32(aService.mTtl));
+    EXPECT_NO_ERROR(error = aDecoder.ReadUint32(aRequestId));
+    EXPECT_NO_ERROR(error = aDecoder.ReadData(aCallbackData, aCallbackDataLen));
+
+exit:
+    return error;
+}
+
+otError DecodeDnssdKey(Decoder              &aDecoder,
+                       otPlatDnssdKey       &aKey,
+                       otPlatDnssdRequestId &aRequestId,
+                       const uint8_t       *&aCallbackData,
+                       uint16_t             &aCallbackDataLen)
+{
+    otError error = OT_ERROR_NONE;
+
+    EXPECT_NO_ERROR(error = aDecoder.ReadUtf8(aKey.mName));
+    EXPECT_NO_ERROR(error = aDecoder.OpenStruct());
+    if (!aDecoder.IsAllReadInStruct())
+    {
+        EXPECT_NO_ERROR(error = aDecoder.ReadUtf8(aKey.mServiceType));
+    }
+    else
+    {
+        aKey.mServiceType = nullptr;
+    }
+    EXPECT_NO_ERROR(error = aDecoder.CloseStruct());
+    EXPECT_NO_ERROR(error = aDecoder.ReadDataWithLen(aKey.mKeyData, aKey.mKeyDataLength));
+    EXPECT_NO_ERROR(error = aDecoder.ReadUint16(aKey.mClass));
+    EXPECT_NO_ERROR(error = aDecoder.ReadUint32(aKey.mTtl));
+    EXPECT_NO_ERROR(error = aDecoder.ReadUint32(aRequestId));
+    EXPECT_NO_ERROR(error = aDecoder.ReadData(aCallbackData, aCallbackDataLen));
+
+exit:
+    return error;
+}
+
+} // namespace Spinel
+} // namespace ot

src/lib/spinel/spinel_prop_codec.hpp

@@ -0,0 +1,119 @@
+/*
+ *  Copyright (c) 2024, The OpenThread Authors.
+ *  All rights reserved.
+ *
+ *  Redistribution and use in source and binary forms, with or without
+ *  modification, are permitted provided that the following conditions are met:
+ *  1. Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ *  2. Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in the
+ *     documentation and/or other materials provided with the distribution.
+ *  3. Neither the name of the copyright holder nor the
+ *     names of its contributors may be used to endorse or promote products
+ *     derived from this software without specific prior written permission.
+ *
+ *  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ *  AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ *  IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ *  ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ *  LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ *  CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ *  SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ *  INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ *  CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ *  ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ *  POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/**
+ * @file This file includes definitions for spinel property encoding and decoding functions.
+ */
+
+#ifndef SPINEL_PROP_CODEC_HPP_
+#define SPINEL_PROP_CODEC_HPP_
+
+#include <openthread/platform/dnssd.h>
+
+#include "lib/spinel/spinel.h"
+#include "lib/spinel/spinel_decoder.hpp"
+#include "lib/spinel/spinel_encoder.hpp"
+
+namespace ot {
+namespace Spinel {
+
+/**
+ * Use Spinel::Encoder to encode a Dnssd object.
+ *
+ * A Spinel header and command MUST have been encoded by the encoder.
+ *
+ * @param[in] aEncoder    A reference to the encoder object.
+ * @param[in] aObj        A reference to the dnssd object. (otPlatDnssdHost, otPlatDnssdService or otPlatDnssdKey).
+ * @param[in] aRequestId  The request id.
+ * @param[in] aCallback   A callback to receive the dnssd update result.
+ */
+template <typename DnssdObjType>
+otError EncodeDnssd(Encoder                    &aEncoder,
+                    const DnssdObjType         &aObj,
+                    otPlatDnssdRequestId        aRequestId,
+                    otPlatDnssdRegisterCallback aCallback);
+
+/**
+ * Use Spinel::Decoder to decode a SPINEL_PROP_DNSSD_HOST message to a otPlatDnssdHost.
+ *
+ * The decoder MUST have read the header, command and property key of the frame.
+ *
+ * @param[in]  aDecoder      A reference to the decoder object.
+ * @param[out] aHost         A reference to the dnssd host.
+ * @param[out] aRequestId    A reference to the request id.
+ * @param[out] aCallback     A reference to the pointer to the callback data.
+ * @param[out] aCallbackLen  A reference to the callback data length.
+ */
+otError DecodeDnssdHost(Decoder              &aDecoder,
+                        otPlatDnssdHost      &aHost,
+                        otPlatDnssdRequestId &aRequestId,
+                        const uint8_t       *&aCallbackData,
+                        uint16_t             &aCallbackDataLen);
+
+/**
+ * Use Spinel::Decoder to decode a SPINEL_PROP_DNSSD_SERVICE message to a otPlatDnssdService.
+ *
+ * The decoder MUST have read the header, command and property key of the frame.
+ *
+ * @param[in]  aDecoder        A reference to the decoder object.
+ * @param[out] aService        A reference to the dnssd service.
+ * @param[out] aRequestId      A reference to the request id.
+ * @param[out] aSubTypeLabels  A pointer to the array of sub-type labels.
+ * @param[out] aSubTypeCount   The number of sub-type labels.
+ * @param[out] aCallback       A reference to the pointer to the callback data.
+ * @param[out] aCallbackLen    A reference to the callback data length.
+ */
+otError DecodeDnssdService(Decoder              &aDecoder,
+                           otPlatDnssdService   &aService,
+                           const char          **aSubTypeLabels,
+                           uint16_t             &aSubTypeLabelsCount,
+                           otPlatDnssdRequestId &aRequestId,
+                           const uint8_t       *&aCallbackData,
+                           uint16_t             &aCallbackDataLen);
+
+/**
+ * Use Spinel::Decoder to decode a SPINEL_PROP_DNSSD_KEY_RECORD message to a otPlatDnssdKey.
+ *
+ * The decoder MUST have read the header, command and property key of the frame.
+ *
+ * @param[in]  aDecoder      A reference to the decoder object.
+ * @param[out] aKey          A reference to the dnssd key.
+ * @param[out] aRequestId    A reference to the request id.
+ * @param[out] aCallback     A reference to the pointer to the callback data.
+ * @param[out] aCallbackLen  A reference to the callback data length.
+ */
+otError DecodeDnssdKey(Decoder              &aDecoder,
+                       otPlatDnssdKey       &aKey,
+                       otPlatDnssdRequestId &aRequestId,
+                       const uint8_t       *&aCallbackData,
+                       uint16_t             &aCallbackDataLen);
+
+} // namespace Spinel
+} // namespace ot
+
+#endif // SPINEL_PROP_CODEC_HPP_

src/ncp/ncp_base.hpp

@@ -62,6 +62,7 @@
 #include "lib/spinel/spinel_buffer.hpp"
 #include "lib/spinel/spinel_decoder.hpp"
 #include "lib/spinel/spinel_encoder.hpp"
+#include "lib/spinel/spinel_prop_codec.hpp"
 
 #if OPENTHREAD_CONFIG_MULTIPAN_RCP_ENABLE
 #define SPINEL_HEADER_IID_BROADCAST OPENTHREAD_SPINEL_CONFIG_BROADCAST_IID
@@ -849,7 +850,7 @@ class NcpBase
 #if OPENTHREAD_CONFIG_NCP_DNSSD_ENABLE && OPENTHREAD_CONFIG_PLATFORM_DNSSD_ENABLE
 
     template <typename DnssdObjType>
-    void DnssdUpdate(const DnssdObjType         *Obj,
+    void DnssdUpdate(const DnssdObjType         *aObj,
                      otPlatDnssdRequestId        aRequestId,
                      otPlatDnssdRegisterCallback aCallback,
                      bool                        aRegister)
@@ -858,12 +859,11 @@ class NcpBase
         uint8_t          header = SPINEL_HEADER_FLAG | SPINEL_HEADER_TX_NOTIFICATION_IID;
         spinel_command_t cmd    = aRegister ? SPINEL_CMD_PROP_VALUE_INSERTED : SPINEL_CMD_PROP_VALUE_REMOVED;
 
+        VerifyOrExit(aObj != nullptr, error = OT_ERROR_INVALID_ARGS);
         VerifyOrExit(mDnssdState == OT_PLAT_DNSSD_READY, error = OT_ERROR_INVALID_STATE);
 
         SuccessOrExit(error = mEncoder.BeginFrame(header, cmd));
-        SuccessOrExit(error = EncodeDnssd(Obj));
-        SuccessOrExit(error = mEncoder.WriteUint32(aRequestId));
-        SuccessOrExit(error = mEncoder.WriteData(reinterpret_cast<const uint8_t *>(&aCallback), sizeof(aCallback)));
+        SuccessOrExit(error = Spinel::EncodeDnssd(mEncoder, *aObj, aRequestId, aCallback));
         SuccessOrExit(error = mEncoder.EndFrame());
 
     exit:
@@ -873,11 +873,9 @@ class NcpBase
         }
     }
 
-    template <typename DnssdObjType> otError EncodeDnssd(const DnssdObjType *aObj);
-
     otPlatDnssdState mDnssdState;
-#endif
-#endif
+#endif // OPENTHREAD_CONFIG_NCP_DNSSD_ENABLE && OPENTHREAD_CONFIG_PLATFORM_DNSSD_ENABLE
+#endif // OPENTHREAD_FTD
 
 #if OPENTHREAD_CONFIG_DIAG_ENABLE
     char    *mDiagOutput;