bk commit into 5.1 tree (anozdrin:1.2612) BUG#30472

[Alexander Nozdrin]

Below is the list of changes that have just been committed into a local
5.1 repository of alik. When alik 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, 2007-09-28 15:42:37+04:00, anozdrin@station. +4 -0
  Prerequisite patch for BUG#30472: libmysql doesn't reset charset,
  insert_id after succ. mysql_change_user() call.
  
  Supply a correct packet length to dispatch command.

  sql/sp_head.cc@stripped, 2007-09-28 15:42:35+04:00, anozdrin@station. +1 -1
    Fix packet length.

  sql/sql_parse.cc@stripped, 2007-09-28 15:42:35+04:00, anozdrin@station. +65 -47
    Fix packet length.

  sql/sql_prepare.cc@stripped, 2007-09-28 15:42:35+04:00, anozdrin@station. +6 -5
    Fix packet length.

  tests/mysql_client_test.c@stripped, 2007-09-28 15:42:35+04:00, anozdrin@station. +200 -1
    Test case for COM_CHANGE_USER.

diff -Nrup a/sql/sp_head.cc b/sql/sp_head.cc
--- a/sql/sp_head.cc	2007-08-31 22:13:22 +04:00
+++ b/sql/sp_head.cc	2007-09-28 15:42:35 +04:00
@@ -2699,7 +2699,7 @@ sp_instr_stmt::execute(THD *thd, uint *n
 
   query= thd->query;
   query_length= thd->query_length;
-  if (!(res= alloc_query(thd, m_query.str, m_query.length+1)) &&
+  if (!(res= alloc_query(thd, m_query.str, m_query.length)) &&
       !(res=subst_spvars(thd, this, &m_query)))
   {
     /*
diff -Nrup a/sql/sql_parse.cc b/sql/sql_parse.cc
--- a/sql/sql_parse.cc	2007-09-10 22:53:16 +04:00
+++ b/sql/sql_parse.cc	2007-09-28 15:42:35 +04:00
@@ -321,8 +321,6 @@ void execute_init_command(THD *thd, sys_
     values of init_command_var can't be changed
   */
   rw_rdlock(var_mutex);
-  thd->query= init_command_var->value;
-  thd->query_length= init_command_var->value_length;
   save_client_capabilities= thd->client_capabilities;
   thd->client_capabilities|= CLIENT_MULTI_QUERIES;
   /*
@@ -332,7 +330,9 @@ void execute_init_command(THD *thd, sys_
   save_vio= thd->net.vio;
   thd->net.vio= 0;
   thd->net.no_send_error= 0;
-  dispatch_command(COM_QUERY, thd, thd->query, thd->query_length+1);
+  dispatch_command(COM_QUERY, thd,
+                   init_command_var->value,
+                   init_command_var->value_length);
   rw_unlock(var_mutex);
   thd->client_capabilities= save_client_capabilities;
   thd->net.vio= save_vio;
@@ -691,30 +691,39 @@ bool do_command(THD *thd)
     net->error= 0;
     DBUG_RETURN(FALSE);
   }
-  else
+
+  packet= (char*) net->read_pos;
+  /*
+    'packet_length' contains length of data, as it was stored in packet
+    header. In case of malformed header, my_net_read returns zero.
+    If packet_length is not zero, my_net_read ensures that the returned
+    number of bytes was actually read from network.
+    There is also an extra safety measure in my_net_read:
+    it sets packet[packet_length]= 0, but only for non-zero packets.
+  */
+  if (packet_length == 0)                       /* safety */
   {
-    packet=(char*) net->read_pos;
-    command = (enum enum_server_command) (uchar) packet[0];
-    if (command >= COM_END)
-      command= COM_END;				// Wrong command
-    DBUG_PRINT("info",("Command on %s = %d (%s)",
-		       vio_description(net->vio), command,
-		       command_name[command].str));
+    /* Initialize with COM_SLEEP packet */
+    packet[0]= (uchar) COM_SLEEP;
+    packet_length= 1;
   }
+  /* Do not rely on my_net_read, extra safety against programming errors. */
+  packet[packet_length]= '\0';                  /* safety */
+
+  command= (enum enum_server_command) (uchar) packet[0];
+
+  if (command >= COM_END)
+    command= COM_END;				// Wrong command
+
+  DBUG_PRINT("info",("Command on %s = %d (%s)",
+                     vio_description(net->vio), command,
+                     command_name[command].str));
 
   /* Restore read timeout value */
   my_net_set_read_timeout(net, thd->variables.net_read_timeout);
 
-  /*
-    packet_length contains length of data, as it was stored in packet
-    header. In case of malformed header, packet_length can be zero.
-    If packet_length is not zero, my_net_read ensures that this number
-    of bytes was actually read from network. Additionally my_net_read
-    sets packet[packet_length]= 0 (thus if packet_length == 0,
-    command == packet[0] == COM_SLEEP).
-    In dispatch_command packet[packet_length] points beyond the end of packet.
-  */
-  DBUG_RETURN(dispatch_command(command,thd, packet+1, (uint) packet_length));
+  DBUG_ASSERT(packet_length);
+  DBUG_RETURN(dispatch_command(command, thd, packet+1, (uint) (packet_length-1)));
 }
 #endif  /* EMBEDDED_LIBRARY */
 
@@ -727,9 +736,7 @@ bool do_command(THD *thd)
     thd             connection handle
     command         type of command to perform 
     packet          data for the command, packet is always null-terminated
-    packet_length   length of packet + 1 (to show that data is
-                    null-terminated) except for COM_SLEEP, where it
-                    can be zero.
+    packet_length   length of packet. Can be zero, e.g. in case of COM_SLEEP.
   RETURN VALUE
     0   ok
     1   request of thread shutdown, i. e. if command is
@@ -773,7 +780,7 @@ bool dispatch_command(enum enum_server_c
     LEX_STRING tmp;
     status_var_increment(thd->status_var.com_stat[SQLCOM_CHANGE_DB]);
     thd->convert_string(&tmp, system_charset_info,
-			packet, packet_length-1, thd->charset());
+			packet, packet_length, thd->charset());
     if (!mysql_change_db(thd, &tmp, FALSE))
     {
       general_log_print(thd, command, "%s",thd->db);
@@ -793,14 +800,16 @@ bool dispatch_command(enum enum_server_c
   {
     char *tbl_name;
     LEX_STRING db;
+    /* Safe because there is always a trailing \0 at the end of the packet */
     uint db_len= *(uchar*) packet;
-    if (db_len >= packet_length || db_len > NAME_LEN)
+    if (db_len + 1 > packet_length || db_len > NAME_LEN)
     {
       my_message(ER_UNKNOWN_COM_ERROR, ER(ER_UNKNOWN_COM_ERROR), MYF(0));
       break;
     }
+    /* Safe because there is always a trailing \0 at the end of the packet */
     uint tbl_len= *(uchar*) (packet + db_len + 1);
-    if (db_len+tbl_len+2 > packet_length || tbl_len > NAME_LEN)
+    if (db_len + tbl_len + 2 > packet_length || tbl_len > NAME_LEN)
     {
       my_message(ER_UNKNOWN_COM_ERROR, ER(ER_UNKNOWN_COM_ERROR), MYF(0));
       break;
@@ -823,7 +832,8 @@ bool dispatch_command(enum enum_server_c
   case COM_CHANGE_USER:
   {
     status_var_increment(thd->status_var.com_other);
-    char *user= (char*) packet, *packet_end= packet+ packet_length;
+    char *user= (char*) packet, *packet_end= packet + packet_length;
+    /* Safe because there is always a trailing \0 at the end of the packet */
     char *passwd= strend(user)+1;
 
     thd->change_user();
@@ -840,6 +850,15 @@ bool dispatch_command(enum enum_server_c
     char db_buff[NAME_LEN+1];                 // buffer to store db in utf8
     char *db= passwd;
     char *save_db;
+    /*
+      If there is no password supplied, the packet must contain '\0',
+      in any type of handshake (4.1 or pre-4.1).
+     */
+    if (passwd >= packet_end)
+    {
+      my_message(ER_UNKNOWN_COM_ERROR, ER(ER_UNKNOWN_COM_ERROR), MYF(0));
+      break;
+    }
     uint passwd_len= (thd->client_capabilities & CLIENT_SECURE_CONNECTION ?
                       (uchar)(*passwd++) : strlen(passwd));
     uint dummy_errors, save_db_length, db_length;
@@ -848,22 +867,17 @@ bool dispatch_command(enum enum_server_c
     USER_CONN *save_user_connect;
 
     db+= passwd_len + 1;
-#ifndef EMBEDDED_LIBRARY
-    /* Small check for incoming packet */
-    if ((uint) ((uchar*) db - net->read_pos) > packet_length)
+    /*
+      Database name is always NUL-terminated, so in case of empty database
+      the packet must contain at least the trailing '\0'.
+    */
+    if (db >= packet_end)
     {
       my_message(ER_UNKNOWN_COM_ERROR, ER(ER_UNKNOWN_COM_ERROR), MYF(0));
       break;
     }
-#endif
+    db_length= packet_end - db - 1; /* do not count the trailing '\0'  */
     /* Convert database name to utf8 */
-    /*
-      Handle problem with old bug in client protocol where db had an extra
-      \0
-    */
-    db_length= (packet_end - db);
-    if (db_length > 0 && db[db_length-1] == 0)
-      db_length--;
     db_buff[copy_and_convert(db_buff, sizeof(db_buff)-1,
                              system_charset_info, db, db_length,
                              thd->charset(), &dummy_errors)]= 0;
@@ -999,7 +1013,7 @@ bool dispatch_command(enum enum_server_c
     break;
 #else
   {
-    char *fields, *packet_end= packet + packet_length - 1, *arg_end;
+    char *fields, *packet_end= packet + packet_length, *arg_end;
     /* Locked closure of all tables */
     TABLE_LIST table_list;
     LEX_STRING conv_name;
@@ -1074,7 +1088,7 @@ bool dispatch_command(enum enum_server_c
       HA_CREATE_INFO create_info;
 
       status_var_increment(thd->status_var.com_stat[SQLCOM_CREATE_DB]);
-      if (thd->make_lex_string(&db, packet, packet_length - 1, FALSE) ||
+      if (thd->make_lex_string(&db, packet, packet_length, FALSE) ||
           thd->make_lex_string(&alias, db.str, db.length, FALSE) ||
           check_db_name(&db))
       {
@@ -1095,7 +1109,7 @@ bool dispatch_command(enum enum_server_c
       status_var_increment(thd->status_var.com_stat[SQLCOM_DROP_DB]);
       LEX_STRING db;
 
-      if (thd->make_lex_string(&db, packet, packet_length - 1, FALSE) ||
+      if (thd->make_lex_string(&db, packet, packet_length, FALSE) ||
           check_db_name(&db))
       {
 	my_error(ER_WRONG_DB_NAME, MYF(0), db.str ? db.str : "NULL");
@@ -1164,7 +1178,7 @@ bool dispatch_command(enum enum_server_c
       break; /* purecov: inspected */
     /*
       If the client is < 4.1.3, it is going to send us no argument; then
-      packet_length is 1, packet[0] is the end 0 of the packet. Note that
+      packet_length is 0, packet[0] is the end 0 of the packet. Note that
       SHUTDOWN_DEFAULT is 0. If client is >= 4.1.3, the shutdown level is in
       packet[0].
     */
@@ -1522,9 +1536,8 @@ int prepare_schema_table(THD *thd, LEX *
 
 bool alloc_query(THD *thd, const char *packet, uint packet_length)
 {
-  packet_length--;				// Remove end null
   /* Remove garbage at start and end of query */
-  while (my_isspace(thd->charset(),packet[0]) && packet_length > 0)
+  while (packet_length > 0 && my_isspace(thd->charset(), packet[0]))
   {
     packet++;
     packet_length--;
@@ -7238,7 +7251,12 @@ bool parse_sql(THD *thd,
 
   /* Parse the query. */
 
-  bool err_status= MYSQLparse(thd) != 0 || thd->is_fatal_error;
+  bool mysql_parse_status= MYSQLparse(thd) != 0;
+
+  /* Check that if MYSQLparse() failed, thd->net.report_error is set. */
+
+  DBUG_ASSERT(!mysql_parse_status ||
+              mysql_parse_status && thd->net.report_error);
 
   /* Reset Lex_input_stream. */
 
@@ -7251,7 +7269,7 @@ bool parse_sql(THD *thd,
 
   /* That's it. */
 
-  return err_status;
+  return mysql_parse_status || thd->is_fatal_error;
 }
 
 /**
diff -Nrup a/sql/sql_prepare.cc b/sql/sql_prepare.cc
--- a/sql/sql_prepare.cc	2007-09-10 22:53:17 +04:00
+++ b/sql/sql_prepare.cc	2007-09-28 15:42:35 +04:00
@@ -2107,7 +2107,7 @@ void mysql_sql_stmt_prepare(THD *thd)
     DBUG_VOID_RETURN;
   }
 
-  if (stmt->prepare(query, query_len+1))
+  if (stmt->prepare(query, query_len))
   {
     /* Statement map deletes the statement on erase */
     thd->stmt_map.erase(stmt);
@@ -2270,7 +2270,7 @@ void mysql_stmt_execute(THD *thd, char *
   /* Query text for binary, general or slow log, if any of them is open */
   String expanded_query;
 #ifndef EMBEDDED_LIBRARY
-  uchar *packet_end= packet + packet_length - 1;
+  uchar *packet_end= packet + packet_length;
 #endif
   Prepared_statement *stmt;
   bool error;
@@ -2585,14 +2585,14 @@ void mysql_stmt_get_longdata(THD *thd, c
   Prepared_statement *stmt;
   Item_param *param;
 #ifndef EMBEDDED_LIBRARY
-  char *packet_end= packet + packet_length - 1;
+  char *packet_end= packet + packet_length;
 #endif
   DBUG_ENTER("mysql_stmt_get_longdata");
 
   status_var_increment(thd->status_var.com_stmt_send_long_data);
 #ifndef EMBEDDED_LIBRARY
   /* Minimal size of long data packet is 6 bytes */
-  if (packet_length <= MYSQL_LONG_DATA_HEADER)
+  if (packet_length < MYSQL_LONG_DATA_HEADER)
   {
     my_error(ER_WRONG_ARGUMENTS, MYF(0), "mysql_stmt_send_long_data");
     DBUG_VOID_RETURN;
@@ -2866,6 +2866,7 @@ bool Prepared_statement::prepare(const c
   error= parse_sql(thd, &lip, NULL) ||
          thd->net.report_error ||
          init_param_array(this);
+
   lex->set_trg_event_type_for_tables();
 
   /* Remember the current database. */
@@ -3059,7 +3060,7 @@ bool Prepared_statement::execute(String 
 
   if (expanded_query->length() &&
       alloc_query(thd, (char*) expanded_query->ptr(),
-                  expanded_query->length()+1))
+                  expanded_query->length()))
   {
     my_error(ER_OUTOFMEMORY, 0, expanded_query->length());
     goto error;
diff -Nrup a/tests/mysql_client_test.c b/tests/mysql_client_test.c
--- a/tests/mysql_client_test.c	2007-08-25 12:43:15 +04:00
+++ b/tests/mysql_client_test.c	2007-09-28 15:42:35 +04:00
@@ -13534,7 +13534,7 @@ static void test_bug9478()
 
     {
       char buff[8];
-      /* Fill in the fethc packet */
+      /* Fill in the fetch packet */
       int4store(buff, stmt->stmt_id);
       buff[4]= 1;                               /* prefetch rows */
       rc= ((*mysql->methods->advanced_command)(mysql, COM_STMT_FETCH,
@@ -16214,6 +16214,204 @@ static void test_bug28934()
   myquery(mysql_query(mysql, "drop table t1"));
 }
 
+/*
+  Test mysql_change_user() C API and COM_CHANGE_USER
+*/
+
+static void test_change_user()
+{
+  char buff[256];
+  const char *user_pw= "mysqltest_pw";
+  const char *user_no_pw= "mysqltest_no_pw";
+  const char *pw= "password";
+  const char *db= "mysqltest_user_test_database";
+  int rc;
+
+  DBUG_ENTER("test_change_user");
+  myheader("test_change_user");
+
+  /* Prepare environment */
+  sprintf(buff, "drop database if exists %s", db);
+  rc= mysql_query(mysql, buff);
+  myquery(rc);
+
+  sprintf(buff, "create database %s", db);
+  rc= mysql_query(mysql, buff);
+  myquery(rc);
+
+  sprintf(buff,
+          "grant select on %s.* to %s@'%%' identified by '%s'",
+          db,
+          user_pw,
+          pw);
+  rc= mysql_query(mysql, buff);
+  myquery(rc);
+
+  sprintf(buff,
+          "grant select on %s.* to %s@'%%'",
+          db,
+          user_no_pw);
+  rc= mysql_query(mysql, buff);
+  myquery(rc);
+
+
+  /* Try some combinations */
+  rc= mysql_change_user(mysql, NULL, NULL, NULL);
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+
+  rc= mysql_change_user(mysql, "", NULL, NULL);
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, "", "", NULL);
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, "", "", "");
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, NULL, "", "");
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+
+  rc= mysql_change_user(mysql, NULL, NULL, "");
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, "", NULL, "");
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, user_pw, NULL, "");
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, user_pw, "", "");
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, user_pw, "", NULL);
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, user_pw, NULL, NULL);
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, user_pw, "", db);
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, user_pw, NULL, db);
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, user_pw, pw, db);
+  myquery(rc);
+
+  rc= mysql_change_user(mysql, user_pw, pw, NULL);
+  myquery(rc);
+
+  rc= mysql_change_user(mysql, user_pw, pw, "");
+  myquery(rc);
+
+  rc= mysql_change_user(mysql, user_no_pw, pw, db);
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, user_no_pw, pw, "");
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, user_no_pw, pw, NULL);
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, user_no_pw, "", NULL);
+  myquery(rc);
+
+  rc= mysql_change_user(mysql, user_no_pw, "", "");
+  myquery(rc);
+
+  rc= mysql_change_user(mysql, user_no_pw, "", db);
+  myquery(rc);
+
+  rc= mysql_change_user(mysql, user_no_pw, NULL, db);
+  myquery(rc);
+
+  rc= mysql_change_user(mysql, "", pw, db);
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, "", pw, "");
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, "", pw, NULL);
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, NULL, pw, NULL);
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, NULL, NULL, db);
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, NULL, "", db);
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  rc= mysql_change_user(mysql, "", "", db);
+  DIE_UNLESS(rc);
+  if (! opt_silent)
+    printf("Got error (as expected): %s\n", mysql_error(mysql));
+
+  /* Cleanup the environment */
+
+  mysql_change_user(mysql, opt_user, opt_password, current_db);
+
+  sprintf(buff, "drop database %s", db);
+  rc= mysql_query(mysql, buff);
+  myquery(rc);
+
+  sprintf(buff, "drop user %s@'%%'", user_pw);
+  rc= mysql_query(mysql, buff);
+  myquery(rc);
+
+  sprintf(buff, "drop user %s@'%%'", user_no_pw);
+  rc= mysql_query(mysql, buff);
+  myquery(rc);
+
+  DBUG_VOID_RETURN;
+}
 
 /*
   Bug#27592 (stack overrun when storing datetime value using prepared statements)
@@ -16744,6 +16942,7 @@ static struct my_tests_st my_tests[]= {
   { "test_bug29687", test_bug29687 },
   { "test_bug29692", test_bug29692 },
   { "test_bug29306", test_bug29306 },
+  { "test_change_user", test_change_user },
   { 0, 0 }
 };