From 902004c1ee4e6234e229d7eb890a75b5fac2eaa1 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Thu, 14 Aug 2014 22:33:57 +0300 Subject: [PATCH] Fix to bug #463, http://bugs.skysql.com/show_bug.cgi?id=463 mysql_common.c:gw_MySQL_get_next_packet didn't handle case where an insert command followed by alter table in the same read buffer. It shouldn't been possible without multi-statement being set. --- server/core/dcb.c | 2 + server/modules/protocol/mysql_client.c | 3 +- server/modules/protocol/mysql_common.c | 52 +++++++------------ .../routing/readwritesplit/readwritesplit.c | 2 + 4 files changed, 24 insertions(+), 35 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 090d57554..5a280c3b0 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -814,6 +814,8 @@ int below_water; } spinlock_acquire(&dcb->writeqlock); + + ss_dassert(dcb->state != DCB_STATE_ZOMBIE); if (dcb->writeq != NULL) { diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 0f243fb1d..6ffe4e56f 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -1435,12 +1435,11 @@ static int route_by_statement( ss_dassert(GWBUF_IS_TYPE_MYSQL((*p_readbuf))); packetbuf = gw_MySQL_get_next_packet(p_readbuf); - - ss_dassert(GWBUF_IS_TYPE_MYSQL(packetbuf)); if (packetbuf != NULL) { CHK_GWBUF(packetbuf); + ss_dassert(GWBUF_IS_TYPE_MYSQL(packetbuf)); /** * This means that buffer includes exactly one MySQL * statement. diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 4e06894d1..f9c0ebdea 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -1514,6 +1514,9 @@ GWBUF* gw_MySQL_get_next_packet( size_t packetlen; size_t totalbuflen; uint8_t* data; + size_t nbytes_copied = 0; + uint8_t* target; + readbuf = *p_readbuf; if (readbuf == NULL) @@ -1540,44 +1543,27 @@ GWBUF* gw_MySQL_get_next_packet( packetbuf = NULL; goto return_packetbuf; } - /** there is one complete packet in the buffer */ - if (packetlen == buflen) - { - packetbuf = gwbuf_clone_portion(readbuf, 0, packetlen); - *p_readbuf = gwbuf_consume(readbuf, packetlen); - goto return_packetbuf; - } + + packetbuf = gwbuf_alloc(packetlen); + target = GWBUF_DATA(packetbuf); + packetbuf->gwbuf_type = readbuf->gwbuf_type; /*< Copy the type too */ /** - * Packet spans multiple buffers. - * Allocate buffer for complete packet - * copy packet parts into it and consume copied bytes - */ - else if (packetlen > buflen) + * Copy first MySQL packet to packetbuf and leave posible other + * packets to read buffer. + */ + while (nbytes_copied < packetlen && totalbuflen > 0) { - size_t nbytes_copied = 0; - uint8_t* target; + uint8_t* src = GWBUF_DATA((*p_readbuf)); + size_t bytestocopy; - packetbuf = gwbuf_alloc(packetlen); - target = GWBUF_DATA(packetbuf); - packetbuf->gwbuf_type = readbuf->gwbuf_type; /*< Copy the type too */ + bytestocopy = MIN(buflen,packetlen-nbytes_copied); - while (nbytes_copied < packetlen) - { - uint8_t* src = GWBUF_DATA(readbuf); - size_t buflen = GWBUF_LENGTH(readbuf); - - memcpy(target+nbytes_copied, src, buflen); - readbuf = gwbuf_consume(readbuf, buflen); - nbytes_copied += buflen; - } - *p_readbuf = readbuf; - ss_dassert(nbytes_copied == packetlen); - } - else - { - packetbuf = gwbuf_clone_portion(readbuf, 0, packetlen); - *p_readbuf = gwbuf_consume(readbuf, packetlen); + memcpy(target+nbytes_copied, src, bytestocopy); + *p_readbuf = gwbuf_consume((*p_readbuf), bytestocopy); + totalbuflen = gwbuf_length((*p_readbuf)); + nbytes_copied += bytestocopy; } + ss_dassert(buflen == 0 || nbytes_copied == packetlen); return_packetbuf: return packetbuf; diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index f09b29ae5..a0c241920 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -1550,6 +1550,8 @@ static void clientReply ( char* cmdstr = (char *)malloc(len+1); /** data+termination character == len */ snprintf(cmdstr, len, "%s", &buf[5]); + + ss_dassert(len+4 == GWBUF_LENGTH(scur->scmd_cur_cmd->my_sescmd_buf)); LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR,