Skip to content

Commit

Permalink
String values in YAML should test edge cases better. (#11687)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Jan 10, 2022
1 parent 7c23cd0 commit 8c5cacd
Show file tree
Hide file tree
Showing 10 changed files with 549 additions and 417 deletions.
16 changes: 7 additions & 9 deletions examples/chip-tool/commands/tests/TestCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 2 additions & 2 deletions examples/chip-tool/commands/tests/TestCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
);
Expand Down
2 changes: 1 addition & 1 deletion src/app/tests/suites/TV_TargetNavigatorCluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,4 @@ tests:
- name: "target"
value: 1
- name: "data"
value: 1
value: "1"
12 changes: 12 additions & 0 deletions src/app/tests/suites/TestCluster.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
13 changes: 13 additions & 0 deletions src/app/zap-templates/common/ClusterTestGeneration.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
//
Expand All @@ -593,3 +605,4 @@ exports.chip_tests_pics = chip_tests_pics;
exports.isTestOnlyCluster = isTestOnlyCluster;
exports.isLiteralNull = isLiteralNull;
exports.expectedValueHasProp = expectedValueHasProp;
exports.octetStringEscapedForCLiteral = octetStringEscapedForCLiteral;
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down Expand Up @@ -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}}";
Expand Down
Loading

0 comments on commit 8c5cacd

Please sign in to comment.