berry json module load HEX error

[darcyg]

https://github.com/arendst/Tasmota/issues/16390

issue

run 1

import json
var s = "{\"abc\":16}"
var j = json.load(s)
print(j)

output:

{'abc': 16}

run 2

import json
var s = "{\"abc\":0x10}"
var j = json.load(s)
print(j)

output:

nil 

run 3

import json
var s = "{\"abc\":{\"hex\":16}}"
var j = json.load(s)
print(j)

output

{'abc': {'hex': 16}}

run 4

import json
var s = "{\"abc\":{\"hex\":0x10}}"
var j = json.load(s)
print(j)

reboot...

patch

diff --git a/lib/libesp32/berry/src/be_strlib.c b/lib/libesp32/berry/src/be_strlib.c
index 23fdf978f..2c5424634 100644
--- a/lib/libesp32/berry/src/be_strlib.c
+++ b/lib/libesp32/berry/src/be_strlib.c
@@ -263,6 +263,9 @@ BERRY_API bint be_str2int(const char *str, const char **endstr)
         while ((c = be_char2hex(*str++)) >= 0) {
             sum = sum * 16 + c;
         }
+        if (endstr) {
+            *endstr = str - 1;
+        }
         return sum;
     } else {
         /* decimal literal */

[s-hadinger]

This is as designed. {"abc":{"hex":0x10}} is not valid json. Json does not support hex integers. You can double check with jsonlint.com

If you want hex, you need to wrap into a string. Yet Berry can transparently convert to an int using int(). If you force conversion to int it will work in both cases.

import json
var s = '{"abc":{"hex":"0x10","dec":16}}'
var j = json.load(s)
print(j)
# {'abc': {'hex': '0x10', 'dec': 16}}
print(int(j['abc']['hex']))
# 16
print(int(j['abc']['dec']))
# 16

[sfromis]

JSON5 exists as a proposed unofficial extension to JSON, including support for hex data. But it seems to be a fork without mainstream acceptance, and not great for compatibility.

[darcyg]

When I tested the modbus bridge function, I found that entering the hex value will cause the motherboard to reboot There will be hex in json because, when debugging, it is introduced for the convenience of debugging faults.

This bug occurs only with json parsing But the error is caused by the basic function be_str2int. Personally, considering the versatility of be_str2int, regardless of the json rules. This patch is still necessary.

[sfromis]

There is no bug. No patch is necessary.

You are just using invalid JSON, and I'd call it an error to break compatibility with standard JSON. Sure, extending the format may be convenient for your specific use case, but that does not make it reasonable, unless you aim for full JSON5 support, which I'm not completely convinced of being a good plan either.

[sfromis]

Hmmmm.... There may be two questions here. One is the focus on hex data in JSON being a syntax error which should fail, meaning that run 2 is "ok, but not easy to debug". Friendliness of how the error shows up can be a consideration, but causing a crash is a bad way. It was not very clear if this was what the "reboot..." in your original description (run 4) meant?

[s-hadinger]

I can reproduce the crash and this is definitely wrong. Thanks for the fix, let me check a thing or two before integrating it.

[s-hadinger]

Aha. The patch you provided solves the crash, but also makes accidentally the parsing of hex working. I suppose we will have to live with that, Berry accepting hex numbers although they are invalid JSON.

It will require additional code to make it break on hex strings...

Also the json parser used by Tasmota jsmn is overly tolerant, so we already have such a case of accepting invalid JSON

[hiperiondev]

This was merged?

[sfromis]

Yes, the links above shows that such a code change was merged. I'd still recommend against using the syntax error of JSON data with unquoted hex numbers, as JSON is a standardized format which should adhere to specifications.

[hiperiondev]

Yes, the links above shows that such a code change was merged. I'd still recommend against using the syntax error of JSON data with unquoted hex numbers, as JSON is a standardized format which should adhere to specifications.

Then this issue should be closed?