ibireme / yyjson

The fastest JSON library in C
https://ibireme.github.io/yyjson/doc/doxygen/html/
MIT License
2.98k stars 262 forks source link

incorrectly formatted bool fields. #161

Closed sbezverk closed 3 months ago

sbezverk commented 4 months ago

Describe the bug When a relatively large number of bool fields are added to the mutable json document and then generating char * with yyjson_mut_write(doc, YYJSON_WRITE_PRETTY, NULL); some fields look incorrectly formated. Example missing trailing comma and \n.

Here is an example:

                                                                                                                   "is_set_cos3_profile": true,
                                                                                                                    "is_set_cos4_profile": true,
                                                                                                                    "is_set_cos5_profile": false,
                                                                                                                    "is_set_cos6_profile": true                                                                                                                    "is_set_cos7_profile": true,                                                                                                                    "is_set_cos0_rate_profile_obj": true,
                                                                                                                    "is_set_cos1_rate_profile_obj": true,
                                                                                                                    "is_set_cos2_rate_profile_obj": false,
                                                                                                                    "is_set_cos3_rate_profile_obj": false,
                                                                                                                    "is_set_cos4_rate_profile_obj": false,
                                                                                                                    "is_set_cos5_rate_profile_obj": false,
                                                                                                                    "is_set_cos6_rate_profile_obj": false,
                                                                                                                    "is_set_cos7_rate_profile_obj": false,
                                                                                                                    "is_set_filter_entry": false,
                                                                                                                    "is_set_is_subinterface": true           < --- missing comma                                                                                                         "is_set_tc_match_flags": false,
                                                                                                                    "is_set_sys_port": true,
                                                                                                                    "is_set_sys_port_arr": true,                < -- missing \n                                                                                                    "is_set_remote_mac_port_arr": false,
                                                                                                                    "is_set_remote_pp_port": false,
                                                                                                                    "is_set_q_prof": false,
                                                                                                                    "is_set_initiate_flush": true                                                                                                                    "is_set_parent_tmport_ref_obj": true,
                                                                                                                    "is_set_l3if_ref_obj": false,
                                                                                                                    "is_set_voqgrp_obj": true,
                                                                                                                    "is_set_activate": true,                                                                                                                    "is_set_next_aggport": false,
                                                                                                                    "is_set_for_etm_rcy": false,
                                                                                                                    "is_set_local_voq_switching": true,
                                                                                                                    "is_set_pt_if_id": true                                                                                                                    "is_set_pt_tt_type": true,
                                                                                                                    "is_set_npu_bmap": true                                                                                                                    "is_set_l1port_refhdl": true,
                                                                                                                    "is_set_l1port_refkey": tru.     < ----- missing 'e'

Your environment

Additional context Add any other context about the problem here. this issue is 100% reproducible on large json documents.

ibireme commented 4 months ago

I believe I've reproduced the output:

yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL);
yyjson_mut_val *root = yyjson_mut_obj(doc);
yyjson_mut_doc_set_root(doc, root);
yyjson_mut_val *val;

val = yyjson_mut_bool(doc, false);
yyjson_mut_obj_add_val(doc, root, "is", val);
val = yyjson_mut_bool(doc, false);
val->tag |= 0x10;
yyjson_mut_obj_add_val(doc, root, "is", val);

val = yyjson_mut_bool(doc, false);
yyjson_mut_obj_add_val(doc, root, "is", val);
val = yyjson_mut_bool(doc, true);
val->tag |= 0x10;
yyjson_mut_obj_add_val(doc, root, "is", val);

val = yyjson_mut_bool(doc, true);
yyjson_mut_obj_add_val(doc, root, "is", val);
val = yyjson_mut_bool(doc, true);
val->tag |= 0x10;
yyjson_mut_obj_add_val(doc, root, "is", val);

usize json_len = 0;
const char *json = yyjson_mut_write(doc, YYJSON_WRITE_PRETTY, &json_len);
printf("%s\n", json);

output:

{
    "is": false,
    "is": true,    "is": false,   < -- missing \n
    "is": true    "is": true,     < -- missing comma
    "is": tr                      < -- missing 'ue'
}

Here's why it's happening:

The bool type only holds 0 or 1. Typically, when you pass non-bool values to a bool parameter, they get converted to 1 if they're non-zero, and everything works fine. But if your compiler doesn't automatically handle this conversion (some environments define bool as unsigned char), and you feed a non-0/1 value directly as a bool, it might cause the JSON bool to have values like 0/1/2/3, potentially missing 1 or 2 characters.

