From 79f3e4f201172b614bc26e0596c8170d69105ac5 Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Mon, 14 Feb 2022 10:30:32 -0500
Subject: [PATCH] Error out if large (not-exactly-representable) integers are
 used in YAML. (#15102)

Otherwise we get silent value corruption.
---
 src/app/tests/suites/TestCluster.yaml         |  4 +--
 .../common/ClusterTestGeneration.js           | 32 +++++++++++++++----
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/src/app/tests/suites/TestCluster.yaml b/src/app/tests/suites/TestCluster.yaml
index bd6e8af8e58db6..a7593bd53e5d1e 100644
--- a/src/app/tests/suites/TestCluster.yaml
+++ b/src/app/tests/suites/TestCluster.yaml
@@ -611,13 +611,13 @@ tests:
       command: "writeAttribute"
       attribute: "float_double"
       arguments:
-          value: 1.7e200
+          value: "1.7e+200"
 
     - label: "Read attribute DOUBLE large Value"
       command: "readAttribute"
       attribute: "float_double"
       response:
-          value: 1.7e200
+          value: "1.7e+200"
 
     - label: "Write attribute DOUBLE small Value"
       command: "writeAttribute"
diff --git a/src/app/zap-templates/common/ClusterTestGeneration.js b/src/app/zap-templates/common/ClusterTestGeneration.js
index 21b15f94dcffad..a44ad17be47c35 100644
--- a/src/app/zap-templates/common/ClusterTestGeneration.js
+++ b/src/app/zap-templates/common/ClusterTestGeneration.js
@@ -390,7 +390,7 @@ function parse(filename)
 
 function printErrorAndExit(context, msg)
 {
-  console.log(context.testName, ': ', context.label);
+  console.log("\nERROR:\n", context.testName, ': ', context.label);
   console.log(msg);
   process.exit(1);
 }
@@ -574,22 +574,26 @@ function chip_tests_config_get_type(name, options)
 // test_cluster_command_value and test_cluster_value-equals are recursive partials using #each. At some point the |global|
 // context is lost and it fails. Make sure to attach the global context as a property of the | value |
 // that is evaluated.
-function attachGlobal(global, value)
+//
+// errorContext should have "thisVal" and "name" properties that will be used
+// for error reporting via printErrorAndExit.
+function attachGlobal(global, value, errorContext)
 {
   if (Array.isArray(value)) {
-    value = value.map(v => attachGlobal(global, v));
+    value = value.map(v => attachGlobal(global, v, errorContext));
   } else if (value instanceof Object) {
     for (key in value) {
       if (key == "global") {
         continue;
       }
-      value[key] = attachGlobal(global, value[key]);
+      value[key] = attachGlobal(global, value[key], errorContext);
     }
   } else if (value === null) {
     value = new NullObject();
   } else {
     switch (typeof value) {
     case 'number':
+      checkNumberSanity(value, errorContext);
       value = new Number(value);
       break;
     case 'string':
@@ -607,6 +611,21 @@ function attachGlobal(global, value)
   return value;
 }
 
+/**
+ * Ensure the given value is not a possibly-corrupted-by-going-through-double
+ * integer.  If it is, tell the user (using that errorContext.name to describe
+ * it) and die.
+ */
+function checkNumberSanity(value, errorContext)
+{
+  // Number.isInteger is false for non-Numbers.
+  if (Number.isInteger(value) && !Number.isSafeInteger(value)) {
+    printErrorAndExit(errorContext.thisVal,
+        `${errorContext.name} value ${
+            value} is too large to represent exactly as an integer in YAML.  Put quotes around it to treat it as a string.\n\n`);
+  }
+}
+
 function chip_tests_item_parameters(options)
 {
   const commandValues = this.arguments.values;
@@ -634,7 +653,8 @@ function chip_tests_item_parameters(options)
             'Missing "' + commandArg.name + '" in arguments list: \n\t* '
                 + commandValues.map(command => command.name).join('\n\t* '));
       }
-      commandArg.definedValue = attachGlobal(this.global, expected.value);
+
+      commandArg.definedValue = attachGlobal(this.global, expected.value, { thisVal : this, name : commandArg.name });
 
       return commandArg;
     });
@@ -663,7 +683,7 @@ function chip_tests_item_response_parameters(options)
         const expected = responseValues.splice(expectedIndex, 1)[0];
         if ('value' in expected) {
           responseArg.hasExpectedValue = true;
-          responseArg.expectedValue    = attachGlobal(this.global, expected.value);
+          responseArg.expectedValue    = attachGlobal(this.global, expected.value, { thisVal : this, name : responseArg.name });
         }
 
         if ('constraints' in expected) {