Skip to content

Commit

Permalink
MDEV-26923 Check all invalid config options
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@tonychen2001 @grooverdan
tonychen2001 authored and grooverdan committed Mar 1, 2024
1 parent 9696697 commit 3254687
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 23 deletions.
8 changes: 8 additions & 0 deletions mysql-test/main/mysqld_option_err.result
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@ Test bad binlog format.
Test bad default storage engine.
Test non-numeric value passed to number option.
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 --not-known-option --help --verbose gives error
Done.
12 changes: 12 additions & 0 deletions mysql-test/main/mysqld_option_err.test
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,18 @@ mkdir $MYSQLTEST_VARDIR/tmp/mysqld_option_err;
--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

--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
#
Expand Down
50 changes: 27 additions & 23 deletions mysys/my_getopt.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ double getopt_ulonglong2double(ulonglong v)
return u.dbl;
}

#define SET_HO_ERROR_AND_CONTINUE(e) { ho_error= (e); continue; }

/**
Handle command line options.
Sort options.
Expand Down Expand Up @@ -202,7 +204,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
const char *UNINIT_VAR(prev_found);
const struct my_option *optp;
void *value;
int error, i;
int ho_error= 0, error, i;
my_bool is_cmdline_arg= 1;
DBUG_ENTER("handle_options");

Expand All @@ -216,7 +218,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,

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 *cur_arg= *pos;
Expand Down Expand Up @@ -305,7 +307,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
my_progname, special_opt_prefix[i],
opt_str, special_opt_prefix[i],
prev_found);
DBUG_RETURN(EXIT_AMBIGUOUS_OPTION);
SET_HO_ERROR_AND_CONTINUE(EXIT_AMBIGUOUS_OPTION)
}
switch (i) {
case OPT_SKIP:
Expand Down Expand Up @@ -350,7 +352,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
"%s: unknown variable '%s'",
my_progname, cur_arg);
if (!option_is_loose)
DBUG_RETURN(EXIT_UNKNOWN_VARIABLE);
SET_HO_ERROR_AND_CONTINUE(EXIT_UNKNOWN_VARIABLE)
}
else
{
Expand All @@ -360,7 +362,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
"%s: unknown option '--%s'",
my_progname, cur_arg);
if (!option_is_loose)
DBUG_RETURN(EXIT_UNKNOWN_OPTION);
SET_HO_ERROR_AND_CONTINUE(EXIT_UNKNOWN_OPTION)
}
if (option_is_loose)
{
Expand All @@ -377,7 +379,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
my_getopt_error_reporter(ERROR_LEVEL,
"%s: variable prefix '%s' is not unique",
my_progname, opt_str);
DBUG_RETURN(EXIT_VAR_PREFIX_NOT_UNIQUE);
SET_HO_ERROR_AND_CONTINUE(EXIT_VAR_PREFIX_NOT_UNIQUE)
}
else
{
Expand All @@ -386,7 +388,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
"%s: ambiguous option '--%s' (%s, %s)",
my_progname, opt_str, prev_found,
optp->name);
DBUG_RETURN(EXIT_AMBIGUOUS_OPTION);
SET_HO_ERROR_AND_CONTINUE(EXIT_AMBIGUOUS_OPTION)
}
}
if ((optp->var_type & GET_TYPE_MASK) == GET_DISABLED)
Expand All @@ -400,14 +402,14 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
(*argc)--;
continue;
}
DBUG_RETURN(EXIT_OPTION_DISABLED);
SET_HO_ERROR_AND_CONTINUE(EXIT_OPTION_DISABLED)
}
error= 0;
value= optp->var_type & GET_ASK_ADDR
? (*my_getopt_get_addr)(key_name, (uint)strlen(key_name), optp, &error)
: optp->value;
if (error)
DBUG_RETURN(error);
SET_HO_ERROR_AND_CONTINUE(error)

if (optp->arg_type == NO_ARG)
{
Expand All @@ -422,7 +424,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
my_getopt_error_reporter(ERROR_LEVEL,
"%s: option '--%s' cannot take an argument",
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)
{
Expand Down Expand Up @@ -451,7 +453,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
if (get_one_option(optp, *((my_bool*) value) ?
enabled_my_option : disabled_my_option,
filename))
DBUG_RETURN(EXIT_ARGUMENT_INVALID);
SET_HO_ERROR_AND_CONTINUE(EXIT_ARGUMENT_INVALID)
continue;
}
argument= optend;
Expand All @@ -465,7 +467,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
"option '--%s' cannot take an argument",
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))
{
Expand All @@ -475,7 +477,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
"unsupported by option '--%s'",
my_progname, optp->name);
if (!option_is_loose)
DBUG_RETURN(EXIT_ARGUMENT_INVALID);
SET_HO_ERROR_AND_CONTINUE(EXIT_ARGUMENT_INVALID)
continue;
}
else
Expand All @@ -494,7 +496,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
my_getopt_error_reporter(ERROR_LEVEL,
"%s: option '--%s' requires an argument",
my_progname, optp->name);
DBUG_RETURN(EXIT_ARGUMENT_REQUIRED);
SET_HO_ERROR_AND_CONTINUE(EXIT_ARGUMENT_REQUIRED)
}
argument= *pos;
(*argc)--;
Expand All @@ -519,14 +521,14 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
fprintf(stderr,
"%s: ERROR: Option '-%c' used, but is disabled\n",
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 &&
optp->arg_type == NO_ARG)
{
*((my_bool*) optp->value)= (my_bool) 1;
if (get_one_option(optp, argument, filename))
DBUG_RETURN(EXIT_UNSPECIFIED_ERROR);
SET_HO_ERROR_AND_CONTINUE(EXIT_UNSPECIFIED_ERROR)
continue;
}
else if (optp->arg_type == REQUIRED_ARG ||
Expand All @@ -546,7 +548,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
if (optp->var_type == GET_BOOL)
*((my_bool*) optp->value)= (my_bool) 1;
if (get_one_option(optp, argument, filename))
DBUG_RETURN(EXIT_UNSPECIFIED_ERROR);
SET_HO_ERROR_AND_CONTINUE(EXIT_UNSPECIFIED_ERROR)
continue;
}
/* Check if there are more arguments after this one */
Expand All @@ -556,7 +558,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
my_getopt_error_reporter(ERROR_LEVEL,
"%s: option '-%c' requires an argument",
my_progname, optp->id);
DBUG_RETURN(EXIT_ARGUMENT_REQUIRED);
SET_HO_ERROR_AND_CONTINUE(EXIT_ARGUMENT_REQUIRED)
}
argument= *++pos;
(*argc)--;
Expand All @@ -565,9 +567,9 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
}
if ((error= setval(optp, optp->value, argument,
set_maximum_value)))
DBUG_RETURN(error);
SET_HO_ERROR_AND_CONTINUE(error)
if (get_one_option(optp, argument, filename))
DBUG_RETURN(EXIT_UNSPECIFIED_ERROR);
SET_HO_ERROR_AND_CONTINUE(EXIT_UNSPECIFIED_ERROR)
break;
}
}
Expand Down Expand Up @@ -601,7 +603,7 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
my_getopt_error_reporter(ERROR_LEVEL,
"%s: unknown option '-%c'",
my_progname, *optend);
DBUG_RETURN(EXIT_UNKNOWN_OPTION);
SET_HO_ERROR_AND_CONTINUE(EXIT_UNKNOWN_OPTION)
}
}
}
Expand All @@ -612,15 +614,17 @@ int handle_options(int *argc, char ***argv, const struct my_option *longopts,
if ((!option_is_autoset) &&
((error= setval(optp, value, argument, set_maximum_value))) &&
!option_is_loose)
DBUG_RETURN(error);
SET_HO_ERROR_AND_CONTINUE(error)
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 */
}
else /* non-option found */
(*argv)[argvpos++]= cur_arg;
}
if (ho_error)
DBUG_RETURN(ho_error);
/*
Destroy the first, already handled option, so that programs that look
for arguments in 'argv', without checking 'argc', know when to stop.
Expand Down

0 comments on commit 3254687

Please sign in to comment.