Here's some yyjson's internal code:

// set bool
yyjson_mut_bool(yyjson_mut_doc *doc, bool val) {
    ...
    val->tag = YYJSON_TYPE_BOOL | (uint8_t)((uint8_t)_val << 3);
    ...
}

// get bool
bool unsafe_yyjson_get_bool(void *val) {
    uint8_t tag = unsafe_yyjson_get_tag(val);
    return (bool)((tag & YYJSON_SUBTYPE_MASK) >> YYJSON_TYPE_BIT);
}

// write bool
u8 *write_bool(u8 *cur, bool val) {
    ...
    return cur + 5 - val; //< val shoule be 0 or 1
}

I'd suggest you check for this scenario in your code. If you find it, you can make sure the value is treated as a bool like this yyjson_mut_bool(doc, !!val).

I'll also fix yyjson later to avoid this problem.

sbezverk commented 4 months ago

Thank you very much for such quick response, really appreciate it. Since I do not have control which value is used to indicate ‘true’ I just applied the workaround you suggested and it works perfectly.

Thank you again and I would appreciate a headsup when fixed version will be published.

Serguei

From: ibireme @.> Reply to: ibireme/yyjson @.> Date: Thursday, 7 March 2024 at 07:29 To: ibireme/yyjson @.> Cc: sbezverk @.>, Author @.***> Subject: Re: [ibireme/yyjson] incorrectly formatted bool fields. (Issue #161)

I believe I've reproduced the output:

yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL);

yyjson_mut_val *root = yyjson_mut_obj(doc);

yyjson_mut_doc_set_root(doc, root);

yyjson_mut_val *val;

val = yyjson_mut_bool(doc, false);

yyjson_mut_obj_add_val(doc, root, "is", val);

val = yyjson_mut_bool(doc, false);

val->tag |= 0x10;

yyjson_mut_obj_add_val(doc, root, "is", val);

val = yyjson_mut_bool(doc, false);

yyjson_mut_obj_add_val(doc, root, "is", val);

val = yyjson_mut_bool(doc, true);

val->tag |= 0x10;

yyjson_mut_obj_add_val(doc, root, "is", val);

val = yyjson_mut_bool(doc, true);

yyjson_mut_obj_add_val(doc, root, "is", val);

val = yyjson_mut_bool(doc, true);

val->tag |= 0x10;

yyjson_mut_obj_add_val(doc, root, "is", val);

usize json_len = 0;

const char *json = yyjson_mut_write(doc, YYJSON_WRITE_PRETTY, &json_len);

printf("%s\n", json);

output:

{

"is": false,

"is": true,    "is": false,   < -- missing \n

"is": true    "is": true,     < -- missing comma

"is": tr                      < -- missing 'ue'

}

Here's why it's happening:

The bool type only holds 0 or 1. Typically, when you pass non-bool values to a bool parameter, they get converted to 1 if they're non-zero, and everything works fine. But if your compiler doesn't automatically handle this conversion (some environments define bool as unsigned char), and you feed a non-0/1 value directly as a bool, it might cause the JSON bool to have values like 0/1/2/3, potentially missing 1 or 2 characters.

Here's some yyjson's internal code:

// set bool

yyjson_mut_bool(yyjson_mut_doc *doc, bool val) {

...

val->tag = YYJSON_TYPE_BOOL | (uint8_t)((uint8_t)_val << 3);

...

}

// get bool

bool unsafe_yyjson_get_bool(void *val) {

uint8_t tag = unsafe_yyjson_get_tag(val);

return (bool)((tag & YYJSON_SUBTYPE_MASK) >> YYJSON_TYPE_BIT);

}

// write bool

u8 write_bool(u8 cur, bool val) {

...

return cur + 5 - val; //< val shoule be 0 or 1

}

I'd suggest you check for this scenario in your code. If you find it, you can make sure the value is treated as a bool like this yyjson_mut_bool(doc, !!val).

I'll also fix yyjson later to avoid this problem.

— Reply to this email directly, view it on GitHubhttps://github.com/ibireme/yyjson/issues/161#issuecomment-1982550281, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AD4E7UDVVQMBJLNILGCTQQTYXACKTAVCNFSM6AAAAABEJY26SOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBSGU2TAMRYGE. You are receiving this because you authored the thread.Message ID: @.***>

ibireme commented 4 months ago

Fixed in master branch.