List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:December 1 2010 10:12am
Subject:bzr push into mysql-trunk-bugfixing branch (Dmitry.Lenev:3392 to 3393)
Bug#27480
View as plain text  
 3393 Dmitry Lenev	2010-12-01
      Prerequisite patch for Bug#27480 (Extend CREATE TEMPORARY TABLES
      privilege to allow temp table operations).
      
      Review fixes in progress. Fixed handling of administrative commands,
      extended test-coverage.

    modified:
      mysql-test/r/grant4.result
      mysql-test/t/grant4.test
      sql/sql_admin.cc
      sql/sql_parse.cc
 3392 Dmitry Lenev	2010-11-30
      Prerequisite patch for Bug#27480 (Extend CREATE TEMPORARY TABLES
      privilege to allow temp table operations).
      
      Review fixes in progress.

    modified:
      sql/sp_head.cc
      sql/sql_view.cc
=== modified file 'mysql-test/r/grant4.result'
--- a/mysql-test/r/grant4.result	2010-02-20 10:07:32 +0000
+++ b/mysql-test/r/grant4.result	2010-12-01 10:07:22 +0000
@@ -121,3 +121,60 @@ View	Create View	character_set_client	co
 v3	CREATE ALGORITHM=UNDEFINED DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v3` AS select `t_select_priv`.`a` AS `a`,`t_select_priv`.`b` AS `b` from `t_select_priv`	latin1	latin1_swedish_ci
 drop database mysqltest_db1;
 drop user mysqltest_u1@localhost;
+#
+# Additional coverage for refactoring which is made as part
+# of fix for bug #27480 "Extend CREATE TEMPORARY TABLES privilege
+# to allow temp table operations".
+# 
+# Check that for statements like CHECK/REPAIR and OPTIMIZE TABLE
+# privileges for all tables involved are checked before processing
+# any tables. Doing otherwise, i.e. checking privileges for table
+# right before processing it might result in lost results for tables
+# which were processed by the time when table for which privileges
+# are insufficient are discovered.
+#
+call mtr.add_suppression("Got an error from thread_id=.*ha_myisam.cc:");
+call mtr.add_suppression("MySQL thread id .*, query id .* localhost.*mysqltest_u1 Checking table");
+drop database if exists mysqltest_db1;
+create database mysqltest_db1;
+# Create tables which we are going to CHECK/REPAIR.
+create table mysqltest_db1.t1 (a int, key(a)) engine=myisam;
+create table mysqltest_db1.t2 (b int);
+insert into mysqltest_db1.t1 values (1), (2);
+insert into mysqltest_db1.t2 values (1);
+# Create user which will try to do this.
+create user mysqltest_u1@localhost;
+grant insert, select on mysqltest_db1.t1 to mysqltest_u1@localhost;
+# Corrupt t1 by replacing t1.MYI with a corrupt + unclosed one created
+# by doing: 'create table t1 (a int key(a))'
+#           head -c1024 t1.MYI > corrupt_t1.MYI 
+flush table mysqltest_db1.t1;
+# Switching to connection 'con1'.
+check table mysqltest_db1.t1;
+Table	Op	Msg_type	Msg_text
+mysqltest_db1.t1	check	warning	1 client is using or hasn't closed the table properly
+mysqltest_db1.t1	check	error	Size of indexfile is: 1024        Should be: 2048
+mysqltest_db1.t1	check	warning	Size of datafile is: 14       Should be: 7
+mysqltest_db1.t1	check	error	Corrupt
+# The below statement should fail before repairing t1.
+# Otherwise info about such repair will be missing from its result-set.
+repair table mysqltest_db1.t1, mysqltest_db1.t2;
+ERROR 42000: SELECT, INSERT command denied to user 'mysqltest_u1'@'localhost' for table 't2'
+# The same is true for CHECK TABLE statement.
+check table mysqltest_db1.t1, mysqltest_db1.t2;
+ERROR 42000: SELECT command denied to user 'mysqltest_u1'@'localhost' for table 't2'
+check table mysqltest_db1.t1;
+Table	Op	Msg_type	Msg_text
+mysqltest_db1.t1	check	warning	Table is marked as crashed
+mysqltest_db1.t1	check	warning	1 client is using or hasn't closed the table properly
+mysqltest_db1.t1	check	error	Size of indexfile is: 1024        Should be: 2048
+mysqltest_db1.t1	check	warning	Size of datafile is: 14       Should be: 7
+mysqltest_db1.t1	check	error	Corrupt
+repair table mysqltest_db1.t1;
+Table	Op	Msg_type	Msg_text
+mysqltest_db1.t1	repair	warning	Number of rows changed from 1 to 2
+mysqltest_db1.t1	repair	status	OK
+# Clean-up.
+# Switching to connection 'default'.
+drop database mysqltest_db1;
+drop user mysqltest_u1@localhost;

=== modified file 'mysql-test/t/grant4.test'
--- a/mysql-test/t/grant4.test	2009-10-19 12:58:13 +0000
+++ b/mysql-test/t/grant4.test	2010-12-01 10:07:22 +0000
@@ -144,3 +144,62 @@ connection default;
 disconnect con1;
 drop database mysqltest_db1;
 drop user mysqltest_u1@localhost;
+
+
+--echo #
+--echo # Additional coverage for refactoring which is made as part
+--echo # of fix for bug #27480 "Extend CREATE TEMPORARY TABLES privilege
+--echo # to allow temp table operations".
+--echo # 
+--echo # Check that for statements like CHECK/REPAIR and OPTIMIZE TABLE
+--echo # privileges for all tables involved are checked before processing
+--echo # any tables. Doing otherwise, i.e. checking privileges for table
+--echo # right before processing it might result in lost results for tables
+--echo # which were processed by the time when table for which privileges
+--echo # are insufficient are discovered.
+--echo #
+call mtr.add_suppression("Got an error from thread_id=.*ha_myisam.cc:");
+call mtr.add_suppression("MySQL thread id .*, query id .* localhost.*mysqltest_u1 Checking table");
+--disable_warnings
+drop database if exists mysqltest_db1;
+--enable_warnings
+let $MYSQLD_DATADIR = `SELECT @@datadir`;
+create database mysqltest_db1;
+--echo # Create tables which we are going to CHECK/REPAIR.
+create table mysqltest_db1.t1 (a int, key(a)) engine=myisam;
+create table mysqltest_db1.t2 (b int);
+insert into mysqltest_db1.t1 values (1), (2);
+insert into mysqltest_db1.t2 values (1);
+--echo # Create user which will try to do this.
+create user mysqltest_u1@localhost;
+grant insert, select on mysqltest_db1.t1 to mysqltest_u1@localhost;
+connect (con1,localhost,mysqltest_u1,,);
+connection default;
+
+--echo # Corrupt t1 by replacing t1.MYI with a corrupt + unclosed one created
+--echo # by doing: 'create table t1 (a int key(a))'
+--echo #           head -c1024 t1.MYI > corrupt_t1.MYI 
+flush table mysqltest_db1.t1;
+--remove_file $MYSQLD_DATADIR/mysqltest_db1/t1.MYI
+--copy_file std_data/corrupt_t1.MYI $MYSQLD_DATADIR/mysqltest_db1/t1.MYI
+
+--echo # Switching to connection 'con1'.
+connection con1;
+check table mysqltest_db1.t1;
+--echo # The below statement should fail before repairing t1.
+--echo # Otherwise info about such repair will be missing from its result-set.
+--error ER_TABLEACCESS_DENIED_ERROR
+repair table mysqltest_db1.t1, mysqltest_db1.t2;
+--echo # The same is true for CHECK TABLE statement.
+--error ER_TABLEACCESS_DENIED_ERROR
+check table mysqltest_db1.t1, mysqltest_db1.t2;
+check table mysqltest_db1.t1;
+repair table mysqltest_db1.t1;
+
+--echo # Clean-up.
+disconnect con1;
+--source include/wait_until_disconnected.inc
+--echo # Switching to connection 'default'.
+connection default;
+drop database mysqltest_db1;
+drop user mysqltest_u1@localhost;

=== modified file 'sql/sql_admin.cc'
--- a/sql/sql_admin.cc	2010-11-29 14:13:07 +0000
+++ b/sql/sql_admin.cc	2010-12-01 10:07:22 +0000
@@ -261,8 +261,6 @@ static bool mysql_admin_table(THD* thd, 
                               HA_CHECK_OPT* check_opt,
                               const char *operator_name,
                               thr_lock_type lock_type,
-                              bool any_priv_will_do,
-                              ulong op_privs,
                               bool open_for_modify,
                               bool no_warnings_for_error,
                               uint extra_open_options,
@@ -293,6 +291,12 @@ static bool mysql_admin_table(THD* thd, 
                             Protocol::SEND_NUM_ROWS | Protocol::SEND_EOF))
     DBUG_RETURN(TRUE);
 
+  /*
+    Close all temporary tables which were pre-open to simplify
+    privilege checking.
+  */
+  close_thread_tables(thd);
+
   for (table= tables; table; table= table->next_local)
   {
     char table_name[NAME_LEN*2+2];
@@ -300,29 +304,6 @@ static bool mysql_admin_table(THD* thd, 
     bool fatal_error=0;
     bool open_error;
 
-    /*
-      Here we need to open list of tables (one table is not enough) because
-      of merge tables: children of merge tables must be opened here.
-    */
-
-    TABLE_LIST *next_global_tl= table->next_global;
-
-    if (open_and_process_temporary_table_list(thd, table))
-      DBUG_RETURN(TRUE);
-
-    TABLE_LIST *last_added_tl;
-
-    for (last_added_tl= table;
-         last_added_tl && last_added_tl->next_global != next_global_tl;
-         last_added_tl= last_added_tl->next_global)
-      ;
-
-    if (op_privs)
-    {
-      if (check_table_access(thd, op_privs, table, any_priv_will_do, 1, FALSE))
-        DBUG_RETURN(TRUE);
-    }
-
     DBUG_PRINT("admin", ("table: '%s'.'%s'", table->db, table->table_name));
     DBUG_PRINT("admin", ("extra_open_options: %u", extra_open_options));
     strxmov(table_name, db, ".", table->table_name, NullS);
@@ -338,10 +319,11 @@ static bool mysql_admin_table(THD* thd, 
                                 MDL_SHARED_NO_READ_WRITE : MDL_SHARED_READ);
     /* open only one table from local list of command */
     {
-      TABLE_LIST *next_global_saved= last_added_tl->next_global;
-      TABLE_LIST *next_local_saved= last_added_tl->next_local;
-      last_added_tl->next_global= NULL;
-      last_added_tl->next_local= NULL;
+      TABLE_LIST *save_next_global, *save_next_local;
+      save_next_global= table->next_global;
+      table->next_global= 0;
+      save_next_local= table->next_local;
+      table->next_local= 0;
       select->table_list.first= table;
       /*
         Time zone tables and SP tables can be add to lex->query_tables list,
@@ -362,10 +344,14 @@ static bool mysql_admin_table(THD* thd, 
       if (view_operator_func == NULL)
         table->required_type=FRMTYPE_TABLE;
 
-      open_error= open_and_lock_tables(thd, table, TRUE, 0);
+      open_error= open_and_process_temporary_table_list(thd, table);
+
+      if (! open_error)
+        open_error= open_and_lock_tables(thd, table, TRUE, 0);
+
       thd->no_warnings_for_error= 0;
-      last_added_tl->next_global= next_global_saved;
-      last_added_tl->next_local= next_local_saved;
+      table->next_global= save_next_global;
+      table->next_local= save_next_local;
       thd->open_options&= ~extra_open_options;
     }
 
@@ -926,10 +912,8 @@ bool mysql_assign_to_keycache(THD* thd, 
   mysql_mutex_unlock(&LOCK_global_system_variables);
   check_opt.key_cache= key_cache;
   DBUG_RETURN(mysql_admin_table(thd, tables, &check_opt,
-                                "assign_to_keycache", TL_READ_NO_INSERT,
-                                FALSE, 0,
-                                FALSE, FALSE, 0, NULL,
-                                &handler::assign_to_keycache, NULL));
+                                "assign_to_keycache", TL_READ_NO_INSERT, 0, 0,
+                                0, 0, &handler::assign_to_keycache, 0));
 }
 
 
@@ -955,10 +939,8 @@ bool mysql_preload_keys(THD* thd, TABLE_
     outdated information if parallel inserts into cache blocks happen.
   */
   DBUG_RETURN(mysql_admin_table(thd, tables, 0,
-                                "preload_keys", TL_READ_NO_INSERT,
-                                FALSE, 0,
-                                FALSE, FALSE, 0, NULL,
-                                &handler::preload_keys, NULL));
+                                "preload_keys", TL_READ_NO_INSERT, 0, 0, 0, 0,
+                                &handler::preload_keys, 0));
 }
 
 
@@ -969,12 +951,13 @@ bool Sql_cmd_analyze_table::execute(THD 
   thr_lock_type lock_type = TL_READ_NO_INSERT;
   DBUG_ENTER("Sql_cmd_analyze_table::execute");
 
+  if (check_table_access(thd, SELECT_ACL | INSERT_ACL, first_table,
+                         FALSE, UINT_MAX, FALSE))
+    goto error;
   thd->enable_slow_log= opt_log_slow_admin_statements;
   res= mysql_admin_table(thd, first_table, &thd->lex->check_opt,
-                         "analyze", lock_type,
-                         FALSE, SELECT_ACL | INSERT_ACL,
-                         TRUE, FALSE, 0, NULL, &handler::ha_analyze, NULL);
-
+                         "analyze", lock_type, 1, 0, 0, 0,
+                         &handler::ha_analyze, 0);
   /* ! we write after unlocking the table */
   if (!res && !thd->lex->no_write_to_binlog)
   {
@@ -986,6 +969,7 @@ bool Sql_cmd_analyze_table::execute(THD 
   thd->lex->select_lex.table_list.first= first_table;
   thd->lex->query_tables= first_table;
 
+error:
   DBUG_RETURN(res);
 }
 
@@ -1003,9 +987,7 @@ bool Sql_cmd_check_table::execute(THD *t
   thd->enable_slow_log= opt_log_slow_admin_statements;
 
   res= mysql_admin_table(thd, first_table, &thd->lex->check_opt, "check",
-                         lock_type,
-                         TRUE, SELECT_ACL,
-                         FALSE, FALSE, HA_OPEN_FOR_REPAIR, NULL,
+                         lock_type, 0, 0, HA_OPEN_FOR_REPAIR, 0,
                          &handler::ha_check, &view_checksum);
 
   thd->lex->select_lex.table_list.first= first_table;
@@ -1029,9 +1011,8 @@ bool Sql_cmd_optimize_table::execute(THD
   res= (specialflag & (SPECIAL_SAFE_MODE | SPECIAL_NO_NEW_FUNC)) ?
     mysql_recreate_table(thd, first_table) :
     mysql_admin_table(thd, first_table, &thd->lex->check_opt,
-                      "optimize", TL_WRITE,
-                      FALSE, SELECT_ACL | INSERT_ACL,
-                      TRUE, FALSE, 0, NULL, &handler::ha_optimize, 0);
+                      "optimize", TL_WRITE, 1, 0, 0, 0,
+                      &handler::ha_optimize, 0);
   /* ! we write after unlocking the table */
   if (!res && !thd->lex->no_write_to_binlog)
   {
@@ -1059,12 +1040,10 @@ bool Sql_cmd_repair_table::execute(THD *
     goto error; /* purecov: inspected */
   thd->enable_slow_log= opt_log_slow_admin_statements;
   res= mysql_admin_table(thd, first_table, &thd->lex->check_opt, "repair",
-                         TL_WRITE,
-                         FALSE, SELECT_ACL| INSERT_ACL,
-                         TRUE,
+                         TL_WRITE, 1,
                          test(thd->lex->check_opt.sql_flags & TT_USEFRM),
                          HA_OPEN_FOR_REPAIR, &prepare_for_repair,
-                         &handler::ha_repair, NULL);
+                         &handler::ha_repair, 0);
 
   /* ! we write after unlocking the table */
   if (!res && !thd->lex->no_write_to_binlog)

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	2010-11-29 14:13:07 +0000
+++ b/sql/sql_parse.cc	2010-12-01 10:07:22 +0000
@@ -476,6 +476,12 @@ void init_update_queries(void)
   sql_command_flags[SQLCOM_DO]|=              CF_PREOPEN_TMP_TABLES;
   sql_command_flags[SQLCOM_CALL]|=            CF_PREOPEN_TMP_TABLES;
   sql_command_flags[SQLCOM_CHECKSUM]|=        CF_PREOPEN_TMP_TABLES;
+  sql_command_flags[SQLCOM_ANALYZE]|=         CF_PREOPEN_TMP_TABLES;
+  sql_command_flags[SQLCOM_CHECK]|=           CF_PREOPEN_TMP_TABLES;
+  sql_command_flags[SQLCOM_OPTIMIZE]|=        CF_PREOPEN_TMP_TABLES;
+  sql_command_flags[SQLCOM_REPAIR]|=          CF_PREOPEN_TMP_TABLES;
+  sql_command_flags[SQLCOM_PRELOAD_KEYS]|=    CF_PREOPEN_TMP_TABLES;
+  sql_command_flags[SQLCOM_ASSIGN_TO_KEYCACHE]|= CF_PREOPEN_TMP_TABLES;
 
   /*
     DDL statements should start with closing opened handlers.

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-trunk-bugfixing branch (Dmitry.Lenev:3392 to 3393)Bug#27480Dmitry Lenev1 Dec