From ef22e3bff04ff6e00daf70e63d9eba1f105fac98 Mon Sep 17 00:00:00 2001
From: Martin Turon <mturon@google.com>
Date: Tue, 2 Nov 2021 10:41:15 -0700
Subject: [PATCH] [msg] Update message field order to match spec.

---
 src/transport/raw/MessageHeader.cpp           |   6 +-
 src/transport/raw/tests/TestMessageHeader.cpp | 108 ++++++++++++++++++
 2 files changed, 111 insertions(+), 3 deletions(-)

diff --git a/src/transport/raw/MessageHeader.cpp b/src/transport/raw/MessageHeader.cpp
index b31b27da51d6d3..5ce789b6cef55b 100644
--- 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
index 74ed1bd22068d5..f3def81d8b6f02 100644
--- a/src/transport/raw/tests/TestMessageHeader.cpp
+++ b/src/transport/raw/tests/TestMessageHeader.cpp
@@ -298,6 +298,112 @@ 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 +415,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