Re: bk commit into 5.0 tree (kaa:1.2517) BUG#30164

[Marc Alff]

Hi Alexey


Alexey Kopytov wrote:
> Below is the list of changes that have just been committed into a local
> 5.0 repository of kaa. When kaa does a push these changes will
> be propagated to the main repository and, within 24 hours after the
> push, to the public repository.
> For information on how to access the public repository
> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>
> ChangeSet@stripped, 2007-08-24 18:11:41+04:00, kaa@stripped +3 -0
>   Bug #30164: Using client side macro inside server side comments generates broken
> queries
>   
>   Problem:
>   
>   In cases when a client-side macro appears inside a server-side comment, the
> add_line() function in mysql.cc discarded all characters until the next delimiter to
> remove macro arguments from the query string. This resulted in broken queries being sent
> to the server when the next delimiter character appeared past the comment's boundaries,
> because the comment closing sequence ('*/') was discarded.
>   
>   Fix:
>   
>   If a client-side macro appears inside a server-side comment, discard all characters
> in the comment after the macro (that is, until the end of the comment rather than the next
> delimiter).
>   This is a minimal fix to allow only simple cases used by the mysqlbinlog utility.
> Limitations that are worth documenting:
>   
>   - Nested server-side and/or client-side comments are not supported by mysql.cc
>   - Using client-side macros in multi-line server-side comments is not supported
>   - All characters after a client-side macro in a server-side comment will be omitted
> from the query string (and thus, will not be sent to server).
>
>   

The comments in the "Fix:" part of the change set are very valuable,
please add them in the code also.


>   client/mysql.cc@stripped, 2007-08-24 18:11:37+04:00, kaa@stripped +32 -17
>     If a client-side macro appears inside a server-side comment, discard all
> characters in the comment after the macro.
>
>   mysql-test/r/mysql.result@stripped, 2007-08-24 18:11:37+04:00, kaa@stripped +2 -0
>     Added a test case for bug #30164.
>
>   mysql-test/t/mysql.test@stripped, 2007-08-24 18:11:37+04:00, kaa@stripped +5 -0
>     Added a test case for bug #30164.
>
> diff -Nrup a/client/mysql.cc b/client/mysql.cc
> --- a/client/mysql.cc	2007-07-10 11:43:10 +04:00
> +++ b/client/mysql.cc	2007-08-24 18:11:37 +04:00
> @@ -1245,6 +1245,7 @@ static bool add_line(String &buffer,char
>    char buff[80], *pos, *out;
>    COMMANDS *com;
>    bool need_space= 0;
> +  bool ss_comment= 0;
>    DBUG_ENTER("add_line");
>  
>    if (!line[0] && buffer.is_empty())
> @@ -1293,22 +1294,31 @@ static bool add_line(String &buffer,char
>        }
>        if ((com=find_command(NullS,(char) inchar)))
>        {
> -	const String tmp(line,(uint) (out-line), charset_info);
> -	buffer.append(tmp);
> -	if ((*com->func)(&buffer,pos-1) > 0)
> -	  DBUG_RETURN(1);                       // Quit
> -	if (com->takes_params)
> -	{
> -	  for (pos++ ;
> -	       *pos && (*pos != *delimiter ||
> -			!is_prefix(pos + 1, delimiter + 1)) ; pos++)
> -	    ;	// Remove parameters
> -	  if (!*pos)
> -	    pos--;
> -	  else 
> -	    pos+= delimiter_length - 1; // Point at last delim char
> -	}
> -	out=line;
> +        const String tmp(line,(uint) (out-line), charset_info);
> +        buffer.append(tmp);
> +        if ((*com->func)(&buffer,pos-1) > 0)
> +          DBUG_RETURN(1);                       // Quit
> +        if (com->takes_params)
> +        {
> +          if (ss_comment)
> +          {
> +            for (pos++; *pos && (*pos != '*' || *(pos + 1) != '/'); pos++)
> +              ;
> +            pos--;
> +          }
> +          else
> +          {
> +            for (pos++ ;
> +                 *pos && (*pos != *delimiter ||
> +                          !is_prefix(pos + 1, delimiter + 1)) ; pos++)
> +              ;	// Remove parameters
> +            if (!*pos)
> +              pos--;
> +            else 
> +              pos+= delimiter_length - 1; // Point at last delim char
> +          }
> +        }
> +        out=line;
>        }
>        else
>        {
> @@ -1368,7 +1378,7 @@ static bool add_line(String &buffer,char
>          out=line;
>        }
>      }
> -    else if (*ml_comment && inchar == '*' && *(pos + 1) == '/')
> +    else if (*ml_comment && !ss_comment && inchar == '*' &&
> *(pos + 1) == '/')
>      {
>        pos++;
>        *ml_comment= 0;
> @@ -1376,6 +1386,11 @@ static bool add_line(String &buffer,char
>      }      
>      else
>      {						// Add found char to buffer
> +      if (!*in_string && inchar == '/' && *(pos + 1) == '*'
> &&
> +          *(pos + 2) == '!')
> +        ss_comment= 1;
> +      else if (!*in_string && ss_comment && inchar == '*' &&
> *(pos + 1) == '/')
> +        ss_comment= 0;
>        if (inchar == *in_string)
>  	*in_string= 0;
>        else if (!*ml_comment && !*in_string &&
> diff -Nrup a/mysql-test/r/mysql.result b/mysql-test/r/mysql.result
> --- a/mysql-test/r/mysql.result	2007-04-23 14:58:34 +04:00
> +++ b/mysql-test/r/mysql.result	2007-08-24 18:11:37 +04:00
> @@ -176,4 +176,6 @@ ERROR at line 1: DELIMITER cannot contai
>  ERROR at line 1: DELIMITER cannot contain a backslash character
>  1
>  1
> +1
> +1
>  End of 5.0 tests
> diff -Nrup a/mysql-test/t/mysql.test b/mysql-test/t/mysql.test
> --- a/mysql-test/t/mysql.test	2007-08-07 13:40:02 +04:00
> +++ b/mysql-test/t/mysql.test	2007-08-24 18:11:37 +04:00
> @@ -276,4 +276,9 @@ remove_file $MYSQLTEST_VARDIR/tmp/bug214
>  --exec $MYSQL
> --pager="540bytelengthstringxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
> -e "select 1" > /dev/null 2>&1
>  --exec $MYSQL
> --character-sets-dir="540bytelengthstringxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
> -e "select 1" 2>&1
>  
> +#
> +# bug #30164: Using client side macro inside server side comments generates broken
> queries
> +#
> +--exec $MYSQL test -e '/*! \C latin1 */ select 1;'
> +
>  --echo End of 5.0 tests
>
>   

Also verified that this fix is compatible with the pending patch for
Bug#28779,
which depends on this.

Ok to push.

Please push in the runtime team tree if possible,
so that Bug#28779 can be resolved sooner.

Thanks,
-- Marc