Re: bk commit into 5.0 tree (gluh:1.2611) BUG#35924

[Alexander Nozdrin]

Hi,

thanks for working on this.

I think, your patch is generally Ok -- I just have a few notes. If you're fine
with them, you can push. Otherwise, plz submit another patch and we'll discuss
it.

On 21 April 2008 14:15:01 gluh@stripped wrote:
> ChangeSet@stripped, 2008-04-21 15:14:58+05:00, gluh@stripped +5 -0
>   Bug#35924 DEFINER should be stored 'quoted' in I_S
>   The '@' symbol can not be used in the host name according to rfc952.
>   The fix:
>   added function check_host_name(LEX_STRING *str)
>   which checks that all symbols in host name string are valid and

So, that means, we're introducing backward-incompatible change, even though it
is a bug fix. I'd draw more attention of docs team to this fact.

>   host name length is not more than max host name length(HOSTNAME_LENGTH).

Could you plz clarify that you just moved that check of the length out of
parser.

>   sql/mysql_priv.h@stripped, 2008-04-21 15:14:56+05:00, gluh@stripped +1 -0
>     added function check_host_name(LEX_STRING *str)
> 
>   sql/sql_parse.cc@stripped, 2008-04-21 15:14:56+05:00, gluh@stripped +32 -0
>     added function check_host_name(LEX_STRING *str)
>     which checks that all symbols in host name string are valid and
>     host name length is not more than max host name length(HOSTNAME_LENGTH).
> 
>   sql/sql_yacc.yy@stripped, 2008-04-21 15:14:56+05:00, gluh@stripped +1 -2
>     added function check_host_name(LEX_STRING *str)

Actually, it's more like "using newly added function check_host_name()".

> diff -Nrup a/mysql-test/t/create.test b/mysql-test/t/create.test
> --- a/mysql-test/t/create.test	2008-02-01 12:00:39 +04:00
> +++ b/mysql-test/t/create.test	2008-04-21 15:14:56 +05:00
> @@ -1171,5 +1171,10 @@ CREATE TABLE t1(c1 VARCHAR(33), KEY USIN
>  SHOW INDEX FROM t1;
>  DROP TABLE t1;
>  
> +#
> +# Bug#35924 DEFINER should be stored 'quoted' in I_S
> +#
> +--error ER_UNKNOWN_ERROR
> +create user mysqltest_1@'test@test';

I think we should introduce a new error code for this.
'Unknown error' must die. :)

> diff -Nrup a/sql/sql_parse.cc b/sql/sql_parse.cc
> --- a/sql/sql_parse.cc	2008-03-21 19:23:14 +04:00
> +++ b/sql/sql_parse.cc	2008-04-21 15:14:56 +05:00
> @@ -7943,3 +7943,35 @@ static bool test_if_data_home_dir(const 
>    DBUG_RETURN(0);
>  }
>  
> +
> +/*
> +  Check that host name string is valid.
> +
> +  SYNOPSIS
> +    check_host_name()
> +      str         string to be checked
> +
> +  RETURN
> +    FALSE   host name is ok
> +    TRUE    host name string is longer than max_length or has invalid symbols
> +*/

Please use doxygen style for new comments.

> +bool check_host_name(LEX_STRING *str)
> +{
> +  const char *name= str->str;
> +  const char *end= str->str + str->length;
> +  if (check_string_length(str, ER(ER_HOSTNAME), HOSTNAME_LENGTH))
> +    return TRUE;
> +
> +  while (name != end)
> +  {
> +    if (*name == '@')
> +    {
> +      my_printf_error(ER_UNKNOWN_ERROR, 
> +                      "Illegal symbol '@' in the host name",MYF(0));

Again, I do believe, that should be a new error. Something like:
  ER_MALFORMED_HOSTNAME: "Malformed hostname (illegal symbol: '%-.1s')"

-- 
Alexander Nozdrin, Software Developer
MySQL AB, Moscow, Russia, www.mysql.com