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

JSON array encoding fails on array of objects #50976

Closed
lucasdietrich opened this issue Oct 4, 2022 · 5 comments
Closed

JSON array encoding fails on array of objects #50976

lucasdietrich opened this issue Oct 4, 2022 · 5 comments
Assignees
Labels
area: JSON bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug Stale

Comments

@lucasdietrich
Copy link
Contributor

Hi,

When I try to encode an array of objects. If the descriptors of the inner object doesn't cover all the fields of the object structure, then the array is wrongly encoded.

Normal case:

#define MY_ARRAY_SIZE 4u

struct mystruct_obj
{
	uint32_t a;
	/* Uncomment following line to get an incorrect result */
	// uint32_t b;
	uint32_t n;
};

static const struct json_obj_descr descr_mystruct_obj[] = {
	JSON_OBJ_DESCR_PRIM(struct mystruct_obj, a, JSON_TOK_NUMBER),
	JSON_OBJ_DESCR_PRIM(struct mystruct_obj, n, JSON_TOK_NUMBER),
};

struct mystruct_arr
{
	struct mystruct_obj items[MY_ARRAY_SIZE];
	size_t count;
};

static const struct json_obj_descr descr_mystruct_arr[] = {
	JSON_OBJ_DESCR_OBJ_ARRAY(
		struct mystruct_arr,
		items,
		MY_ARRAY_SIZE,
		count,
		descr_mystruct_obj,
		ARRAY_SIZE(descr_mystruct_obj)
	)
};

int encode_to(char *buf, size_t len)
{
	struct mystruct_arr arr = { .count = 0u };

	for (uint8_t i = 0u; i < MY_ARRAY_SIZE; i++) {
		arr.items[i].a = 3;
                // Uncomment following line to highlight that the encoder reads at incorrect offsets.
		// arr.items[i].b = 0xFFFFFFFF;
		arr.items[i].n = i;
		arr.count++;
	}

	return json_arr_encode_buf(descr_mystruct_arr, &arr, buf, len);
}

With the previous code, the json is properly encoded :

[
	{
		"a": 3,
		"n": 0
	},
	{
		"a": 3,
		"n": 1
	},
	{
		"a": 3,
		"n": 2
	},
	{
		"a": 3,
		"n": 3
	}
]

But if I uncomment the line uint32_t b;.
Here, "b" member is still not encoded in the JSON, but I would have expect the result to be the same, however I get the following result. Array is incorrectly encoded.

[
	{
		"a": 3,
		"n": 0
	},
	{
		"a": 0,
		"n": 0
	},
	{
		"a": 0,
		"n": 3
	},
	{
		"a": 3,
		"n": 2
	}
]

If I set "b" to "0xFFFFFFFF" in initialization loop, it seems that the value of "b" is used sporadically. This highlights that the encoder reads the values at incorrect offsets while encoding the array.

[
	{
		"a": 3,
		"n": 0
	},
	{
		"a": 0,
		"n": -1
	},
	{
		"a": -1,
		"n": 3
	},
	{
		"a": 3,
		"n": 2
	}
]

Is this just an undocummented unhandled case ? I do understand this case has not been foreseen.

Impact
Impact is that we cannot use C structure (for JSON encoding) which are not exhaustively described using descriptors. In my case, I will not be able to use existing structure if there are fields I don't want to encode, I would need to create structures dedicated to JSON encoding.

I haven't done much debugging yet, maybe I will dig into this when I have time.

Environment:

  • Linux
  • Zephyr v3.1.0
  • QEMU x86
@lucasdietrich lucasdietrich added the bug The issue is a bug, or the PR is fixing a bug label Oct 4, 2022
@mrfuchs
Copy link
Contributor

mrfuchs commented Oct 5, 2022

Without having it debugged, I would say the reason is that arr_encode() iterates over the array items by incrementing the data pointer by elem_size.

field = (char *)field + elem_size;

elem_size is retrieved via get_elem_size() which reconstructs an element's size by the information given in the JSON descriptor. As b is part of the structure but missing from the descriptor, encoding will fail.

zephyr/lib/os/json.c

Lines 834 to 838 in 87d7c8f

