From 967546f197c1552e9e845d802f0598213ad35a91 Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Thu, 17 Feb 2022 09:21:51 -0500
Subject: [PATCH] Fix ZAP variable lookups to not mis-treat non-strings as
 variable names. (#15276)

[nodeId] was being treated as a lookup of the nodeId varible and
ending up with a number, not an array containing a number.  Which
silently got treated as empty array!

Also adds sanity-checking so that "treat non-array as empty array"
silent thing does not happen anymore.
---
 .../partials/test_cluster_command_value.zapt     |  1 +
 .../partials/test_cluster_value_equals.zapt      |  1 +
 .../common/ClusterTestGeneration.js              | 16 ++++++++++++++++
 .../templates/partials/check_test_value.zapt     |  1 +
 .../CHIP/templates/partials/test_value.zapt      |  1 +
 5 files changed, 20 insertions(+)

diff --git a/examples/chip-tool/templates/partials/test_cluster_command_value.zapt b/examples/chip-tool/templates/partials/test_cluster_command_value.zapt
index 809ab075be2668..e5e7c2b0943c7c 100644
--- a/examples/chip-tool/templates/partials/test_cluster_command_value.zapt
+++ b/examples/chip-tool/templates/partials/test_cluster_command_value.zapt
@@ -10,6 +10,7 @@
   {{/if}}
 {{else if isArray}}
 
+  {{ensureIsArray definedValue~}}
   {{! forceNotList=true because we really want the type of a single item here.
       Similarly, forceNotOptional=true and forceNotNullable=true because we
       have accounted for those already. }}
diff --git a/examples/chip-tool/templates/partials/test_cluster_value_equals.zapt b/examples/chip-tool/templates/partials/test_cluster_value_equals.zapt
index 74ef700d7ad8ca..6d688c411018ed 100644
--- a/examples/chip-tool/templates/partials/test_cluster_value_equals.zapt
+++ b/examples/chip-tool/templates/partials/test_cluster_value_equals.zapt
@@ -21,6 +21,7 @@
     {{/if}}
   {{/if}}
 {{else if isArray}}
+    {{ensureIsArray expected~}}
     {
       auto iter_{{depth}} = {{actual}}.begin();
       {{#each expected}}
diff --git a/src/app/zap-templates/common/ClusterTestGeneration.js b/src/app/zap-templates/common/ClusterTestGeneration.js
index 603860e0bbe2fa..1bfe3b5de4fd26 100644
--- a/src/app/zap-templates/common/ClusterTestGeneration.js
+++ b/src/app/zap-templates/common/ClusterTestGeneration.js
@@ -518,6 +518,12 @@ function chip_tests_items(options)
 
 function getVariable(context, key, name)
 {
+  if (!(typeof name == "string" || (typeof name == "object" && (name instanceof String)))) {
+    // Non-string key; don't try to look it up.  Could end up looking like a
+    // variable name by accident when stringified.
+    return null;
+  }
+
   while (!('variables' in context) && context.parent) {
     context = context.parent;
   }
@@ -756,6 +762,15 @@ function if_include_struct_item_value(structValue, name, options)
   return options.inverse(this);
 }
 
+// To be used to verify that things are actually arrays before trying to use
+// #each with them, since that silently treats non-arrays as empty arrays.
+function ensureIsArray(value, options)
+{
+  if (!(value instanceof Array)) {
+    printErrorAndExit(this, `Expected array but instead got ${typeof value}: ${JSON.stringify(value)}\n`);
+  }
+}
+
 //
 // Module exports
 //
@@ -775,3 +790,4 @@ exports.isTestOnlyCluster                   = isTestOnlyCluster;
 exports.isLiteralNull                       = isLiteralNull;
 exports.octetStringEscapedForCLiteral       = octetStringEscapedForCLiteral;
 exports.if_include_struct_item_value        = if_include_struct_item_value;
+exports.ensureIsArray                       = ensureIsArray;
diff --git a/src/darwin/Framework/CHIP/templates/partials/check_test_value.zapt b/src/darwin/Framework/CHIP/templates/partials/check_test_value.zapt
index e8eb5ac4b59b40..cc74ea08e874d7 100644
--- a/src/darwin/Framework/CHIP/templates/partials/check_test_value.zapt
+++ b/src/darwin/Framework/CHIP/templates/partials/check_test_value.zapt
@@ -19,6 +19,7 @@
     {{/if}}
   {{/if}}
 {{else if isArray}}
+    {{ensureIsArray expected~}}
     XCTAssertEqual([{{actual}} count], {{expected.length}});
     {{#each expected}}
       {{>check_test_value actual=(concat ../actual "[" @index "]") expected=this cluster=../cluster isArray=false type=../type parent=../parent}}
diff --git a/src/darwin/Framework/CHIP/templates/partials/test_value.zapt b/src/darwin/Framework/CHIP/templates/partials/test_value.zapt
index ee35fe93db5f35..77438fd32fb7f3 100644
--- a/src/darwin/Framework/CHIP/templates/partials/test_value.zapt
+++ b/src/darwin/Framework/CHIP/templates/partials/test_value.zapt
@@ -11,6 +11,7 @@
     {{>test_value target=target definedValue=definedValue cluster=cluster isNullable=false depth=(incrementDepth depth)}}
   {{/if}}
 {{else if isArray}}
+  {{ensureIsArray definedValue~}}
   {
     NSMutableArray * temp_{{depth}} = [[NSMutableArray alloc] init];
     {{#each definedValue}}