Rotating device id representation in TLV Additional Data needs to be aligned with the spec

examples/chip-tool/commands/payload/AdditionalDataParseCommand.cpp

@@ -17,6 +17,7 @@
  */
 
 #include "AdditionalDataParseCommand.h"
+#include <math.h>
 #include <setup_payload/AdditionalDataPayload.h>
 #include <setup_payload/AdditionalDataPayloadParser.h>
 #include <string>
@@ -26,22 +27,16 @@ using namespace ::chip::SetupPayloadData;
 
 CHIP_ERROR AdditionalDataParseCommand::Run()
 {
-    std::vector<uint8_t> payloadData;
     AdditionalDataPayload resultPayload;
     CHIP_ERROR err = CHIP_NO_ERROR;
-    std::string payloadString(mPayload);
 
-    // Decode input payload
-    size_t len = payloadString.length();
+    size_t additionalDataPayloadBytesLength = static_cast<size_t>(ceil(strlen(mPayload) / 2));
+    std::unique_ptr<uint8_t[]> additionalDataPayloadBytes(new uint8_t[additionalDataPayloadBytesLength]);
 
-    for (size_t i = 0; i < len; i += 2)
-    {
-        auto str  = payloadString.substr(i, 2);
-        uint8_t x = (uint8_t) stoi(str, 0, 16);
-        payloadData.push_back(x);
-    }
+    size_t bufferSize =
+        chip::Encoding::HexToBytes(mPayload, strlen(mPayload), additionalDataPayloadBytes.get(), additionalDataPayloadBytesLength);
 
-    err = AdditionalDataPayloadParser(payloadData.data(), (uint32_t) payloadData.size()).populatePayload(resultPayload);
+    err = AdditionalDataPayloadParser(additionalDataPayloadBytes.get(), bufferSize).populatePayload(resultPayload);
     SuccessOrExit(err);
 
     ChipLogProgress(chipTool, "AdditionalDataParseCommand, RotatingDeviceId=%s", resultPayload.rotatingDeviceId.c_str());

src/app/server/Mdns.cpp

@@ -32,6 +32,7 @@
 #if CHIP_ENABLE_ROTATING_DEVICE_ID
 #include <setup_payload/AdditionalDataPayloadGenerator.h>
 #endif
+#include <system/TimeSource.h>
 #include <transport/FabricTable.h>
 
 #include <app/server/Server.h>

src/setup_payload/AdditionalDataPayloadGenerator.cpp

@@ -125,6 +125,7 @@ CHIP_ERROR AdditionalDataPayloadGenerator::generateRotatingDeviceIdAsHexString(
                                                           rotatingDeviceIdBufferTemp, ArraySize(rotatingDeviceIdBufferTemp),
                                                           rotatingDeviceIdBufferSizeTemp));
 
+    VerifyOrReturnError(rotatingDeviceIdBufferSize >= RotatingDeviceId::kHexMaxLength, CHIP_ERROR_BUFFER_TOO_SMALL);
     ReturnErrorOnFailure(BytesToUppercaseHexString(rotatingDeviceIdBufferTemp, rotatingDeviceIdBufferSizeTemp,
                                                    rotatingDeviceIdBuffer, rotatingDeviceIdBufferSize));
     rotatingDeviceIdValueOutputSize = rotatingDeviceIdBufferSizeTemp * 2;

src/setup_payload/AdditionalDataPayloadGenerator.h

@@ -98,6 +98,7 @@ class AdditionalDataPayloadGenerator
      * @param rotatingDeviceIdBuffer rotating device id buffer
      * @param rotatingDeviceIdBufferSize the current size of the supplied buffer
      * @param rotatingDeviceIdValueOutputSize the number of chars making up the actual value of the returned rotating device id
+     * excluding the null terminator
      *
      * @return Returns a CHIP_ERROR on error, CHIP_NO_ERROR otherwise.
      *

src/setup_payload/AdditionalDataPayloadParser.h

