revision-id: c231e2f986bba5d5d92225fd062966e08d530391 (mariadb-10.6.1-163-gc231e2f986b) parent(s): e3ad985cbb83fb24627f09918ed0e39eaa9f522d author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2021-10-10 11:51:04 +0300 message: MDEV-26724 Endless loop in json_escape_to_string upon ... empty string Part#3: - make json_escape() return different errors on conversion error and on out-of-space condition. - Make histogram code handle conversion errors. --- include/json_lib.h | 14 ++++++++++---- mysql-test/main/statistics_json.result | 16 ++++++++++++++++ mysql-test/main/statistics_json.test | 14 ++++++++++++++ sql/opt_histogram_json.cc | 5 ++++- sql/sql_statistics.cc | 24 +++++++++++++++++------- strings/json_lib.c | 8 ++++---- 6 files changed, 65 insertions(+), 16 deletions(-) diff --git a/include/json_lib.h b/include/json_lib.h index e9f3deea415..34385bd8217 100644 --- a/include/json_lib.h +++ b/include/json_lib.h @@ -382,6 +382,9 @@ int json_find_paths_first(json_engine_t *je, json_find_paths_t *state, int json_find_paths_next(json_engine_t *je, json_find_paths_t *state); +#define JSON_ERROR_OUT_OF_SPACE (-1) +#define JSON_ERROR_ILLEGAL_SYMBOL (-2) + /* Converst JSON string constant into ordinary string constant which can involve unpacking json escapes and changing character set. @@ -394,10 +397,13 @@ int json_unescape(CHARSET_INFO *json_cs, uchar *res, uchar *res_end); /* - Converst ordinary string constant into JSON string constant. - which can involve appropriate escaping and changing character set. - Returns negative integer in the case of an error, - the length of the result otherwise. + Convert a string constant into JSON string constant. + This can involve appropriate escaping and changing the character set. + Returns the length of the result on success, + on error returns a negative error code. + Some error codes: + JSON_ERROR_OUT_OF_SPACE Not enough space in the provided buffer + JSON_ERROR_ILLEGAL_SYMBOL Source symbol cannot be represented in JSON */ int json_escape(CHARSET_INFO *str_cs, const uchar *str, const uchar *str_end, CHARSET_INFO *json_cs, uchar *json, uchar *json_end); diff --git a/mysql-test/main/statistics_json.result b/mysql-test/main/statistics_json.result index 1edfa474453..5bcbd94939e 100644 --- a/mysql-test/main/statistics_json.result +++ b/mysql-test/main/statistics_json.result @@ -7927,6 +7927,22 @@ a Ñ drop table t1; # +# Another testcase: use a character that cannot be represented in utf8: +# +create table t1 ( a varchar(100) character set cp1251); +insert into t1 values ( _cp1251 x'88'),( _cp1251 x'98'); +analyze table t1 persistent for all; +Table Op Msg_type Msg_text +test.t1 analyze status Engine-independent statistics collected +test.t1 analyze status OK +# Command succeeded but no histogram was collected: +select hist_type, histogram +from mysql.column_stats +where db_name=database() and table_name='t1'; +hist_type histogram +NULL NULL +drop table t1; +# # ASAN use-after-poison my_strnxfrm_simple_internal / Histogram_json_hb::range_selectivity ... # (Just the testcase) # diff --git a/mysql-test/main/statistics_json.test b/mysql-test/main/statistics_json.test index 878c1344d8f..37629452ff4 100644 --- a/mysql-test/main/statistics_json.test +++ b/mysql-test/main/statistics_json.test @@ -228,6 +228,20 @@ where db_name=database() and table_name='t1'; select * from t1; drop table t1; +--echo # +--echo # Another testcase: use a character that cannot be represented in utf8: +--echo # +create table t1 ( a varchar(100) character set cp1251); +insert into t1 values ( _cp1251 x'88'),( _cp1251 x'98'); +analyze table t1 persistent for all; + +--echo # Command succeeded but no histogram was collected: +select hist_type, histogram +from mysql.column_stats +where db_name=database() and table_name='t1'; + +drop table t1; + --echo # --echo # ASAN use-after-poison my_strnxfrm_simple_internal / Histogram_json_hb::range_selectivity ... --echo # (Just the testcase) diff --git a/sql/opt_histogram_json.cc b/sql/opt_histogram_json.cc index 7d270ba7191..8bea920ecbf 100644 --- a/sql/opt_histogram_json.cc +++ b/sql/opt_histogram_json.cc @@ -81,7 +81,10 @@ static bool json_escape_to_string(const String *str, String* out) return false; // Ok } - // We get here if the escaped string didn't fit into memory. + if (res != JSON_ERROR_OUT_OF_SPACE) + return true; // Some conversion error + + // Out of space error. Try with a bigger buffer if (out->alloc(out->alloced_length()*2)) return true; } diff --git a/sql/sql_statistics.cc b/sql/sql_statistics.cc index 1a61244935f..e1b6380d965 100644 --- a/sql/sql_statistics.cc +++ b/sql/sql_statistics.cc @@ -1771,19 +1771,24 @@ class Count_distinct_field: public Sql_alloc @brief Calculate a histogram of the tree */ - void walk_tree_with_histogram(ha_rows rows) + bool walk_tree_with_histogram(ha_rows rows) { Histogram_base *hist= table_field->collected_stats->histogram; Histogram_builder *hist_builder= hist->create_builder(table_field, tree_key_length, rows); - tree->walk(table_field->table, histogram_build_walk, - (void *) hist_builder); + if (tree->walk(table_field->table, histogram_build_walk, + (void*)hist_builder)) + { + delete hist_builder; + return true; // Error + } hist_builder->finalize(); distincts= hist_builder->counters.get_count_distinct(); distincts_single_occurence= hist_builder->counters. get_count_single_occurence(); delete hist_builder; + return false; } ulonglong get_count_distinct() @@ -2497,7 +2502,13 @@ void Column_statistics_collected::finish(MEM_ROOT *mem_root, ha_rows rows, if (!have_histogram) count_distinct->walk_tree(); else - count_distinct->walk_tree_with_histogram(rows - nulls); + { + if (count_distinct->walk_tree_with_histogram(rows - nulls)) + { + delete histogram; + histogram= NULL; + } + } ulonglong distincts= count_distinct->get_count_distinct(); ulonglong distincts_single_occurence= @@ -2535,12 +2546,11 @@ void Column_statistics_collected::finish(MEM_ROOT *mem_root, ha_rows rows, have_histogram= false; set_not_null(COLUMN_STAT_HIST_SIZE); - if (have_histogram && distincts) + if (have_histogram && distincts && histogram) { set_not_null(COLUMN_STAT_HIST_TYPE); - histogram= count_distinct->get_histogram(); set_not_null(COLUMN_STAT_HISTOGRAM); - } + } delete count_distinct; count_distinct= NULL; } diff --git a/strings/json_lib.c b/strings/json_lib.c index 58efac30dc0..0e7611528e1 100644 --- a/strings/json_lib.c +++ b/strings/json_lib.c @@ -1637,7 +1637,7 @@ int json_escape(CHARSET_INFO *str_cs, if (c_len < 0) { /* JSON buffer is depleted. */ - return -1; + return JSON_ERROR_OUT_OF_SPACE; } /* JSON charset cannot convert this character. */ @@ -1649,7 +1649,7 @@ int json_escape(CHARSET_INFO *str_cs, json+= c_len, json_end)) <= 0) { /* JSON buffer is depleted. */ - return -1; + return JSON_ERROR_OUT_OF_SPACE; } json+= c_len; @@ -1682,11 +1682,11 @@ int json_escape(CHARSET_INFO *str_cs, continue; } /* JSON buffer is depleted. */ - return -1; + return JSON_ERROR_OUT_OF_SPACE; } } else /* c_len == 0, an illegal symbol. */ - return -1; + return JSON_ERROR_ILLEGAL_SYMBOL; } return (int)(json - json_start);