Merge branch '2.2' into develop

Documentation/About/Limitations.md

@@ -217,6 +217,9 @@ SELECT ..INTO variable|OUTFILE|DUMPFILE
 SET autocommit=1|0
 ```
 
+Session commands that are 2²⁴ - 1 bytes or longer are not supported and
+cause the session to be closed.
+
 There is a possibility for misbehavior. If `USE mytable` is executed in one of
 the slaves and fails, it may be due to replication lag rather than the database
 not existing. Thus, the same command may produce different result in different

maxscale-system-test/CMakeLists.txt

@@ -681,6 +681,14 @@ add_test_executable(mxs1786_statistics.cpp mxs1786_statistics replication LABELS
 # https://jira.mariadb.org/browse/MXS-1787
 add_test_executable(mxs1787_call_ps.cpp mxs1787_call_ps replication LABELS readwritesplit REPL_BACKEND)
 
+# MXS-1804: request 16M-1 stmt_prepare command packet connect hang
+# https://jira.mariadb.org/browse/MXS-1804
+add_test_executable(mxs1804_long_ps_hang.cpp mxs1804_long_ps_hang replication LABELS readwritesplit REPL_BACKEND)
+
+# MXS-1808: Crash with mysql_stmt_send_long_data
+# https://jira.mariadb.org/browse/MXS-1808
+add_test_executable(mxs1808_long_data.cpp mxs1808_long_data replication LABELS readwritesplit REPL_BACKEND)
+
 # 'namedserverfilter' test
 add_test_executable(namedserverfilter.cpp namedserverfilter namedserverfilter LABELS namedserverfilter LIGHT REPL_BACKEND)
 

maxscale-system-test/mxs1804_long_ps_hang.cpp

@@ -0,0 +1,41 @@
+/**
+ * MXS-1804: request 16M-1 stmt_prepare command packet connect hang
+ *
+ * https://jira.mariadb.org/browse/MXS-1804
+ */
+
+#include "testconnections.h"
+
+int sql_str_size(int sqlsize)
+{
+    char prefx[] = "select ''";
+    return sqlsize - strlen(prefx) - 1;
+}
+
+void gen_select_sqlstr(char *sqlstr, unsigned int strsize, int sqlsize)
+{
+    strcpy(sqlstr, "select '");
+    memset(sqlstr + strlen("select '"), 'f', strsize);
+    sqlstr[sqlsize - 2] = '\'';
+    sqlstr[sqlsize - 1] = '\0';
+}
+
+int main(int argc, char** argv)
+{
+    TestConnections test(argc, argv);
+    int sqlsize = 16777215;
+    int strsize = sql_str_size(sqlsize);
+
+    char* sqlstr = (char *)malloc(sqlsize);
+    gen_select_sqlstr(sqlstr, strsize, sqlsize);
+
+    test.set_timeout(30);
+    test.maxscales->connect();
+
+    MYSQL_STMT* stmt = mysql_stmt_init(test.maxscales->conn_rwsplit[0]);
+    test.assert(mysql_stmt_prepare(stmt, sqlstr, strlen(sqlstr)) != 0, "Prepare should fail in 2.2 but not hang",
+                mysql_stmt_error(stmt));
+    mysql_stmt_close(stmt);
+
+    return test.global_result;
+}

maxscale-system-test/mxs1808_long_data.cpp

@@ -0,0 +1,86 @@
+#include "testconnections.h"
+#include <iostream>
+
+using namespace std;
+
+void print_stmt_error(MYSQL_STMT *stmt, const char* msg)
+{
+    cout << "Error: " << msg << ": " << mysql_stmt_error(stmt) << endl;
+}
+
+static int test_long_data(MYSQL *conn, int sqlsize)
+{
+    int data1size = sqlsize / 2;
+
+    char * data1 = (char*) malloc(data1size);
+    memset(data1, 97, data1size);
+    char * data3 = (char*) malloc(sqlsize);
+    memset(data3, 99, sqlsize);
+
+    MYSQL_STMT *stmt;
+    stmt = mysql_stmt_init(conn);
+    int        rc, int_data;
+    MYSQL_RES  *result;
+    MYSQL_BIND my_bind[1];
+
+    rc = mysql_autocommit(conn, 1);
+
+    if (NULL == stmt)
+    {
+        fprintf(stderr, "%s", mysql_error(conn));
+        return 0;
+    }
+    if (mysql_stmt_prepare(stmt, "select ?", strlen("select ?")) != 0)
+    {
+        print_stmt_error(stmt, "stmt prepare fail");
+        return 0;
+    }
+
+    memset((char*) my_bind, 0, sizeof(my_bind));
+
+    my_bind[0].buffer = (void *)&int_data;
+    my_bind[0].buffer_type = MYSQL_TYPE_STRING;
+
+    if (mysql_stmt_bind_param(stmt, my_bind) != 0)
+    {
+        print_stmt_error(stmt, "bind param error");
+        return 0;
+    }
+
+    /* supply data in pieces */
+    if (mysql_stmt_send_long_data(stmt, 0, data1, data1size) != 0)
+    {
+        print_stmt_error(stmt, "send long data1 failed");
+        return 0;
+    }
+    if (mysql_stmt_send_long_data(stmt, 0, data3, sqlsize) != 0)
+    {
+        print_stmt_error(stmt, "send long data3 failed");
+        return 0;
+    }
+
+    /* execute */
+    if (mysql_stmt_execute(stmt) != 0)
+    {
+        print_stmt_error(stmt, "execute prepare stmt failed");
+        return 0;
+    }
+    /* get the result */
+    result = mysql_store_result(conn);
+    mysql_free_result(result);
+    mysql_stmt_close(stmt);
+    free(data1);
+    free(data3);
+    return 1;
+}
+
+int main(int argc, char** argv)
+{
+    TestConnections test(argc, argv);
+
+    test.maxscales->connect();
+    test.assert(test_long_data(test.maxscales->conn_rwsplit[0], 123456), "Test should work");
+    test.maxscales->disconnect();
+
+    return test.global_result;
+}

server/modules/routing/binlogrouter/binlog_common.c

@@ -78,11 +78,11 @@ bool binlog_next_file_exists(const char* binlogdir, const char* binlog)
         {
             char buf[BLRM_BINLOG_NAME_STR_LEN + 1];
             char filename[PATH_MAX + 1];
-            char next_file[BLRM_BINLOG_NAME_STR_LEN + 1];
+            char next_file[BLRM_BINLOG_NAME_STR_LEN + 1 + 20];
             int offset = sptr - binlog;
             memcpy(buf, binlog, offset);
             buf[offset] = '\0';
-            sprintf(next_file, BINLOG_NAMEFMT, buf, filenum);
+            snprintf(next_file, sizeof(next_file), BINLOG_NAMEFMT, buf, filenum);
             snprintf(filename, PATH_MAX, "%s/%s", binlogdir, next_file);
             filename[PATH_MAX] = '\0';
 

server/modules/routing/binlogrouter/blr_file.c

@@ -240,7 +240,7 @@ typedef struct start_encryption_event
 int
 blr_file_init(ROUTER_INSTANCE *router)
 {
-    char path[PATH_MAX + 1] = "";
+    char path[PATH_MAX + 1 - BINLOG_FILE_EXTRA_INFO - BINLOG_FNAMELEN - 2] = "";
     char filename[PATH_MAX + 1] = "";
     int file_found, n = 1;
     int root_len, i;
@@ -262,9 +262,7 @@ blr_file_init(ROUTER_INSTANCE *router)
             return 0;
         }
 
-        strcpy(path, datadir);
-        strcat(path, "/");
-        strcat(path, router->service->name);
+        snprintf(path, sizeof(path), "%s/%s", datadir, router->service->name);
 
         if (access(path, R_OK) == -1)
         {

server/modules/routing/binlogrouter/blr_slave.c

@@ -7047,9 +7047,9 @@ static bool blr_slave_gtid_request(ROUTER_INSTANCE *router,
                             SQLITE_OPEN_READONLY,
                             NULL) != SQLITE_OK)
         {
-            char errmsg[BINLOG_ERROR_MSG_LEN + 1];
+            char errmsg[BINLOG_ERROR_MSG_LEN + sizeof(dbpath) + 1];
             snprintf(errmsg,
-                     BINLOG_ERROR_MSG_LEN,
+                     sizeof(errmsg),
                      "Slave %lu: failed to open GTID maps db '%s': %s",
                       (unsigned long)slave->serverid,
                       dbpath,
@@ -8180,7 +8180,7 @@ static bool blr_handle_admin_stmt(ROUTER_INSTANCE *router,
             else
             {
                 int rc;
-                char error_string[BINLOG_ERROR_MSG_LEN + 1] = "";
+                char error_string[BINLOG_ERROR_MSG_LEN + 1 + BINLOG_ERROR_MSG_LEN + 1] = "";
                 MASTER_SERVER_CFG *current_master = NULL;
 
                 current_master = (MASTER_SERVER_CFG *)MXS_CALLOC(1, sizeof(MASTER_SERVER_CFG));
@@ -8228,7 +8228,7 @@ static bool blr_handle_admin_stmt(ROUTER_INSTANCE *router,
 
                         spinlock_release(&router->lock);
 
-                        snprintf(error_string, BINLOG_ERROR_MSG_LEN,
+                        snprintf(error_string, sizeof(error_string),
                                  "Error writing into %s/master.ini: %s",
                                  router->binlogdir,
                                  error);

server/modules/routing/readwritesplit/rwsplit_route_stmt.cc

@@ -192,7 +192,13 @@ bool RWSplitSession::route_single_stmt(GWBUF *querybuf)
             if ((target = handle_slave_is_target(command, stmt_id)))
             {
                 succp = true;
-                store_stmt = m_config.retry_failed_reads;
+
+                if (m_config.retry_failed_reads &&
+                    (command == MXS_COM_QUERY || command == MXS_COM_STMT_EXECUTE))
+                {
+                    // Only commands that can contain an SQL statement should be stored
+                    store_stmt = true;
+                }
             }
         }
         else if (TARGET_IS_MASTER(route_target))
@@ -258,6 +264,18 @@ bool RWSplitSession::route_single_stmt(GWBUF *querybuf)
     return succp;
 }
 
+static inline bool is_large_query(GWBUF* buf)
+{
+    uint32_t buflen = gwbuf_length(buf);
+
+    // The buffer should contain at most (2^24 - 1) + 4 bytes ...
+    ss_dassert(buflen <= MYSQL_HEADER_LEN + GW_MYSQL_MAX_PACKET_LEN);
+    // ... and the payload should be buflen - 4 bytes
+    ss_dassert(MYSQL_GET_PAYLOAD_LEN(GWBUF_DATA(buf)) == buflen - MYSQL_HEADER_LEN);
+
+    return buflen == MYSQL_HEADER_LEN + GW_MYSQL_MAX_PACKET_LEN;
+}
+
 /**
  * Purge session command history
  *
@@ -326,6 +344,13 @@ void RWSplitSession::purge_history(mxs::SSessionCommand& sescmd)
  */
 bool RWSplitSession::route_session_write(GWBUF *querybuf, uint8_t command, uint32_t type)
 {
+    if (is_large_query(querybuf))
+    {
+        MXS_ERROR("Session command is too large, session cannot continue. "
+                  "Large session commands are not supported in 2.2.");
+        return false;
+    }
+
     /** The SessionCommand takes ownership of the buffer */
     uint64_t id = m_sescmd_count++;
     mxs::SSessionCommand sescmd(new mxs::SessionCommand(querybuf, id));
@@ -848,18 +873,6 @@ bool RWSplitSession::handle_master_is_target(SRWBackend* dest)
     return succp;
 }
 
-static inline bool is_large_query(GWBUF* buf)
-{
-    uint32_t buflen = gwbuf_length(buf);
-
-    // The buffer should contain at most (2^24 - 1) + 4 bytes ...
-    ss_dassert(buflen <= MYSQL_HEADER_LEN + GW_MYSQL_MAX_PACKET_LEN);
-    // ... and the payload should be buflen - 4 bytes
-    ss_dassert(MYSQL_GET_PAYLOAD_LEN(GWBUF_DATA(buf)) == buflen - MYSQL_HEADER_LEN);
-
-    return buflen == MYSQL_HEADER_LEN + GW_MYSQL_MAX_PACKET_LEN;
-}
-
 /*
  * Add a wait gitd query in front of user's query to achive causal read;
  *