chip-tool: do not enforce triple quoting of custom arguments #19529
Assignees
Comments
bzbarsky-apple
added a commit
to bzbarsky-apple/connectedhomeip
that referenced
this issue
Jun 13, 2022
Specific changes:
1) Use CharReaderBuilder/CharReader, because that allows us to set 'failIfExtra'
to true, so we fail out on trailing garbage.
2) Since now we can, set 'allowSingleQuotes' to true, so people can use single
quotes in their JSON if that's more convenient.
3) Add checking for duplicated keys by setting rejectDupKeys to true.
4) For complex arguments, enforce that the value is in fact an array or object,
not a primitive value.
5) Factour out the parsing logic from ComplexArgument so we can use it in
CustomArgument too.
6) Make CustomArgument fail out on JSON parse errors instead of silently doing
unexpected things.
7) For CustomArgument, detect an argument starting with one of our type prefixes
("hex:", "i:", "u:", "f:", "d:") and if so just treat it as a string value
instead of trying to do a full JSON parse (which would fail anyway).
8) For CustomArgument, detect an argument starting with "0x" and treat it as a
string starting "u:0x", so it will be parsed as a hex number instead of just
failing to parse.
This leads to the following behavior:
./chip-tool any write-by-id 0x130AFC01 0x130A0001 123 17 1
Before: sends 123.
After: sends 123 as signed int..
./chip-tool any write-by-id 0x130AFC01 0x130A0001 0x123 17 1
Before: sends 0.
After: sends 291 as unsigned int.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 u:123 17 1
Before: sends null.
After: sends 123 as unsigned int.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 hex:121314 17 1
Before: sends null.
After: sends byte string with bytes 0x12, 0x13, 0x14.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 "[1, 2, 3], [4, 5, 6]" 17 1
Before: sends list [1, 2, 3].
After: errors out on trailing garbage.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 "[1 2, 3]" 17 1
Before: sends list [1].
After: errors out with "Missing ',' or ']' in array declaration".
./chip-tool accesscontrol write acl '[{"privilege": 5, "authMode": 2, "subjects": [112233], "targets": [{"cluster": null, "endpoint": null, "deviceType": 0x100}]}]' 17 0
Before: Errors out on the invalid hex number not in quotes, shows the error location.
After: same.
./chip-tool accesscontrol write acl '[{"privilege": 5, "authMode": 2, "subjects": [112233], "targets": []}], [{"targets": [{"cluster": null, "endpoint": null, "deviceType": 0x100}]}]' 17 0
Before: sends the first array and ignores the second one.
After: errors out on trailing garbage.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 '{"1": 0x100 }' 17 1
Before: Sends {"1": 0}
After: Parse error on invalid hex.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 '{"1": "u:0x100" }' 17 1
Before: sends {"1": 256}
After: same.
Fixes project-chip#19529
bzbarsky-apple
added a commit
to bzbarsky-apple/connectedhomeip
that referenced
this issue
Jun 13, 2022
Specific changes:
1) Use CharReaderBuilder/CharReader, because that allows us to set 'failIfExtra'
to true, so we fail out on trailing garbage.
2) Since now we can, set 'allowSingleQuotes' to true, so people can use single
quotes in their JSON if that's more convenient.
3) Add checking for duplicated keys by setting rejectDupKeys to true.
4) For complex arguments, enforce that the value is in fact an array or object,
not a primitive value.
5) Factour out the parsing logic from ComplexArgument so we can use it in
CustomArgument too.
6) Make CustomArgument fail out on JSON parse errors instead of silently doing
unexpected things.
7) For CustomArgument, detect an argument starting with one of our type prefixes
("hex:", "i:", "u:", "f:", "d:") and if so just treat it as a string value
instead of trying to do a full JSON parse (which would fail anyway).
8) For CustomArgument, detect an argument starting with "0x" and treat it as a
string starting "u:0x", so it will be parsed as a hex number instead of just
failing to parse.
This leads to the following behavior:
./chip-tool any write-by-id 0x130AFC01 0x130A0001 123 17 1
Before: sends 123 as signed int.
After: same.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 0x123 17 1
Before: sends 0.
After: sends 291 as unsigned int.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 u:123 17 1
Before: sends null.
After: sends 123 as unsigned int.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 hex:121314 17 1
Before: sends null.
After: sends byte string with bytes 0x12, 0x13, 0x14.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 "[1, 2, 3], [4, 5, 6]" 17 1
Before: sends list [1, 2, 3].
After: errors out on trailing garbage.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 "[1 2, 3]" 17 1
Before: sends list [1].
After: errors out with "Missing ',' or ']' in array declaration".
./chip-tool accesscontrol write acl '[{"privilege": 5, "authMode": 2, "subjects": [112233], "targets": [{"cluster": null, "endpoint": null, "deviceType": 0x100}]}]' 17 0
Before: Errors out on the invalid hex number not in quotes, shows the error location.
After: same.
./chip-tool accesscontrol write acl '[{"privilege": 5, "authMode": 2, "subjects": [112233], "targets": []}], [{"targets": [{"cluster": null, "endpoint": null, "deviceType": 0x100}]}]' 17 0
Before: sends the first array and ignores the second one.
After: errors out on trailing garbage.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 '{"1": 0x100 }' 17 1
Before: Sends {"1": 0}
After: Parse error on invalid hex.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 '{"1": "u:0x100" }' 17 1
Before: sends {"1": 256}
After: same.
Fixes project-chip#19529
bzbarsky-apple
added a commit
to bzbarsky-apple/connectedhomeip
that referenced
this issue
Jun 13, 2022
Specific changes:
1) Use CharReaderBuilder/CharReader, because that allows us to set 'failIfExtra'
to true, so we fail out on trailing garbage.
2) Since now we can, set 'allowSingleQuotes' to true, so people can use single
quotes in their JSON if that's more convenient.
3) Add checking for duplicated keys by setting rejectDupKeys to true.
4) For complex arguments, enforce that the value is in fact an array or object,
not a primitive value.
5) Factour out the parsing logic from ComplexArgument so we can use it in
CustomArgument too.
6) Make CustomArgument fail out on JSON parse errors instead of silently doing
unexpected things.
7) For CustomArgument, detect an argument starting with one of our type prefixes
("hex:", "i:", "u:", "f:", "d:") and if so just treat it as a string value
instead of trying to do a full JSON parse (which would fail anyway).
8) For CustomArgument, detect an argument starting with "0x" and treat it as a
string starting "u:0x", so it will be parsed as a hex number instead of just
failing to parse.
This leads to the following behavior:
./chip-tool any write-by-id 0x130AFC01 0x130A0001 123 17 1
Before: sends 123 as signed int.
After: same.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 0x123 17 1
Before: sends 0.
After: sends 291 as unsigned int.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 u:123 17 1
Before: sends null.
After: sends 123 as unsigned int.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 hex:121314 17 1
Before: sends null.
After: sends byte string with bytes 0x12, 0x13, 0x14.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 "[1, 2, 3], [4, 5, 6]" 17 1
Before: sends list [1, 2, 3].
After: errors out on trailing garbage.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 "[1 2, 3]" 17 1
Before: sends list [1].
After: errors out with "Missing ',' or ']' in array declaration".
./chip-tool accesscontrol write acl '[{"privilege": 5, "authMode": 2, "subjects": [112233], "targets": [{"cluster": null, "endpoint": null, "deviceType": 0x100}]}]' 17 0
Before: Errors out on the invalid hex number not in quotes, shows the error location.
After: same.
./chip-tool accesscontrol write acl '[{"privilege": 5, "authMode": 2, "subjects": [112233], "targets": []}], [{"targets": [{"cluster": null, "endpoint": null, "deviceType": 0x100}]}]' 17 0
Before: sends the first array and ignores the second one.
After: errors out on trailing garbage.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 '{"1": 0x100 }' 17 1
Before: Sends {"1": 0}
After: Parse error on invalid hex.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 '{"1": "u:0x100" }' 17 1
Before: sends {"1": 256}
After: same.
Fixes project-chip#19529
andy31415
pushed a commit
that referenced
this issue
Jun 14, 2022
Specific changes:
1) Use CharReaderBuilder/CharReader, because that allows us to set 'failIfExtra'
to true, so we fail out on trailing garbage.
2) Since now we can, set 'allowSingleQuotes' to true, so people can use single
quotes in their JSON if that's more convenient.
3) Add checking for duplicated keys by setting rejectDupKeys to true.
4) For complex arguments, enforce that the value is in fact an array or object,
not a primitive value.
5) Factour out the parsing logic from ComplexArgument so we can use it in
CustomArgument too.
6) Make CustomArgument fail out on JSON parse errors instead of silently doing
unexpected things.
7) For CustomArgument, detect an argument starting with one of our type prefixes
("hex:", "i:", "u:", "f:", "d:") and if so just treat it as a string value
instead of trying to do a full JSON parse (which would fail anyway).
8) For CustomArgument, detect an argument starting with "0x" and treat it as a
string starting "u:0x", so it will be parsed as a hex number instead of just
failing to parse.
This leads to the following behavior:
./chip-tool any write-by-id 0x130AFC01 0x130A0001 123 17 1
Before: sends 123 as signed int.
After: same.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 0x123 17 1
Before: sends 0.
After: sends 291 as unsigned int.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 u:123 17 1
Before: sends null.
After: sends 123 as unsigned int.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 hex:121314 17 1
Before: sends null.
After: sends byte string with bytes 0x12, 0x13, 0x14.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 "[1, 2, 3], [4, 5, 6]" 17 1
Before: sends list [1, 2, 3].
After: errors out on trailing garbage.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 "[1 2, 3]" 17 1
Before: sends list [1].
After: errors out with "Missing ',' or ']' in array declaration".
./chip-tool accesscontrol write acl '[{"privilege": 5, "authMode": 2, "subjects": [112233], "targets": [{"cluster": null, "endpoint": null, "deviceType": 0x100}]}]' 17 0
Before: Errors out on the invalid hex number not in quotes, shows the error location.
After: same.
./chip-tool accesscontrol write acl '[{"privilege": 5, "authMode": 2, "subjects": [112233], "targets": []}], [{"targets": [{"cluster": null, "endpoint": null, "deviceType": 0x100}]}]' 17 0
Before: sends the first array and ignores the second one.
After: errors out on trailing garbage.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 '{"1": 0x100 }' 17 1
Before: Sends {"1": 0}
After: Parse error on invalid hex.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 '{"1": "u:0x100" }' 17 1
Before: sends {"1": 256}
After: same.
Fixes #19529
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Problem
The following sends 4 bytes of octstr to the device:
./chip-tool any write-by-id 0x130AFC01 0x130A0001 '"hex:05025555"' 12344321 1The following sends a value of null to the device. No warning that parsing failed and no error either in the output.
./chip-tool any write-by-id 0x130AFC01 0x130A0001 hex:05025555 12344321 1Proposed Solution
Triple quoting should not be required. If argument starts with hex: then treat it as a hex string. Parsing errors should not be silently ignored. Probably easiest to fix in
CHIP_ERROR Parse(const char * label, const char * json)Make hexadecimal numbers work as well e.g. 0x123The text was updated successfully, but these errors were encountered: