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

@@ -91,4 +91,43 @@ create table mysql_test.`#sql-347f_7` (f1 int);
 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

mysql-test/t/drop.test

@@ -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

sql/sql_acl.cc

@@ -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 *thd, GRANT_NAME *grant_name,
           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_LIST *table_list, bool is_proc,
     }
 
     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 <LEX_USER> &list)
 	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 <LEX_USER> &list)
 }
 
 
-/*
-  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, const char *sp_db, const char *sp_name,
   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, const char *sp_db, const char *sp_name,
 	  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,
+
+	if (replace_routine_table(thd,grant_proc,tables[4].table,lex_user,
 				  grant_proc->db, grant_proc->tname,
-                                   is_proc, ~(ulong)0, 1))
+                                  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, const char *sp_db, const char *sp_name,
   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());
 }