Skip to content

Commit

Permalink
Error out if large (not-exactly-representable) integers are used in Y…
Browse files Browse the repository at this point in the history
…AML. (#15102)

Otherwise we get silent value corruption.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Feb 22, 2022
1 parent 2f535fa commit 79f3e4f
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 8 deletions.
4 changes: 2 additions & 2 deletions src/app/tests/suites/TestCluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
32 changes: 26 additions & 6 deletions src/app/zap-templates/common/ClusterTestGeneration.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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':
Expand All @@ -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;
Expand Down Expand Up @@ -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;
});
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 79f3e4f

Please sign in to comment.