[msg] Update message field order to match spec. (#11330)

project-chip · Dec 1, 2021 · 3937764 · 3937764
1 parent 2d6c868
commit 3937764
Show file tree

Hide file tree

Showing 2 changed files with 113 additions and 3 deletions.
diff --git a/src/transport/raw/MessageHeader.cpp b/src/transport/raw/MessageHeader.cpp
@@ -145,12 +145,12 @@ CHIP_ERROR PacketHeader::Decode(const uint8_t * const data, uint16_t size, uint1
     VerifyOrExit(version == kMsgHeaderVersion, err = CHIP_ERROR_VERSION_MISMATCH);
     SetMessageFlags(msgFlags);
 
+    SuccessOrExit(err = reader.Read16(&mSessionId).StatusCode());
+
     uint8_t securityFlags;
     SuccessOrExit(err = reader.Read8(&securityFlags).StatusCode());
     SetSecurityFlags(securityFlags);
 
-    SuccessOrExit(err = reader.Read16(&mSessionId).StatusCode());
-
     SuccessOrExit(err = reader.Read32(&mMessageCounter).StatusCode());
 
     if (mMsgFlags.Has(Header::MsgFlagValues::kSourceNodeIdPresent))
@@ -293,8 +293,8 @@ CHIP_ERROR PacketHeader::Encode(uint8_t * data, uint16_t size, uint16_t * encode
 
     uint8_t * p = data;
     Write8(p, msgFlags);
-    Write8(p, secFlags);
     LittleEndian::Write16(p, mSessionId);
+    Write8(p, secFlags);
     LittleEndian::Write32(p, mMessageCounter);
     if (mSourceNodeId.HasValue())
     {

diff --git a/src/transport/raw/tests/TestMessageHeader.cpp b/src/transport/raw/tests/TestMessageHeader.cpp
@@ -298,6 +298,114 @@ void TestPayloadHeaderEncodeDecodeBounds(nlTestSuite * inSuite, void * inContext
     }
 }
 
+struct SpecComplianceTestVector
+{
+    uint8_t encoded[8 + 8 + 8]; // Fixed header + max source id + max dest id
+    uint8_t messageFlags;
+    uint16_t sessionId;
+    uint8_t sessionType;
+    uint8_t securityFlags;
+    uint32_t messageCounter;
+
+    bool isSecure;
+    uint8_t size;
+
+    int groupId; // negative means no value
+};
+
+struct SpecComplianceTestVector theSpecComplianceTestVector[] = {
+    {
+        // Secure unicast message
+        .encoded        = { 0x00, 0x88, 0x77, 0x00, 0x44, 0x33, 0x22, 0x11 },
+        .messageFlags   = 0x00,
+        .sessionId      = 0x7788,
+        .sessionType    = 0x00,
+        .securityFlags  = 0x00,
+        .messageCounter = 0x11223344,
+        .isSecure       = true,
+        .size           = 8,
+
+        .groupId = -1,
+    },
+    {
+        // Secure group message
+        .encoded        = { 0x02, 0xEE, 0xDD, 0xC1, 0x40, 0x30, 0x20, 0x10, 0x56, 0x34 },
+        .messageFlags   = 0x02,
+        .sessionId      = 0xDDEE,
+        .sessionType    = 0x01,
+        .securityFlags  = 0xC1,
+        .messageCounter = 0x10203040,
+        .isSecure       = true,
+        .size           = 10,
+
+        .groupId = 0x3456,
+    },
+    {
+        // Unsecured message
+        .encoded        = { 0x00, 0x00, 0x00, 0x00, 0x40, 0x30, 0x20, 0x10 },
+        .messageFlags   = 0x00,
+        .sessionId      = 0x0000,
+        .sessionType    = 0x00,
+        .securityFlags  = 0x00,
+        .messageCounter = 0x10203040,
+        .isSecure       = false,
+        .size           = 8,
+
+        .groupId = -1,
+    },
+};
+
+const unsigned theSpecComplianceTestVectorLength = sizeof(theSpecComplianceTestVector) / sizeof(struct SpecComplianceTestVector);
+
+#define MAX_HEADER_SIZE (8 + 8 + 8)
+
+void TestSpecComplianceEncode(nlTestSuite * inSuite, void * inContext)
+{
+    struct SpecComplianceTestVector * testEntry;
+    uint8_t buffer[MAX_HEADER_SIZE];
+    uint16_t encodeSize;
+
+    for (unsigned i = 0; i < theSpecComplianceTestVectorLength; i++)
+    {
+        PacketHeader packetHeader;
+        testEntry = &theSpecComplianceTestVector[i];
+
+        packetHeader.SetMessageFlags(testEntry->messageFlags);
+        packetHeader.SetSecurityFlags(testEntry->securityFlags);
+        packetHeader.SetSessionId(testEntry->sessionId);
+        packetHeader.SetMessageCounter(testEntry->messageCounter);
+
+        if (testEntry->groupId >= 0)
+        {
+            packetHeader.SetDestinationGroupId(static_cast<GroupId>(testEntry->groupId));
+        }
+
+        NL_TEST_ASSERT(inSuite, packetHeader.Encode(buffer, sizeof(buffer), &encodeSize) == CHIP_NO_ERROR);
+        NL_TEST_ASSERT(inSuite, encodeSize == testEntry->size);
+        NL_TEST_ASSERT(inSuite, memcmp(buffer, testEntry->encoded, encodeSize) == 0);
+    }
+}
+
+void TestSpecComplianceDecode(nlTestSuite * inSuite, void * inContext)
+{
+    struct SpecComplianceTestVector * testEntry;
+    PacketHeader packetHeader;
+    uint16_t decodeSize;
+
+    for (unsigned i = 0; i < theSpecComplianceTestVectorLength; i++)
+    {
+        testEntry = &theSpecComplianceTestVector[i];
+
+        NL_TEST_ASSERT(inSuite, packetHeader.Decode(testEntry->encoded, testEntry->size, &decodeSize) == CHIP_NO_ERROR);
+        NL_TEST_ASSERT(inSuite, decodeSize == testEntry->size);
+        NL_TEST_ASSERT(inSuite, packetHeader.GetMessageFlags() == testEntry->messageFlags);
+        NL_TEST_ASSERT(inSuite, packetHeader.GetSecurityFlags() == testEntry->securityFlags);
+        NL_TEST_ASSERT(inSuite, packetHeader.GetSessionId() == testEntry->sessionId);
+        NL_TEST_ASSERT(inSuite, packetHeader.GetMessageCounter() == testEntry->messageCounter);
+        NL_TEST_ASSERT(inSuite, packetHeader.IsEncrypted() == testEntry->isSecure);
+    }
+}
+
 } // namespace
 
 // clang-format off
@@ -309,6 +417,8 @@ static const nlTest sTests[] =
     NL_TEST_DEF("PayloadEncodeDecode", TestPayloadHeaderEncodeDecode),
     NL_TEST_DEF("PacketEncodeDecodeBounds", TestPacketHeaderEncodeDecodeBounds),
     NL_TEST_DEF("PayloadEncodeDecodeBounds", TestPayloadHeaderEncodeDecodeBounds),
+    NL_TEST_DEF("SpecComplianceEncode", TestSpecComplianceEncode),
+    NL_TEST_DEF("SpecComplianceDecode", TestSpecComplianceDecode),
     NL_TEST_SENTINEL()
 };
 // clang-format on