A fix and a test case for Bug#15752 "Lost connection to MySQL server

when calling a SP from C API"

The bug was caused by lack of checks for misuse in mysql_real_query. 
A stored procedure always returns at least one result, which is the 
status of execution of the procedure itself.
This result, or so-called OK packet, is similar to a result
returned by INSERT/UPDATE/CREATE operations: it contains the overall
status of execution, the number of affected rows and the number of
warnings. The client test program attached to the bug did not read this 
result and ivnoked the next query. In turn, libmysql had no check for 
such scenario and mysql_real_query was simply trying to send that query 
without reading the pending response, thus messing up the communication
protocol.

The fix is to return an error from mysql_real_query when it's called
prior to retrieval of all pending results.


client/mysqlbinlog.cc:
  net_safe_read -> cli_safe_read
include/mysql.h:
  Remove a private function from the public header.
include/mysql_com.h:
  Remove a define that is never used.
include/sql_common.h:
  Add a declaration for cli_safe_read - a function that reads one packet
  from the server.
libmysql/libmysql.c:
  net_safe_read -> cli_safe_read
  Return CR_COMMANDS_OUT_OF_SYNC on attempt to execute a statement
  using a connection which has pending result sets.
sql-common/client.c:
  Actual fix for Bug#15752: if the server has pending result sets for
  the client, return CR_COMMANDS_OUT_OF_SYNC on attempt to execute
  another query. Similarly to the behaviour of mysql_use_result(),
  multiple result sets block the connection and must be fetched
  before it can be used for another query.
  This uncovered an error in the protocol: the server doesn't drop
  SERVER_MORE_RESULTS_EXISTS status flag upon an error, so in case of
  a multi-query like SELECT 1; SELECT syntax_error; SELECT 2; 
  the client has no way to know that the server won't ever come to 
  execution of the third query and won't return any result sets for it.
  For now, fix it in cli_safe_read, as a proper fix requires extension
  of the client-server protocol.
sql/protocol.cc:
  Remove a name that is never used.
sql/slave.cc:
  net_safe_read -> cli_safe_read
tests/mysql_client_test.c:
  Make 'query' a local variable to avoid name clash.
  Add a test case for Bug#15752 "Lost connection to MySQL server when
  calling an SP from C API"

1 files changed, 40 insertions, 29 deletions

diff --git a/sql-common/client.c b/sql-common/client.c
index 31e85475f08..7c5460dd859 100644
--- a/sql-common/client.c
+++ b/sql-common/client.c
@@ -584,7 +584,7 @@ err:
 *****************************************************************************/
 
 ulong
-net_safe_read(MYSQL *mysql)
+cli_safe_read(MYSQL *mysql)
 {
   NET *net= &mysql->net;
   ulong len=0;
@@ -627,6 +627,16 @@ net_safe_read(MYSQL *mysql)
     }
     else
       set_mysql_error(mysql, CR_UNKNOWN_ERROR, unknown_sqlstate);
+    /*
+      Cover a protocol design error: error packet does not
+      contain the server status. Therefore, the client has no way
+      to find out whether there are more result sets of
+      a multiple-result-set statement pending. Luckily, in 5.0 an
+      error always aborts execution of a statement, wherever it is
+      a multi-statement or a stored procedure, so it should be
+      safe to unconditionally turn off the flag here.
+    */
+    mysql->server_status&= ~SERVER_MORE_RESULTS_EXISTS;
 
     DBUG_PRINT("error",("Got error: %d/%s (%s)",
 			net->last_errno, net->sqlstate, net->last_error));
