Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/json_lib.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ enum json_errors {
JE_ESCAPING= -6, /* Error in the escaping. */

JE_DEPTH= -7, /* The limit on the JSON depth was overrun. */
JE_KILLED= -8, /* Killed during processing */
};


Expand Down
4 changes: 4 additions & 0 deletions mysql-test/main/func_json.result
Original file line number Diff line number Diff line change
Expand Up @@ -1779,6 +1779,8 @@ Warning 4042 Syntax error in JSON path in argument 2 to function 'json_extract'
SELECT JSON_EXISTS(CONCAT('[', REPEAT('[', 4000), 'Y', REPEAT(']', 4000), ', 1]'), '$[100]') as ex;
ex
NULL
Warnings:
Warning 4040 Limit of 32 on JSON nested structures depth is reached in argument 1 to function 'json_exists' at position 131
#
# MDEV-35548 UBSAN: runtime error: index -1 out of bounds for type 'json_path_step_t[32]'
# (aka 'struct st_json_path_step_t[32]')
Expand All @@ -1794,6 +1796,8 @@ JSON_EXTRACT('0E+0','$')
SELECT JSON_EXISTS(CONCAT('[', REPEAT('[', 4000), 'Y', REPEAT(']', 4000), ', 1]'), '$[100]') as je;
je
NULL
Warnings:
Warning 4040 Limit of 32 on JSON nested structures depth is reached in argument 1 to function 'json_exists' at position 131
# End of 10.6 tests
SELECT json_extract(t.j, '$')
FROM (SELECT json_extract('["a",1]', '$') AS j) AS t;
Expand Down
14 changes: 14 additions & 0 deletions mysql-test/main/func_json_notembedded.result
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,18 @@ select json_insert(@obj, '$.meta', 1);
ERROR 70100: Query execution was interrupted (max_statement_time exceeded)
select json_compact(@arr);
ERROR 70100: Query execution was interrupted (max_statement_time exceeded)
select json_contains(@obj, '"d"', '$.c');
ERROR 70100: Query execution was interrupted (max_statement_time exceeded)
select json_contains_path(@obj, 'one', '$.c');
ERROR 70100: Query execution was interrupted (max_statement_time exceeded)
select json_exists(@obj, '$.c');
ERROR 70100: Query execution was interrupted (max_statement_time exceeded)
select json_extract(@obj, '$.c');
ERROR 70100: Query execution was interrupted (max_statement_time exceeded)
select json_detailed(@arr);
ERROR 70100: Query execution was interrupted (max_statement_time exceeded)
select json_keys(@obj, '$.c');
ERROR 70100: Query execution was interrupted (max_statement_time exceeded)
select json_loose(@arr);
ERROR 70100: Query execution was interrupted (max_statement_time exceeded)
select json_merge(@obj, @arr);
Expand All @@ -30,12 +40,16 @@ select json_merge_patch(@obj, @obj);
ERROR 70100: Query execution was interrupted (max_statement_time exceeded)
select json_merge_preserve(@obj, @arr);
ERROR 70100: Query execution was interrupted (max_statement_time exceeded)
select json_overlaps(@obj, @arr);
ERROR 70100: Query execution was interrupted (max_statement_time exceeded)
select json_remove(@obj,'$.foo');
ERROR 70100: Query execution was interrupted (max_statement_time exceeded)
select json_replace(@obj,'$.foo',1);
ERROR 70100: Query execution was interrupted (max_statement_time exceeded)
select json_set(@arr,'$[1000]',1);
ERROR 70100: Query execution was interrupted (max_statement_time exceeded)
select ST_AsTEXT(ST_GeomFromGeoJSON(JSON_OBJECT("type", "Point", "coordinates", @arr),2)) as exp;
ERROR 70100: Query execution was interrupted (max_statement_time exceeded)
SET debug_dbug= @old_debug;
disconnect u;
connection default;
Expand Down
7 changes: 7 additions & 0 deletions mysql-test/main/func_json_notembedded.test
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,21 @@ select json_array_append(@arr, '$[0]', 1);
select json_array_insert(@arr, '$[0]', 1);
select json_insert(@obj, '$.meta', 1);
select json_compact(@arr);
select json_contains(@obj, '"d"', '$.c');
select json_contains_path(@obj, 'one', '$.c');
select json_exists(@obj, '$.c');
select json_extract(@obj, '$.c');
select json_detailed(@arr);
select json_keys(@obj, '$.c');
select json_loose(@arr);
select json_merge(@obj, @arr);
select json_merge_patch(@obj, @obj);
select json_merge_preserve(@obj, @arr);
select json_overlaps(@obj, @arr);
select json_remove(@obj,'$.foo');
select json_replace(@obj,'$.foo',1);
select json_set(@arr,'$[1000]',1);
select ST_AsTEXT(ST_GeomFromGeoJSON(JSON_OBJECT("type", "Point", "coordinates", @arr),2)) as exp;
enable_abort_on_error;
SET debug_dbug= @old_debug;
disconnect u;
Expand Down
1 change: 1 addition & 0 deletions sql/item_geofunc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ String *Item_func_geometry_from_json::val_str(String *str)

