BUG#51277 SUPER_ACL should be checked unconditionally (binlog_format and binlog_direct)

SUPER_ACL should be checked unconditionally while verifying if the binlog_format
or the binlog_direct_non_transactional_updates might be changed.

Roughly speaking, both session values cannot be changed in the context of a
transaction or a stored function. Note that changing the global value does
not cause any effect until a new connection is created.

So, we fixed the problem by first checking the permissions and right after further
verifications are ignored if the global value is being updated. In this patch, we
also re-structure the test case to make it more readable.

1 files changed, 21 insertions, 22 deletions

diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc
index 8f71d27e8d4..d8bdf35fd51 100644
--- a/sql/sys_vars.cc
+++ b/sql/sys_vars.cc
@@ -240,6 +240,12 @@ static bool check_has_super(sys_var *self, THD *thd, set_var *var)
 }
 static bool binlog_format_check(sys_var *self, THD *thd, set_var *var)
 {
+  if (check_has_super(self, thd, var))
+    return true;
+
+  if (var->type == OPT_GLOBAL)
+    return false;
+
   /*
      If RBR and open temporary tables, their CREATE TABLE may not be in the
      binlog, so we can't toggle to SBR in this connection.
@@ -271,18 +277,12 @@ static bool binlog_format_check(sys_var *self, THD *thd, set_var *var)
   /*
     Make the session variable 'binlog_format' read-only inside a transaction.
   */
-  if (thd->active_transaction() && (var->type == OPT_SESSION))
+  if (thd->active_transaction())
   {
     my_error(ER_INSIDE_TRANSACTION_PREVENTS_SWITCH_BINLOG_FORMAT, MYF(0));
     return true;
   }
 
-  if (check_has_super(self, thd, var))
-    return true;
-  if (var->type == OPT_GLOBAL ||
-      (thd->variables.binlog_format == var->save_result.ulonglong_value))
-    return false;
-
   return false;
 }
 
@@ -311,32 +311,31 @@ static Sys_var_enum Sys_binlog_format(
 
 static bool binlog_direct_check(sys_var *self, THD *thd, set_var *var)
 {
+  if (check_has_super(self, thd, var))
+    return true;
+
+  if (var->type == OPT_GLOBAL)
+    return false;
+
    /*
      Makes the session variable 'binlog_direct_non_transactional_updates'
-     read-only inside a transaction.
+     read-only if within a procedure, trigger or function.
    */
-   if (thd->active_transaction() && (var->type == OPT_SESSION))
+   if (thd->in_sub_stmt)
    {
-     my_error(ER_INSIDE_TRANSACTION_PREVENTS_SWITCH_BINLOG_DIRECT, MYF(0));
-     return 1;
+     my_error(ER_STORED_FUNCTION_PREVENTS_SWITCH_BINLOG_DIRECT, MYF(0));
+     return true;
    }
    /*
      Makes the session variable 'binlog_direct_non_transactional_updates'
-     read-only if within a procedure, trigger or function.
+     read-only inside a transaction.
    */
-   if (thd->in_sub_stmt)
+   if (thd->active_transaction())
    {
-     my_error(ER_STORED_FUNCTION_PREVENTS_SWITCH_BINLOG_DIRECT, MYF(0));
-     return 1;
+     my_error(ER_INSIDE_TRANSACTION_PREVENTS_SWITCH_BINLOG_DIRECT, MYF(0));
+     return true;
    }
 
-  if (check_has_super(self, thd, var))
-    return true;
-  if (var->type == OPT_GLOBAL ||
-      (thd->variables.binlog_direct_non_trans_update ==
-       static_cast<my_bool>(var->save_result.ulonglong_value)))
-    return false;
-
   return false;
 }