json: Fix multidimensional array support #50816

mrfuchs · 2022-09-29T11:40:59Z

This patch fixes support for encoding and decoding multidimensional arrays
as described by the JSON_OBJ_DESCR_ARRAY_ARRAY() macro.

Fixes #50801

carlescufi · 2022-09-30T10:25:46Z

@mrfuchs can you take a look at the CI failures?

andyross

Saw this languishing and figured I should give it an approval, even though it's not really my area (we don't really have a maintainer for JSON right now, I guess?).

Language wonks (@stephanosio @keith-packard ?) might care about the unicode string literals, though they seem harmless to me.

tests/lib/json/src/main.c

dariis · 2022-12-02T12:48:12Z

@mrfuchs Thank you, it worked for me 👍

mrfuchs · 2022-12-05T10:21:55Z

Saw this languishing and figured I should give it an approval, even though it's not really my area

Thank you! Unfortunately, I had to update this PR. Would you review it once more, please?

(we don't really have a maintainer for JSON right now, I guess?).

Yes, it's difficult to get JSON PRs approved - my last PR #47989 also got closed due to a lack of reviewers.
@Sir-Branch asked for being the official JSON maintainer, but has become inactive lately. Sometimes @martinjaeger helps reviewing changes to the JSON library.

Language wonks (@stephanosio @keith-packard ?) might care about the unicode string literals, though they seem harmless to me.

I didn't really pay attention to that. I just reused the test data that was already there.

mrfuchs · 2022-12-05T10:22:36Z

I noticed that my original patch would still not fix parsing arrays of objects. That's why I moved these lines inside the loop...

zephyr/lib/os/json.c

Lines 578 to 583 in 55dc398

    
           		/* For nested arrays, update value to current field, 
        
           		 * so it matches descriptor's offset to length field 
        
           		 */ 
        
           		if (elem_descr->type == JSON_TOK_ARRAY_START) { 
        
           			value = field; 
        
           		}

... and added a test for it (cf96b83). However, after adding the new unit test, CI failed again (for 64-bit targets only, cf. #36696 and #24372#issuecomment-617392567). So, I fixed this issue in 55dc398 to make CI work again.

github-actions · 2023-02-04T00:30:10Z

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

ofirshe · 2023-06-26T09:00:34Z

Hey,

I would like to inquire about the current status of this fix as I am experiencing the same issue as described here [(https://github.com//issues/59445)].

mrfuchs · 2023-06-26T09:08:56Z

I would like to inquire about the current status of this fix as I am experiencing the same issue [...]

Will update this PR soon to follow @keith-packard's suggestion re. non-ascii text in the unit tests.

mrfuchs · 2023-06-27T12:10:15Z

@andyross, @keith-packard, I updated the unit tests. Could you review? Thanks!

keith-packard

The commit message on the first patch (fixing the bugs) provides no information about what was broken and how the change addresses that. Makes reviewing the patch pretty hard.

keith-packard · 2023-06-27T18:54:34Z

@andyross, @keith-packard, I updated the unit tests. Could you review? Thanks!

Unit test changes to use octal escapes look good!

This patch fixes support for encoding and decoding multidimensional arrays as described by the JSON_OBJ_DESCR_ARRAY_ARRAY() macro. Currently, the JSON array encoding and decoding functions, arr_encode() and arr_parse(), expect array elements to be of object or primitive type. However, arrays may be nested and so an array's elements may also be arrays. In order to support nested arrays, two special cases must be considered: 1. The array of objects/arrays sub-descriptor is described by two `json_obj_descr` structs and so two instead of one `json_obj_descr` structs must be skipped when iterating over the JSON descriptor to get to an array's elements. 2. The implicit array item count field has to be considered for the parent itself and all its child array items when calculating an element's size. Fixes zephyrproject-rtos#50801 Signed-off-by: Markus Fuchs <[email protected]>

Switch the non-ascii text in the tests to use octal escape sequences to avoid potential compiler issues on platforms not defaulting to utf-8 text encodings. Signed-off-by: Markus Fuchs <[email protected]>

Fix encoded JSON string in test_json_decoding_array_array() test so it matches the described array object and add a test case for encoding arrays of arrays. Signed-off-by: Markus Fuchs <[email protected]>

Add tests for encoding and decoding two-dimensional arrays as described by the JSON_OBJ_DESCR_ARRAY_ARRAY() macro. Signed-off-by: Markus Fuchs <[email protected]>

Add tests for encoding and decoding nested arrays of objects located inside a parent object at a non-zero offset. Signed-off-by: Markus Fuchs <[email protected]>

This patch fixes encoding arrays of objects on 64-bit targets. Fixes zephyrproject-rtos#36696 Signed-off-by: Markus Fuchs <[email protected]>

mrfuchs · 2023-06-28T14:58:42Z

The commit message on the first patch (fixing the bugs) provides no information about what was broken and how the change addresses that.

I updated the commit message so it contains more information on what was missing and what was changed.

After the changes introduced by zephyrproject-rtos#50816 the UpdateHub could not decode anymore the JSON object. This introduce missing parsing definitions to allow JSON parser undertood the correct UpdateHub probe object. Fixes zephyrproject-rtos#69297 Signed-off-by: Gerson Fernando Budke <[email protected]>

After the changes introduced by #50816 the UpdateHub could not decode anymore the JSON object. This introduce missing parsing definitions to allow JSON parser undertood the correct UpdateHub probe object. Fixes #69297 Signed-off-by: Gerson Fernando Budke <[email protected]>

After the changes introduced by zephyrproject-rtos#50816 the UpdateHub could not decode anymore the JSON object. This introduce missing parsing definitions to allow JSON parser undertood the correct UpdateHub probe object. Fixes zephyrproject-rtos#69297 Signed-off-by: Gerson Fernando Budke <[email protected]> (cherry picked from commit 5fb62ca)

After the changes introduced by #50816 the UpdateHub could not decode anymore the JSON object. This introduce missing parsing definitions to allow JSON parser undertood the correct UpdateHub probe object. Fixes #69297 Signed-off-by: Gerson Fernando Budke <[email protected]> (cherry picked from commit 5fb62ca)

After the changes introduced by zephyrproject-rtos#50816 the UpdateHub could not decode anymore the JSON object. This introduce missing parsing definitions to allow JSON parser undertood the correct UpdateHub probe object. Fixes zephyrproject-rtos#69297 Signed-off-by: Gerson Fernando Budke <[email protected]>

After the changes introduced by #50816 the UpdateHub could not decode anymore the JSON object. This introduce missing parsing definitions to allow JSON parser undertood the correct UpdateHub probe object. Fixes #69297 Signed-off-by: Gerson Fernando Budke <[email protected]> (cherry picked from commit 5fb62ca)

mrfuchs requested review from nashif, dcpleung and andyross as code owners September 29, 2022 11:40

zephyrbot added area: JSON area: Base OS Base OS Library (lib/os) labels Sep 29, 2022

zephyrbot requested a review from Sir-Branch September 29, 2022 11:41

zephyrbot assigned andyross Sep 29, 2022

mrfuchs force-pushed the json_arr-arr branch 6 times, most recently from 556c752 to fda1247 Compare September 29, 2022 12:57

carlescufi force-pushed the json_arr-arr branch from fda1247 to f514b8b Compare September 30, 2022 09:40

mrfuchs force-pushed the json_arr-arr branch 3 times, most recently from f13877f to 5217b6a Compare October 4, 2022 13:44

andyross previously approved these changes Oct 4, 2022

View reviewed changes

tests/lib/json/src/main.c Outdated Show resolved Hide resolved

mrfuchs dismissed andyross’s stale review via a609d42 November 1, 2022 16:01

mrfuchs force-pushed the json_arr-arr branch 6 times, most recently from 2a388d8 to 55dc398 Compare November 3, 2022 13:23

mrfuchs force-pushed the json_arr-arr branch 2 times, most recently from 14b6bc0 to a0d34b3 Compare June 27, 2023 11:13

mrfuchs mentioned this pull request Jun 27, 2023

json: Improve tolerance to uncovered C members during JSON encoding #53333

Closed

keith-packard reviewed Jun 27, 2023

View reviewed changes

mrfuchs added 6 commits June 28, 2023 16:05

tests: lib: json: Switch non-ascii text to octal escape sequences

063cbbc

Switch the non-ascii text in the tests to use octal escape sequences to avoid potential compiler issues on platforms not defaulting to utf-8 text encodings. Signed-off-by: Markus Fuchs <[email protected]>

tests: lib: json: Fix and enhance test for array of arrays

7dad45a

Fix encoded JSON string in test_json_decoding_array_array() test so it matches the described array object and add a test case for encoding arrays of arrays. Signed-off-by: Markus Fuchs <[email protected]>

tests: lib: json: Add tests for handling two-dimensional arrays

af44f0b

Add tests for encoding and decoding two-dimensional arrays as described by the JSON_OBJ_DESCR_ARRAY_ARRAY() macro. Signed-off-by: Markus Fuchs <[email protected]>

tests: lib: json: Add test for array of objects

773f903

Add tests for encoding and decoding nested arrays of objects located inside a parent object at a non-zero offset. Signed-off-by: Markus Fuchs <[email protected]>

json: Fix 64-bit support

065d0b1

This patch fixes encoding arrays of objects on 64-bit targets. Fixes zephyrproject-rtos#36696 Signed-off-by: Markus Fuchs <[email protected]>

mrfuchs force-pushed the json_arr-arr branch from a0d34b3 to 065d0b1 Compare June 28, 2023 14:06

keith-packard approved these changes Jun 28, 2023

View reviewed changes

pdgendt approved these changes Jun 29, 2023

View reviewed changes

carlescufi merged commit 8757c71 into zephyrproject-rtos:main Jun 29, 2023
17 checks passed

mrfuchs mentioned this pull request Jun 30, 2023

Json JSON_OBJ_DESCR_OBJ_ARRAY misbehaving #59445

Closed

nandojve mentioned this pull request Apr 23, 2024

Zephyr UpdateHub sample issues #69297

Closed

nandojve mentioned this pull request Apr 27, 2024

mgmt: updatehub: Fix json arrays #72036

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

json: Fix multidimensional array support #50816

json: Fix multidimensional array support #50816

mrfuchs commented Sep 29, 2022

carlescufi commented Sep 30, 2022

andyross left a comment

dariis commented Dec 2, 2022

mrfuchs commented Dec 5, 2022

mrfuchs commented Dec 5, 2022 •

edited

Loading

github-actions bot commented Feb 4, 2023

ofirshe commented Jun 26, 2023

mrfuchs commented Jun 26, 2023

mrfuchs commented Jun 27, 2023

keith-packard left a comment

keith-packard commented Jun 27, 2023

mrfuchs commented Jun 28, 2023

json: Fix multidimensional array support #50816

json: Fix multidimensional array support #50816

Conversation

mrfuchs commented Sep 29, 2022

carlescufi commented Sep 30, 2022

andyross left a comment

Choose a reason for hiding this comment

dariis commented Dec 2, 2022

mrfuchs commented Dec 5, 2022

mrfuchs commented Dec 5, 2022 • edited Loading

github-actions bot commented Feb 4, 2023

ofirshe commented Jun 26, 2023

mrfuchs commented Jun 26, 2023

mrfuchs commented Jun 27, 2023

keith-packard left a comment

Choose a reason for hiding this comment

keith-packard commented Jun 27, 2023

mrfuchs commented Jun 28, 2023

mrfuchs commented Dec 5, 2022 •

edited

Loading