MBug#423035: error in parsing enum value for plugin variable in mysqld command-line option

Fix parsing of invalid plugin enum option value.

Previous patch to fix plugin enum option parsing on big-endian introduced another bug due
to incorrect comparison of unsigned value. This would cause an incorrect value to be
parsed as value 0.

See also MySQL Bug#41010 and Bug#32034.


mysql-test/mysql-test-run.pl:
  Add a facility for test case to run the mysqld binary (to test that invalid startup options
  are rejected correctly).
mysql-test/r/mysqld_option_err.result:
  Add a test case to check that invalid startup options for mysqld are rejected.
  This is needed to test MBug#423035.
  Also add a few other similar tests, as this was completely untested before this patch.
mysql-test/t/mysqld_option_err.test:
  Add a test case to check that invalid startup options for mysqld are rejected.
  This is needed to test MBug#423035.
  Also add a few other similar tests, as this was completely untested before this patch.
mysys/my_getopt.c:
  Fix parsing of invalid plugin enum option value.

4 files changed, 72 insertions, 1 deletions

diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl
index cd92d4367ee..f5c22ca2154 100755
--- a/mysql-test/mysql-test-run.pl
+++ b/mysql-test/mysql-test-run.pl
@@ -1771,6 +1771,20 @@ sub tool_arguments ($$) {
   return mtr_args2str($exe, @$args);
 }
 
+# This is not used to actually start a mysqld server, just to allow test
+# scripts to run the mysqld binary to test invalid server startup options.
+sub mysqld_client_arguments () {
+  my $default_mysqld= default_mysqld();
+  my $exe = find_mysqld($basedir);
+  my $args;
+  mtr_init_args(\$args);
+  mtr_add_arg($args, "--no-defaults");
+  mtr_add_arg($args, "--basedir=%s", $basedir);
+  mtr_add_arg($args, "--character-sets-dir=%s", $default_mysqld->value("character-sets-dir"));
+  mtr_add_arg($args, "--language=%s", $default_mysqld->value("language"));
+  return mtr_args2str($exe, @$args);
+}
+
 
 sub have_maria_support () {
   my $maria_var= $mysqld_variables{'maria'};
@@ -1968,6 +1982,7 @@ sub environment_setup {
   $ENV{'MYSQLADMIN'}=               native_path($exe_mysqladmin);
   $ENV{'MYSQL_CLIENT_TEST'}=        mysql_client_test_arguments();
   $ENV{'MYSQL_FIX_SYSTEM_TABLES'}=  mysql_fix_arguments();
+  $ENV{'MYSQLD'}=                   mysqld_client_arguments();
   $ENV{'EXE_MYSQL'}=                $exe_mysql;
 
   # ----------------------------------------------------
diff --git a/mysql-test/r/mysqld_option_err.result b/mysql-test/r/mysqld_option_err.result
new file mode 100644
index 00000000000..255f109b788
--- /dev/null
+++ b/mysql-test/r/mysqld_option_err.result
@@ -0,0 +1,6 @@
+Test that unknown option is not silently ignored.
+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.
+Done.
diff --git a/mysql-test/t/mysqld_option_err.test b/mysql-test/t/mysqld_option_err.test
new file mode 100644
index 00000000000..2b35a2eb38e
--- /dev/null
+++ b/mysql-test/t/mysqld_option_err.test
@@ -0,0 +1,47 @@
+#
+# Test error checks on mysqld command line option parsing.
+#
+# Call mysqld with different invalid options, and check that it fails in each case.
+#
+# This means that a test failure results in mysqld starting up, which is only
+# caught when the test case times out. This is not ideal, but I did not find an
+# easy way to have the server shut down after a successful startup.
+#
+
+--source include/not_embedded.inc
+
+# We have not run (and do not need) bootstrap of the server. We just
+# give it a dummy data directory (for log files etc).
+
+mkdir $MYSQLTEST_VARDIR/tmp/mysqld_option_err;
+
+
+--echo Test that unknown option is not silently ignored.
+--error 2
+--exec $MYSQLD --skip-networking --datadir=$MYSQLTEST_VARDIR/tmp/mysqld_option_err --skip-grant-tables --nonexistentoption >$MYSQLTEST_VARDIR/tmp/mysqld_option_err/mysqltest.log 2>&1
+
+
+--echo Test bad binlog format.
+--error 1
+--exec $MYSQLD --skip-networking --datadir=$MYSQLTEST_VARDIR/tmp/mysqld_option_err --skip-grant-tables --log-bin --binlog-format=badformat >>$MYSQLTEST_VARDIR/tmp/mysqld_option_err/mysqltest.log 2>&1
+
+
+--echo Test bad default storage engine.
+--error 1
+--exec $MYSQLD --skip-networking --datadir=$MYSQLTEST_VARDIR/tmp/mysqld_option_err --skip-grant-tables --default-storage-engine=nonexistentengine >>$MYSQLTEST_VARDIR/tmp/mysqld_option_err/mysqltest.log 2>&1
+
+
+--echo Test non-numeric value passed to number option.
+--error 1
+--exec $MYSQLD --skip-networking --datadir=$MYSQLTEST_VARDIR/tmp/mysqld_option_err --skip-grant-tables  --min-examined-row-limit=notanumber >>$MYSQLTEST_VARDIR/tmp/mysqld_option_err/mysqltest.log 2>&1
+
+
+# Test for MBug#423035: error in parsing enum value for plugin
+# variable in mysqld command-line option.
+# See also Bug#32034.
+--echo Test that bad value for plugin enum option is rejected correctly.
+--error 7
+--exec $MYSQLD --skip-networking --datadir=$MYSQLTEST_VARDIR/tmp/mysqld_option_err --skip-grant-tables $EXAMPLE_PLUGIN_OPT --plugin-load=EXAMPLE=ha_example.so --plugin-example-enum-var=noexist >>$MYSQLTEST_VARDIR/tmp/mysqld_option_err/mysqltest.log 2>&1
+
+
+--echo Done.
diff --git a/mysys/my_getopt.c b/mysys/my_getopt.c
index 0de80b01c4f..d44ec162b93 100644
--- a/mysys/my_getopt.c
+++ b/mysys/my_getopt.c
@@ -603,6 +603,7 @@ static int setval(const struct my_option *opts, uchar* *value, char *argument,
 		  my_bool set_maximum_value)
 {
   int err= 0;
+  int pos;
 
   if (value && argument)
   {
@@ -647,7 +648,9 @@ static int setval(const struct my_option *opts, uchar* *value, char *argument,
 	return EXIT_OUT_OF_MEMORY;
       break;
     case GET_ENUM:
-      if (((*(ulong *)result_pos)= find_type(argument, opts->typelib, 2) - 1) < 0)
+      pos= find_type(argument, opts->typelib, 2) - 1;
+      (*(ulong *)result_pos)= pos;
+      if (pos < 0)
         return EXIT_ARGUMENT_INVALID;
       break;
     case GET_SET: