Skip to content

Commit 40a4f80

Browse files
gkodinovgkodinov
committed
Bug#34897517: Can't set report_host to NULL
Setting variable values on the command line doesn't support DB NULL values. Hence it should not be allowed to persist a DB NULL as a values for one of the variables that takes its persisted values from the command line at startup. Added a check and an error message at SET PERSIST[_ONLY] time. Added a list of exempted variables. Change-Id: Ic95cd946314cd31fe2f0217d28a20b45ea674e3e
1 parent 2cd10ce commit 40a4f80

10 files changed

+150
-62
lines changed

‎mysql-test/include/persist_only_variables.sql

+3-3
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ set @@persist_only.table_open_cache_instances=16;
108108
set @@persist_only.thread_handling='one-thread-per-connection';
109109
set @@persist_only.thread_stack=286720;
110110
set @@persist_only.tls_version='TLSv1.2,TLSv1.3';
111-
set @@persist_only.report_host=NULL;
111+
set @@persist_only.report_host='';
112112
set @@persist_only.report_port=21000;
113-
set @@persist_only.report_password=NULL;
114-
set @@persist_only.report_user=NULL;
113+
set @@persist_only.report_password='';
114+
set @@persist_only.report_user='';

‎mysql-test/r/persisted_variables_bugs.result

+10
Original file line numberDiff line numberDiff line change
@@ -521,4 +521,14 @@ RESET PERSIST test_component.bool_ro_sys_var;
521521
RESET PERSIST replica_preserve_commit_order;
522522
SET GLOBAL replica_preserve_commit_order = DEFAULT;
523523
UNINSTALL COMPONENT "file://component_test_sys_var_service";
524+
#
525+
# Bug #34897517: Can't set report_host to NULL
526+
#
527+
SET PERSIST_ONLY report_host = NULL;
528+
ERROR HY000: Trying to persist a NULL into a variable 'report_host' that's settable only on command line is not supported
529+
SET PERSIST_ONLY report_host = DEFAULT;
530+
ERROR HY000: Trying to persist a NULL into a variable 'report_host' that's settable only on command line is not supported
531+
SET PERSIST ssl_crl = NULL;
532+
RESET PERSIST ssl_crl;
533+
SET GLOBAL ssl_crl = DEFAULT;
524534
# End of the 8.0 tests

‎mysql-test/r/read_only_persisted_plugin_variables.result

+3-3
Original file line numberDiff line numberDiff line change
@@ -103,10 +103,10 @@ set @@persist_only.table_open_cache_instances=16;
103103
set @@persist_only.thread_handling='one-thread-per-connection';
104104
set @@persist_only.thread_stack=286720;
105105
set @@persist_only.tls_version='TLSv1.2,TLSv1.3';
106-
set @@persist_only.report_host=NULL;
106+
set @@persist_only.report_host='';
107107
set @@persist_only.report_port=21000;
108-
set @@persist_only.report_password=NULL;
109-
set @@persist_only.report_user=NULL;
108+
set @@persist_only.report_password='';
109+
set @@persist_only.report_user='';
110110
# Must return 100 rows.
111111
SELECT count(*) from performance_schema.persisted_variables;
112112
count(*)

‎mysql-test/t/persisted_variables_bugs.test

+15
Original file line numberDiff line numberDiff line change
@@ -551,4 +551,19 @@ SET GLOBAL replica_preserve_commit_order = DEFAULT;
551551
UNINSTALL COMPONENT "file://component_test_sys_var_service";
552552

553553

554+
--echo #
555+
--echo # Bug #34897517: Can't set report_host to NULL
556+
--echo #
557+
558+
--error ER_NULL_CANT_BE_PERSISTED_FOR_READONLY
559+
SET PERSIST_ONLY report_host = NULL;
560+
561+
--error ER_NULL_CANT_BE_PERSISTED_FOR_READONLY
562+
SET PERSIST_ONLY report_host = DEFAULT;
563+
564+
SET PERSIST ssl_crl = NULL;
565+
RESET PERSIST ssl_crl;
566+
SET GLOBAL ssl_crl = DEFAULT;
567+
568+
554569
--echo # End of the 8.0 tests

‎share/messages_to_clients.txt

+3
Original file line numberDiff line numberDiff line change
@@ -9980,6 +9980,9 @@ ER_EXPLAIN_INTO_IMPLICIT_FORMAT_NOT_SUPPORTED 0A000
99809980
ER_EXPLAIN_INTO_FORMAT_NOT_SUPPORTED 0A000
99819981
eng "EXPLAIN INTO does not support FORMAT=%s."
99829982

9983+
ER_NULL_CANT_BE_PERSISTED_FOR_READONLY
9984+
eng "Trying to persist a NULL into a variable '%-.192s' that's settable only on command line is not supported"
9985+
99839986
#
99849987
# End of 8.0 error messages (server-to-client).
99859988
# Do NOT add messages intended for the error log above!

‎sql/persisted_variable.cc

+1
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,7 @@ bool Persisted_variables_cache::set_variable(THD *thd, set_var *setvar) {
391391
String bool_str;
392392
if (setvar->value) {
393393
res = setvar->value->val_str(&str);
394+
if (setvar->value->is_null()) is_null = true;
394395
if (system_var->get_var_type() == GET_BOOL) {
395396
if (res == nullptr ||
396397
check_boolean_value(res->c_ptr_quick(), bool_str)) {

‎sql/set_var.cc

+66-1
Original file line numberDiff line numberDiff line change
@@ -1288,7 +1288,9 @@ void System_variable_tracker::cache_metadata(THD *thd, sys_var *v) const {
12881288
if (m_cache.has_value()) {
12891289
return;
12901290
}
1291-
m_cache = Cache{v->show_type(), v->is_sensitive()};
1291+
m_cache = Cache{
1292+
v->show_type(), v->is_sensitive(),
1293+
v->is_persist_readonly() || v->is_readonly() || v->is_parse_early()};
12921294
if (v->is_sensitive()) {
12931295
thd->lex->set_rewrite_required();
12941296
}
@@ -1442,6 +1444,69 @@ int sql_set_variables(THD *thd, List<set_var_base> *var_list, bool opened) {
14421444
it.rewind();
14431445
while ((var = it++)) {
14441446
if ((error = var->check(thd))) goto err;
1447+
1448+
set_var *setvar = dynamic_cast<set_var *>(var);
1449+
if (setvar &&
1450+
(setvar->type == OPT_PERSIST || setvar->type == OPT_PERSIST_ONLY) &&
1451+
setvar->m_var_tracker.cached_is_applied_as_command_line()) {
1452+
/*
1453+
There are certain variables that can process NULL as a default value
1454+
TODO: there should be no exceptions!
1455+
*/
1456+
static const std::set<std::string> exceptions = {"basedir",
1457+
"character_sets_dir",
1458+
"ft_stopword_file",
1459+
"lc_messages_dir",
1460+
"plugin_dir",
1461+
"relay_log",
1462+
"relay_log_info_file",
1463+
"replica_load_tmpdir",
1464+
"socket",
1465+
"tmpdir",
1466+
"init_file",
1467+
"admin_ssl_ca",
1468+
"admin_ssl_capath",
1469+
"admin_ssl_cert",
1470+
"admin_ssl_cipher",
1471+
"admin_tls_ciphersuites",
1472+
"admin_ssl_key",
1473+
"admin_ssl_crl",
1474+
"admin_ssl_crlpath",
1475+
"ssl_ca",
1476+
"ssl_capath",
1477+
"ssl_cert",
1478+
"ssl_cipher",
1479+
"tls_ciphersuites",
1480+
"ssl_key",
1481+
"ssl_crl",
1482+
"ssl_crlpath"};
1483+
1484+
if (setvar->value && setvar->value->is_null() &&
1485+
exceptions.find(setvar->m_var_tracker.get_var_name()) ==
1486+
exceptions.end()) {
1487+
/* an explicit NULL value */
1488+
my_error(ER_NULL_CANT_BE_PERSISTED_FOR_READONLY, MYF(0),
1489+
setvar->m_var_tracker.get_var_name());
1490+
error = 1;
1491+
goto err;
1492+
} else if (!setvar->value &&
1493+
setvar->m_var_tracker.cached_show_type() == SHOW_CHAR_PTR &&
1494+
exceptions.find(setvar->m_var_tracker.get_var_name()) ==
1495+
exceptions.end()) {
1496+
/* SET = DEFAULT for a CHARPTR variable, check the default value */
1497+
auto f = [&error, thd](const System_variable_tracker &, sys_var *lvar) {
1498+
assert(lvar->show_type() == SHOW_CHAR_PTR);
1499+
char *ptr = (char *)(intptr)lvar->get_option()->def_value;
1500+
if (!ptr) {
1501+
my_error(ER_NULL_CANT_BE_PERSISTED_FOR_READONLY, MYF(0),
1502+
lvar->name.str);
1503+
error = 1;
1504+
}
1505+
};
1506+
setvar->m_var_tracker.access_system_variable(thd, f);
1507+
if (error) goto err;
1508+
}
1509+
}
14451510
}
14461511
if ((error = thd->is_error())) goto err;
14471512

‎sql/set_var.h

+6
Original file line numberDiff line numberDiff line change
@@ -846,6 +846,11 @@ class System_variable_tracker final {
846846
return m_cache.value().m_cached_is_sensitive;
847847
}
848848

849+
bool cached_is_applied_as_command_line() const {
850+
if (!m_cache.has_value()) my_abort();
851+
return m_cache.value().m_cached_is_applied_as_command_line;
852+
}
853+
849854
/** Number of system variable elements to preallocate. */
850855
static constexpr size_t SYSTEM_VARIABLE_PREALLOC = 200;
851856

@@ -893,6 +898,7 @@ class System_variable_tracker final {
893898
struct Cache {
894899
SHOW_TYPE m_cached_show_type;
895900
bool m_cached_is_sensitive;
901+
bool m_cached_is_applied_as_command_line;
896902
};
897903
mutable std::optional<Cache> m_cache;
898904

‎sql/sql_plugin_var.cc

+40-54
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,45 @@ SHOW_TYPE pluginvar_show_type(SYS_VAR *plugin_var) {
183183
}
184184
}
185185

186+
const void *pluginvar_default_value(SYS_VAR *plugin_var) {
187+
switch (plugin_var->flags & (PLUGIN_VAR_TYPEMASK | PLUGIN_VAR_THDLOCAL)) {
188+
case PLUGIN_VAR_INT:
189+
return &((sysvar_uint_t *)plugin_var)->def_val;
190+
case PLUGIN_VAR_LONG:
191+
return &((sysvar_ulong_t *)plugin_var)->def_val;
192+
case PLUGIN_VAR_LONGLONG:
193+
return &((sysvar_ulonglong_t *)plugin_var)->def_val;
194+
case PLUGIN_VAR_ENUM:
195+
return &((sysvar_enum_t *)plugin_var)->def_val;
196+
case PLUGIN_VAR_SET:
197+
return &((sysvar_set_t *)plugin_var)->def_val;
198+
case PLUGIN_VAR_BOOL:
199+
return &((sysvar_bool_t *)plugin_var)->def_val;
200+
case PLUGIN_VAR_STR:
201+
return &((sysvar_str_t *)plugin_var)->def_val;
202+
case PLUGIN_VAR_DOUBLE:
203+
return &((sysvar_double_t *)plugin_var)->def_val;
204+
case PLUGIN_VAR_INT | PLUGIN_VAR_THDLOCAL:
205+
return &((thdvar_uint_t *)plugin_var)->def_val;
206+
case PLUGIN_VAR_LONG | PLUGIN_VAR_THDLOCAL:
207+
return &((thdvar_ulong_t *)plugin_var)->def_val;
208+
case PLUGIN_VAR_LONGLONG | PLUGIN_VAR_THDLOCAL:
209+
return &((thdvar_ulonglong_t *)plugin_var)->def_val;
210+
case PLUGIN_VAR_ENUM | PLUGIN_VAR_THDLOCAL:
211+
return &((thdvar_enum_t *)plugin_var)->def_val;
212+
case PLUGIN_VAR_SET | PLUGIN_VAR_THDLOCAL:
213+
return &((thdvar_set_t *)plugin_var)->def_val;
214+
case PLUGIN_VAR_BOOL | PLUGIN_VAR_THDLOCAL:
215+
return &((thdvar_bool_t *)plugin_var)->def_val;
216+
case PLUGIN_VAR_STR | PLUGIN_VAR_THDLOCAL:
217+
return &((thdvar_str_t *)plugin_var)->def_val;
218+
case PLUGIN_VAR_DOUBLE | PLUGIN_VAR_THDLOCAL:
219+
return &((thdvar_double_t *)plugin_var)->def_val;
220+
default:
221+
return nullptr;
222+
}
223+
}
224+
186225
/*
187226
returns a pointer to the memory which holds the thd-local variable or
188227
a pointer to the global variable if thd==null.
@@ -366,60 +405,7 @@ bool sys_var_pluginvar::global_update(THD *thd, set_var *var) {
366405
void *tgt = real_value_ptr(thd, var->type);
367406
const void *src = &var->save_result;
368407

369-
if (!var->value) {
370-
switch (plugin_var->flags & (PLUGIN_VAR_TYPEMASK | PLUGIN_VAR_THDLOCAL)) {
371-
case PLUGIN_VAR_INT:
372-
src = &((sysvar_uint_t *)plugin_var)->def_val;
373-
break;
374-
case PLUGIN_VAR_LONG:
375-
src = &((sysvar_ulong_t *)plugin_var)->def_val;
376-
break;
377-
case PLUGIN_VAR_LONGLONG:
378-
src = &((sysvar_ulonglong_t *)plugin_var)->def_val;
379-
break;
380-
case PLUGIN_VAR_ENUM:
381-
src = &((sysvar_enum_t *)plugin_var)->def_val;
382-
break;
383-
case PLUGIN_VAR_SET:
384-
src = &((sysvar_set_t *)plugin_var)->def_val;
385-
break;
386-
case PLUGIN_VAR_BOOL:
387-
src = &((sysvar_bool_t *)plugin_var)->def_val;
388-
break;
389-
case PLUGIN_VAR_STR:
390-
src = &((sysvar_str_t *)plugin_var)->def_val;
391-
break;
392-
case PLUGIN_VAR_DOUBLE:
393-
src = &((sysvar_double_t *)plugin_var)->def_val;
394-
break;
395-
case PLUGIN_VAR_INT | PLUGIN_VAR_THDLOCAL:
396-
src = &((thdvar_uint_t *)plugin_var)->def_val;
397-
break;
398-
case PLUGIN_VAR_LONG | PLUGIN_VAR_THDLOCAL:
399-
src = &((thdvar_ulong_t *)plugin_var)->def_val;
400-
break;
401-
case PLUGIN_VAR_LONGLONG | PLUGIN_VAR_THDLOCAL:
402-
src = &((thdvar_ulonglong_t *)plugin_var)->def_val;
403-
break;
404-
case PLUGIN_VAR_ENUM | PLUGIN_VAR_THDLOCAL:
405-
src = &((thdvar_enum_t *)plugin_var)->def_val;
406-
break;
407-
case PLUGIN_VAR_SET | PLUGIN_VAR_THDLOCAL:
408-
src = &((thdvar_set_t *)plugin_var)->def_val;
409-
break;
410-
case PLUGIN_VAR_BOOL | PLUGIN_VAR_THDLOCAL:
411-
src = &((thdvar_bool_t *)plugin_var)->def_val;
412-
break;
413-
case PLUGIN_VAR_STR | PLUGIN_VAR_THDLOCAL:
414-
src = &((thdvar_str_t *)plugin_var)->def_val;
415-
break;
416-
case PLUGIN_VAR_DOUBLE | PLUGIN_VAR_THDLOCAL:
417-
src = &((thdvar_double_t *)plugin_var)->def_val;
418-
break;
419-
default:
420-
assert(0);
421-
}
422-
}
408+
if (!var->value) src = pluginvar_default_value(plugin_var);
423409

424410
if ((plugin_var->flags & PLUGIN_VAR_TYPEMASK) == PLUGIN_VAR_STR &&
425411
plugin_var->flags & PLUGIN_VAR_MEMALLOC) {

‎sql/sql_plugin_var.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ void update_func_str(THD *, SYS_VAR *, void *tgt, const void *save);
141141
void update_func_double(THD *, SYS_VAR *, void *tgt, const void *save);
142142

143143
SHOW_TYPE pluginvar_show_type(SYS_VAR *plugin_var);
144+
const void *pluginvar_default_value(SYS_VAR *plugin_var);
144145

145146
int item_value_type(st_mysql_value *value);
146147
const char *item_val_str(st_mysql_value *value, char *buffer, int *length);
@@ -248,7 +249,8 @@ class sys_var_pluginvar : public sys_var {
248249
: (plugin_var_arg->flags & PLUGIN_VAR_RQCMDARG
249250
? REQUIRED_ARG
250251
: REQUIRED_ARG))),
251-
pluginvar_show_type(plugin_var_arg), 0, nullptr,
252+
pluginvar_show_type(plugin_var_arg),
253+
(intptr)pluginvar_default_value(plugin_var_arg), nullptr,
252254
VARIABLE_NOT_IN_BINLOG,
253255
(plugin_var_arg->flags & PLUGIN_VAR_NODEFAULT) ? on_check_pluginvar
254256
: nullptr,

0 commit comments

Comments
 (0)