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.
