maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Vicentiu!

Looks fine, thanks. Few comments below.

On Feb 13, vicentiu wrote:
> revision-id: 822aa21672ba6650a31a3ec05ab5c1bce7a80c1e (mariadb-10.2.3-198-g822aa21672b)
> parent(s): b745ea489f0698c328383d9b8ec20d2f9777b1b8
> author: Vicențiu Ciorbaru
> committer: Vicențiu Ciorbaru
> timestamp: 2017-02-13 14:44:00 +0200
> message:
> 
> MDEV-11170: MariaDB 10.2 cannot start on MySQL 5.7 datadir
> 
> PART 2 of the fix adds the logic of not using password column, unless it
> exists. If password column is missing we attempt to use plugin &&
> authentication_string columns.
> 
> diff --git a/mysql-test/r/no_password_column-mdev-11170.result b/mysql-test/r/no_password_column-mdev-11170.result
> new file mode 100644
> index 00000000000..c3920a1b530
> --- /dev/null
> +++ b/mysql-test/r/no_password_column-mdev-11170.result
> @@ -0,0 +1,170 @@
...
> +# Reset to final original state.
> +#
> +drop table mysql.user;
> +create table mysql.user like backup_user;
> +insert into mysql.user select * from backup_user;

you could RENAME TABLE backup_user to mysql.user.

> +drop table backup_user;
> +flush privileges;
> diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc
> index 568aff9ac92..7e03253ca98 100644
> --- a/sql/sql_acl.cc
> +++ b/sql/sql_acl.cc
> @@ -2071,7 +2103,7 @@ static bool acl_load(THD *thd, const Grant_tables& tables)
>                                     8, 8, MYF(0));
>  
>        /* check default role, if any */
> -      if (!is_role && user_table.num_fields() > DEFAULT_ROLE_COLUMN_IDX)
> +      if (!is_role && user_table.default_role())

all these changes could've went into the first patch...

>        {
>          user.default_rolename.str=
>            get_field(&acl_memroot, user_table.default_role());
> @@ -4030,11 +4092,16 @@ static int replace_user_table(THD *thd, const User_table &user_table,
>  
>    rights= user_table.get_access();
>  
> -  DBUG_PRINT("info",("table fields: %d",user_table.num_fields()));
> -  if (combo.pwhash.str[0])
> +  DBUG_PRINT("info",("table fields: %d", user_table.num_fields()));
> +  /* If we don't have a password column, we'll use the authentication_string
> +     column later. */
> +  if (combo.pwhash.str[0] && user_table.password())
>      user_table.password()->store(combo.pwhash.str, combo.pwhash.length,
>                                   system_charset_info);
> -  if (user_table.num_fields() >= 31)    /* From 4.0.0 we have more fields */
> +  /* We either have the password column, the plugin column, or both. Otherwise
> +     we have a corrupt user table. */
> +  DBUG_ASSERT(user_table.password() || user_table.plugin());

you cannot assert the table structure. ALTER TABLE shouldn't crash the
server.

> +  if (user_table.ssl_type())    /* From 4.0.0 we have more fields */
>    {
>      /* We write down SSL related ACL stuff */
>      switch (lex->ssl_type) {

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx