Commit 32546877 authored by Tony Chen's avatar Tony Chen Committed by Daniel Black

MDEV-26923 Check all invalid config options

Previously, the behavior was to error out on the first invalid option
encountered. With this change, a best effort approach is made so that
all invalid options processed will be printed before exiting.

There is a caveat. The options are processed many times at varying
stages of server startup because the server is not aware of all valid
options immediately (e.g. plugins have to be loaded first before the
server knows what are the available plugin options). So, there are some
options that the server can determine are invalid "early" on, and there
are some options that the server cannot determine are invalid until
"later" on. For example, the server can determine an option such as
`--a` is an ambiguous option very early on but an option such as
`--this-does-not-match-any-option` cannot be labelled as invalid until
the server is aware of all available options.

Thus, it is possible that the server will still fail before printing out
all "invalid" options. You can see this by passing `--a
--obvious-invalid-option`.

Test cases were added to `mysqld_option_err.test` to validate that
multiple invalid options will be displayed in the error message.

All new code of the whole pull request, including one or several files
that are either new files or modified ones, are contributed under the
BSD-new license. I am contributing on behalf of my employer Amazon Web
Services.
parent 96966976
...@@ -3,6 +3,14 @@ Test bad binlog format. ...@@ -3,6 +3,14 @@ Test bad binlog format.
Test bad default storage engine. Test bad default storage engine.
Test non-numeric value passed to number option. Test non-numeric value passed to number option.
Test that bad value for plugin enum option is rejected correctly. Test that bad value for plugin enum option is rejected correctly.
Test to see if multiple unknown options will be displayed in the error output
unknown option '--nonexistentoption'
unknown option '--alsononexistent'
unknown variable 'nonexistentvariable=1'
Test to see if multiple ambiguous options and invalid arguments will be displayed in the error output
Error while setting value 'invalid_value' to 'sql_mode'
ambiguous option '--character' (character-set-client-handshake, character_sets_dir)
option '--bootstrap' cannot take an argument
Test that --help --verbose works Test that --help --verbose works
Test that --not-known-option --help --verbose gives error Test that --not-known-option --help --verbose gives error
Done. Done.
...@@ -46,6 +46,18 @@ mkdir $MYSQLTEST_VARDIR/tmp/mysqld_option_err; ...@@ -46,6 +46,18 @@ mkdir $MYSQLTEST_VARDIR/tmp/mysqld_option_err;
--error 7 --error 7
--exec $MYSQLD_BOOTSTRAP_CMD --skip-networking --datadir=$MYSQLTEST_VARDIR/tmp/mysqld_option_err --skip-grant-tables --plugin-dir=$MYSQLTEST_VARDIR/plugins --plugin-load=example=ha_example.so --plugin-example-enum-var=noexist >>$MYSQLTEST_VARDIR/tmp/mysqld_option_err/mysqltest.log 2>&1 --exec $MYSQLD_BOOTSTRAP_CMD --skip-networking --datadir=$MYSQLTEST_VARDIR/tmp/mysqld_option_err --skip-grant-tables --plugin-dir=$MYSQLTEST_VARDIR/plugins --plugin-load=example=ha_example.so --plugin-example-enum-var=noexist >>$MYSQLTEST_VARDIR/tmp/mysqld_option_err/mysqltest.log 2>&1
--echo Test to see if multiple unknown options will be displayed in the error output
# Remove the noise to make the test robust
--replace_regex /^((?!nonexistent).)*$// /.*unknown/unknown/
--error 7
--exec $MYSQLD_BOOTSTRAP_CMD --skip-networking --datadir=$MYSQLTEST_VARDIR/tmp/mysqld_option_err --skip-grant-tables --nonexistentoption --alsononexistent --nonexistentvariable=1 2>&1
--echo Test to see if multiple ambiguous options and invalid arguments will be displayed in the error output
# Remove the noise to make the test robust
--replace_regex /^((?!('sql_mode'|'--character'|'--bootstrap')).)*$// /.*Error while setting value/Error while setting value/ /.*ambiguous option/ambiguous option/ /.*option '--bootstrap'/option '--bootstrap'/
--error 1
--exec $MYSQLD_BOOTSTRAP_CMD --skip-networking --datadir=$MYSQLTEST_VARDIR/tmp/mysqld_option_err --skip-grant-tables --getopt-prefix-matching --sql-mode=invalid_value --character --bootstrap=partstoob 2>&1
# #
# Test that an wrong option with --help --verbose gives an error # Test that an wrong option with --help --verbose gives an error
# #
......
...@@ -133,6 +133,8 @@ double getopt_ulonglong2double(ulonglong v) ...@@ -133,6 +133,8 @@ double getopt_ulonglong2double(ulonglong v)
return u.dbl; return u.dbl;
} }
#define SET_HO_ERROR_AND_CONTINUE(e) { ho_error= (e); continue; }
/** /**
Handle command line options. Handle command line options.
Sort options. Sort options.
...@@ -202,7 +204,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -202,7 +204,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
const char *UNINIT_VAR(prev_found); const char *UNINIT_VAR(prev_found);
const struct my_option *optp; const struct my_option *optp;
void *value; void *value;
int error, i; int ho_error= 0, error, i;
my_bool is_cmdline_arg= 1; my_bool is_cmdline_arg= 1;
DBUG_ENTER("handle_options"); DBUG_ENTER("handle_options");
...@@ -216,7 +218,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -216,7 +218,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
is_cmdline_arg= !is_file_marker(**argv); is_cmdline_arg= !is_file_marker(**argv);
for (pos= *argv, pos_end=pos+ *argc; pos != pos_end ; pos++) for (pos= *argv, pos_end=pos+ *argc; pos < pos_end ; pos++)
{ {
char **first= pos; char **first= pos;
char *cur_arg= *pos; char *cur_arg= *pos;
...@@ -305,7 +307,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -305,7 +307,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
my_progname, special_opt_prefix[i], my_progname, special_opt_prefix[i],
opt_str, special_opt_prefix[i], opt_str, special_opt_prefix[i],
prev_found); prev_found);
DBUG_RETURN(EXIT_AMBIGUOUS_OPTION); SET_HO_ERROR_AND_CONTINUE(EXIT_AMBIGUOUS_OPTION)
} }
switch (i) { switch (i) {
case OPT_SKIP: case OPT_SKIP:
...@@ -350,7 +352,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -350,7 +352,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
"%s: unknown variable '%s'", "%s: unknown variable '%s'",
my_progname, cur_arg); my_progname, cur_arg);
if (!option_is_loose) if (!option_is_loose)
DBUG_RETURN(EXIT_UNKNOWN_VARIABLE); SET_HO_ERROR_AND_CONTINUE(EXIT_UNKNOWN_VARIABLE)
} }
else else
{ {
...@@ -360,7 +362,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -360,7 +362,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
"%s: unknown option '--%s'", "%s: unknown option '--%s'",
my_progname, cur_arg); my_progname, cur_arg);
if (!option_is_loose) if (!option_is_loose)
DBUG_RETURN(EXIT_UNKNOWN_OPTION); SET_HO_ERROR_AND_CONTINUE(EXIT_UNKNOWN_OPTION)
} }
if (option_is_loose) if (option_is_loose)
{ {
...@@ -377,7 +379,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -377,7 +379,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
my_getopt_error_reporter(ERROR_LEVEL, my_getopt_error_reporter(ERROR_LEVEL,
"%s: variable prefix '%s' is not unique", "%s: variable prefix '%s' is not unique",
my_progname, opt_str); my_progname, opt_str);
DBUG_RETURN(EXIT_VAR_PREFIX_NOT_UNIQUE); SET_HO_ERROR_AND_CONTINUE(EXIT_VAR_PREFIX_NOT_UNIQUE)
} }
else else
{ {
...@@ -386,7 +388,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -386,7 +388,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
"%s: ambiguous option '--%s' (%s, %s)", "%s: ambiguous option '--%s' (%s, %s)",
my_progname, opt_str, prev_found, my_progname, opt_str, prev_found,
optp->name); optp->name);
DBUG_RETURN(EXIT_AMBIGUOUS_OPTION); SET_HO_ERROR_AND_CONTINUE(EXIT_AMBIGUOUS_OPTION)
} }
} }
if ((optp->var_type & GET_TYPE_MASK) == GET_DISABLED) if ((optp->var_type & GET_TYPE_MASK) == GET_DISABLED)
...@@ -400,14 +402,14 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -400,14 +402,14 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
(*argc)--; (*argc)--;
continue; continue;
} }
DBUG_RETURN(EXIT_OPTION_DISABLED); SET_HO_ERROR_AND_CONTINUE(EXIT_OPTION_DISABLED)
} }
error= 0; error= 0;
value= optp->var_type & GET_ASK_ADDR value= optp->var_type & GET_ASK_ADDR
? (*my_getopt_get_addr)(key_name, (uint)strlen(key_name), optp, &error) ? (*my_getopt_get_addr)(key_name, (uint)strlen(key_name), optp, &error)
: optp->value; : optp->value;
if (error) if (error)
DBUG_RETURN(error); SET_HO_ERROR_AND_CONTINUE(error)
if (optp->arg_type == NO_ARG) if (optp->arg_type == NO_ARG)
{ {
...@@ -422,7 +424,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -422,7 +424,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
my_getopt_error_reporter(ERROR_LEVEL, my_getopt_error_reporter(ERROR_LEVEL,
"%s: option '--%s' cannot take an argument", "%s: option '--%s' cannot take an argument",
my_progname, optp->name); my_progname, optp->name);
DBUG_RETURN(EXIT_NO_ARGUMENT_ALLOWED); SET_HO_ERROR_AND_CONTINUE(EXIT_NO_ARGUMENT_ALLOWED)
} }
if ((optp->var_type & GET_TYPE_MASK) == GET_BOOL) if ((optp->var_type & GET_TYPE_MASK) == GET_BOOL)
{ {
...@@ -451,7 +453,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -451,7 +453,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
if (get_one_option(optp, *((my_bool*) value) ? if (get_one_option(optp, *((my_bool*) value) ?
enabled_my_option : disabled_my_option, enabled_my_option : disabled_my_option,
filename)) filename))
DBUG_RETURN(EXIT_ARGUMENT_INVALID); SET_HO_ERROR_AND_CONTINUE(EXIT_ARGUMENT_INVALID)
continue; continue;
} }
argument= optend; argument= optend;
...@@ -465,7 +467,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -465,7 +467,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
"option '--%s' cannot take an argument", "option '--%s' cannot take an argument",
my_progname, optp->name); my_progname, optp->name);
DBUG_RETURN(EXIT_NO_ARGUMENT_ALLOWED); SET_HO_ERROR_AND_CONTINUE(EXIT_NO_ARGUMENT_ALLOWED)
} }
if (!(optp->var_type & GET_AUTO)) if (!(optp->var_type & GET_AUTO))
{ {
...@@ -475,7 +477,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -475,7 +477,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
"unsupported by option '--%s'", "unsupported by option '--%s'",
my_progname, optp->name); my_progname, optp->name);
if (!option_is_loose) if (!option_is_loose)
DBUG_RETURN(EXIT_ARGUMENT_INVALID); SET_HO_ERROR_AND_CONTINUE(EXIT_ARGUMENT_INVALID)
continue; continue;
} }
else else
...@@ -494,7 +496,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -494,7 +496,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
my_getopt_error_reporter(ERROR_LEVEL, my_getopt_error_reporter(ERROR_LEVEL,
"%s: option '--%s' requires an argument", "%s: option '--%s' requires an argument",
my_progname, optp->name); my_progname, optp->name);
DBUG_RETURN(EXIT_ARGUMENT_REQUIRED); SET_HO_ERROR_AND_CONTINUE(EXIT_ARGUMENT_REQUIRED)
} }
argument= *pos; argument= *pos;
(*argc)--; (*argc)--;
...@@ -519,14 +521,14 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -519,14 +521,14 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
fprintf(stderr, fprintf(stderr,
"%s: ERROR: Option '-%c' used, but is disabled\n", "%s: ERROR: Option '-%c' used, but is disabled\n",
my_progname, optp->id); my_progname, optp->id);
DBUG_RETURN(EXIT_OPTION_DISABLED); SET_HO_ERROR_AND_CONTINUE(EXIT_OPTION_DISABLED)
} }
if ((optp->var_type & GET_TYPE_MASK) == GET_BOOL && if ((optp->var_type & GET_TYPE_MASK) == GET_BOOL &&
optp->arg_type == NO_ARG) optp->arg_type == NO_ARG)
{ {
*((my_bool*) optp->value)= (my_bool) 1; *((my_bool*) optp->value)= (my_bool) 1;
if (get_one_option(optp, argument, filename)) if (get_one_option(optp, argument, filename))
DBUG_RETURN(EXIT_UNSPECIFIED_ERROR); SET_HO_ERROR_AND_CONTINUE(EXIT_UNSPECIFIED_ERROR)
continue; continue;
} }
else if (optp->arg_type == REQUIRED_ARG || else if (optp->arg_type == REQUIRED_ARG ||
...@@ -546,7 +548,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -546,7 +548,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
if (optp->var_type == GET_BOOL) if (optp->var_type == GET_BOOL)
*((my_bool*) optp->value)= (my_bool) 1; *((my_bool*) optp->value)= (my_bool) 1;
if (get_one_option(optp, argument, filename)) if (get_one_option(optp, argument, filename))
DBUG_RETURN(EXIT_UNSPECIFIED_ERROR); SET_HO_ERROR_AND_CONTINUE(EXIT_UNSPECIFIED_ERROR)
continue; continue;
} }
/* Check if there are more arguments after this one */ /* Check if there are more arguments after this one */
...@@ -556,7 +558,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -556,7 +558,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
my_getopt_error_reporter(ERROR_LEVEL, my_getopt_error_reporter(ERROR_LEVEL,
"%s: option '-%c' requires an argument", "%s: option '-%c' requires an argument",
my_progname, optp->id); my_progname, optp->id);
DBUG_RETURN(EXIT_ARGUMENT_REQUIRED); SET_HO_ERROR_AND_CONTINUE(EXIT_ARGUMENT_REQUIRED)
} }
argument= *++pos; argument= *++pos;
(*argc)--; (*argc)--;
...@@ -565,9 +567,9 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -565,9 +567,9 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
} }
if ((error= setval(optp, optp->value, argument, if ((error= setval(optp, optp->value, argument,
set_maximum_value))) set_maximum_value)))
DBUG_RETURN(error); SET_HO_ERROR_AND_CONTINUE(error)
if (get_one_option(optp, argument, filename)) if (get_one_option(optp, argument, filename))
DBUG_RETURN(EXIT_UNSPECIFIED_ERROR); SET_HO_ERROR_AND_CONTINUE(EXIT_UNSPECIFIED_ERROR)
break; break;
} }
} }
...@@ -601,7 +603,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -601,7 +603,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
my_getopt_error_reporter(ERROR_LEVEL, my_getopt_error_reporter(ERROR_LEVEL,
"%s: unknown option '-%c'", "%s: unknown option '-%c'",
my_progname, *optend); my_progname, *optend);
DBUG_RETURN(EXIT_UNKNOWN_OPTION); SET_HO_ERROR_AND_CONTINUE(EXIT_UNKNOWN_OPTION)
} }
} }
} }
...@@ -612,15 +614,17 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts, ...@@ -612,15 +614,17 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
if ((!option_is_autoset) && if ((!option_is_autoset) &&
((error= setval(optp, value, argument, set_maximum_value))) && ((error= setval(optp, value, argument, set_maximum_value))) &&
!option_is_loose) !option_is_loose)
DBUG_RETURN(error); SET_HO_ERROR_AND_CONTINUE(error)
if (get_one_option(optp, argument, filename)) if (get_one_option(optp, argument, filename))
DBUG_RETURN(EXIT_UNSPECIFIED_ERROR); SET_HO_ERROR_AND_CONTINUE(EXIT_UNSPECIFIED_ERROR)
(*argc)--; /* option handled (long), decrease argument count */ (*argc)--; /* option handled (long), decrease argument count */
} }
else /* non-option found */ else /* non-option found */
(*argv)[argvpos++]= cur_arg; (*argv)[argvpos++]= cur_arg;
} }
if (ho_error)
DBUG_RETURN(ho_error);
/* /*
Destroy the first, already handled option, so that programs that look Destroy the first, already handled option, so that programs that look
for arguments in 'argv', without checking 'argc', know when to stop. for arguments in 'argv', without checking 'argc', know when to stop.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment