From 6f42abd3b5ea0d06a06e6e53b42d46856d2184ff Mon Sep 17 00:00:00 2001
From: Jerry Johns <johnsj@google.com>
Date: Wed, 8 Dec 2021 12:18:35 -0800
Subject: [PATCH] Fix mismatched expectations on index retrieval in Ember
 cluster lookup

AttributePathExpandIterator uses emberAfClusterIndex to look up the
index of a cluster within a given endpoint. This in turn calls into
emberAfFindClusterInTypeWithMfgCode to find that index. This returns an
index that indexes into all clusters in that endpoint regardless of
type.

Later on however, it uses emberAfGetNthClusterId to retrieve the actual
cluster index for a given index. However, this function applies the
index as though it's indexing into a list of clusters that match the
masking criteria (i.e server or client). This results in it retrieving
the wrong cluster in cases where there is a mix of client and server
clusters on a given endpoint.

Since most places use this 'scoped index', this PR fixes
emberAfFindClusterInTypeWithMfgCode to return a scoped index instead.

Problem:

Marc noted that when reading the WifiDiagnostics cluster, that on the
server, the logs indicated a retrieval of a ClusterId 0x36, but the
subsequent logs in the reporting engine seemed to indicate it was
picking up ClusterId 0x37 oddly enough.

Testing:

Validated that the above bug no longer happens, as well as reading out
the entire device using wildcards in the REPL to ensure everything still
works.
---
 src/app/util/attribute-storage.cpp | 25 +++++++++++++------------
 src/app/util/attribute-storage.h   | 11 +++++++++++
 2 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/src/app/util/attribute-storage.cpp b/src/app/util/attribute-storage.cpp
index 235d4f52b92ebc..12a0b795f1bdbe 100644
--- a/src/app/util/attribute-storage.cpp
+++ b/src/app/util/attribute-storage.cpp
@@ -643,32 +643,33 @@ EmberAfStatus emAfReadOrWriteAttribute(EmberAfAttributeSearchRecord * attRecord,
     return EMBER_ZCL_STATUS_UNSUPPORTED_ATTRIBUTE; // Sorry, attribute was not found.
 }
 
-// Check if a cluster is implemented or not. If yes, the cluster is returned.
-// If the cluster is not manufacturerSpecific [ClusterId < FC00] then
-// manufacturerCode argument is ignored otherwise checked.
-//
-// mask = 0 -> find either client or server
-// mask = CLUSTER_MASK_CLIENT -> find client
-// mask = CLUSTER_MASK_SERVER -> find server
 EmberAfCluster * emberAfFindClusterInTypeWithMfgCode(EmberAfEndpointType * endpointType, ClusterId clusterId,
                                                      EmberAfClusterMask mask, uint16_t manufacturerCode, uint8_t * index)
 {
     uint8_t i;
+    uint8_t scopedIndex = 0;
+
     for (i = 0; i < endpointType->clusterCount; i++)
     {
         EmberAfCluster * cluster = &(endpointType->cluster[i]);
-        if (cluster->clusterId == clusterId &&
-            (mask == 0 || (mask == CLUSTER_MASK_CLIENT && emberAfClusterIsClient(cluster)) ||
+
+        if ((mask == 0 || (mask == CLUSTER_MASK_CLIENT && emberAfClusterIsClient(cluster)) ||
              (mask == CLUSTER_MASK_SERVER && emberAfClusterIsServer(cluster))))
         {
-            if (index)
+            if (cluster->clusterId == clusterId)
             {
-                *index = i;
+                if (index)
+                {
+                    *index = scopedIndex;
+                }
+
+                return cluster;
             }
 
-            return cluster;
+            scopedIndex++;
         }
     }
+
     return NULL;
 }
 
diff --git a/src/app/util/attribute-storage.h b/src/app/util/attribute-storage.h
index ca2c4fbf21fdd6..873a4eeb437d6c 100644
--- a/src/app/util/attribute-storage.h
+++ b/src/app/util/attribute-storage.h
@@ -127,6 +127,17 @@ EmberAfStatus emAfReadOrWriteAttribute(EmberAfAttributeSearchRecord * attRecord,
 bool emAfMatchCluster(EmberAfCluster * cluster, EmberAfAttributeSearchRecord * attRecord);
 bool emAfMatchAttribute(EmberAfCluster * cluster, EmberAfAttributeMetadata * am, EmberAfAttributeSearchRecord * attRecord);
 
+// Check if a cluster is implemented or not. If yes, the cluster is returned.
+// If the cluster is not manufacturerSpecific [ClusterId < FC00] then
+// manufacturerCode argument is ignored otherwise checked.
+//
+// mask = 0 -> find either client or server
+// mask = CLUSTER_MASK_CLIENT -> find client
+// mask = CLUSTER_MASK_SERVER -> find server
+//
+// If a pointer to an index is provided, it will be updated to point to the relative index of the cluster
+// within the set of clusters that match the mask criteria.
+//
 EmberAfCluster * emberAfFindClusterInTypeWithMfgCode(EmberAfEndpointType * endpointType, chip::ClusterId clusterId,
                                                      EmberAfClusterMask mask, uint16_t manufacturerCode, uint8_t * index = nullptr);