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:
  Fix packet length.
sql/sql_parse.cc:
  Fix packet length.
sql/sql_prepare.cc:
  Fix packet length.
tests/mysql_client_test.c:
  Test case for COM_CHANGE_USER.

sql/sp_head.cc

@@ -2699,7 +2699,7 @@ sp_instr_stmt::execute(THD *thd, uint *nextp)
 
   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)))
   {
     /*

sql/sql_parse.cc

@@ -321,8 +321,6 @@ void execute_init_command(THD *thd, sys_var_str *init_command_var,
     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_var_str *init_command_var,
   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];
+    /* 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_command command, THD *thd,
     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_command command, THD *thd,
   {
     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_command command, THD *thd,
   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_command command, THD *thd,
     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_command command, THD *thd,
     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_command command, THD *thd,
     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_command command, THD *thd,
       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_command command, THD *thd,
       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_command command, THD *thd,
       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 *lex, Table_ident *table_ident,
 
 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;
 }
 
 /**

sql/sql_prepare.cc

@@ -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 *packet_arg, uint packet_length)
   /* 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, char *packet, ulong packet_length)
   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 char *packet, uint packet_len)
   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 *expanded_query, bool open_cursor)
 
   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;

tests/mysql_client_test.c

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