maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

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@xxxxxxxxxxx