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