static int arr_encode(const struct json_obj_descr *elem_descr,
const void *field, const void *val,
json_append_bytes_t append_bytes, void *data)
{
ptrdiff_t elem_size = get_elem_size(elem_descr);

zephyr/lib/os/json.c

Lines 508 to 523 in 87d7c8f

static ptrdiff_t get_elem_size(const struct json_obj_descr *descr)
{
switch (descr->type) {
case JSON_TOK_NUMBER:
return sizeof(int32_t);
case JSON_TOK_OPAQUE:
case JSON_TOK_FLOAT:
case JSON_TOK_OBJ_ARRAY:
return sizeof(struct json_obj_token);
case JSON_TOK_STRING:
return sizeof(char *);
case JSON_TOK_TRUE:
case JSON_TOK_FALSE:
return sizeof(bool);
case JSON_TOK_ARRAY_START:
return descr->array.n_elements * get_elem_size(descr->array.element_descr);

lucasdietrich added a commit to lucasdietrich/zephyr that referenced this issue Oct 5, 2022
@lucasdietrich
Copy link
Contributor Author

lucasdietrich commented Oct 5, 2022

Hi Markus,

Thank you for your quick analysis. After debugging this, your explanation perfectly matches my observations.

Now the question is: does this behavior deserves to be enhanced or not ? This is probably not a must have. But anyone could unintentionally end up with JSON descriptors not matching C structure. So telling explicitly somewhere in the documentation that we can't do that would be a plus.

I'll try to work on it (because I need it anyway), let's see if I can enchance this behavior without breaking everything.
I have a first dirty but working solution https://github.com/lucasdietrich/zephyr/commits/json_uncovered_struct.


Totally aside of this:
I'm actually a bit confused about what kind of value the "val" argument of arr_parse() and decode_value() should hold.
When debugging test case test_json_decoding_array_array of tests/lib/json, obj_parse() is called twice (recursively):

  • When parsing the outer-most array with "val=0x10e000 <obj_array_array_ts>"

  • When parsing the nested array with "val=NULL"

  • "val" argument of decode_value() seems to be used only for array decoding.

@github-actions
Copy link

This issue 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 issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Dec 11, 2022
lucasdietrich added a commit to lucasdietrich/zephyr that referenced this issue Dec 24, 2022
This enhancement fix encoding of JSON objects for which descriptors do
not covers all members of the C structure represented.

Fix zephyrproject-rtos#50976

Signed-off-by: Lucas Dietrich <ld.adecy@gmail.com>
@nashif nashif removed the Stale label Feb 17, 2023
@teburd teburd reopened this Mar 2, 2023
@teburd
Copy link
Collaborator

teburd commented Mar 2, 2023

Not stale, pending PR

lucasdietrich added a commit to lucasdietrich/zephyr that referenced this issue Mar 3, 2023
This enhancement fix encoding of JSON objects for which descriptors do
not covers all members of the C structure represented.

Fix zephyrproject-rtos#50976

Signed-off-by: Lucas Dietrich <ld.adecy@gmail.com>
lucasdietrich added a commit to lucasdietrich/zephyr that referenced this issue Apr 3, 2023
This enhancement fix encoding of JSON objects for which descriptors do
not covers all members of the C structure represented.

Fixes zephyrproject-rtos#50976

Signed-off-by: Lucas Dietrich <ld.adecy@gmail.com>
lucasdietrich added a commit to lucasdietrich/zephyr that referenced this issue Apr 3, 2023
This enhancement fix encoding of JSON objects for which descriptors do
not covers all members of the C structure represented.

Introduce SIZEOF_FIELD helper macro

Fixes zephyrproject-rtos#50976

Signed-off-by: Lucas Dietrich <ld.adecy@gmail.com>
lucasdietrich added a commit to lucasdietrich/zephyr that referenced this issue Apr 12, 2023
This enhancement fix encoding of JSON objects for which descriptors do
not covers all members of the C structure represented.

Introduce SIZEOF_FIELD helper macro

Fixes zephyrproject-rtos#50976

Signed-off-by: Lucas Dietrich <ld.adecy@gmail.com>
@github-actions
Copy link

github-actions bot commented May 2, 2023

This issue 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 issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label May 2, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 16, 2023
lucasdietrich added a commit to lucasdietrich/zephyr that referenced this issue May 30, 2023
This enhancement fix encoding of JSON objects for which descriptors do
not covers all members of the C structure represented.

Fix zephyrproject-rtos#50976

Signed-off-by: Lucas Dietrich <ld.adecy@gmail.com>
lucasdietrich added a commit to lucasdietrich/zephyr that referenced this issue Jun 17, 2023
This enhancement fix encoding of JSON objects for which descriptors do
not covers all members of the C structure represented.

Fix zephyrproject-rtos#50976

Signed-off-by: Lucas Dietrich <ld.adecy@gmail.com>
lucasdietrich added a commit to lucasdietrich/zephyr that referenced this issue Jun 27, 2023
This enhancement fix encoding of JSON objects for which descriptors do
not covers all members of the C structure represented.

Fixes zephyrproject-rtos#50976

Signed-off-by: Lucas Dietrich <ld.adecy@gmail.com>
lucasdietrich added a commit to lucasdietrich/zephyr that referenced this issue Jul 26, 2023
Add special handling for nested arrays

This enhancement fix encoding of JSON objects for which descriptors do
not covers all members of the C structure represented.

Fixes zephyrproject-rtos#50976

Signed-off-by: Lucas Dietrich <ld.adecy@gmail.com>
lucasdietrich added a commit to lucasdietrich/zephyr that referenced this issue Jul 26, 2023
Add special handling for nested arrays

This enhancement fix encoding of JSON objects for which descriptors do
not covers all members of the C structure represented.

Fixes zephyrproject-rtos#50976

Signed-off-by: Lucas Dietrich <ld.adecy@gmail.com>
lucasdietrich added a commit to lucasdietrich/zephyr that referenced this issue Jul 26, 2023
Add special handling for nested arrays

This enhancement fix encoding of JSON objects for which descriptors do
not covers all members of the C structure represented.

Fixes zephyrproject-rtos#50976

Signed-off-by: Lucas Dietrich <ld.adecy@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: JSON bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug Stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants