From: Ramil Kalimullin
Date: November 12 2008 8:11am
Subject: Re: bzr commit into mysql-5.0-bugteam branch (v.narayanan:2712)
 Bug#39616
List-Archive: http://lists.mysql.com/commits/58521
Message-Id: <491A8FA9.6040106@sun.com>
MIME-Version: 1.0
Content-Type: text/plain; format=flowed; charset=ISO-8859-1
Content-Transfer-Encoding: 7BIT

Hi!

on 11.11.2008 16:53 V Narayanan wrote:
> #At file:///home/narayanan/Work/mysql/W-M/mysql-5.0-bugteam-39616/
> 
>  2712 V Narayanan	2008-11-11
>       Bug#39616: Missing quotes from .CSV crashes server

The patch looks good, see minor suggestions below.
Ok to push with a test case (you can either use a stored data
file or create one in the test).

>       When a CSV file contained comma separated elements 
>       that were not enclosed in quotes, it was causing the
>       mysql server to crash.
>       
>       The algorithm that parsed the content of a row in
>       mysql 5.0 was assuming that the values of the fields
>       in a .CSV file will be enclosed in quotes and will be
>       separated by commas.
>       
>       This was causing the algorithm to fail when the content
>       of the file resembled the following
>       3,"with quotes"
>       The CSV engine that is part of mysql 5.0 was expecting
>       the above to be
>       "3","with quotes"
>       
>       The above is just one example of where the engine was
>       failing for what would be recognized as a valid .CSV 
>       file content otherwise.
>       
>       The proposed fix changes the previous algorithm being used
>       to parse rows from the .CSV file to handle two separate
>       cases
>       
>       1) When the current field of the row is enclosed in quotes
>       2) When the current field of the row is not enclosed in quotes

Please add a note (for the Doc team) about '/X' chars handling.

> --- a/sql/examples/ha_tina.cc	2008-03-29 15:50:46 +0000
> +++ b/sql/examples/ha_tina.cc	2008-11-11 12:53:48 +0000
> @@ -419,34 +419,68 @@ int ha_tina::find_current_row(byte *buf)
>    for (Field **field=table->field ; *field ; field++)
>    {
>      buffer.length(0);
> -    mapped_ptr++; // Increment past the first quote
> -    for(;mapped_ptr != end_ptr; mapped_ptr++)
> +    /* Handle the case where the first character begins with a quote */
> +    if (*mapped_ptr == '"')
>      {
> -      //Need to convert line feeds!
> -      if (*mapped_ptr == '"' && 
> -          (((mapped_ptr[1] == ',') && (mapped_ptr[2] == '"')) || (mapped_ptr == end_ptr -1 )))
> +      /* Increment past the first quote */
> +      mapped_ptr++;
> +      /* Loop through the row to extract the values for the current field */
> +      for(;mapped_ptr != end_ptr; mapped_ptr++)
please, add a space after the first ';'

>        {
> -        mapped_ptr += 2; // Move past the , and the "
> -        break;
> -      } 
> -      if (*mapped_ptr == '\\' && mapped_ptr != (end_ptr - 1)) 
> -      {
> -        mapped_ptr++;
> -        if (*mapped_ptr == 'r')
> -          buffer.append('\r');
> -        else if (*mapped_ptr == 'n' )
> -          buffer.append('\n');
> -        else if ((*mapped_ptr == '\\') || (*mapped_ptr == '"'))
> -          buffer.append(*mapped_ptr);
> -        else  /* This could only happed with an externally created file */
> +        /* check for end of the current field */
> +        if (*mapped_ptr == '"' && 
> +            (mapped_ptr[1] == ',' || mapped_ptr == end_ptr -1 ))
> +        {
> +          /* Move past the , and the " */
> +          mapped_ptr += 2;
> +          break;
> +        } 
> +        if (*mapped_ptr == '\\' && mapped_ptr != (end_ptr - 1)) 
>          {
> -          buffer.append('\\');
> +          mapped_ptr++;
> +          if (*mapped_ptr == 'r')
> +            buffer.append('\r');
> +          else if (*mapped_ptr == 'n' )
> +            buffer.append('\n');
> +          else if ((*mapped_ptr == '\\') || (*mapped_ptr == '"'))
> +            buffer.append(*mapped_ptr);
> +          else  /* This could only happed with an externally created file */
> +          {
> +            buffer.append('\\');
> +            buffer.append(*mapped_ptr);
> +          }
> +        } 
> +        else
> +        {
> +          /* 
> +             If end of row occurs here, it means there has been an error
> +             in parsing
> +           */
Remove an extra space before "*/".
Please add a note that no last qoute was found - it's the real problem here.

> +          if (mapped_ptr == end_ptr -1) DBUG_RETURN(HA_ERR_END_OF_FILE);
DBUG_RETURN(HA_ERR_END_OF_FILE); should be on a separate line.

> +          /* Store current character in the buffer for the field */
>            buffer.append(*mapped_ptr);
>          }
> -      } 
> -      else
> +      }
> +    }
> +    else
> +    {
> +      /* Handle the case where the current row does not start with quotes */
> +        
> +      /* Loop through the row to extract the values for the current field */
> +      for (;mapped_ptr != end_ptr; mapped_ptr++)
add a space after ';'

Thank you,
Ramil.