From d3a79ce7c40167d90a0eade6404ea477a9b85828 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Thu, 14 Aug 2014 13:05:05 +0300 Subject: [PATCH 1/4] Partial fix to #463, http://bugs.skysql.com/show_bug.cgi?id=463 log_manager.cc: fixed block buffer overflow. Queries are logged to trace log and long queries exceed the bufsize in length. Those were written beyond allocated memory areas. mysql_client_server_protocol.h: added mysql_protocol_state_t to indicate whether MySQL protocol object is allocated, usable or freed. Freed means that memory allocations made by the protocol are freed. That is, command history etc. mysql_backend.c: gw_backend_hangup and gw_error_backend_event used to call error handling function although session was already closing. Added check for session state. mysql_client.c: route_by_statement lost some packets in case where query was sent in multiple packets. mysql_common.c: gw_MySQL_get_next_packet failed in packet handling with route_by_statement. When multi-packet query was merged into one, packet type wasn't copied. protocol_archive_srv_command and mysql_protocol_done didn't have proper locking in place which lead to occasional crashes. --- log_manager/log_manager.cc | 26 +++--- server/include/dcb.h | 4 +- .../include/mysql_client_server_protocol.h | 7 ++ server/modules/protocol/mysql_backend.c | 79 +++++++++++++++---- server/modules/protocol/mysql_client.c | 20 ++--- server/modules/protocol/mysql_common.c | 56 +++++++++---- .../routing/readwritesplit/readwritesplit.c | 2 +- 7 files changed, 137 insertions(+), 57 deletions(-) diff --git a/log_manager/log_manager.cc b/log_manager/log_manager.cc index c8f96266e..733e9fffe 100644 --- a/log_manager/log_manager.cc +++ b/log_manager/log_manager.cc @@ -255,11 +255,7 @@ static int logmanager_write_log( static blockbuf_t* blockbuf_init(logfile_id_t id); static void blockbuf_node_done(void* bb_data); static char* blockbuf_get_writepos( -#if 0 - int** refcount, -#else blockbuf_t** p_bb, -#endif logfile_id_t id, size_t str_len, bool flush); @@ -653,7 +649,16 @@ static int logmanager_write_log( int safe_str_len; timestamp_len = get_timestamp_len(); - safe_str_len = MIN(timestamp_len-1+str_len, lf->lf_buf_size); + + /** Findout how much can be safely written with current block size */ + if (timestamp_len-1+str_len > lf->lf_buf_size) + { + safe_str_len = lf->lf_buf_size; + } + else + { + safe_str_len = timestamp_len-1+str_len; + } /** * Seek write position and register to block buffer. * Then print formatted string to write position. @@ -673,9 +678,9 @@ static int logmanager_write_log( * of the timestamp string. */ if (use_valist) { - vsnprintf(wp+timestamp_len, safe_str_len, str, valist); + vsnprintf(wp+timestamp_len, safe_str_len-timestamp_len, str, valist); } else { - snprintf(wp+timestamp_len, safe_str_len, "%s", str); + snprintf(wp+timestamp_len, safe_str_len-timestamp_len, "%s", str); } /** write to syslog */ @@ -694,12 +699,7 @@ static int logmanager_write_log( break; } } - - /** remove double line feed */ - if (wp[timestamp_len+str_len-2] == '\n') { - wp[timestamp_len+str_len-2]=' '; - } - wp[timestamp_len+str_len-1]='\n'; + wp[safe_str_len-1] = '\n'; blockbuf_unregister(bb); /** diff --git a/server/include/dcb.h b/server/include/dcb.h index 88964fc31..4f8be99d4 100644 --- a/server/include/dcb.h +++ b/server/include/dcb.h @@ -145,9 +145,9 @@ typedef enum { DCB_STATE_POLLING, /*< Waiting in the poll loop */ DCB_STATE_LISTENING, /*< The DCB is for a listening socket */ DCB_STATE_DISCONNECTED, /*< The socket is now closed */ - DCB_STATE_FREED, /*< Memory freed */ DCB_STATE_NOPOLLING, /*< Removed from poll mask */ - DCB_STATE_ZOMBIE /*< DCB is no longer active, waiting to free it */ + DCB_STATE_ZOMBIE, /*< DCB is no longer active, waiting to free it */ + DCB_STATE_FREED /*< Memory freed */ } dcb_state_t; typedef enum { diff --git a/server/modules/include/mysql_client_server_protocol.h b/server/modules/include/mysql_client_server_protocol.h index 9e2147d05..c875a1232 100644 --- a/server/modules/include/mysql_client_server_protocol.h +++ b/server/modules/include/mysql_client_server_protocol.h @@ -102,6 +102,12 @@ typedef enum { MYSQL_IDLE } mysql_auth_state_t; +typedef enum { + MYSQL_PROTOCOL_ALLOC, + MYSQL_PROTOCOL_ACTIVE, + MYSQL_PROTOCOL_DONE +} mysql_protocol_state_t; + /* * MySQL session specific data @@ -270,6 +276,7 @@ typedef struct { server_command_t protocol_command; /*< session command list */ server_command_t* protocol_cmd_history; /*< session command history */ mysql_auth_state_t protocol_auth_state; /*< Authentication status */ + mysql_protocol_state_t protocol_state; /*< Protocol struct status */ uint8_t scramble[MYSQL_SCRAMBLE_LEN]; /*< server scramble, * created or received */ uint32_t server_capabilities; /*< server capabilities, diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index 5d8088d4e..903c17022 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -762,12 +762,13 @@ return_rc: */ static int gw_error_backend_event(DCB *dcb) { - SESSION* session; - void* rsession; - ROUTER_OBJECT* router; - ROUTER* router_instance; - GWBUF* errbuf; - bool succp; + SESSION* session; + void* rsession; + ROUTER_OBJECT* router; + ROUTER* router_instance; + GWBUF* errbuf; + bool succp; + session_state_t ses_state; CHK_DCB(dcb); session = dcb->session; @@ -787,7 +788,7 @@ static int gw_error_backend_event(DCB *dcb) * closed by router and COM_QUIT sent or there was an error which * have already been handled. */ - if (dcb->session != DCB_STATE_POLLING) + if (dcb->state != DCB_STATE_POLLING) { return 1; } @@ -796,6 +797,29 @@ static int gw_error_backend_event(DCB *dcb) 0, "Lost connection to backend server."); + spinlock_acquire(&session->ses_lock); + ses_state = session->state; + spinlock_release(&session->ses_lock); + + /** + * Session might be initialized when DCB already is in the poll set. + * Thus hangup can occur in the middle of session initialization. + * Only complete and successfully initialized sessions allow for + * calling error handler. + */ + while (ses_state == SESSION_STATE_READY) + { + spinlock_acquire(&session->ses_lock); + ses_state = session->state; + spinlock_release(&session->ses_lock); + } + + if (ses_state != SESSION_STATE_ROUTER_READY) + { + gwbuf_free(errbuf); + goto retblock; + } + router->handleError(router_instance, rsession, errbuf, @@ -811,6 +835,7 @@ static int gw_error_backend_event(DCB *dcb) } dcb_close(dcb); +retblock: return 1; } @@ -924,12 +949,13 @@ return_fd: static int gw_backend_hangup(DCB *dcb) { - SESSION* session; - void* rsession; - ROUTER_OBJECT* router; - ROUTER* router_instance; - bool succp; - GWBUF* errbuf; + SESSION* session; + void* rsession; + ROUTER_OBJECT* router; + ROUTER* router_instance; + bool succp; + GWBUF* errbuf; + session_state_t ses_state; CHK_DCB(dcb); session = dcb->session; @@ -950,7 +976,29 @@ gw_backend_hangup(DCB *dcb) 1, 0, "Lost connection to backend server."); - + + spinlock_acquire(&session->ses_lock); + ses_state = session->state; + spinlock_release(&session->ses_lock); + + /** + * Session might be initialized when DCB already is in the poll set. + * Thus hangup can occur in the middle of session initialization. + * Only complete and successfully initialized sessions allow for + * calling error handler. + */ + while (ses_state == SESSION_STATE_READY) + { + spinlock_acquire(&session->ses_lock); + ses_state = session->state; + spinlock_release(&session->ses_lock); + } + + if (ses_state != SESSION_STATE_ROUTER_READY) + { + gwbuf_free(errbuf); + goto retblock; + } router->handleError(router_instance, rsession, errbuf, @@ -972,7 +1020,8 @@ gw_backend_hangup(DCB *dcb) } dcb_close(dcb); - return 1; +retblock: + return 1; } /** diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index 6cc9027de..fbe0589a5 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -65,7 +65,7 @@ static int gw_client_hangup_event(DCB *dcb); int mysql_send_ok(DCB *dcb, int packet_number, int in_affected_rows, const char* mysql_message); int MySQLSendHandshake(DCB* dcb); static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue); -static int route_by_statement(SESSION *, GWBUF *); +static int route_by_statement(SESSION *, GWBUF **); /* * The "module object" for the mysqld client protocol module. @@ -534,7 +534,7 @@ int gw_read_client_event( if (nbytes_read == 0) { goto return_rc; - } + } /** * if read queue existed appent read to it. * if length of read buffer is less than 3 or less than mysql packet @@ -783,7 +783,7 @@ int gw_read_client_event( * Feed each statement completely and separately * to router. */ - rc = route_by_statement(session, read_buffer); + rc = route_by_statement(session, &read_buffer); if (read_buffer != NULL) { @@ -1383,7 +1383,7 @@ gw_client_hangup_event(DCB *dcb) #if defined(SS_DEBUG) LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, - "Client hangup error handling."))); + "Client hangup error handling."))); #endif dcb_close(dcb); return 1; @@ -1399,7 +1399,9 @@ gw_client_hangup_event(DCB *dcb) * Return 1 in success. If the last packet is incomplete return success but * leave incomplete packet to readbuf. */ -static int route_by_statement(SESSION *session, GWBUF *readbuf) +static int route_by_statement( + SESSION* session, + GWBUF** p_readbuf) { int rc = -1; GWBUF* packetbuf; @@ -1407,7 +1409,7 @@ static int route_by_statement(SESSION *session, GWBUF *readbuf) gwbuf_type_t prevtype; GWBUF* tmpbuf; - tmpbuf = readbuf; + tmpbuf = *p_readbuf; while (tmpbuf != NULL) { ss_dassert(GWBUF_IS_TYPE_MYSQL(tmpbuf)); @@ -1416,9 +1418,9 @@ static int route_by_statement(SESSION *session, GWBUF *readbuf) #endif do { - ss_dassert(GWBUF_IS_TYPE_MYSQL(readbuf)); + ss_dassert(GWBUF_IS_TYPE_MYSQL((*p_readbuf))); - packetbuf = gw_MySQL_get_next_packet(&readbuf); + packetbuf = gw_MySQL_get_next_packet(p_readbuf); ss_dassert(GWBUF_IS_TYPE_MYSQL(packetbuf)); @@ -1447,7 +1449,7 @@ static int route_by_statement(SESSION *session, GWBUF *readbuf) goto return_rc; } } - while (readbuf != NULL); + while (*p_readbuf != NULL); return_rc: return rc; diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 70050a13c..ef9991554 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -79,6 +79,7 @@ MySQLProtocol* mysql_protocol_init( strerror(eno)))); goto return_p; } + p->protocol_state = MYSQL_PROTOCOL_ALLOC; p->protocol_auth_state = MYSQL_ALLOC; p->protocol_command.scom_cmd = MYSQL_COM_UNDEFINED; p->protocol_command.scom_nresponse_packets = 0; @@ -90,6 +91,7 @@ MySQLProtocol* mysql_protocol_init( /*< Assign fd with protocol */ p->fd = fd; p->owner_dcb = dcb; + p->protocol_state = MYSQL_PROTOCOL_ACTIVE; CHK_PROTOCOL(p); return_p: return p; @@ -107,15 +109,25 @@ return_p: void mysql_protocol_done ( DCB* dcb) { - server_command_t* scmd = ((MySQLProtocol *)dcb->protocol)->protocol_cmd_history; + MySQLProtocol* p; + server_command_t* scmd; server_command_t* scmd2; + p = (MySQLProtocol *)dcb->protocol; + + spinlock_acquire(&p->protocol_lock); + + scmd = p->protocol_cmd_history; + while (scmd != NULL) { scmd2 = scmd->scom_next; free(scmd); scmd = scmd2; } + p->protocol_state = MYSQL_PROTOCOL_DONE; + + spinlock_release(&p->protocol_lock); } @@ -886,7 +898,7 @@ int mysql_send_com_quit( if (buf == NULL) { return 0; - } + } nbytes = dcb->func.write(dcb, buf); return nbytes; @@ -1542,16 +1554,18 @@ GWBUF* gw_MySQL_get_next_packet( packetbuf = gwbuf_alloc(packetlen); target = GWBUF_DATA(packetbuf); - + packetbuf->gwbuf_type = readbuf->gwbuf_type; /*< Copy the type too */ + while (nbytes_copied < packetlen) { uint8_t* src = GWBUF_DATA(readbuf); size_t buflen = GWBUF_LENGTH(readbuf); - + memcpy(target+nbytes_copied, src, buflen); - *p_readbuf = gwbuf_consume(readbuf, buflen); + readbuf = gwbuf_consume(readbuf, buflen); nbytes_copied += buflen; } + *p_readbuf = readbuf; ss_dassert(nbytes_copied == packetlen); } else @@ -1625,11 +1639,16 @@ void protocol_archive_srv_command( MySQLProtocol* p) { server_command_t* s1; - server_command_t** s2; + server_command_t* h1; int len = 0; spinlock_acquire(&p->protocol_lock); + if (p->protocol_state != MYSQL_PROTOCOL_ACTIVE) + { + goto retblock; + } + s1 = &p->protocol_command; LOGIF(LT, (skygw_log_write( @@ -1639,18 +1658,19 @@ void protocol_archive_srv_command( p->owner_dcb->fd))); /** Copy to history list */ - s2 = &p->protocol_cmd_history; - - if (*s2 != NULL) - { - while ((*s2)->scom_next != NULL) - { - *s2 = (*s2)->scom_next; - len += 1; - } + if ((h1 = p->protocol_cmd_history) == NULL) + { + p->protocol_cmd_history = server_command_copy(s1); } - *s2 = server_command_copy(s1); - + else + { + while (h1->scom_next != NULL) + { + h1 = h1->scom_next; + } + h1->scom_next = server_command_copy(s1); + } + /** Keep history limits, remove oldest */ if (len > MAX_CMD_HISTORY) { @@ -1669,6 +1689,8 @@ void protocol_archive_srv_command( p->protocol_command = *(s1->scom_next); free(s1->scom_next); } + +retblock: spinlock_release(&p->protocol_lock); } diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index 9d4b1b8ff..c10aca134 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -1549,7 +1549,7 @@ static void clientReply ( size_t len = MYSQL_GET_PACKET_LEN(buf); char* cmdstr = (char *)malloc(len+1); - snprintf(cmdstr, len+1, "%s", &buf[5]); + snprintf(cmdstr, len, "%s", &buf[5]); LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, From 77e5525436c180d70d417425f33317afb909891b Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Thu, 14 Aug 2014 15:15:22 +0300 Subject: [PATCH 2/4] mysql_client.c:gw_error_client_event & gw_client_hangup_event: added session state check, if session is already closing, don't start redundant call to dcb_close. mysql_common.c:mysql_protocol_done: added protocol state check. Used not to check it which caused double free of allocated memory. --- server/core/dcb.c | 7 +++--- server/modules/protocol/mysql_backend.c | 25 +++++++++---------- server/modules/protocol/mysql_client.c | 14 +++++++++++ server/modules/protocol/mysql_common.c | 5 ++++ .../routing/readwritesplit/readwritesplit.c | 2 +- 5 files changed, 36 insertions(+), 17 deletions(-) diff --git a/server/core/dcb.c b/server/core/dcb.c index 1dfde0b58..090d57554 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -978,9 +978,10 @@ int below_water; } if (dolog) { - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Error : Writing to %s socket failed due %d, %s.", + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [dcb_write] Writing to %s socket failed due %d, %s.", + pthread_self(), dcb_isclient(dcb) ? "client" : "backend server", saved_errno, strerror(saved_errno)))); diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index 903c17022..da00de083 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -777,11 +777,6 @@ static int gw_error_backend_event(DCB *dcb) router = session->service->router; router_instance = session->service->router_instance; -#if defined(SS_DEBUG) - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Backend error event handling."))); -#endif /** * Avoid running redundant error handling procedure. * dcb_close is already called for the DCB. Thus, either connection is @@ -820,6 +815,11 @@ static int gw_error_backend_event(DCB *dcb) goto retblock; } +#if defined(SS_DEBUG) + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Backend error event handling."))); +#endif router->handleError(router_instance, rsession, errbuf, @@ -963,14 +963,7 @@ gw_backend_hangup(DCB *dcb) rsession = session->router_session; router = session->service->router; - router_instance = session->service->router_instance; - -#if defined(SS_DEBUG) - LOGIF(LE, (skygw_log_write_flush( - LOGFILE_ERROR, - "Backend hangup error handling."))); -#endif - + router_instance = session->service->router_instance; errbuf = mysql_create_custom_error( 1, @@ -999,6 +992,12 @@ gw_backend_hangup(DCB *dcb) gwbuf_free(errbuf); goto retblock; } +#if defined(SS_DEBUG) + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Backend hangup error handling."))); +#endif + router->handleError(router_instance, rsession, errbuf, diff --git a/server/modules/protocol/mysql_client.c b/server/modules/protocol/mysql_client.c index fbe0589a5..0f243fb1d 100644 --- a/server/modules/protocol/mysql_client.c +++ b/server/modules/protocol/mysql_client.c @@ -1300,12 +1300,19 @@ static int gw_error_client_event( STRDCBSTATE(dcb->state), (session != NULL ? session : NULL)))); + if (session != NULL && session->state == SESSION_STATE_STOPPING) + { + goto retblock; + } + #if defined(SS_DEBUG) LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, "Client error event handling."))); #endif dcb_close(dcb); + +retblock: return 1; } @@ -1380,12 +1387,19 @@ gw_client_hangup_event(DCB *dcb) { CHK_SESSION(session); } + + if (session != NULL && session->state == SESSION_STATE_STOPPING) + { + goto retblock; + } #if defined(SS_DEBUG) LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, "Client hangup error handling."))); #endif dcb_close(dcb); + +retblock: return 1; } diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index ef9991554..cd1e5c5f9 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -117,6 +117,10 @@ void mysql_protocol_done ( spinlock_acquire(&p->protocol_lock); + if (p->protocol_state != MYSQL_PROTOCOL_ACTIVE) + { + goto retblock; + } scmd = p->protocol_cmd_history; while (scmd != NULL) @@ -127,6 +131,7 @@ void mysql_protocol_done ( } p->protocol_state = MYSQL_PROTOCOL_DONE; +retblock: spinlock_release(&p->protocol_lock); } diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index c10aca134..f09b29ae5 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -1548,7 +1548,7 @@ static void clientReply ( (uint8_t *)GWBUF_DATA((scur->scmd_cur_cmd->my_sescmd_buf)); size_t len = MYSQL_GET_PACKET_LEN(buf); char* cmdstr = (char *)malloc(len+1); - + /** data+termination character == len */ snprintf(cmdstr, len, "%s", &buf[5]); LOGIF(LE, (skygw_log_write_flush( From 2393ac57e9dabb15da51061ab02ce28fc21fac02 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Thu, 14 Aug 2014 17:23:46 +0300 Subject: [PATCH 3/4] mysql_common.c:protocol_add_srv_command didn't check that protocol status was MYSQL_PROTOCOL_ACTIVE and wrote to freed memory. --- server/modules/protocol/mysql_backend.c | 3 ++- server/modules/protocol/mysql_common.c | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index da00de083..ce7b6ef97 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -1006,7 +1006,8 @@ gw_backend_hangup(DCB *dcb) &succp); /** There are not required backends available, close session. */ - if (!succp) { + if (!succp) + { #if defined(SS_DEBUG) LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index cd1e5c5f9..4e06894d1 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -1712,6 +1712,10 @@ void protocol_add_srv_command( spinlock_acquire(&p->protocol_lock); + if (p->protocol_state != MYSQL_PROTOCOL_ACTIVE) + { + goto retblock; + } /** this is the only server command in protocol */ if (p->protocol_command.scom_cmd == MYSQL_COM_UNDEFINED) { @@ -1744,6 +1748,7 @@ void protocol_add_srv_command( c = c->scom_next; } #endif +retblock: spinlock_release(&p->protocol_lock); } From 902004c1ee4e6234e229d7eb890a75b5fac2eaa1 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Thu, 14 Aug 2014 22:33:57 +0300 Subject: [PATCH 4/4] 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,