Re: [Maria-developers] [Commits] 5dd6eb4: MDEV-9117: Client Server capability negotiation for MariaDB specific functionality
Hi, Sanja! On Jan 28, OleksandrByelkin wrote:
revision-id: 5dd6eb45c418f61c749a7884d7b3e13d62f3bfb9 (mariadb-10.1.8-113-g5dd6eb4) parent(s): 14d79a5e99f9992cabb7489203989ccdaacadb78 committer: Oleksandr Byelkin timestamp: 2016-01-28 14:58:12 +0100 message:
MDEV-9117: Client Server capability negotiation for MariaDB specific functionality
Just a couple of comments:
New capability flags space. Removed old progress flag, added new one.
diff --git a/client/mysql.cc b/client/mysql.cc index 3e08c87..715eed8 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;
ulong width is not portable, remember trubles we had with ulong sysvars? better use ulonglong for 64bit and uint for 32bit.
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/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 | \
really? CLIENT_MYSQL? Is it because they aren't using Connector/C yet?
CLIENT_LONG_FLAG | \ CLIENT_TRANSACTIONS | \ CLIENT_PROTOCOL_41 | \ diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 2bcd351..dcb12e6 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -11730,18 +11731,27 @@ 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 + ulonglong ext_client_capabilities= + (((ulonglong)uint4korr(net->read_pos + 28)) << 32); + client_capabilities|= ext_client_capabilities; + }
As I wrote earlier, please add here else if (client_capabilities & CLIENT_PROGRESS_OBSOLETE) client_capabilities|= MARIADB_CLIENT_PROGRESS;
}
/* 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));
why " 1" ?
if (thd->client_capabilities & CLIENT_SSL) { unsigned long errptr __attribute__((unused));
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
On 01.02.2016 22:34, Sergei Golubchik wrote:
Hi, Sanja!
On Jan 28, OleksandrByelkin wrote:
revision-id: 5dd6eb45c418f61c749a7884d7b3e13d62f3bfb9 (mariadb-10.1.8-113-g5dd6eb4) parent(s): 14d79a5e99f9992cabb7489203989ccdaacadb78 committer: Oleksandr Byelkin timestamp: 2016-01-28 14:58:12 +0100 message:
MDEV-9117: Client Server capability negotiation for MariaDB specific functionality Just a couple of comments:
New capability flags space. Removed old progress flag, added new one.
diff --git a/client/mysql.cc b/client/mysql.cc index 3e08c87..715eed8 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; ulong width is not portable, remember trubles we had with ulong sysvars? better use ulonglong for 64bit and uint for 32bit.
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/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 | \ really? CLIENT_MYSQL? Is it because they aren't using Connector/C yet?
Because libmysql do not support extended flags (only connector-C)
CLIENT_LONG_FLAG | \ CLIENT_TRANSACTIONS | \ CLIENT_PROTOCOL_41 | \ diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 2bcd351..dcb12e6 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -11730,18 +11731,27 @@ 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 + ulonglong ext_client_capabilities= + (((ulonglong)uint4korr(net->read_pos + 28)) << 32); + client_capabilities|= ext_client_capabilities; + }
As I wrote earlier, please add here
else if (client_capabilities & CLIENT_PROGRESS_OBSOLETE) client_capabilities|= MARIADB_CLIENT_PROGRESS;
OK
}
/* 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));
why " 1" ?
probably earlier it was 2 bytes, I'll fix.
if (thd->client_capabilities & CLIENT_SSL) { unsigned long errptr __attribute__((unused));
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
participants (2)
-
Oleksandr Byelkin -
Sergei Golubchik