← Back to team overview

maria-developers team mailing list archive

Re: [Commits] 2606b87: MDEV-9117: Client Server capability negotiation for MariaDB specific functionality

 

Hi, Serg!

On 24.11.2015 20:38, Sergei Golubchik wrote:
[skip]
Ok, but I think it'll be needed in Connector/C, when it connects to the
old MariaDB server.
I informed Georg and he found it reasonable.

While I am trying to fix git triggers for mail, here is the after-review patch

diff --git a/client/mysql.cc b/client/mysql.cc
index 3e08c87..de5ecb1 100644
--- a/client/mysql.cc
+++ b/client/mysql.cc
@@ -152,7 +152,7 @@ static ulong opt_max_allowed_packet, opt_net_buffer_length;
 static uint verbose=0,opt_silent=0,opt_mysql_port=0, opt_local_infile=0;
 static uint my_end_arg;
 static char * opt_mysql_unix_port=0;
-static int connect_flag=CLIENT_INTERACTIVE;
+static ulong connect_flag=CLIENT_INTERACTIVE;
 static my_bool opt_binary_mode= FALSE;
 static int interrupted_query= 0;
 static char *current_host,*current_db,*current_user=0,*opt_password=0,
diff --git a/include/mysql_com.h b/include/mysql_com.h
index aa6ab0f..57092dc 100644
--- a/include/mysql_com.h
+++ b/include/mysql_com.h
@@ -188,7 +188,8 @@ enum enum_server_command
 #define REFRESH_GENERIC         (1ULL << 30)
 #define REFRESH_FAST            (1ULL << 31) /* Intern flag */

-#define CLIENT_LONG_PASSWORD    1    /* new more secure passwords */
+#define CLIENT_LONG_PASSWORD    0    /* obsolete flag */
+#define CLIENT_MYSQL 1 /* mysql/old mariadb server/client */
 #define CLIENT_FOUND_ROWS    2    /* Found instead of affected rows */
 #define CLIENT_LONG_FLAG    4    /* Get all column flags */
 #define CLIENT_CONNECT_WITH_DB    8    /* One can specify db on connect */
@@ -215,7 +216,8 @@ enum enum_server_command
 /* Don't close the connection for a connection with expired password. */
 #define CLIENT_CAN_HANDLE_EXPIRED_PASSWORDS (1UL << 22)

-#define CLIENT_PROGRESS (1UL << 29) /* Client support progress indicator */
+#define CLIENT_PROGRESS_OBSOLETE  (1UL << 29)
+#define CLIENT_PROGRESS           0
 #define CLIENT_SSL_VERIFY_SERVER_CERT (1UL << 30)
 /*
   It used to be that if mysql_real_connect() failed, it would delete any
@@ -228,6 +230,13 @@ enum enum_server_command
 */
 #define CLIENT_REMEMBER_OPTIONS (1UL << 31)

+/* MariaDB extended capability flags */
+#define MARIADB_CLIENT_FLAGS_MASK 0xffffffff00000000ULL
+/* Client support progress indicator */
+#define MARIADB_CLIENT_PROGRESS (1ULL << 32)
+/* Client support mariaDB extended flags */
+#define MARIADB_CLIENT_EXTENDED_FLAGS (1ULL << 63)
+
 #ifdef HAVE_COMPRESS
 #define CAN_CLIENT_COMPRESS CLIENT_COMPRESS
 #else
@@ -235,7 +244,7 @@ enum enum_server_command
 #endif

 /* Gather all possible capabilites (flags) supported by the server */
-#define CLIENT_ALL_FLAGS  (CLIENT_LONG_PASSWORD | \
+#define CLIENT_ALL_FLAGS  (CLIENT_MYSQL | \
                            CLIENT_FOUND_ROWS | \
                            CLIENT_LONG_FLAG | \
                            CLIENT_CONNECT_WITH_DB | \
@@ -256,7 +265,7 @@ enum enum_server_command
                            CLIENT_PS_MULTI_RESULTS | \
                            CLIENT_SSL_VERIFY_SERVER_CERT | \
                            CLIENT_REMEMBER_OPTIONS | \
-                           CLIENT_PROGRESS | \
+                           MARIADB_CLIENT_PROGRESS | \
                            CLIENT_PLUGIN_AUTH | \
                            CLIENT_PLUGIN_AUTH_LENENC_CLIENT_DATA | \
                            CLIENT_CONNECT_ATTRS)
diff --git a/libmysql/client_settings.h b/libmysql/client_settings.h
index b233614..2577870 100644
--- a/libmysql/client_settings.h
+++ b/libmysql/client_settings.h
@@ -27,7 +27,7 @@ extern char *    mysql_unix_port;
  When adding capabilities here, consider if they should be also added to
  the server's version.
 */
-#define CLIENT_CAPABILITIES (CLIENT_LONG_PASSWORD | \
+#define CLIENT_CAPABILITIES (CLIENT_MYSQL | \
                              CLIENT_LONG_FLAG |     \
                              CLIENT_TRANSACTIONS |  \
                              CLIENT_PROTOCOL_41 | \
diff --git a/sql-common/client.c b/sql-common/client.c
index 2a73eae..0ec0842 100644
--- a/sql-common/client.c
+++ b/sql-common/client.c
@@ -2486,7 +2486,7 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio,
   if (mysql->client_flag & CLIENT_PROTOCOL_41)
   {
     /* 4.1 server and 4.1 client has a 32 byte option flag */
-    int4store(buff,mysql->client_flag);
+    int4store(buff, mysql->client_flag);
     int4store(buff+4, net->max_packet_size);
     buff[8]= (char) mysql->charset->number;
     bzero(buff+9, 32-9);
@@ -2494,7 +2494,7 @@ static int send_client_reply_packet(MCPVIO_EXT *mpvio,
   }
   else
   {
-    int2store(buff, mysql->client_flag);
+    int2store(buff, (mysql->client_flag));
     int3store(buff+2, net->max_packet_size);
     end= buff+5;
   }
diff --git a/sql/client_settings.h b/sql/client_settings.h
index d6a157f..cc90d6a 100644
--- a/sql/client_settings.h
+++ b/sql/client_settings.h
@@ -28,7 +28,7 @@
  When adding capabilities here, consider if they should be also added to
  the libmysql version.
 */
-#define CLIENT_CAPABILITIES (CLIENT_LONG_PASSWORD | \
+#define CLIENT_CAPABILITIES (CLIENT_MYSQL | \
                              CLIENT_LONG_FLAG |     \
                              CLIENT_TRANSACTIONS |  \
                              CLIENT_PROTOCOL_41 |   \
diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
index 1240d5d..d07e0a4 100644
--- a/sql/sql_acl.cc
+++ b/sql/sql_acl.cc
@@ -11245,7 +11245,8 @@ static bool send_server_handshake_packet(MPVIO_EXT *mpvio,

   *end++= protocol_version;

-  thd->client_capabilities= CLIENT_BASIC_FLAGS;
+  thd->client_capabilities=
+    (CLIENT_BASIC_FLAGS | MARIADB_CLIENT_EXTENDED_FLAGS);

   if (opt_using_transactions)
     thd->client_capabilities|= CLIENT_TRANSACTIONS;
@@ -11313,7 +11314,8 @@ static bool send_server_handshake_packet(MPVIO_EXT *mpvio,
   int2store(end+5, thd->client_capabilities >> 16);
   end[7]= data_len;
   DBUG_EXECUTE_IF("poison_srv_handshake_scramble_len", end[7]= -100;);
-  bzero(end + 8, 10);
+  bzero(end + 8, 6);
+  int4store(end + 14, thd->client_capabilities >> 32);
   end+= 18;
   /* write scramble tail */
   end= (char*) memcpy(end, data + SCRAMBLE_LENGTH_323,
@@ -11729,18 +11731,38 @@ static ulong parse_client_handshake_packet(MPVIO_EXT *mpvio,
   */
   DBUG_ASSERT(net->read_pos[pkt_len] == 0);

-  ulong client_capabilities= uint2korr(net->read_pos);
+  ulonglong client_capabilities= uint2korr(net->read_pos);
+  compile_time_assert(sizeof(client_capabilities) >= 8);
   if (client_capabilities & CLIENT_PROTOCOL_41)
   {
-    if (pkt_len < 4)
+    if (pkt_len < 32)
       return packet_error;
     client_capabilities|= ((ulong) uint2korr(net->read_pos+2)) << 16;
+    if (!(client_capabilities & CLIENT_MYSQL))
+    {
+      // it is client with mariadb extensions
+      client_capabilities|= CLIENT_MYSQL;
+      ulonglong ext_client_capabilities=
+        (((ulonglong)uint4korr(net->read_pos + 28)) << 32);
+      if (ext_client_capabilities & MARIADB_CLIENT_EXTENDED_FLAGS)
+        client_capabilities|= ext_client_capabilities;
+      else
+      {
+        DBUG_PRINT("error", ("CLIENT_PROTOCOL_41: on, "
+                             "CLIENT_LONG_PASSWORD/CLIENT_MYSQL off, "
+                             "but MARIADB_CLIENT_EXTENDED_FLAGS is off. "
+                             "flags: %llx ext flags %llx",
+ client_capabilities, ext_client_capabilities));
+        return packet_error;
+      }
+    }
   }

   /* Disable those bits which are not supported by the client. */
+  compile_time_assert(sizeof(thd->client_capabilities) >= 8);
   thd->client_capabilities&= client_capabilities;

- DBUG_PRINT("info", ("client capabilities: %lu", thd->client_capabilities)); + DBUG_PRINT("info", ("client capabilities 1: %llu", thd->client_capabilities));
   if (thd->client_capabilities & CLIENT_SSL)
   {
     unsigned long errptr __attribute__((unused));
@@ -11768,8 +11790,6 @@ static ulong parse_client_handshake_packet(MPVIO_EXT *mpvio,

   if (client_capabilities & CLIENT_PROTOCOL_41)
   {
-    if (pkt_len < 32)
-      return packet_error;
     thd->max_client_packet_length= uint4korr(net->read_pos+4);
DBUG_PRINT("info", ("client_character_set: %d", (uint) net->read_pos[8]));
     if (thd_init_client_charset(thd, (uint) net->read_pos[8]))
@@ -12546,7 +12566,7 @@ bool acl_authenticate(THD *thd, uint com_change_user_pkt_len)
   }

   DBUG_PRINT("info",
-             ("Capabilities: %lu  packet_length: %ld  Host: '%s'  "
+             ("Capabilities: %llu  packet_length: %ld  Host: '%s' "
               "Login user: '%s' Priv_user: '%s'  Using password: %s "
               "Access: %lu  db: '%s'",
               thd->client_capabilities, thd->max_client_packet_length,
diff --git a/sql/sql_class.cc b/sql/sql_class.cc
index e95e32a..b496e4c 100644
--- a/sql/sql_class.cc
+++ b/sql/sql_class.cc
@@ -4330,7 +4330,7 @@ extern "C" void thd_progress_init(MYSQL_THD thd, uint max_stage)
     is a high level command (like ALTER TABLE) and we are not in a
     stored procedure
   */
-  thd->progress.report= ((thd->client_capabilities & CLIENT_PROGRESS) &&
+ thd->progress.report= ((thd->client_capabilities & MARIADB_CLIENT_PROGRESS) &&
                          thd->progress.report_to_client &&
                          !thd->in_sub_stmt);
   thd->progress.next_report_time= 0;
diff --git a/sql/sql_class.h b/sql/sql_class.h
index aee6d56..b13e5f6 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -1410,7 +1410,7 @@ class Sub_statement_state
   Discrete_intervals_list auto_inc_intervals_forced;
   ulonglong limit_found_rows;
   ha_rows    cuted_fields, sent_row_count, examined_row_count;
-  ulong client_capabilities;
+  ulonglong client_capabilities;
   ulong query_plan_flags;
   uint in_sub_stmt;
   bool enable_slow_log;
@@ -2043,7 +2043,7 @@ class THD :public Statement,
   /* Needed by MariaDB semi sync replication */
   Trans_binlog_info *semisync_info;

-  ulong client_capabilities;        /* What the client supports */
+  ulonglong client_capabilities;  /* What the client supports */
   ulong max_client_packet_length;

   HASH        handler_tables_hash;
@@ -2085,7 +2085,7 @@ class THD :public Statement,
     bool       report_to_client;
     /*
       true, if we will send progress report packets to a client
-      (client has requested them, see CLIENT_PROGRESS; report_to_client
+ (client has requested them, see MARIADB_CLIENT_PROGRESS; report_to_client
       is true; not in sub-statement)
     */
     bool       report;
diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index bc26780..28e9b3b 100644
--- a/sql/sql_parse.cc
+++ b/sql/sql_parse.cc
@@ -637,7 +637,7 @@ void execute_init_command(THD *thd, LEX_STRING *init_command,
                           mysql_rwlock_t *var_lock)
 {
   Vio* save_vio;
-  ulong save_client_capabilities;
+  ulonglong save_client_capabilities;

   mysql_rwlock_rdlock(var_lock);
   if (!init_command->length)
diff --git a/tests/mysql_client_fw.c b/tests/mysql_client_fw.c
index 990fdb1..f26de62 100644
--- a/tests/mysql_client_fw.c
+++ b/tests/mysql_client_fw.c
@@ -326,7 +326,8 @@ mysql_simple_prepare(MYSQL *mysql_arg, const char *query)

    @return pointer to initialized and connected MYSQL object
 */
-static MYSQL* client_connect(ulong flag, uint protocol, my_bool auto_reconnect)
+static MYSQL* client_connect(ulonglong flag, uint protocol,
+                             my_bool auto_reconnect)
 {
   MYSQL* mysql;
   int  rc;
diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c
index a1a52e8..31fca0f 100644
--- a/tests/mysql_client_test.c
+++ b/tests/mysql_client_test.c
@@ -15336,7 +15336,7 @@ static void test_mysql_insert_id()

   myheader("test_mysql_insert_id");

-  rc= mysql_query(mysql, "drop table if exists t1");
+  rc= mysql_query(mysql, "drop table if exists t1,t2");
   myquery(rc);
   /* table without auto_increment column */
rc= mysql_query(mysql, "create table t1 (f1 int, f2 varchar(255), key(f1))");
@@ -18700,7 +18700,8 @@ static void test_progress_reporting()
   myheader("test_progress_reporting");

   conn= client_connect(CLIENT_PROGRESS, MYSQL_PROTOCOL_TCP, 0);
-  DIE_UNLESS(conn->client_flag & CLIENT_PROGRESS);
+  if (!(conn->client_flag & CLIENT_PROGRESS))
+    return;

   mysql_options(conn, MYSQL_PROGRESS_CALLBACK, (void*) report_progress);
   rc= mysql_query(conn, "set @save=@@global.progress_report_time");



Follow ups

References