10.4-MDEV-31470 wsrep_sst_method variable validity checking

This commit checks the validity of value change of wsrep_sst_method variable.
The validity check is same as happens in donor node when incoming SST request
is parsed.

The commit has also a mtr test: wsrep.wsrep_variables_sst_method which verifies
that wsrep_sst_method can be succesfully changed to acceptable values and that
the SET command results in error if invalid value was entered.

Signed-off-by: Julius Goryavsky <julius.goryavsky@mariadb.com>

mysql-test/suite/wsrep/r/wsrep_variables_sst_method.result

@@ -0,0 +1,61 @@
+SELECT @@wsrep_on;
+@@wsrep_on
+1
+SET @wsrep_sst_method_saved = @@global.wsrep_sst_method;
+/* test currently supported methods */
+SET GLOBAL wsrep_sst_method=DEFAULT;
+SELECT @@global.wsrep_sst_method;
+@@global.wsrep_sst_method
+rsync
+SET GLOBAL wsrep_sst_method='rsync';
+SELECT @@global.wsrep_sst_method;
+@@global.wsrep_sst_method
+rsync
+SET GLOBAL wsrep_sst_method='mysqldump';
+SELECT @@global.wsrep_sst_method;
+@@global.wsrep_sst_method
+mysqldump
+SET GLOBAL wsrep_sst_method='mariabackup';
+SELECT @@global.wsrep_sst_method;
+@@global.wsrep_sst_method
+mariabackup
+SET GLOBAL wsrep_sst_method='backup';
+SELECT @@global.wsrep_sst_method;
+@@global.wsrep_sst_method
+backup
+SET GLOBAL wsrep_sst_method='backup1.sh';
+SELECT @@global.wsrep_sst_method;
+@@global.wsrep_sst_method
+backup1.sh
+SET GLOBAL wsrep_sst_method='my method';
+ERROR 42000: Variable 'wsrep_sst_method' can't be set to the value of 'my method'
+SELECT @@global.wsrep_sst_method;
+@@global.wsrep_sst_method
+backup1.sh
+SHOW WARNINGS;
+Level	Code	Message
+Error	1231	Variable 'wsrep_sst_method' can't be set to the value of 'my method'
+SET GLOBAL wsrep_sst_method='/method';
+ERROR 42000: Variable 'wsrep_sst_method' can't be set to the value of '/method'
+SELECT @@global.wsrep_sst_method;
+@@global.wsrep_sst_method
+backup1.sh
+SHOW WARNINGS;
+Level	Code	Message
+Error	1231	Variable 'wsrep_sst_method' can't be set to the value of '/method'
+SET GLOBAL wsrep_sst_method='method!';
+ERROR 42000: Variable 'wsrep_sst_method' can't be set to the value of 'method!'
+SELECT @@global.wsrep_sst_method;
+@@global.wsrep_sst_method
+backup1.sh
+SHOW WARNINGS;
+Level	Code	Message
+Error	1231	Variable 'wsrep_sst_method' can't be set to the value of 'method!'
+SET GLOBAL wsrep_sst_method='method;';
+ERROR 42000: Variable 'wsrep_sst_method' can't be set to the value of 'method;'
+SELECT @@global.wsrep_sst_method;
+@@global.wsrep_sst_method
+backup1.sh
+SHOW WARNINGS;
+Level	Code	Message
+Error	1231	Variable 'wsrep_sst_method' can't be set to the value of 'method;'

mysql-test/suite/wsrep/t/wsrep_variables_sst_method.cnf

@@ -0,0 +1,7 @@
+!include ../my.cnf
+
+[mysqld.1]
+wsrep-on=ON
+wsrep-cluster-address=gcomm://
+wsrep-provider=@ENV.WSREP_PROVIDER
+binlog-format=ROW

mysql-test/suite/wsrep/t/wsrep_variables_sst_method.test

