A patch for Bug#34820: log_output can be set to illegal value.

We have "set" variables, which can accept empty values
(like sql_mode), and which can not (like log_output). The problem
was that the code does not distinguish them and allow empty
values for every set variable.

The fix is to introduce an attribute of a set variable telling
whether it can accept empty values.

mysql-test/r/variables.result

@@ -1014,3 +1014,13 @@ Variable_name='table_lock_wait_timeout';
 Variable_name	Value
 table_definition_cache	#
 table_lock_wait_timeout	#
+
+# --
+# -- Bug#34820: log_output can be set to illegal value.
+# --
+SET GLOBAL log_output = '';
+ERROR 42000: Variable 'log_output' can't be set to the value of ''
+SET GLOBAL log_output = 0;
+ERROR 42000: Variable 'log_output' can't be set to the value of '0'
+
+# -- End of Bug#34820.

mysql-test/t/variables.test

@@ -778,3 +778,20 @@ set global thread_cache_size         =@my_thread_cache_size;
 --replace_column 2 #
 show global variables where Variable_name='table_definition_cache' or
 Variable_name='table_lock_wait_timeout';
+
+###########################################################################
+
+--echo
+--echo # --
+--echo # -- Bug#34820: log_output can be set to illegal value.
+--echo # --
+
+--error ER_WRONG_VALUE_FOR_VAR
+SET GLOBAL log_output = '';
+
+--error ER_WRONG_VALUE_FOR_VAR
+SET GLOBAL log_output = 0;
+
+--echo
+--echo # -- End of Bug#34820.
+

sql/set_var.cc

@@ -1664,6 +1664,14 @@ bool sys_var::check_set(THD *thd, set_var *var, TYPELIB *enum_names)
       strmov(buff, "NULL");
       goto err;
     }
+
+    if (!m_allow_empty_value &&
+        res->length() == 0)
+    {
+      buff[0]= 0;
+      goto err;
+    }
+
     var->save_result.ulong_value= ((ulong)
 				   find_set(enum_names, res->c_ptr(),
 					    res->length(),
@@ -1679,10 +1687,19 @@ bool sys_var::check_set(THD *thd, set_var *var, TYPELIB *enum_names)
   else
   {
     ulonglong tmp= var->value->val_int();
-   /*
-     For when the enum is made to contain 64 elements, as 1ULL<<64 is
-     undefined, we guard with a "count<64" test.
-   */
+
+    if (!m_allow_empty_value &&
+        tmp == 0)
+    {
+      buff[0]= '0';
+      buff[1]= 0;
+      goto err;
+    }
+
+    /*
+      For when the enum is made to contain 64 elements, as 1ULL<<64 is
+      undefined, we guard with a "count<64" test.
+    */
     if (unlikely((tmp >= ((ULL(1)) << enum_names->count)) &&
                  (enum_names->count < 64)))
     {

sql/set_var.h

@@ -74,7 +74,8 @@ public:
   sys_var(const char *name_arg, sys_after_update_func func= NULL,
           Binlog_status_enum binlog_status_arg= NOT_IN_BINLOG)
     :name(name_arg), after_update(func), no_support_one_shot(1),
-    binlog_status(binlog_status_arg)
+    binlog_status(binlog_status_arg),
+    m_allow_empty_value(TRUE)
   {}
   virtual ~sys_var() {}
   void chain_sys_var(sys_var_chain *chain_arg)
@@ -109,8 +110,16 @@ public:
   virtual bool is_readonly() const { return 0; }
   virtual sys_var_pluginvar *cast_pluginvar() { return 0; }
 
+protected:
+  void set_allow_empty_value(bool allow_empty_value)
+  {
+    m_allow_empty_value= allow_empty_value;
+  }
+
 private:
   const Binlog_status_enum binlog_status;
+
+  bool m_allow_empty_value;
 };
 
 
@@ -878,8 +887,11 @@ public:
   sys_var_log_output(sys_var_chain *chain, const char *name_arg, ulong *value_arg,
                      TYPELIB *typelib, sys_after_update_func func)
     :sys_var(name_arg,func), value(value_arg), enum_names(typelib)
-  { chain_sys_var(chain); }
-  bool check(THD *thd, set_var *var)
+  {
+    chain_sys_var(chain);
+    set_allow_empty_value(FALSE);
+  }
+  virtual bool check(THD *thd, set_var *var)
   {
     return check_set(thd, var, enum_names);
   }