[Asterisk-code-review] func_json: Enhance parsing capabilities of JSON_DECODE (asterisk[master])
Kevin Harwell
asteriskteam at digium.com
Fri Apr 8 16:27:24 CDT 2022
Attention is currently required from: N A, Joshua Colp.
Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/18012 )
Change subject: func_json: Enhance parsing capabilities of JSON_DECODE
......................................................................
Patch Set 4: Code-Review-1
(2 comments)
File funcs/func_json.c:
https://gerrit.asterisk.org/c/asterisk/+/18012/comment/53d1a2f3_05169c4e
PS4, Line 90: static int parse_node(char **key, char *currentkey, char *nestchar, int count, struct ast_json *json, char *buf, size_t len)
: {
: const char *result = NULL;
: char *previouskey;
: struct ast_json *jsonval = json;
:
: snprintf(buf, len, "%s", ""); /* clear the buffer from previous round if necessary */
: if (!json) { /* no error or warning should be thrown */
: ast_debug(1, "Could not find key '%s' in parsed JSON\n", currentkey);
: return -1;
: }
:
: switch(ast_json_typeof(jsonval)) {
: unsigned long int size;
: int r;
: char *result2;
:
: case AST_JSON_STRING:
: ast_debug(1, "Got JSON string\n");
: result = ast_json_string_get(jsonval);
: snprintf(buf, len, "%s", result);
: break;
: case AST_JSON_INTEGER:
: ast_debug(1, "Got JSON integer\n");
: r = ast_json_integer_get(jsonval);
: snprintf(buf, len, "%d", r); /* the snprintf below is mutually exclusive with this one */
: break;
: case AST_JSON_ARRAY:
: ast_debug(1, "Got JSON array\n");
: previouskey = currentkey;
: currentkey = strsep(key, nestchar); /* retrieve the desired index */
: size = ast_json_array_size(jsonval);
: ast_debug(1, "Parsed JSON array of size %lu, key: %s\n", size, currentkey);
: if (!currentkey) { /* this is the end, so just dump the array */
: if (count) {
: ast_debug(1, "No key on which to index in the array, so returning count: %lu\n", size);
: snprintf(buf, len, "%lu", size);
: return 0;
: } else {
: char *jsonarray;
: ast_debug(1, "No key on which to index in the array, so dumping '%s' array\n", previouskey);
: jsonarray = ast_json_dump_string(jsonval);
: ast_copy_string(buf, jsonarray, len);
: ast_json_free(jsonarray);
: }
: } else if (ast_str_to_int(currentkey, &r) || r < 0) {
: ast_debug(1, "Requested index '%s' is not numeric or is invalid\n", currentkey);
: } else if (r >= size) {
: ast_debug(1, "Requested index '%d' does not exist in parsed array\n", r);
: } else {
: struct ast_json *json2 = ast_json_array_get(jsonval, r);
: if (!json2) {
: ast_debug(1, "Array index %d contains empty item\n", r);
: return -1;
: }
: previouskey = currentkey;
: currentkey = strsep(key, nestchar); /* get the next subkey */
: ast_debug(1, "Recursing on index %d in array (key was '%s' and is now '%s')\n", r, previouskey, currentkey);
: /* json2 is a borrowed ref. That's fine, since json won't get freed until recursing is over */
: /* If there are keys remaining, then parse the next object we can get. Otherwise, just dump the child */
: if (parse_node(key, currentkey, nestchar, count, currentkey ? ast_json_object_get(json2, currentkey) : json2, buf, len)) { /* recurse on this node */
: return -1;
: }
: }
: break;
: default:
: ast_debug(1, "Got generic JSON object\n");
: result2 = ast_json_dump_string(jsonval);
: snprintf(buf, len, "%s", result2);
: ast_json_free(result2);
: }
: return 0;
: }
I think this function can be simplified, made more generic, and then moved into json.c.
For example here is some code that will recursively search a given json object, looking for the first given "key". If found it returns the associated value as a json object (NOTE, untested):
static struct ast_json *ast_json_find_by_key(struct ast_json *obj, const char *key)
{
struct ast_json *res;
struct ast_json_iter *iter;
size_t i;
switch(ast_json_typeof(obj)) {
case AST_JSON_OBJECT:
/* Breadth first */
res = ast_json_object_get(obj, key);
if (res) {
return res;
}
iter = ast_json_object_iter(obj);
while ((iter = ast_json_object_iter_next(obj, iter))) {
res = ast_json_find_by_key(ast_json_object_iter_value(iter), key);
if (res) {
return res;
}
}
break;
case AST_JSON_ARRAY:
for (i = 0; i < ast_json_array_size(obj); ++i) {
res = ast_json_find_by_key(ast_json_array_get(obj, i), key);
if (res) {
return res;
}
}
break;
default:
return obj;
}
return NULL;
}
Then from your code you can just do something like this:
struct ast_json *val;
char s_val;
val = ast_json_find_by_key(json, "hello");
if (!val) {
/* cleanup, log, and return */
}
s_val = ast_json_dump_string(val);
if (!s_val) {
/* cleanup, log, and return */
}
snprintf(buf, len, "%s", s_val);
/* cleanup and return */
Note too it'd be easy to add another function that calls the above to search over multiple keys.
Things to consider with this approach and your current one. Any time you recursively call another function you need to be careful not to "blow the stack". Since the JSON is coming from an external source someone may be able to craft the JSON in a way to crash your system. I'd probably add a max depth to the search to alleviate that problem.
https://gerrit.asterisk.org/c/asterisk/+/18012/comment/690061eb_8a802efe
PS4, Line 235: json = ast_json_load_str(str, NULL);
There is no reason you should have to load the json multiple times. You can craft things in a way to search, and return a json object and then search into that object if multiple keys are given. See above comment.
--
To view, visit https://gerrit.asterisk.org/c/asterisk/+/18012
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I603940b216a3911b498fc6583b18934011ef5d5b
Gerrit-Change-Number: 18012
Gerrit-PatchSet: 4
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-CC: Joshua Colp <jcolp at sangoma.com>
Gerrit-Attention: N A <mail at interlinked.x10host.com>
Gerrit-Attention: Joshua Colp <jcolp at sangoma.com>
Gerrit-Comment-Date: Fri, 08 Apr 2022 21:27:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220408/4376744d/attachment-0001.html>
More information about the asterisk-code-review
mailing list