@@ -0,0 +1,50 @@
+--source include/have_wsrep.inc
+--source include/have_innodb.inc
+
+SELECT @@wsrep_on;
+
+SET @wsrep_sst_method_saved = @@global.wsrep_sst_method;
+
+/* test currently supported methods */
+SET GLOBAL wsrep_sst_method=DEFAULT;
+SELECT @@global.wsrep_sst_method;
+
+SET GLOBAL wsrep_sst_method='rsync';
+SELECT @@global.wsrep_sst_method;
+
+SET GLOBAL wsrep_sst_method='mysqldump';
+SELECT @@global.wsrep_sst_method;
+
+SET GLOBAL wsrep_sst_method='mariabackup';
+SELECT @@global.wsrep_sst_method;
+
+SET GLOBAL wsrep_sst_method='backup';
+SELECT @@global.wsrep_sst_method;
+
+SET GLOBAL wsrep_sst_method='backup1.sh';
+SELECT @@global.wsrep_sst_method;
+
+--error ER_WRONG_VALUE_FOR_VAR
+SET GLOBAL wsrep_sst_method='my method';
+SELECT @@global.wsrep_sst_method;
+SHOW WARNINGS;
+
+--error ER_WRONG_VALUE_FOR_VAR
+SET GLOBAL wsrep_sst_method='/method';
+SELECT @@global.wsrep_sst_method;
+SHOW WARNINGS;
+
+--error ER_WRONG_VALUE_FOR_VAR
+SET GLOBAL wsrep_sst_method='method!';
+SELECT @@global.wsrep_sst_method;
+SHOW WARNINGS;
+
+--error ER_WRONG_VALUE_FOR_VAR
+SET GLOBAL wsrep_sst_method='method;';
+SELECT @@global.wsrep_sst_method;
+SHOW WARNINGS;
+
+
+--disable_query_log
+SET @@global.wsrep_sst_method = @wsrep_sst_method_saved;
+--enable_query_log

sql/wsrep_sst.cc

@@ -172,6 +172,36 @@ static void* wsrep_sst_joiner_monitor_thread(void *arg __attribute__((unused)))
   return NULL;
 }
 
+/* return true if character can be a part of a filename */
+static bool filename_char(int const c)
+{
+  return isalnum(c) || (c == '-') || (c == '_') || (c == '.');
+}
+
+/* return true if character can be a part of an address string */
+static bool address_char(int const c)
+{
+  return filename_char(c) ||
+         (c == ':') || (c == '[') || (c == ']') || (c == '/');
+}
+
+static bool check_request_str(const char* const str,
+                              bool (*check) (int c),
+                              bool log_warn = true)
+{
+  for (size_t i(0); str[i] != '\0'; ++i)
+  {
+    if (!check(str[i]))
+    {
+      if (log_warn) WSREP_WARN("Illegal character in state transfer request: %i (%c).",
+                               str[i], str[i]);
+      return true;
+    }
+  }
+
+  return false;
+}
+
 bool wsrep_sst_method_check (sys_var *self, THD* thd, set_var* var)
 {
   if ((! var->save_result.string_value.str) ||
@@ -183,6 +213,16 @@ bool wsrep_sst_method_check (sys_var *self, THD* thd, set_var* var)
     return 1;
   }
 
+  /* check also that method name is alphanumeric string  */
+  if (check_request_str(var->save_result.string_value.str,
+                        filename_char, false))
+  {
+    my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), var->var->name.str,
+             var->save_result.string_value.str ?
+             var->save_result.string_value.str : "NULL");
+    return 1;
+  }
+
   return 0;
 }
 
@@ -1920,35 +1960,6 @@ static int sst_donate_other (const char*        method,
   return arg.err;
 }
 
-/* return true if character can be a part of a filename */
-static bool filename_char(int const c)
-{
-  return isalnum(c) || (c == '-') || (c == '_') || (c == '.');
-}
-
-/* return true if character can be a part of an address string */
-static bool address_char(int const c)
-{
-  return filename_char(c) ||
-         (c == ':') || (c == '[') || (c == ']') || (c == '/');
-}
-
-static bool check_request_str(const char* const str,
-                              bool (*check) (int c))
-{
-  for (size_t i(0); str[i] != '\0'; ++i)
-  {
-    if (!check(str[i]))
-    {
-      WSREP_WARN("Illegal character in state transfer request: %i (%c).",
-                 str[i], str[i]);
-      return true;
-    }
-  }
-
-  return false;
-}
-
 int wsrep_sst_donate(const std::string& msg,
                      const wsrep::gtid& current_gtid,
                      const bool         bypass)
@@ -1956,7 +1967,7 @@ int wsrep_sst_donate(const std::string& msg,
   const char* method= msg.data();
   size_t method_len= strlen (method);
 
-  if (check_request_str(method, filename_char))
+  if (check_request_str(method, filename_char, true))
   {
     WSREP_ERROR("Bad SST method name. SST canceled.");
     return WSREP_CB_FAILURE;
@@ -1978,7 +1989,7 @@ int wsrep_sst_donate(const std::string& msg,
     addr= data;
   }
 
-  if (check_request_str(addr, address_char))
+  if (check_request_str(addr, address_char, true))
   {
     WSREP_ERROR("Bad SST address string. SST canceled.");
     return WSREP_CB_FAILURE;