Skip to content

Commit

Permalink
Fixed "Packets out of order" warning message on stdout in clients,
Browse files Browse the repository at this point in the history
compiled for debugging, when the server goes down

This happens in the following scenario:
- Server gets a shutdown message
- Servers sends error ER_CONNECTION_KILLED to the clients connection
- The client sends a query to the server, before the server has time to
  close the connection to the client
- Client reads the ER_CONNECTION_KILLED error message

In the above case, the packet number for the reply is one less than
what the client expected and the client prints "Packets out of order".

Fixed the following way:
- The client accepts now error packages with a packet number
  one less than expected.
- To ensure that this issue can be detected early in my_real_read(), error
  messages sent to the client are not compressed, even when compressed protocol is used.
  • Loading branch information
montywi committed Aug 21, 2016
1 parent 6f31dd0 commit 5932fa7
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 36 deletions.
2 changes: 1 addition & 1 deletion include/mysql.h.pp
Expand Up @@ -27,7 +27,7 @@
char save_char;
char net_skip_rest_factor;
my_bool thread_specific_malloc;
my_bool compress;
unsigned char compress;
my_bool unused3;
unsigned char *unused;
unsigned int last_errno;
Expand Down
2 changes: 1 addition & 1 deletion include/mysql_com.h
Expand Up @@ -386,7 +386,7 @@ typedef struct st_net {
char save_char;
char net_skip_rest_factor;
my_bool thread_specific_malloc;
my_bool compress;
unsigned char compress;
my_bool unused3; /* Please remove with the next incompatible ABI change. */
/*
Pointer to query object in query cache, do not equal NULL (0) for
Expand Down
2 changes: 1 addition & 1 deletion mysql-test/include/wait_until_connected_again.inc
Expand Up @@ -14,7 +14,7 @@ while ($mysql_errno)
# Strangely enough, the server might return "Too many connections"
# while being shutdown, thus 1040 is an "allowed" error
# See BUG#36228
--error 0,1040,1053,2002,2003,2005,2006,2013
--error 0,1040,1053,2002,2003,2005,2006,2013,1927
show status;

dec $counter;
Expand Down
2 changes: 1 addition & 1 deletion mysql-test/include/wait_until_disconnected.inc
Expand Up @@ -12,7 +12,7 @@ while (!$mysql_errno)
# Strangely enough, the server might return "Too many connections"
# while being shutdown, thus 1040 is an "allowed" error.
# See BUG#36228.
--error 0,1040,1053,2002,2003,2005,2006,2013
--error 0,1040,1053,2002,2003,2005,2006,2013,1927
show status;

dec $counter;
Expand Down
97 changes: 68 additions & 29 deletions sql/net_serv.cc
Expand Up @@ -540,7 +540,7 @@ net_write_buff(NET *net, const uchar *packet, ulong len)
left_length= (ulong) (net->buff_end - net->write_pos);

#ifdef DEBUG_DATA_PACKETS
DBUG_DUMP("data", packet, len);
DBUG_DUMP("data_written", packet, len);
#endif
if (len > left_length)
{
Expand Down Expand Up @@ -629,7 +629,8 @@ net_real_write(NET *net,const uchar *packet, size_t len)
}
memcpy(b+header_length,packet,len);

if (my_compress(b+header_length, &len, &complen))
/* Don't compress error packets (compress == 2) */
if (net->compress == 2 || my_compress(b+header_length, &len, &complen))
complen=0;
int3store(&b[NET_HEADER_SIZE],complen);
int3store(b,len);
Expand All @@ -640,7 +641,7 @@ net_real_write(NET *net,const uchar *packet, size_t len)
#endif /* HAVE_COMPRESS */

#ifdef DEBUG_DATA_PACKETS
DBUG_DUMP("data", packet, len);
DBUG_DUMP("data_written", packet, len);
#endif

#ifndef NO_ALARM
Expand Down Expand Up @@ -830,6 +831,7 @@ my_real_read(NET *net, size_t *complen,
size_t length;
uint i,retry_count=0;
ulong len=packet_error;
my_bool expect_error_packet __attribute((unused))= 0;
thr_alarm_t alarmed;
#ifndef NO_ALARM
ALARM alarm_buff;
Expand Down Expand Up @@ -878,6 +880,7 @@ my_real_read(NET *net, size_t *complen,

if (i== 0 && thd_net_is_killed())
{
DBUG_PRINT("info", ("thd is killed"));
len= packet_error;
net->error= 0;
net->last_errno= ER_CONNECTION_KILLED;
Expand Down Expand Up @@ -947,39 +950,34 @@ my_real_read(NET *net, size_t *complen,
pos+= length;
update_statistics(thd_increment_bytes_received(length));
}

#ifdef DEBUG_DATA_PACKETS
DBUG_DUMP("data_read", net->buff+net->where_b, length);
#endif
if (i == 0)
{ /* First parts is packet length */
ulong helping;
#ifndef DEBUG_DATA_PACKETS
DBUG_DUMP("packet_header", net->buff+net->where_b,
NET_HEADER_SIZE);
#endif
if (net->buff[net->where_b + 3] != (uchar) net->pkt_nr)
{
if (net->buff[net->where_b] != (uchar) 255)
{
DBUG_PRINT("error",
("Packets out of order (Found: %d, expected %u)",
(int) net->buff[net->where_b + 3],
net->pkt_nr));
/*
We don't make noise server side, since the client is expected
to break the protocol for e.g. --send LOAD DATA .. LOCAL where
the server expects the client to send a file, but the client
may reply with a new command instead.
*/
{
#ifndef MYSQL_SERVER
EXTRA_DEBUG_fflush(stdout);
EXTRA_DEBUG_fprintf(stderr,"Error: Packets out of order (Found: %d, expected %d)\n",
(int) net->buff[net->where_b + 3],
(uint) (uchar) net->pkt_nr);
EXTRA_DEBUG_fflush(stderr);
if (net->buff[net->where_b + 3] == (uchar) (net->pkt_nr -1))
{
/*
If the server was killed then the server may have missed the
last sent client packet and the packet numbering may be one off.
*/
DBUG_PRINT("warning", ("Found possible out of order packets"));
expect_error_packet= 1;
}
else
#endif
}
len= packet_error;
/* Not a NET error on the client. XXX: why? */
MYSQL_SERVER_my_error(ER_NET_PACKETS_OUT_OF_ORDER, MYF(0));
goto end;
}
net->compress_pkt_nr= ++net->pkt_nr;
goto packets_out_of_order;
}
net->compress_pkt_nr= ++net->pkt_nr;
#ifdef HAVE_COMPRESS
if (net->compress)
{
Expand Down Expand Up @@ -1027,6 +1025,21 @@ my_real_read(NET *net, size_t *complen,
}
#endif
}
#ifndef MYSQL_SERVER
else if (expect_error_packet)
{
/*
This check is safe both for compressed and not compressed protocol
as for the compressed protocol errors are not compressed anymore.
*/
if (net->buff[net->where_b] != (uchar) 255)
{
/* Restore pkt_nr to original value */
net->pkt_nr--;
goto packets_out_of_order;
}
}
#endif
}

end:
Expand All @@ -1040,7 +1053,7 @@ my_real_read(NET *net, size_t *complen,
net->reading_or_writing=0;
#ifdef DEBUG_DATA_PACKETS
if (len != packet_error)
DBUG_DUMP("data", net->buff+net->where_b, len);
DBUG_DUMP("data_read", net->buff+net->where_b, len);
#endif
#ifdef MYSQL_SERVER
if (server_extension != NULL)
Expand All @@ -1051,9 +1064,35 @@ my_real_read(NET *net, size_t *complen,
}
#endif
return(len);

packets_out_of_order:
{
DBUG_PRINT("error",
("Packets out of order (Found: %d, expected %u)",
(int) net->buff[net->where_b + 3],
net->pkt_nr));
DBUG_ASSERT(0);
/*
We don't make noise server side, since the client is expected
to break the protocol for e.g. --send LOAD DATA .. LOCAL where
the server expects the client to send a file, but the client
may reply with a new command instead.
*/
#ifndef MYSQL_SERVER
EXTRA_DEBUG_fflush(stdout);
EXTRA_DEBUG_fprintf(stderr,"Error: Packets out of order (Found: %d, expected %d)\n",
(int) net->buff[net->where_b + 3],
(uint) (uchar) net->pkt_nr);
EXTRA_DEBUG_fflush(stderr);
#endif
len= packet_error;
MYSQL_SERVER_my_error(ER_NET_PACKETS_OUT_OF_ORDER, MYF(0));
goto end;
}
}



/* Old interface. See my_net_read_packet() for function description */

#undef my_net_read
Expand Down
15 changes: 12 additions & 3 deletions sql/protocol.cc
Expand Up @@ -373,7 +373,8 @@ bool net_send_error_packet(THD *thd, uint sql_errno, const char *err,
uint error;
char converted_err[MYSQL_ERRMSG_SIZE];
char buff[2+1+SQLSTATE_LENGTH+MYSQL_ERRMSG_SIZE], *pos;

my_bool ret;
uint8 save_compress;
DBUG_ENTER("send_error_packet");

if (net->vio == 0)
Expand Down Expand Up @@ -401,8 +402,16 @@ bool net_send_error_packet(THD *thd, uint sql_errno, const char *err,
/* Converted error message is always null-terminated. */
length= (uint) (strmake(pos, converted_err, MYSQL_ERRMSG_SIZE - 1) - buff);

DBUG_RETURN(net_write_command(net,(uchar) 255, (uchar*) "", 0, (uchar*) buff,
length));
/*
Ensure that errors are not compressed. This is to ensure we can
detect out of bands error messages in the client
*/
if ((save_compress= net->compress))
net->compress= 2;
ret= net_write_command(net,(uchar) 255, (uchar*) "", 0, (uchar*) buff,
length);
net->compress= save_compress;
DBUG_RETURN(ret);
}

#endif /* EMBEDDED_LIBRARY */
Expand Down

0 comments on commit 5932fa7

Please sign in to comment.