MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Chad MILLER Date:February 19 2008 10:23pm
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-02-19 17:23:50-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.
  
  This patch moves the reporting of the message up to the three callers
  and in the one where it is not an error, it reports it as a warning.

  mysql-test/r/drop.result@stripped, 2008-02-19 17:23:46-05:00, cmiller@stripped +49 -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-02-19 17:23:46-05:00, cmiller@stripped +69 -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-02-19 17:23:46-05:00, cmiller@stripped +56 -14
    Change this local function, replace_routine_table, so that it
    may also defer error reporting to the caller.  Send up the 
    
    Also, disable a segment of code that makes a faulty assumption
    about the existence of a routine's defining user, until that 
    assumption becomes true.
    
    When removing a routine, it a missing ACL grant is now a warning
    instead of an error.

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-02-19 17:23:46 -05:00
@@ -91,4 +91,53 @@ 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';
+FLUSH PRIVILEGES;
+	
+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
+SHOW FUNCTION STATUS WHERE DEFINER='userbug33464@localhost';
+Db	Name	Type	Definer	Modified	Created	Security_type	Comment	character_set_client	collation_connection	Database Collation
+dbbug33464	fn1	FUNCTION	userbug33464@localhost	date	date	INVOKER		latin1	latin1_swedish_ci	latin1_swedish_ci
+dbbug33464	fn2	FUNCTION	userbug33464@localhost	date	date	DEFINER		latin1	latin1_swedish_ci	latin1_swedish_ci
+SELECT fn1();
+fn1()
+1
+SELECT fn2();
+fn2()
+2
+DROP USER 'userbug33464'@'localhost';
+SHOW FUNCTION STATUS WHERE DEFINER='userbug33464@localhost';
+Db	Name	Type	Definer	Modified	Created	Security_type	Comment	character_set_client	collation_connection	Database Collation
+dbbug33464	fn1	FUNCTION	userbug33464@localhost	date	date	INVOKER		latin1	latin1_swedish_ci	latin1_swedish_ci
+dbbug33464	fn2	FUNCTION	userbug33464@localhost	date	date	DEFINER		latin1	latin1_swedish_ci	latin1_swedish_ci
+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';
+SHOW FUNCTION STATUS WHERE DEFINER='userbug33464@localhost';
+Db	Name	Type	Definer	Modified	Created	Security_type	Comment	character_set_client	collation_connection	Database Collation
+use test;
 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-02-19 17:23:46 -05:00
@@ -134,4 +134,73 @@ 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';
+FLUSH PRIVILEGES;
+
+--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
+
+--replace_column 5 date 6 date 
+SHOW FUNCTION STATUS WHERE DEFINER='userbug33464@localhost';
+SELECT fn1();
+SELECT fn2();
+
+--error 0, ER_CANNOT_USER
+DROP USER 'userbug33464'@'localhost';
+
+--replace_column 5 date 6 date 
+SHOW FUNCTION STATUS WHERE DEFINER='userbug33464@localhost';
+DROP FUNCTION fn1;
+DROP FUNCTION fn2;
+DROP PROCEDURE sp3;
+
+--error 0, ER_CANNOT_USER
+DROP USER 'userbug33464'@'localhost';
+
+--replace_column 5 date 6 date 
+SHOW FUNCTION STATUS WHERE DEFINER='userbug33464@localhost';
+
+use test;
+
 --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-02-19 17:23:46 -05:00
@@ -2779,6 +2779,11 @@ table_error:
 }
 
 
