Re: bzr commit into mysql-5.1-bugteam branch (chad:2841) Bug#39559

[Marc Alff]

Hi Chad

This is a good fix,
OK to push.

Thanks,
-- Marc


Chad MILLER wrote:
> #At file:///home/cmiller/work/mysqlbzr/5.1-bugteam--bug39559/ based on
> revid:patrick.crews@stripped
> 
>  2841 Chad MILLER	2009-04-09
>       Bug#39559: dump of stored procedures / functions with C-style \
>       	comment can't be read back
>       
>       A change to the lexer in 5.1 caused slash-asterisk-bang-version
>       sections to be terminated early if there exists a slash-asterisk-
>       style comment inside it.  Nesting comments is usually illegal,
>       but we rely on versioned comment blocks in mysqldump, and the
>       contents of those sections must be allowed to have comments.
>       
>       The problem was that when encountering open-comment tokens and
>       consuming -or- passing through the contents, the "in_comment"
>       state at the end was clobbered with the not-in-a-comment value,
>       regardless of whether we were in a comment before this or not.  
>       
>       So, """/*!VER one /* two */ three */""" would lose its in-comment
>       state between "two" and "three".  Save the echo and in-comment
>       state, and restore it at the end of the comment if we consume a 
>       comment.
> 
>     modified:
>       mysql-test/r/parser.result
>       mysql-test/t/parser.test
>       sql/sql_lex.cc
>       sql/sql_lex.h
> === modified file 'mysql-test/r/parser.result'
> --- a/mysql-test/r/parser.result	2009-02-14 16:04:16 +0000
> +++ b/mysql-test/r/parser.result	2009-04-10 02:18:18 +0000
> @@ -615,3 +615,55 @@ UPDATE t3 SET a4={d '1789-07-14'} WHERE 
>  SELECT a1, a4 FROM t2 WHERE a4 LIKE {fn UCASE('1789-07-14')};
>  a1	a4
>  DROP TABLE t1, t2, t3;
> +#
> +# Bug#39559: dump of stored procedures / functions with C-style 
> +#     comment can't be read back
> +#
> ++----------+--------+
> +| expected | result |
> ++----------+--------+
> +|        2 |      2 | 
> ++----------+--------+
> ++----------+--------+
> +| expected | result |
> ++----------+--------+
> +|        1 |      1 | 
> ++----------+--------+
> ++----------+--------+
> +| expected | result |
> ++----------+--------+
> +|        3 |      3 | 
> ++----------+--------+
> ++----------+--------+
> +| expected | result |
> ++----------+--------+
> +|        2 |      2 | 
> ++----------+--------+
> ++----------+--------+
> +| expected | result |
> ++----------+--------+
> +|        7 |      7 | 
> ++----------+--------+
> ++----------+--------+
> +| expected | result |
> ++----------+--------+
> +|        8 |      8 | 
> ++----------+--------+
> ++----------+--------+
> +| expected | result |
> ++----------+--------+
> +|        7 |      7 | 
> ++----------+--------+
> ++----------+--------+
> +| expected | result |
> ++----------+--------+
> +|        4 |      4 | 
> ++----------+--------+
> ++----------+--------+
> +| expected | result |
> ++----------+--------+
> +|        4 |      4 | 
> ++----------+--------+
> +#
> +# End of 5.1 tests
> +#
> 
> === modified file 'mysql-test/t/parser.test'
> --- a/mysql-test/t/parser.test	2008-07-19 08:31:33 +0000
> +++ b/mysql-test/t/parser.test	2009-04-10 02:18:18 +0000
> @@ -724,3 +724,31 @@ SELECT {fn CONCAT(a1,a2)} FROM t1;
>  UPDATE t3 SET a4={d '1789-07-14'} WHERE a1=0;
>  SELECT a1, a4 FROM t2 WHERE a4 LIKE {fn UCASE('1789-07-14')};
>  DROP TABLE t1, t2, t3;
> +
> +###########################################################################
> +--echo #
> +--echo # Bug#39559: dump of stored procedures / functions with C-style 
> +--echo #     comment can't be read back
> +--echo #
> +
> +--write_file $MYSQLTEST_VARDIR/tmp/bug39559.sql
> +select 2 as expected, /*!01000/**/*/ 2 as result;
> +select 1 as expected, /*!99998/**/*/ 1 as result;
> +select 3 as expected, /*!01000 1 + */ 2 as result;
> +select 2 as expected, /*!99990 1 + */ 2 as result;
> +select 7 as expected, /*!01000 1 + /* 8 + */ 2 + */ 4 as result;
> +select 8 as expected, /*!99998 1 + /* 2 + */ 4 + */ 8 as result;
> +select 7 as expected, /*!01000 1 + /*!01000 8 + */ 2 + */ 4 as result;
> +select 7 as expected, /*!01000 1 + /*!99998 8 + */ 2 + */ 4 as result;
> +select 4 as expected, /*!99998 1 + /*!99998 8 + */ 2 + */ 4 as result;
> +select 4 as expected, /*!99998 1 + /*!01000 8 + */ 2 + */ 4 as result;
> +select 7 as expected, /*!01000 1 + /*!01000 8 + /*!01000 error */ 16 + */ 2 + */ 4
> as result;
> +select 4 as expected, /* 1 + /*!01000 8 + */ 2 + */ 4;
> +EOF
> +
> +--exec $MYSQL --comment --force --table test <$MYSQLTEST_VARDIR/tmp/bug39559.sql
> +--remove_file $MYSQLTEST_VARDIR/tmp/bug39559.sql
> +
> +--echo #
> +--echo # End of 5.1 tests
> +--echo #
> 
> === modified file 'sql/sql_lex.cc'
> --- a/sql/sql_lex.cc	2009-03-05 14:22:33 +0000
> +++ b/sql/sql_lex.cc	2009-04-10 02:18:18 +0000
> @@ -712,6 +712,53 @@ static inline uint int_token(const char 
>    return ((uchar) str[-1] <= (uchar) cmp[-1]) ? smaller : bigger;
>  }
>  
> +
> +/**
> +  Given a stream that is advanced to the first contained character in 
> +  an open comment, consume the comment.  Optionally, if we are allowed, 
> +  recurse so that we understand comments within this current comment.
> +
> +  At this level, we do not support version-condition comments.  We might 
> +  have been called with having just passed one in the stream, though.  In 
> +  that case, we probably want to tolerate mundane comments inside.  Thus,
> +  the case for recursion.
> +
> +  @retval  Whether EOF reached before comment is closed.
> +*/
> +bool consume_comment(Lex_input_stream *lip, int remaining_recursions_permitted)
> +{
> +  reg1 uchar c;
> +  while (! lip->eof())
> +  {
> +    c= lip->yyGet();
> +
> +    if (remaining_recursions_permitted > 0)
> +    {
> +      if ((c == '/') && (lip->yyPeek() == '*'))
> +      {
> +        lip->yySkip(); /* Eat asterisk */
> +        consume_comment(lip, remaining_recursions_permitted-1);
> +        continue;
> +      }
> +    }
> +
> +    if (c == '*')
> +    {
> +      if (lip->yyPeek() == '/')
> +      {
> +        lip->yySkip(); /* Eat slash */
> +        return FALSE;
> +      }
> +    }
> +
> +    if (c == '\n')
> +      lip->yylineno++;
> +  }
> +
> +  return TRUE;
> +}
> +
> +
>  /*
>    MYSQLlex remember the following states from the following MYSQLlex()
>  
> @@ -1204,6 +1251,8 @@ int MYSQLlex(void *arg, void *yythd)
>        /* Reject '/' '*', since we might need to turn off the echo */
>        lip->yyUnget();
>  
> +      lip->save_in_comment_state();
> +
>        if (lip->yyPeekn(2) == '!')
>        {
>          lip->in_comment= DISCARD_COMMENT;
> @@ -1246,11 +1295,17 @@ int MYSQLlex(void *arg, void *yythd)
>              /* Expand the content of the special comment as real code */
>              lip->set_echo(TRUE);
>              state=MY_LEX_START;
> -            break;
> +            break;  /* Do not treat contents as a comment.  */
> +          }
> +          else
> +          {
> +            comment_closed= ! consume_comment(lip, 1);
> +            /* version allowed to have one level of comment inside. */
>            }
>          }
>          else
>          {
> +          /* Not a version comment. */
>            state=MY_LEX_START;
>            lip->set_echo(TRUE);
>            break;
> @@ -1261,38 +1316,30 @@ int MYSQLlex(void *arg, void *yythd)
>          lip->in_comment= PRESERVE_COMMENT;
>          lip->yySkip();                  // Accept /
>          lip->yySkip();                  // Accept *
> +        comment_closed= ! consume_comment(lip, 0);
> +        /* regular comments can have zero comments inside. */
>        }
>        /*
>          Discard:
>          - regular '/' '*' comments,
>          - special comments '/' '*' '!' for a future version,
>          by scanning until we find a closing '*' '/' marker.
> -        Note: There is no such thing as nesting comments,
> -        the first '*' '/' sequence seen will mark the end.
> +
> +        Nesting regular comments isn't allowed.  The first 
> +        '*' '/' returns the parser to the previous state.
> +
> +        /#!VERSI oned containing /# regular #/ is allowed #/
> +
> +		Inside one versioned comment, another versioned comment
> +		is treated as a regular discardable comment.  It gets
> +		no special parsing.
>        */
> -      comment_closed= FALSE;
> -      while (! lip->eof())
> -      {
> -        c= lip->yyGet();
> -        if (c == '*')
> -        {
> -          if (lip->yyPeek() == '/')
> -          {
> -            lip->yySkip();
> -            comment_closed= TRUE;
> -            state = MY_LEX_START;
> -            break;
> -          }
> -        }
> -        else if (c == '\n')
> -          lip->yylineno++;
> -      }
> +
>        /* Unbalanced comments with a missing '*' '/' are a syntax error */
>        if (! comment_closed)
>          return (ABORT_SYM);
>        state = MY_LEX_START;             // Try again
> -      lip->in_comment= NO_COMMENT;
> -      lip->set_echo(TRUE);
> +      lip->restore_in_comment_state();
>        break;
>      case MY_LEX_END_LONG_COMMENT:
>        if ((lip->in_comment != NO_COMMENT) && lip->yyPeek() == '/')
> 
> === modified file 'sql/sql_lex.h'
> --- a/sql/sql_lex.h	2009-03-05 14:22:33 +0000
> +++ b/sql/sql_lex.h	2009-04-10 02:18:18 +0000
> @@ -1160,6 +1160,18 @@ public:
>      m_echo= echo;
>    }
>  
> +  void save_in_comment_state()
> +  {
> +    m_echo_saved= m_echo;
> +    in_comment_saved= in_comment;
> +  }
> +
> +  void restore_in_comment_state()
> +  {
> +    m_echo= m_echo_saved;
> +    in_comment= in_comment_saved;
> +  }
> +
>    /**
>      Skip binary from the input stream.
>      @param n number of bytes to accept.
> @@ -1417,6 +1429,7 @@ private:
>  
>    /** Echo the parsed stream to the pre-processed buffer. */
>    bool m_echo;
> +  bool m_echo_saved;
>  
>    /** Pre-processed buffer. */
>    char *m_cpp_buf;
> @@ -1479,6 +1492,7 @@ public:
>  
>    /** State of the lexical analyser for comments. */
>    enum_comment_state in_comment;
> +  enum_comment_state in_comment_saved;
>  
>    /**
>      Starting position of the TEXT_STRING or IDENT in the pre-processed
> 
> 
> 
> ------------------------------------------------------------------------
> 
>