String values in YAML should test edge cases better. (#11687)

This adds garbage to the ends of all the strings and octet strings
being sent in yaml tests, and makes sure that garbage is not included
in the length, so the span is correct.  The idea is to catch cases
where someone uses a span's pointer without checking its length.

This also adds a way to embed a 0 byte in an octet string (by using
\x00 in the yaml string) and adds support for both sending and
checking for such values.

Author: bzbarsky-apple

examples/chip-tool/commands/tests/TestCommand.cpp

@@ -109,25 +109,23 @@ bool TestCommand::CheckConstraintMaxLength(const char * itemName, uint64_t curre
     return true;
 }
 
-bool TestCommand::CheckValueAsString(const char * itemName, const chip::ByteSpan current, const char * expected)
+bool TestCommand::CheckValueAsString(const char * itemName, chip::ByteSpan current, chip::ByteSpan expected)
 {
-    const chip::ByteSpan expectedArgument = chip::ByteSpan(chip::Uint8::from_const_char(expected), strlen(expected));
-
-    if (!current.data_equal(expectedArgument))
+    if (!current.data_equal(expected))
     {
-        Exit(std::string(itemName) + " value mismatch, expecting " + std::string(expected));
+        Exit(std::string(itemName) + " value mismatch, expecting " +
+             std::string(chip::Uint8::to_const_char(expected.data()), expected.size()));
         return false;
     }
 
     return true;
 }
 
-bool TestCommand::CheckValueAsString(const char * itemName, const chip::CharSpan current, const char * expected)
+bool TestCommand::CheckValueAsString(const char * itemName, chip::CharSpan current, chip::CharSpan expected)
 {
-    const chip::CharSpan expectedArgument(expected, strlen(expected));
-    if (!current.data_equal(expectedArgument))
+    if (!current.data_equal(expected))
     {
-        Exit(std::string(itemName) + " value mismatch, expected '" + expected + "' but got '" +
+        Exit(std::string(itemName) + " value mismatch, expected '" + std::string(expected.data(), expected.size()) + "' but got '" +
              std::string(current.data(), current.size()) + "'");
         return false;
     }

examples/chip-tool/commands/tests/TestCommand.h

@@ -171,9 +171,9 @@ class TestCommand : public CHIPCommand
         return false;
     }
 
-    bool CheckValueAsString(const char * itemName, chip::ByteSpan current, const char * expected);
+    bool CheckValueAsString(const char * itemName, chip::ByteSpan current, chip::ByteSpan expected);
 
-    bool CheckValueAsString(const char * itemName, chip::CharSpan current, const char * expected);
+    bool CheckValueAsString(const char * itemName, chip::CharSpan current, chip::CharSpan expected);
 
     template <typename T>
     bool CheckValuePresent(const char * itemName, const chip::Optional<T> & value)

examples/chip-tool/templates/partials/test_cluster_command_value.zapt

@@ -32,9 +32,9 @@
     {{#if_chip_enum type}}
       static_cast<{{zapTypeToEncodableClusterObjectType type ns=ns}}>({{definedValue}});
     {{else if (isCharString type)}}
-      chip::Span<const char>("{{definedValue}}", strlen("{{definedValue}}"));
+      chip::Span<const char>("{{definedValue}}garbage: not in length on purpose", {{definedValue.length}});
     {{else if (isOctetString type)}}
-      chip::ByteSpan(chip::Uint8::from_const_char("{{definedValue}}"), strlen("{{definedValue}}"));
+      chip::ByteSpan(chip::Uint8::from_const_char("{{octetStringEscapedForCLiteral definedValue}}garbage: not in length on purpose"), {{definedValue.length}});
     {{else}}
       {{#if_is_bitmap type}}
       static_cast<{{zapTypeToEncodableClusterObjectType type ns=ns}}>({{definedValue}});

examples/chip-tool/templates/partials/test_cluster_value_equals.zapt

@@ -29,7 +29,8 @@
         than "global") that are not present in the struct ? }}
   {{else}}
     VerifyOrReturn(CheckValue
-      {{~#if (isString type)}}AsString("{{label}}", {{actual}}, "{{expected}}")
+      {{~#if (isOctetString type)}}AsString("{{label}}", {{actual}}, chip::ByteSpan(chip::Uint8::from_const_char("{{octetStringEscapedForCLiteral expected}}"), {{expected.length}}))
+      {{else if (isCharString type)}}AsString("{{label}}", {{actual}}, chip::CharSpan("{{expected}}", {{expected.length}}))
       {{else}}<{{chipType}}>("{{label}}", {{actual}}, {{expected}}{{asTypeLiteralSuffix type}})
       {{/if}}
     );

src/app/tests/suites/TV_TargetNavigatorCluster.yaml

@@ -36,4 +36,4 @@ tests:
               - name: "target"
                 value: 1
               - name: "data"
-                value: 1
+                value: "1"

src/app/tests/suites/TestCluster.yaml

@@ -594,6 +594,18 @@ tests:
       response:
           value: ""
 
+    - label: "Write attribute OCTET_STRING with embedded null"
+      command: "writeAttribute"
+      attribute: "octet_string"
+      arguments:
+          value: "Tes\x00ti\x00ng"
+
+    - label: "Read attribute OCTET_STRING with embedded null"
+      command: "readAttribute"
+      attribute: "octet_string"
+      response:
+          value: "Tes\x00ti\x00ng"
+
     - label: "Write attribute OCTET_STRING"
       command: "writeAttribute"
       attribute: "octet_string"

src/app/zap-templates/common/ClusterTestGeneration.js

@@ -582,6 +582,18 @@ function expectedValueHasProp(value, name)
   return name in value;
 }
 
+function octetStringEscapedForCLiteral(value)
+{
+  return value.replace(/\p{Control}/gu, ch => {
+    let code = ch.charCodeAt(0);
+    code     = code.toString();
+    if (code.length == 1) {
+      code = "0" + code;
+    }
+    return "\\x" + code;
+  });
+}
+
 //
 // Module exports
 //
@@ -593,3 +605,4 @@ exports.chip_tests_pics                     = chip_tests_pics;
 exports.isTestOnlyCluster                   = isTestOnlyCluster;
 exports.isLiteralNull                       = isLiteralNull;
 exports.expectedValueHasProp                = expectedValueHasProp;
+exports.octetStringEscapedForCLiteral       = octetStringEscapedForCLiteral;

src/darwin/Framework/CHIP/templates/partials/test_cluster.zapt

@@ -21,8 +21,7 @@ bool testSendCluster{{parent.filename}}_{{asTestIndex index}}_{{asUpperCamelCase
     {{#chip_tests_item_parameters}}
     {{#if (isString type)}}
     {{#if (isOctetString type)}}
-    NSString * {{asLowerCamelCase name}}ArgumentString= @"{{definedValue}}";
-    NSData * {{asLowerCamelCase name}}Argument = [{{asLowerCamelCase name}}ArgumentString dataUsingEncoding:NSUTF8StringEncoding];
+    NSData * {{asLowerCamelCase name}}Argument = [[NSData alloc] initWithBytes:"{{octetStringEscapedForCLiteral definedValue}}" length:{{definedValue.length}}];
     {{else}}
     NSString * {{asLowerCamelCase name}}Argument= @"{{definedValue}}";
     {{/if}}
@@ -67,8 +66,7 @@ bool testSendCluster{{parent.filename}}_{{asTestIndex index}}_{{asUpperCamelCase
         {{else}}
         {{#if (isString type)}}
         {{#if (isOctetString type)}}
-        NSString * {{asLowerCamelCase name}}ArgumentString= @"{{expectedValue}}";
-        NSData * {{asLowerCamelCase name}}Argument = [{{asLowerCamelCase name}}ArgumentString dataUsingEncoding:NSUTF8StringEncoding];
+        NSData * {{asLowerCamelCase name}}Argument = [[NSData alloc] initWithBytes:"{{octetStringEscapedForCLiteral expectedValue}}" length:{{expectedValue.length}}];
         XCTAssertTrue([values[@"{{#if parent.isAttribute}}value{{else}}{{name}}{{/if}}"] isEqualToData:{{asLowerCamelCase name}}Argument]);
         {{else}}
         NSString * {{asLowerCamelCase name}}Argument= @"{{expectedValue}}";