@@ -652,7 +662,7 @@ cli_advanced_command(MYSQL *mysql, enum enum_server_command command,
   NET *net= &mysql->net;
   my_bool result= 1;
   init_sigpipe_variables
-    DBUG_ENTER("cli_advanced_command");
+  DBUG_ENTER("cli_advanced_command");
 
   /* Don't give sigpipe errors if the client doesn't want them */
   set_sigpipe(mysql);
@@ -662,7 +672,8 @@ cli_advanced_command(MYSQL *mysql, enum enum_server_command command,
     if (mysql_reconnect(mysql))
       DBUG_RETURN(1);
   }
-  if (mysql->status != MYSQL_STATUS_READY)
+  if (mysql->status != MYSQL_STATUS_READY ||
+      mysql->server_status & SERVER_MORE_RESULTS_EXISTS)
   {
     DBUG_PRINT("error",("state: %d", mysql->status));
     set_mysql_error(mysql, CR_COMMANDS_OUT_OF_SYNC, unknown_sqlstate);
@@ -701,7 +712,7 @@ cli_advanced_command(MYSQL *mysql, enum enum_server_command command,
   }
   result=0;
   if (!skip_check)
-    result= ((mysql->packet_length=net_safe_read(mysql)) == packet_error ?
+    result= ((mysql->packet_length=cli_safe_read(mysql)) == packet_error ?
 	     1 : 0);
 end:
   reset_sigpipe(mysql);
@@ -753,7 +764,7 @@ static void cli_flush_use_result(MYSQL *mysql)
   for (;;)
   {
     ulong pkt_len;
-    if ((pkt_len=net_safe_read(mysql)) == packet_error)
+    if ((pkt_len=cli_safe_read(mysql)) == packet_error)
       break;
     if (pkt_len <= 8 && mysql->net.read_pos[0] == 254)
     {
@@ -1275,7 +1286,7 @@ MYSQL_DATA *cli_read_rows(MYSQL *mysql,MYSQL_FIELD *mysql_fields,
   NET *net = &mysql->net;
   DBUG_ENTER("cli_read_rows");
 
-  if ((pkt_len= net_safe_read(mysql)) == packet_error)
+  if ((pkt_len= cli_safe_read(mysql)) == packet_error)
     DBUG_RETURN(0);
   if (!(result=(MYSQL_DATA*) my_malloc(sizeof(MYSQL_DATA),
 				       MYF(MY_WME | MY_ZEROFILL))))
@@ -1340,7 +1351,7 @@ MYSQL_DATA *cli_read_rows(MYSQL *mysql,MYSQL_FIELD *mysql_fields,
       }
     }
     cur->data[field]=to;			/* End of last field */
-    if ((pkt_len=net_safe_read(mysql)) == packet_error)
+    if ((pkt_len=cli_safe_read(mysql)) == packet_error)
     {
       free_rows(result);
       DBUG_RETURN(0);
@@ -1372,7 +1383,7 @@ read_one_row(MYSQL *mysql,uint fields,MYSQL_ROW row, ulong *lengths)
   uchar *pos, *prev_pos, *end_pos;
   NET *net= &mysql->net;
 
-  if ((pkt_len=net_safe_read(mysql)) == packet_error)
+  if ((pkt_len=cli_safe_read(mysql)) == packet_error)
     return -1;
   if (pkt_len <= 8 && net->read_pos[0] == 254)
   {
@@ -1649,23 +1660,23 @@ static MYSQL_RES *cli_use_result(MYSQL *mysql);
 
 static MYSQL_METHODS client_methods=
 {
-  cli_read_query_result,
-  cli_advanced_command,
-  cli_read_rows,
-  cli_use_result,
-  cli_fetch_lengths,
-  cli_flush_use_result
+  cli_read_query_result,                       /* read_query_result */
+  cli_advanced_command,                        /* advanced_command */
+  cli_read_rows,                               /* read_rows */
+  cli_use_result,                              /* use_result */
+  cli_fetch_lengths,                           /* fetch_lengths */
+  cli_flush_use_result                         /* flush_use_result */
 #ifndef MYSQL_SERVER
-  ,cli_list_fields,
-  cli_read_prepare_result,
-  cli_stmt_execute,
-  cli_read_binary_rows,
-  cli_unbuffered_fetch,
-  NULL,
-  cli_read_statistics,
-  cli_read_query_result,
-  cli_read_change_user_result,
-  cli_read_binary_rows
+  ,cli_list_fields,                            /* list_fields */
+  cli_read_prepare_result,                     /* read_prepare_result */
+  cli_stmt_execute,                            /* stmt_execute */
+  cli_read_binary_rows,                        /* read_binary_rows */
+  cli_unbuffered_fetch,                        /* unbuffered_fetch */
+  NULL,                                        /* free_embedded_thd */
+  cli_read_statistics,                         /* read_statistics */
+  cli_read_query_result,                       /* next_result */
+  cli_read_change_user_result,                 /* read_change_user_result */
+  cli_read_binary_rows                         /* read_rows_from_cursor */
 #endif
 };
 
@@ -1999,7 +2010,7 @@ CLI_MYSQL_REAL_CONNECT(MYSQL *mysql,const char *host, const char *user,
     Part 1: Connection established, read and parse first packet
   */
 
-  if ((pkt_length=net_safe_read(mysql)) == packet_error)
+  if ((pkt_length=cli_safe_read(mysql)) == packet_error)
     goto error;
 
   /* Check if version of protocol matches current one */
@@ -2223,7 +2234,7 @@ CLI_MYSQL_REAL_CONNECT(MYSQL *mysql,const char *host, const char *user,
     OK-packet, or re-request scrambled password.
   */
 
-  if ((pkt_length=net_safe_read(mysql)) == packet_error)
+  if ((pkt_length=cli_safe_read(mysql)) == packet_error)
     goto error;
 
   if (pkt_length == 1 && net->read_pos[0] == 254 && 
@@ -2240,7 +2251,7 @@ CLI_MYSQL_REAL_CONNECT(MYSQL *mysql,const char *host, const char *user,
       goto error;
     }
     /* Read what server thinks about out new auth message report */
-    if (net_safe_read(mysql) == packet_error)
+    if (cli_safe_read(mysql) == packet_error)
       goto error;
   }
 
@@ -2564,7 +2575,7 @@ static my_bool cli_read_query_result(MYSQL *mysql)
   */
   mysql = mysql->last_used_con;
 
-  if ((length = net_safe_read(mysql)) == packet_error)
+  if ((length = cli_safe_read(mysql)) == packet_error)
     DBUG_RETURN(1);
   free_old_query(mysql);		/* Free old result */
 #ifdef MYSQL_CLIENT			/* Avoid warn of unused labels*/
@@ -2599,7 +2610,7 @@ get_info:
   if (field_count == NULL_LENGTH)		/* LOAD DATA LOCAL INFILE */
   {
     int error=handle_local_infile(mysql,(char*) pos);
-    if ((length=net_safe_read(mysql)) == packet_error || error)
+    if ((length= cli_safe_read(mysql)) == packet_error || error)
       DBUG_RETURN(1);
     goto get_info;				/* Get info packet */
   }