maria-developers team mailing list archive

[Oleksandr Byelkin <sanja@xxxxxxxxxxxxxxxx>]

On 16.01.2016 21:56, Sergei Golubchik wrote:
> Hi, Sanja!
>
> On Nov 25, OleksandrByelkin wrote:
> > revision-id: cae19452eec1d3be47fa04759dd2dda6d061543e (mariadb-10.1.8-69-gcae1945)
> > parent(s): 55e67c3e344317c6f349f5391e5d117ec51ae062
> > committer: Oleksandr Byelkin
> > timestamp: 2015-11-25 18:09:39 +0100
> > message:
> >
> > MDEV-9117: Client Server capability negotiation for MariaDB specific functionality
> >
> > New capability flags space.
> > Removed old progress flag, added new one.
> >
> > 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
> > @@ -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)
> ok
>
> > +#define CLIENT_PROGRESS           0
> why is that?
Because old flag will not work with the new server.
> >   #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)
> how can this work, if client_flag and server_capabilities are
> only 32-bit variables?
it is not anymore (on the server) on non-connector client it will be 
lost but who cares if it doesnot support it ((I removed support by your 
request).
> > +
> >   #ifdef HAVE_COMPRESS
> >   #define CAN_CLIENT_COMPRESS CLIENT_COMPRESS
> >   #else
> > 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
> > @@ -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;
> Why do you need that (MARIADB_CLIENT_EXTENDED_FLAGS and the check)?
Server read or do not read more flags depend on it.
> > +      }
> > +    }
> here you need to check for old MariaDB clients - they will have
> CLIENT_MYSQL and might have CLIENT_PROGRESS_OBSOLETE - do something like
>
>     else if (client_capabilities & CLIENT_PROGRESS_OBSOLETE)
>     {
>       client_capabilities |= MARIADB_CLIENT_PROGRESS;
>     }
>
> >     }
> >   
> > 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");
> separate commit, please
ok, but it looks too late now, sorry.
> >     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;
> what are you testing here?
yes, I'll fix it.
> Regards,
> Sergei
> Chief Architect MariaDB
> and security@xxxxxxxxxxx