+/**
+  @retval       0  success
+  @retval      -1  error, and message emitted
+  @retval myerrno  error, and message not emitted
+*/
 static int replace_routine_table(THD *thd, GRANT_NAME *grant_name,
 			      TABLE *table, const LEX_USER &combo,
 			      const char *db, const char *routine_name,
@@ -2799,15 +2804,19 @@ static int replace_routine_table(THD *th
   strxmov(grantor, thd->security_ctx->user, "@",
           thd->security_ctx->host_or_ip, NullS);
 
+#ifdef FIXED_BUG_34676
   /*
-    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);
   }
+#endif
 
   table->use_all_columns();
   restore_record(table, s->default_values);		// Get empty record
@@ -2833,9 +2842,8 @@ static int replace_routine_table(THD *th
     */
     if (revoke_grant)
     { // no row, no revoke
-      my_error(ER_NONEXISTING_PROC_GRANT, MYF(0),
-               combo.user.str, combo.host.str, routine_name);
-      DBUG_RETURN(-1);
+      /* Callers differ in whether this is dire.  Let caller decide. */
+      DBUG_RETURN(ER_NONEXISTING_PROC_GRANT);
     }
     old_row_exists= 0;
     restore_record(table,record[1]);			// Get saved record
@@ -3320,9 +3328,17 @@ bool mysql_routine_grant(THD *thd, TABLE
       my_hash_insert(is_proc ? &proc_priv_hash : &func_priv_hash,(uchar*) grant_name);
     }
 
-    if (replace_routine_table(thd, grant_name, tables[1].table, *Str,
-			   db_name, table_name, is_proc, rights, revoke_grant))
+    int returned_myerrno;
+    if ((returned_myerrno= replace_routine_table(thd, grant_name, tables[1].table, *Str,
+			   db_name, table_name, is_proc, rights, revoke_grant)))
     {
+
+      if (returned_myerrno == ER_NONEXISTING_PROC_GRANT)
+        my_error(ER_NONEXISTING_PROC_GRANT, MYF(0),
+                 Str->user.str, Str->host.str, table_name);
+      else
+        assert((returned_myerrno < ER_ERROR_FIRST) || (returned_myerrno > ER_ERROR_LAST));
+
       result= TRUE;
       continue;
     }
@@ -5949,15 +5965,25 @@ 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,
+          int returned_myerrno;
+	  returned_myerrno= 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);
+	  if (!returned_myerrno)
 	  {
 	    revoked= 1;
 	    continue;
 	  }
+          else
+          {
+            if (returned_myerrno == ER_NONEXISTING_PROC_GRANT)
+              my_error(ER_NONEXISTING_PROC_GRANT, MYF(0),
+                       lex_user->user.str, lex_user->host.str, grant_proc->tname);
+            else
+              assert((returned_myerrno < ER_ERROR_FIRST) || (returned_myerrno > ER_ERROR_LAST));
+          }
 	  result= -1;	// Something went wrong
 	}
 	counter++;
@@ -6031,14 +6057,30 @@ 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,
+
+        int returned_myerrno;
+	returned_myerrno= 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);
+
+        /* Make sure replace_routine_table returns something we know about. */
+        if (returned_myerrno == 0)
+        {
 	  revoked= 1;
 	  continue;
-	}
-	result= -1;	// Something went wrong
+        }
+        else if (returned_myerrno == ER_NONEXISTING_PROC_GRANT)
+        { 
+          /* It is not an error to lack the defining user of this routine.  */
+          push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN,
+                              ER_NONEXISTING_PROC_GRANT, ER(ER_NONEXISTING_PROC_GRANT),
+                              lex_user.user.str, lex_user.host.str, grant_proc->tname);
+        } 
+        else
+        {
+          assert((returned_myerrno < ER_ERROR_FIRST) || (returned_myerrno > ER_ERROR_LAST));
+          result= -1;	// Something went wrong
+        }
       }
       counter++;
     }
Thread
bk commit into 5.1 tree (cmiller:1.2523) BUG#33464Chad MILLER19 Feb
  • Re: bk commit into 5.1 tree (cmiller:1.2523) BUG#33464Konstantin Osipov21 Feb
    • Re: bk commit into 5.1 tree (cmiller:1.2523) BUG#33464Chad MILLER21 Feb
      • Re: bk commit into 5.1 tree (cmiller:1.2523) BUG#33464Konstantin Osipov21 Feb