Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SVE2] Octet strings escaped C style in Yaml cause test failure #21961

Closed
bluebin14 opened this issue Aug 17, 2022 · 7 comments · Fixed by #22125
Closed

[SVE2] Octet strings escaped C style in Yaml cause test failure #21961

bluebin14 opened this issue Aug 17, 2022 · 7 comments · Fixed by #22125

Comments

@bluebin14
Copy link
Contributor

Problem

Several Yaml's e.g. Test_TC_CADMIN_1_3.yaml declare octet strings escaped C style. This causes tast failure in all cases because raw binary data containing random control characters etc. is being passed on to chip-tool. Example:

    PakeVerifier:
        type: octet_string
        defaultValue:
          "\xb9\x61\x70\xaa\xe8\x03\x34\x68\x84\x72\x4f\xe9\xa3\xb2\x87\xc3\x03\x30\xc2\xa6\x60\x37\x5d\x17\xbb\x20\x5a\x8c\xf1\xae\xcb\x35\x04\x57\xf8\xab\x79\xee\x25\x3a\xb6\xa8\xe4\x6b\xb0\x9e\x54\x3a\xe4\x22\x73\x6d\xe5\x01\xe3\xdb\x37\xd4\x41\xfe\x34\x49\x20\xd0\x95\x48\xe4\xc1\x82\x40\x63\x0c\x4f\xf4\x91\x3c\x53\x51\x38\x39\xb7\xc0\x7f\xcc\x06\x27\xa1\xb8\x57\x3a\x14\x9f\xcd\x1f\xa4\x66\xcf"

The error in this case is:
- Test Step Error: Error occurred during execution of test case TC_CADMIN_1_3. No closing quotation

THFail

Proposed Solution

Use "hex:b96170aa..." syntax for expressing octet strings containing binary data in yaml.

@tcarmelveilleux
Copy link
Contributor

  • Current YAML test generator never intended the values from the YAML to be parsed by other tools and then put on the command line
  • Current YAML assumptions in the zzz_generated/chip-tool/zap-generated/test/Commands.h generated from examples/chip-tool/templates/tests/partials/octet_string_value.zapt uses the string from YAML as-is

The "hex:" is supported in YAML:

function isHexString(value)
{
  return value && value.startsWith(kHexPrefix); // kHexPrefix == "hex:"
}

Python TH only uses strings on the command line arguments. It assumes that all arguments can be put directly on the command line as-is.

If we do add hex: then it "likely" will work, but there's an overall normalization problem here

@vivien-apple
Copy link
Contributor

| Current YAML test generator never intended the values from the YAML to be parsed by other tools and then put on the command line

Well, current YAML is intended to be a common format for tests. I believe this is fine if other tools than the current generator are using them.

I also suspect that those test cases are only runs manually currently (aka they do not work in CI). And that instead of having to runs each step by hand there is a sort of script that parses them and converts them to python command (pure guess).

In any cases this is not a big deal to convert from C-style to hex: but in general I agree that having a defined common format that across our tool would makes life easier...

@bluebin14
Copy link
Contributor Author

@vivien-apple the test cases are run fully automated. Changing to hex: notation or overriding them in test_parameters with hex: values fixes the problem. So for now switching to hex: notation in all yaml would be a good start.

@bzbarsky-apple
Copy link
Contributor

That said, the C-style escape works in CI (where the YAML is converted to C++ code in chip-tool which is then run), right? That's because the conversion to C++ ensures that non-ASCII characters in YAML strings are properly escaped when creating C strings.

@bluebin14 what is the context in which this fails and how is the YAML being processed there?

@bluebin14
Copy link
Contributor Author

@bzbarsky-apple The simplest context is running the test using Test harness UI interface with default parameters e.g. 20202021/3840 without overriding anything and it fails as seen in the screenshot of this issue above due to binary (non ascii) characters passed on command line to chip-tool

@bzbarsky-apple
Copy link
Contributor

@bluebin14 Is there an issue tracking the test harness doing the wrong thing here? It really needs to not do that...

@sowmyassp
Copy link

@woody-apple This issue has not been solved we need to reopen this issue

It was tested with new image
TH image V 2.4
SDK SHA 8b6c0c0
Backend SHA d67ade00f92ea15db6d929580e03fbde40b35087
Frontend SHA 82c1c96eba39bc8cbe17b4b58f66dbddc40dfaa6

https://github.com/CHIP-Specifications/chip-test-plans/issues/2188

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants