MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Chad MILLER Date:March 5 2008 2:33pm
Subject:bk commit into 5.1 tree (cmiller:1.2523) BUG#33464
View as plain text  
Below is the list of changes that have just been committed into a local
5.1 repository of cmiller.  When cmiller does a push these changes
will be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2008-03-05 09:33:32-05:00, cmiller@stripped +3 -0
  Bug#33464: DROP FUNCTION caused a crash
  
  The cause of the crash is an assertion failure that we do not emit 
  an error message (grant not found) and then return "ok".  The 
  assertion is valid, and we were ignoring the buggy behavior prior 
  to the "Diagnostics" result-verification.
  
  Use an error handler to mutate innocuous missing-grant errors, when 
  removing routines, into warnings.

  mysql-test/r/drop.result@stripped, 2008-03-05 09:33:29-05:00, cmiller@stripped +39 -0
    Show that the crash disappears.  Also prepare for the larger bug to
    be fixed with only minor changes to this test.

  mysql-test/t/drop.test@stripped, 2008-03-05 09:33:29-05:00, cmiller@stripped +62 -0
    Show that the crash disappears.  Also prepare for the larger bug to
    be fixed with only minor changes to this test.

  sql/sql_acl.cc@stripped, 2008-03-05 09:33:29-05:00, cmiller@stripped +83 -22
    Disable a segment of code that makes a faulty assumption
    about the existence of a routine's defining user, until that 
    assumption becomes true.
    
    Push a new handler onto the error-handler stack, so that when 
    removing a routine, a missing ACL grant is now a warning
    instead of an error.  If any unexpected error is raised then tell
    the caller.

diff -Nrup a/mysql-test/r/drop.result b/mysql-test/r/drop.result
--- a/mysql-test/r/drop.result	2007-11-26 03:56:29 -05:00
+++ b/mysql-test/r/drop.result	2008-03-05 09:33:29 -05:00
@@ -91,4 +91,43 @@ create table mysql_test.`#sql-347f_7` (f
 create table mysql_test.`#sql-347f_8` (f1 int);
 drop table mysql_test.`#sql-347f_8`;
 drop database mysql_test;
+CREATE DATABASE dbbug33464;
+CREATE USER 'userbug33464'@'localhost';
+GRANT CREATE ROUTINE ON dbbug33464.* TO 'userbug33464'@'localhost';
+	
+userbug33464@localhost	dbbug33464
+DROP PROCEDURE IF EXISTS sp3;
+DROP FUNCTION IF EXISTS fn1;
+CREATE PROCEDURE sp3(v1 char(20))
+BEGIN
+SELECT * from dbbug33464.t6 where t6.f2= 'xyz';
+END//
+CREATE FUNCTION fn1() returns char(50) SQL SECURITY INVOKER
+BEGIN
+return 1;
+END//
+CREATE FUNCTION fn2() returns char(50) SQL SECURITY DEFINER
+BEGIN
+return 2;
+END//
+USE dbbug33464;
+	
+root@localhost	dbbug33464
+SELECT fn1();
+fn1()
+1
+SELECT fn2();
+fn2()
+2
+DROP USER 'userbug33464'@'localhost';
+DROP FUNCTION fn1;
+Warnings:
+Warning	1403	There is no such grant defined for user 'userbug33464' on host 'localhost' on routine 'fn1'
+DROP FUNCTION fn2;
+Warnings:
+Warning	1403	There is no such grant defined for user 'userbug33464' on host 'localhost' on routine 'fn2'
+DROP PROCEDURE sp3;
+DROP USER 'userbug33464'@'localhost';
+use test;
+DROP DATABASE dbbug33464;
 End of 5.1 tests
diff -Nrup a/mysql-test/t/drop.test b/mysql-test/t/drop.test
--- a/mysql-test/t/drop.test	2007-11-23 09:21:22 -05:00
+++ b/mysql-test/t/drop.test	2008-03-05 09:33:29 -05:00
@@ -134,4 +134,66 @@ drop table mysql_test.`#sql-347f_8`;
 copy_file $MYSQLTEST_VARDIR/master-data/mysql_test/t1.frm $MYSQLTEST_VARDIR/master-data/mysql_test/#sql-347f_6.frm;
 drop database mysql_test;
 
+
+#
+# Bug#33464: DROP FUNCTION caused a crash.
+#
+CREATE DATABASE dbbug33464;
+CREATE USER 'userbug33464'@'localhost';
+
+GRANT CREATE ROUTINE ON dbbug33464.* TO 'userbug33464'@'localhost';
+
+--replace_result $MASTER_MYPORT MYSQL_PORT $MASTER_MYSOCK MYSQL_SOCK
+connect (connbug33464, localhost, userbug33464, , dbbug33464);
+--source suite/funcs_1/include/show_connection.inc
+
+--disable_warnings
+DROP PROCEDURE IF EXISTS sp3;
+DROP FUNCTION IF EXISTS fn1;
+--enable_warnings
+
+delimiter //;
+CREATE PROCEDURE sp3(v1 char(20))
+BEGIN
+   SELECT * from dbbug33464.t6 where t6.f2= 'xyz';
+END//
+delimiter ;//
+
+delimiter //;
+CREATE FUNCTION fn1() returns char(50) SQL SECURITY INVOKER
+BEGIN
+   return 1;
+END//
+delimiter ;//
+
+delimiter //;
+CREATE FUNCTION fn2() returns char(50) SQL SECURITY DEFINER
+BEGIN
+   return 2;
+END//
+delimiter ;//
+
+disconnect connbug33464;
+
+# cleanup
+connection default;
+USE dbbug33464;
+--source suite/funcs_1/include/show_connection.inc
+
+SELECT fn1();
+SELECT fn2();
+
+--error 0, ER_CANNOT_USER
+DROP USER 'userbug33464'@'localhost';
+
+DROP FUNCTION fn1;
+DROP FUNCTION fn2;
+DROP PROCEDURE sp3;
+
+--error 0, ER_CANNOT_USER
+DROP USER 'userbug33464'@'localhost';
+
+use test;
+DROP DATABASE dbbug33464;
+
 --echo End of 5.1 tests
diff -Nrup a/sql/sql_acl.cc b/sql/sql_acl.cc
--- a/sql/sql_acl.cc	2008-02-01 06:15:10 -05:00
+++ b/sql/sql_acl.cc	2008-03-05 09:33:29 -05:00
@@ -2779,6 +2779,10 @@ table_error:
 }
 
 
+/**
+  @retval       0  success
+  @retval      -1  error
+*/
 static int replace_routine_table(THD *thd, GRANT_NAME *grant_name,
 			      TABLE *table, const LEX_USER &combo,
 			      const char *db, const char *routine_name,
@@ -2800,14 +2804,11 @@ static int replace_routine_table(THD *th
           thd->security_ctx->host_or_ip, NullS);
 
   /*
-    The following should always succeed as new users are created before
-    this function is called!
+    New users are created before this function is called.
+
+    There may be some cases where a routine's definer is removed but the
+    routine remains.
   */
-  if (!find_acl_user(combo.host.str, combo.user.str, FALSE))
-  {
-    my_error(ER_PASSWORD_NO_MATCH,MYF(0));
-    DBUG_RETURN(-1);
-  }
 
   table->use_all_columns();
   restore_record(table, s->default_values);		// Get empty record
@@ -3321,7 +3322,8 @@ bool mysql_routine_grant(THD *thd, TABLE
     }
 
     if (replace_routine_table(thd, grant_name, tables[1].table, *Str,
-			   db_name, table_name, is_proc, rights, revoke_grant))
+                              db_name, table_name, is_proc, rights, 
+                              revoke_grant) != 0)
     {
       result= TRUE;
       continue;
@@ -5949,11 +5951,11 @@ bool mysql_revoke_all(THD *thd,  List <L
 	if (!strcmp(lex_user->user.str,user) &&
             !strcmp(lex_user->host.str, host))
 	{
-	  if (!replace_routine_table(thd,grant_proc,tables[4].table,*lex_user,
+	  if (replace_routine_table(thd,grant_proc,tables[4].table,*lex_user,
 				  grant_proc->db,
 				  grant_proc->tname,
                                   is_proc,
-				  ~(ulong)0, 1))
+				  ~(ulong)0, 1) == 0)
 	  {
 	    revoked= 1;
 	    continue;
@@ -5979,17 +5981,73 @@ bool mysql_revoke_all(THD *thd,  List <L
 }
 
 
-/*
-  Revoke privileges for all users on a stored procedure
 
-  SYNOPSIS
-    sp_revoke_privileges()
+
+/**
+  If the defining user for a routine does not exist, then the ACL lookup
+  code should raise two errors which we should intercept.  We convert the more
+  descriptive error into a warning, and consume the other.
+
+  If any other errors are raised, then we set a flag that should indicate
+  that there was some failure we should complain at a higher level.
+*/
+class Silence_routine_definer_errors : public Internal_error_handler
+{
+public:
+  Silence_routine_definer_errors()
+    : is_grave(FALSE)
+  {}
+
+  virtual ~Silence_routine_definer_errors()
+  {}
+
+  virtual bool handle_error(uint sql_errno, const char *message,
+                            MYSQL_ERROR::enum_warning_level level,
+                            THD *thd);
+
+  bool has_errors() { return is_grave; }
+
+private:
+  bool is_grave;
+};
+
+bool
+Silence_routine_definer_errors::handle_error(uint sql_errno,
+                                       const char *message,
+                                       MYSQL_ERROR::enum_warning_level level,
+                                       THD *thd)
+{
+  if (level == MYSQL_ERROR::WARN_LEVEL_ERROR)
+  {
+    switch (sql_errno)
+    {
+      case ER_NONEXISTING_PROC_GRANT:
+        /* Convert the error into a warning. */
+        push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN, sql_errno, message);
+        return TRUE;
+      default:
+        is_grave= TRUE;
+    }
+  }
+
+  return FALSE;
+}
+
+
+/**
+  Revoke privileges for all users on a stored procedure.  Use an error handler
+  that converts errors about missing grants into warnings.
+
+  @param
     thd                         The current thread.
+  @param
     db				DB of the stored procedure
+  @param
     name			Name of the stored procedure
 
-  RETURN
+  @retval
     0           OK.
+  @retval
     < 0         Error. Error message not yet sent.
 */
 
@@ -6000,11 +6058,15 @@ bool sp_revoke_privileges(THD *thd, cons
   int result;
   TABLE_LIST tables[GRANT_TABLES];
   HASH *hash= is_proc ? &proc_priv_hash : &func_priv_hash;
+  Silence_routine_definer_errors error_handler;
   DBUG_ENTER("sp_revoke_privileges");
 
   if ((result= open_grant_tables(thd, tables)))
     DBUG_RETURN(result != 1);
 
+  /* Be sure to pop this before exiting this scope! */
+  thd->push_internal_handler(&error_handler);
+
   rw_wrlock(&LOCK_grant);
   VOID(pthread_mutex_lock(&acl_cache->lock));
 
@@ -6031,14 +6093,14 @@ bool sp_revoke_privileges(THD *thd, cons
 	  grant_proc->host.hostname : (char*)"";
 	lex_user.host.length= grant_proc->host.hostname ?
 	  strlen(grant_proc->host.hostname) : 0;
-	if (!replace_routine_table(thd,grant_proc,tables[4].table,lex_user,
-				   grant_proc->db, grant_proc->tname,
-                                   is_proc, ~(ulong)0, 1))
+
+	if (replace_routine_table(thd,grant_proc,tables[4].table,lex_user,
+				  grant_proc->db, grant_proc->tname,
+                                  is_proc, ~(ulong)0, 1) == 0)
 	{
 	  revoked= 1;
 	  continue;
 	}
-	result= -1;	// Something went wrong
       }
       counter++;
     }
@@ -6048,10 +6110,9 @@ bool sp_revoke_privileges(THD *thd, cons
   rw_unlock(&LOCK_grant);
   close_thread_tables(thd);
 
-  if (result)
-    my_message(ER_REVOKE_GRANTS, ER(ER_REVOKE_GRANTS), MYF(0));
+  thd->pop_internal_handler();
 
-  DBUG_RETURN(result);
+  DBUG_RETURN(error_handler.has_errors());
 }
 
 
Thread
bk commit into 5.1 tree (cmiller:1.2523) BUG#33464Chad MILLER5 Mar