Fix JSON validation issues concerning missing commas, escaped control characters, and hex escaped 0 value. #179
Conversation
|
|
||
| switch( c ) | ||
| { | ||
| case '\0': |
There was a problem hiding this comment.
This will break from the default case - no need for this.
| } | ||
|
|
||
| if( ( i == end ) && ( value > 0U ) ) | ||
| if( i == end ) |
There was a problem hiding this comment.
Allow value of 0 (\u0000) per JSON spec.
|
|
||
| default: | ||
|
|
||
| /* a control character: (NUL,SPACE) */ |
There was a problem hiding this comment.
this is invalid... escaped literal control characters should fail validation per spec, thus I have removed it.
| depth--; | ||
|
|
||
| if( ( skipSpaceAndComma( buf, &i, max ) == true ) && | ||
| isOpenBracket_( stack[ depth ] ) ) |
There was a problem hiding this comment.
I have removed this isOpenBracket_ check as there is no point... stack[ depth ] will always contain an open bracket when depth >= 0.
|
|
||
| if( skipSpaceAndComma( buf, &i, max ) != true ) | ||
| { | ||
| /* After parsing a scalar, we must either have a comma (followed by more content) |
There was a problem hiding this comment.
This fixes cases such as [3[4]] from being parsed as valid.
| { | ||
| /* After closing a nested collection, if there is no comma found when calling | ||
| * skipSpaceAndComma, then we must be at the end of the parent collection. */ | ||
| if( ( i < max ) && !isMatchingBracket_( stack[ depth ], buf[ i ] ) ) |
There was a problem hiding this comment.
This fixes cases where missing commas between collections was considered valid. (e.g. [ { "key1" : "value1"} {"key2":"value2"}])
| /* More details at: https://github.com/FreeRTOS/coreJSON/blob/main/MISRA.md#rule-143 */ | ||
| /* coverity[misra_c_2012_rule_14_3_violation] */ | ||
| end = ( i <= ( SIZE_MAX - HEX_ESCAPE_LENGTH ) ) ? ( i + HEX_ESCAPE_LENGTH ) : SIZE_MAX; | ||
| end = i + HEX_ESCAPE_LENGTH; |
There was a problem hiding this comment.
This previous logic relied on value remaning 0 to determine whether or not a hex escape was skipped. because the value 0 is now allowed, I have updated this logic to always add HEX_ESCAPE_LENGTH to i as the end and added an overflow check.
There was a problem hiding this comment.
that way the i == end logic holds for determining if a hex escape value was parsed.
| uses: FreeRTOS/CI-CD-Github-Actions/complexity@main | ||
| with: | ||
| path: ./ | ||
| horrid_threshold: 12 |
There was a problem hiding this comment.
to be frank here - I did not want to spend extra time reducing the complexity :)
|
|
||
| if( skipSpaceAndComma( buf, &i, max ) == true ) | ||
| { | ||
| + __CPROVER_assume( isOpenBracket_(stack[depth])); |
There was a problem hiding this comment.
I have verified by code inspection that this assumption should hold for the loop - I would ask that reviewers verify this as well - see comment about redundant isOpenBracket_ check.
3 participants
Description
This change fixes the JSON_Validate API:
Test Steps
Ran coreJSON against the y (valid) and n (invalid) JSON documents found in https://github.com/nst/JSONTestSuite/tree/master/test_parsing.
Checklist:
Related Issue
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.