json_scan_start(&je, js->charset(), (const uchar *) js->ptr(),
(const uchar *) js->end());
je.killed_ptr= (uint32_t *) &current_thd->killed;

if ((null_value= !Geometry::create_from_json(&buffer, &je, options==1, str)))
{
Expand Down
53 changes: 40 additions & 13 deletions sql/item_jsonfunc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,10 @@ void report_json_error_ex(const char *js, json_engine_t *je,
n_param, fname, position);
return;

case JE_KILLED:
thd->send_kill_message();
return;

default:
return;
}
Expand Down Expand Up @@ -668,6 +672,9 @@ bool Item_func_json_exists::val_bool()
{
json_engine_t je;
int array_counters[JSON_DEPTH_LIMIT]= {0};
THD *thd= current_thd;

JSON_DO_PAUSE_EXECUTION(thd, 0.0002);

String *js= args[0]->val_json(&tmp_js);

Expand All @@ -690,17 +697,20 @@ bool Item_func_json_exists::val_bool()
null_value= 0;
json_scan_start(&je, js->charset(),(const uchar *) js->ptr(),
(const uchar *) js->ptr() + js->length());
je.killed_ptr= (uint32_t *) &thd->killed;

path.cur_step= path.p.steps;
if (json_find_path(&je, &path.p, &path.cur_step, array_counters))
{
if (je.s.error)
goto err_return;
goto js_error;
return 0;
}

return 1;

js_error:
report_json_error(js, &je, 0);
err_return:
null_value= 1;
return 0;
Expand Down Expand Up @@ -904,6 +914,7 @@ String *Item_func_json_unquote::read_json(json_engine_t *je)
if ((null_value= args[0]->null_value))
return 0;

/* no json_scan_next called so not interuptable */
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This comment is incorrect. Item_func_json_unquote::read_json returns the engine to val_str, which then calls json_read_value(&je). Since json_read_value internally uses json_scan_next, this path is indeed interruptible and je.killed_ptr must be initialized to avoid a crash (due to the missing NULL check in json_lib.c) or a lack of interruptibility.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/gemini - I'm pretty sure that json_read_value doesn't call json_scan_next.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct that Item_func_json_unquote::read_json itself does not call json_scan_next (which is why the comment you added there is accurate). However, json_read_value is the core function used to parse JSON values, and it internally iterates through tokens by calling json_scan_next. Because json_read_value is used in many other functions (like Item_func_json_contains::val_bool), those functions become interruptible once je.killed_ptr is initialized. If you look at the implementation of json_read_value in strings/json_lib.c, you will see the while ((t= json_scan_next(j)) > 0) loop that performs the parsing. Therefore, initializing killed_ptr before calling json_read_value is necessary to ensure those functions respect the KILL signal.

json_scan_start(je, js->charset(),(const uchar *) js->ptr(),
(const uchar *) js->ptr() + js->length());

Expand Down Expand Up @@ -1114,6 +1125,9 @@ String *Item_func_json_extract::read_json(String *str,
int possible_multiple_values;
int array_size_counter[JSON_DEPTH_LIMIT];
uint has_negative_path= 0;
THD *thd= current_thd;

JSON_DO_PAUSE_EXECUTION(thd, 0.0002);

if ((null_value= args[0]->null_value))
return 0;
Expand Down Expand Up @@ -1237,6 +1251,8 @@ String *Item_func_json_extract::read_json(String *str,
js= str;
json_scan_start(&je, js->charset(),(const uchar *) js->ptr(),
(const uchar *) js->ptr() + js->length());
je.killed_ptr= (uint32_t *) &thd->killed;

if (json_nice(&je, &tmp_js, Item_func_json_format::LOOSE))
goto error;

Expand Down Expand Up @@ -1532,6 +1548,9 @@ bool Item_func_json_contains::val_bool()
String *js= args[0]->val_json(&tmp_js);
json_engine_t je, ve;
int result;
THD *thd= current_thd;

JSON_DO_PAUSE_EXECUTION(thd, 0.0002);

if ((null_value= args[0]->null_value))
return 0;
Expand All @@ -1550,6 +1569,7 @@ bool Item_func_json_contains::val_bool()

json_scan_start(&je, js->charset(),(const uchar *) js->ptr(),
(const uchar *) js->ptr() + js->length());
je.killed_ptr= (uint32_t *) &thd->killed;

if (arg_count>2) /* Path specified. */
{
Expand Down Expand Up @@ -1584,6 +1604,7 @@ bool Item_func_json_contains::val_bool()

json_scan_start(&ve, val->charset(),(const uchar *) val->ptr(),
(const uchar *) val->end());
ve.killed_ptr= (uint32_t *) &thd->killed;

if (json_read_value(&je) || json_read_value(&ve))
goto error;
Expand Down Expand Up @@ -1680,6 +1701,9 @@ longlong Item_func_json_contains_path::val_int()
json_engine_t je;
uint n_arg;
longlong result;
THD *thd= current_thd;

JSON_DO_PAUSE_EXECUTION(thd, 0.0002);

if ((null_value= args[0]->null_value))
return 0;
Expand Down Expand Up @@ -1713,6 +1737,7 @@ longlong Item_func_json_contains_path::val_int()

json_scan_start(&je, js->charset(),(const uchar *) js->ptr(),
(const uchar *) js->ptr() + js->length());
je.killed_ptr= (uint32_t *) &thd->killed;

c_path->cur_step= c_path->p.steps;
if (json_find_path(&je, &c_path->p, &c_path->cur_step, array_counters))
Expand Down Expand Up @@ -1760,6 +1785,9 @@ bool Item_func_json_contains_path::val_bool()
int UNINIT_VAR(n_found);
int array_sizes[JSON_DEPTH_LIMIT];
uint has_negative_path= 0;
THD *thd= current_thd;

JSON_DO_PAUSE_EXECUTION(thd, 0.0002);

if ((null_value= args[0]->null_value))
return 0;
Expand Down Expand Up @@ -1792,7 +1820,7 @@ bool Item_func_json_contains_path::val_bool()

json_get_path_start(&je, js->charset(),(const uchar *) js->ptr(),
(const uchar *) js->ptr() + js->length(), &p);

je.killed_ptr= (uint32_t *) &thd->killed;

if (!mode_one)
{
Expand Down Expand Up @@ -2210,8 +2238,6 @@ String *Item_func_json_array_append::val_str(String *str)
report_json_error(js, &je, 0);

return_null:
/* intentionally after return_null label */
thd->check_killed();

null_value= 1;
Comment thread
grooverdan marked this conversation as resolved.
return 0;
Expand Down Expand Up @@ -2731,7 +2757,6 @@ String *Item_func_json_merge::val_str(String *str)
report_json_error(js1, &je1, 0);
if (je2.s.error)
report_json_error(js2, &je2, n_arg);
thd->check_killed(); // to get the error message right
null_return:
null_value= 1;
return NULL;
Expand Down Expand Up @@ -3067,7 +3092,6 @@ String *Item_func_json_merge_patch::val_str(String *str)
report_json_error(js1, &je1, 0);
if (je2.s.error)
report_json_error(js2, &je2, n_arg);
thd->check_killed(); // to get the error message right
null_return:
null_value= 1;
return NULL;
Expand Down Expand Up @@ -3163,7 +3187,6 @@ longlong Item_func_json_length::val_int()

err_return:
report_json_error(js, &je, 0);
current_thd->check_killed(); // to get the error message right
null_return:
null_value= 1;
return 0;
Expand Down Expand Up @@ -3218,7 +3241,6 @@ longlong Item_func_json_depth::val_int()
return depth;

report_json_error(js, &je, 0);
current_thd->check_killed(); // to get the error message right
null_value= 1;
return 0;
}
Expand Down Expand Up @@ -3284,7 +3306,6 @@ String *Item_func_json_type::val_str(String *str)

error:
report_json_error(js, &je, 0);
current_thd->check_killed();
null_value= 1;
return 0;
}
Expand Down Expand Up @@ -3563,7 +3584,6 @@ String *Item_func_json_insert::val_str(String *str)

js_error:
report_json_error(js, &je, 0);
thd->check_killed(); // to get the error message right
return_null:
null_value= 1;
return 0;
Expand Down Expand Up @@ -3765,7 +3785,6 @@ String *Item_func_json_remove::val_str(String *str)
return str;

js_error:
thd->check_killed(); // to get the error message right
report_json_error(js, &je, 0);
null_return:
null_value= 1;
Expand Down Expand Up @@ -3825,12 +3844,16 @@ String *Item_func_json_keys::val_str(String *str)
String *js= args[0]->val_json(&tmp_js);
uint n_keys= 0;
int array_counters[JSON_DEPTH_LIMIT]= {0};
THD *thd= current_thd;

JSON_DO_PAUSE_EXECUTION(thd, 0.0002);

if ((args[0]->null_value))
goto null_return;

json_scan_start(&je, js->charset(),(const uchar *) js->ptr(),
(const uchar *) js->ptr() + js->length());
je.killed_ptr= (uint32_t *) &thd->killed;

if (arg_count < 2)
goto skip_search;
Expand Down Expand Up @@ -4226,7 +4249,6 @@ String *Item_func_json_format::val_str(String *str)
{
null_value= 1;
report_json_error(js, &je, 0);
thd->check_killed(); // to get the error message right
return 0;
}

Expand All @@ -4250,6 +4272,7 @@ int Arg_comparator::compare_json_str_basic(Item *j, Item *s)

if ((js= j->val_str(&value1)))
{
/* doesn't appear to json_scan_next so not interuptable */
Comment thread
grooverdan marked this conversation as resolved.
json_scan_start(&je, js->charset(), (const uchar *) js->ptr(),
(const uchar *) js->ptr()+js->length());
if (json_read_value(&je))
Expand Down Expand Up @@ -4958,6 +4981,9 @@ bool Item_func_json_overlaps::val_bool()
String *js= args[0]->val_json(&tmp_js);
json_engine_t je, ve;
int result;
THD *thd= current_thd;

JSON_DO_PAUSE_EXECUTION(thd, 0.0002);

if ((null_value= args[0]->null_value))
return 0;
Expand All @@ -4976,9 +5002,11 @@ bool Item_func_json_overlaps::val_bool()

json_scan_start(&je, js->charset(), (const uchar *) js->ptr(),
(const uchar *) js->ptr() + js->length());
je.killed_ptr= (uint32_t *) &thd->killed;

json_scan_start(&ve, val->charset(), (const uchar *) val->ptr(),
(const uchar *) val->end());
ve.killed_ptr= (uint32_t *) &thd->killed;

if (json_read_value(&je) || json_read_value(&ve))
goto error;
Expand All @@ -4994,7 +5022,6 @@ bool Item_func_json_overlaps::val_bool()
report_json_error(js, &je, 0);
if (ve.s.error)
report_json_error(val, &ve, 1);
current_thd->check_killed(); // to get the error message right
return 0;
}

Expand Down
7 changes: 6 additions & 1 deletion strings/json_lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -974,7 +974,12 @@ int json_scan_next(json_engine_t *j)
int t_next;

get_first_nonspace(&j->s, &t_next, &j->sav_c_len);
return *j->killed_ptr || json_actions[j->state][t_next](j);
if (j->killed_ptr && *j->killed_ptr)
{
j->s.error= JE_KILLED;
return 1;
}
return json_actions[j->state][t_next](j);
}


Expand Down