@@ -38,7 +38,7 @@ class AdditionalDataPayloadParser
 {
 private:
     const uint8_t * mPayloadBufferData;
-    const uint32_t mPayloadBufferLength;
+    const size_t mPayloadBufferLength;
 
 public:
     /**
@@ -49,7 +49,7 @@ class AdditionalDataPayloadParser
      *                                     it needs to outlive the lifetime of this parse.
      * @param[in] payloadBufferLength      The buffer data length for the additional data payload.
      */
-    AdditionalDataPayloadParser(const uint8_t * payloadBufferData, const uint32_t payloadBufferLength) :
+    AdditionalDataPayloadParser(const uint8_t * payloadBufferData, const size_t payloadBufferLength) :
         mPayloadBufferData(payloadBufferData), mPayloadBufferLength(payloadBufferLength)
     {}
 

src/setup_payload/tests/TestAdditionalDataPayload.cpp

@@ -22,6 +22,8 @@
  *
  */
 
+#include <math.h>
+#include <memory>
 #include <nlunit-test.h>
 #include <stdio.h>
 
@@ -49,6 +51,7 @@ constexpr char kAdditionalDataPayloadWithRotatingDeviceIdAndMaxLifetimeCounter[]
 constexpr char kRotatingDeviceId[]                                               = "0A001998AB7130E38B7E9A401CFE9F7B79AF";
 constexpr uint16_t kLifetimeCounter                                              = 10;
 constexpr uint16_t kAdditionalDataPayloadLength                                  = 51;
+constexpr uint16_t kShortRotatingIdLength                                        = 5;
 
 CHIP_ERROR GenerateAdditionalDataPayload(nlTestSuite * inSuite, uint16_t lifetimeCounter, const char * serialNumberBuffer,
                                          size_t serialNumberBufferSize, BitFlags<AdditionalDataFields> additionalDataFields,
@@ -77,23 +80,14 @@ CHIP_ERROR GenerateAdditionalDataPayload(nlTestSuite * inSuite, uint16_t lifetim
     return err;
 }
 
-CHIP_ERROR ParseAdditionalDataPayload(const char * additionalDataPayload,
+CHIP_ERROR ParseAdditionalDataPayload(const char * additionalDataPayload, size_t additionalDataPayloadLength,
                                       chip::SetupPayloadData::AdditionalDataPayload & outPayload)
 {
-    std::vector<uint8_t> payloadData;
-    std::string payloadString(additionalDataPayload);
-
-    // Decode input payload
-    size_t len = payloadString.length();
-
-    for (size_t i = 0; i < len; i += 2)
-    {
-        auto str  = payloadString.substr(i, 2);
-        uint8_t x = (uint8_t) stoi(str, 0, 16);
-        payloadData.push_back(x);
-    }
-
-    return AdditionalDataPayloadParser(payloadData.data(), (uint32_t) payloadData.size()).populatePayload(outPayload);
+    size_t additionalDataPayloadBytesLength = static_cast<size_t>(ceil(additionalDataPayloadLength / 2));
+    std::unique_ptr<uint8_t[]> additionalDataPayloadBytes(new uint8_t[additionalDataPayloadBytesLength]);
+    size_t bufferSize = chip::Encoding::HexToBytes(additionalDataPayload, additionalDataPayloadLength,
+                                                   additionalDataPayloadBytes.get(), additionalDataPayloadBytesLength);
+    return AdditionalDataPayloadParser(additionalDataPayloadBytes.get(), bufferSize).populatePayload(outPayload);
 }
 
 void TestGeneratingAdditionalDataPayloadWithRotatingDeviceId(nlTestSuite * inSuite, void * inContext)
@@ -175,35 +169,52 @@ void TestGeneratingRotatingDeviceIdAsStringWithNullInputs(nlTestSuite * inSuite,
     NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INVALID_ARGUMENT);
 }
 
+void TestGeneratingRotatingDeviceIdWithSmallBuffer(nlTestSuite * inSuite, void * inContext)
+{
+    CHIP_ERROR err = CHIP_NO_ERROR;
+    char rotatingDeviceIdHexBuffer[kShortRotatingIdLength];
+    size_t rotatingDeviceIdValueOutputSize = 0;
+    err                                    = AdditionalDataPayloadGenerator().generateRotatingDeviceIdAsHexString(
+        kLifetimeCounter, kSerialNumber, strlen(kSerialNumber), rotatingDeviceIdHexBuffer, ArraySize(rotatingDeviceIdHexBuffer),
+        rotatingDeviceIdValueOutputSize);
+    NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_BUFFER_TOO_SMALL);
+}
+
 void TestParsingAdditionalDataPayloadWithRotatingDeviceId(nlTestSuite * inSuite, void * inContext)
 {
     chip::SetupPayloadData::AdditionalDataPayload resultPayload;
-    NL_TEST_ASSERT(inSuite, ParseAdditionalDataPayload(kAdditionalDataPayloadWithRotatingDeviceId, resultPayload) == CHIP_NO_ERROR);
+    NL_TEST_ASSERT(inSuite,
+                   ParseAdditionalDataPayload(kAdditionalDataPayloadWithRotatingDeviceId,
+                                              strlen(kAdditionalDataPayloadWithRotatingDeviceId), resultPayload) == CHIP_NO_ERROR);
     NL_TEST_ASSERT(inSuite, strcmp(resultPayload.rotatingDeviceId.c_str(), kRotatingDeviceId) == 0);
 }
 
 void TestParsingAdditionalDataPayloadWithoutRotatingDeviceId(nlTestSuite * inSuite, void * inContext)
 {
     chip::SetupPayloadData::AdditionalDataPayload resultPayload;
     NL_TEST_ASSERT(inSuite,
-                   ParseAdditionalDataPayload(kAdditionalDataPayloadWithoutRotatingDeviceId, resultPayload) == CHIP_NO_ERROR);
+                   ParseAdditionalDataPayload(kAdditionalDataPayloadWithoutRotatingDeviceId,
+                                              strlen(kAdditionalDataPayloadWithoutRotatingDeviceId),
+                                              resultPayload) == CHIP_NO_ERROR);
     NL_TEST_ASSERT(inSuite, strcmp(resultPayload.rotatingDeviceId.c_str(), "") == 0);
 }
 
 void TestParsingAdditionalDataPayloadWithInvalidRotatingDeviceIdLength(nlTestSuite * inSuite, void * inContext)
 {
     chip::SetupPayloadData::AdditionalDataPayload resultPayload;
     NL_TEST_ASSERT(inSuite,
-                   ParseAdditionalDataPayload(kAdditionalDataPayloadWithInvalidRotatingDeviceIdLength, resultPayload) ==
-                       CHIP_ERROR_TLV_UNDERRUN);
+                   ParseAdditionalDataPayload(kAdditionalDataPayloadWithInvalidRotatingDeviceIdLength,
+                                              strlen(kAdditionalDataPayloadWithInvalidRotatingDeviceIdLength),
+                                              resultPayload) == CHIP_ERROR_TLV_UNDERRUN);
 }
 
 void TestParsingAdditionalDataPayloadWithLongRotatingDeviceId(nlTestSuite * inSuite, void * inContext)
 {
     chip::SetupPayloadData::AdditionalDataPayload resultPayload;
     NL_TEST_ASSERT(inSuite,
-                   ParseAdditionalDataPayload(kAdditionalDataPayloadWithLongRotatingDeviceId, resultPayload) ==
-                       CHIP_ERROR_INVALID_STRING_LENGTH);
+                   ParseAdditionalDataPayload(kAdditionalDataPayloadWithLongRotatingDeviceId,
+                                              strlen(kAdditionalDataPayloadWithLongRotatingDeviceId),
+                                              resultPayload) == CHIP_ERROR_INVALID_STRING_LENGTH);
 }
 
 /**
@@ -218,6 +229,7 @@ const nlTest sTests[] =
     NL_TEST_DEF("Test Generating Additional Data Payload without Rotatin gDevice Id", TestGeneratingAdditionalDataPayloadWithoutRotatingDeviceId),
     NL_TEST_DEF("Test Generating Rotating Device Id as string", TestGeneratingRotatingDeviceIdAsString),
     NL_TEST_DEF("Test Generating Rotating Device Id as string with null/invalid inputs", TestGeneratingRotatingDeviceIdAsStringWithNullInputs),
+    NL_TEST_DEF("Test Generating Rotating Device Id as string with small buffer", TestGeneratingRotatingDeviceIdWithSmallBuffer),
     NL_TEST_DEF("Test Parsing Additional Data Payload with Rotating Device Id", TestParsingAdditionalDataPayloadWithRotatingDeviceId),
     NL_TEST_DEF("Test Parsing Additional Data Payload without Rotating Device Id", TestParsingAdditionalDataPayloadWithoutRotatingDeviceId),
     NL_TEST_DEF("Test Parsing Additional Data Payload with Invalid Rotating Device Id Length", TestParsingAdditionalDataPayloadWithInvalidRotatingDeviceIdLength),