List:Commits« Previous MessageNext Message »
From:Alexey Kopytov Date:March 18 2009 8:18am
Subject:bzr commit into mysql-5.0-bugteam branch (Alexey.Kopytov:2764)
Bug#41486
View as plain text  
#At file:///Users/kaa/src/bzr/bugteam/bug41486/my50-bug41486/ based on revid:sergey.glukhov@stripped

 2764 Alexey Kopytov	2009-03-18
      Fix for bug#41486: extra character appears in BLOB for every 
                         ~40Mb after mysqldump/import 
              
      When the input string exceeds the maximum allowed size for the 
      internal buffer, batch_readline() returns a truncated string. 
      Since there was no way for a caller to determine whether the 
      string was truncated or not, the command line client assumed 
      batch_readline() to always return the whole input string and 
      appended a newline character. This resulted in garbled data 
      when importing dumps containing strings longer than the 
      maximum input buffer size. 
        
      Fixed by adding a flag to the batch_readline() interface to 
      signal a truncated string to the caller. 
        
      Other minor problems fixed during patch implementation: 
       
      - The maximum allowed buffer size for batch_readline() was set 
      up depending on the client's max_allowed_packet value. It does 
      not actully make any sense, as those variables are not 
      related. The input buffer size limit is now always set to 1 
      MB. 
        
      - fill_buffer() did not always set the EOF flag. 
       
      - The input buffer could actually grow twice as the specified 
      limit due to insufficient checks in intern_read_line(). 
      modified:
        client/my_readline.h
        client/mysql.cc
        client/readline.cc
        mysql-test/r/mysql.result
        mysql-test/t/mysql.test

per-file messages:
  client/my_readline.h
    Changed the interface of batch_readline().
  client/mysql.cc
    Honor the truncated flag returned by batch_readline() and do  
    not append the newline character if it was set. Since we can't 
    change the interfaces for readline()/fgets() used in the  
    interactive mode, always assume the returned string was not  
    truncated. In addition, always set the batch_readline()  
    internal buffer to 1 MB, independently from the client's  
    max_allowed_packet.
  client/readline.cc
    Added the 'truncated' argument do batch_readline() to signal 
    truncated string to a caller. 
    Fixed fill_buffer() to set the EOF flag correctly. 
    Fixed checks in intern_read_line() to not allow the internal  
    buffer grow past the specified limit.
  mysql-test/r/mysql.result
    Added a test case for bug #41486.
  mysql-test/t/mysql.test
    Added a test case for bug #41486.
=== modified file 'client/my_readline.h'
--- a/client/my_readline.h	2006-12-23 19:17:15 +0000
+++ b/client/my_readline.h	2009-03-18 08:18:24 +0000
@@ -29,5 +29,5 @@ typedef struct st_line_buffer
 
 extern LINE_BUFFER *batch_readline_init(ulong max_size,FILE *file);
 extern LINE_BUFFER *batch_readline_command(LINE_BUFFER *buffer, my_string str);
-extern char *batch_readline(LINE_BUFFER *buffer);
+extern char *batch_readline(LINE_BUFFER *buffer, bool *truncated);
 extern void batch_readline_end(LINE_BUFFER *buffer);

=== modified file 'client/mysql.cc'
--- a/client/mysql.cc	2009-02-24 13:06:28 +0000
+++ b/client/mysql.cc	2009-03-18 08:18:24 +0000
@@ -112,6 +112,8 @@ extern "C" {
 #define PROMPT_CHAR '\\'
 #define DEFAULT_DELIMITER ";"
 
+#define MAX_BATCH_BUFFER_SIZE (1024L * 1024L)
+
 typedef struct st_status
 {
   int exit_status;
@@ -1035,7 +1037,7 @@ static void fix_history(String *final_co
 
 static COMMANDS *find_command(char *name,char cmd_name);
 static bool add_line(String &buffer,char *line,char *in_string,
-                     bool *ml_comment);
+                     bool *ml_comment, bool truncated);
 static void remove_cntrl(String &buffer);
 static void print_table_data(MYSQL_RES *result);
 static void print_table_data_html(MYSQL_RES *result);
@@ -1117,7 +1119,7 @@ int main(int argc,char *argv[])
     exit(1);
   }
   if (status.batch && !status.line_buff &&
-      !(status.line_buff=batch_readline_init(opt_max_allowed_packet+512,stdin)))
+      !(status.line_buff= batch_readline_init(MAX_BATCH_BUFFER_SIZE, stdin)))
   {
     free_defaults(defaults_argv);
     my_end(0);
@@ -1766,13 +1768,14 @@ static int read_and_execute(bool interac
   ulong line_number=0;
   bool ml_comment= 0;  
   COMMANDS *com;
+  bool truncated= 0;
   status.exit_status=1;
   
   for (;;)
   {
     if (!interactive)
     {
-      line=batch_readline(status.line_buff);
+      line=batch_readline(status.line_buff, &truncated);
       /*
         Skip UTF8 Byte Order Marker (BOM) 0xEFBBBF.
         Editors like "notepad" put this marker in
@@ -1891,7 +1894,7 @@ static int read_and_execute(bool interac
 #endif
       continue;
     }
-    if (add_line(glob_buffer,line,&in_string,&ml_comment))
+    if (add_line(glob_buffer,line,&in_string,&ml_comment, truncated))
       break;
   }
   /* if in batch mode, send last query even if it doesn't end with \g or go */
@@ -1977,7 +1980,7 @@ static COMMANDS *find_command(char *name
 
 
 static bool add_line(String &buffer,char *line,char *in_string,
-                     bool *ml_comment)
+                     bool *ml_comment, bool truncated)
 {
   uchar inchar;
   char buff[80], *pos, *out;
@@ -2224,9 +2227,10 @@ static bool add_line(String &buffer,char
   {
     uint length=(uint) (out-line);
 
-    if (length < 9 || 
-        my_strnncoll (charset_info, 
-                      (uchar *)line, 9, (const uchar *) "delimiter", 9))
+    if (!truncated &&
+        (length < 9 || 
+         my_strnncoll (charset_info, 
+                       (uchar *)line, 9, (const uchar *) "delimiter", 9)))
     {
       /* 
         Don't add a new line in case there's a DELIMITER command to be 
@@ -3886,7 +3890,7 @@ static int com_source(String *buffer, ch
     return put_info(buff, INFO_ERROR, 0);
   }
 
-  if (!(line_buff=batch_readline_init(opt_max_allowed_packet+512,sql_file)))
+  if (!(line_buff= batch_readline_init(MAX_BATCH_BUFFER_SIZE, sql_file)))
   {
     my_fclose(sql_file,MYF(0));
     return put_info("Can't initialize batch_readline", INFO_ERROR, 0);

=== modified file 'client/readline.cc'
--- a/client/readline.cc	2006-12-23 19:17:15 +0000
+++ b/client/readline.cc	2009-03-18 08:18:24 +0000
@@ -24,7 +24,7 @@ static bool init_line_buffer(LINE_BUFFER
 			    ulong max_size);
 static bool init_line_buffer_from_string(LINE_BUFFER *buffer,my_string str);
 static uint fill_buffer(LINE_BUFFER *buffer);
-static char *intern_read_line(LINE_BUFFER *buffer,ulong *out_length);
+static char *intern_read_line(LINE_BUFFER *buffer, ulong *out_length, bool *truncated);
 
 
 LINE_BUFFER *batch_readline_init(ulong max_size,FILE *file)
@@ -42,12 +42,13 @@ LINE_BUFFER *batch_readline_init(ulong m
 }
 
 
-char *batch_readline(LINE_BUFFER *line_buff)
+char *batch_readline(LINE_BUFFER *line_buff, bool *truncated)
 {
   char *pos;
   ulong out_length;
+  DBUG_ASSERT(truncated != NULL);
 
-  if (!(pos=intern_read_line(line_buff,&out_length)))
+  if (!(pos=intern_read_line(line_buff,&out_length, truncated)))
     return 0;
   if (out_length && pos[out_length-1] == '\n')
     if (--out_length && pos[out_length-1] == '\r')  /* Remove '\n' */
@@ -149,6 +150,14 @@ static uint fill_buffer(LINE_BUFFER *buf
     read_count=(buffer->bufread - bufbytes)/IO_SIZE;
     if ((read_count*=IO_SIZE))
       break;
+    if (buffer->bufread * 2 > buffer->max_size)
+    {
+      /*
+        So we must grow the buffer but we cannot due to the max_size limit.
+        Return 0 w/o setting buffer->eof to signal this condition.
+      */
+      return 0;
+    }
     buffer->bufread *= 2;
     if (!(buffer->buffer = (char*) my_realloc(buffer->buffer,
 					      buffer->bufread+1,
@@ -172,11 +181,15 @@ static uint fill_buffer(LINE_BUFFER *buf
 
   DBUG_PRINT("fill_buff", ("Got %d bytes", read_count));
 
-  /* Kludge to pretend every nonempty file ends with a newline. */
-  if (!read_count && bufbytes && buffer->end[-1] != '\n')
+  if (!read_count)
   {
-    buffer->eof = read_count = 1;
-    *buffer->end = '\n';
+    buffer->eof = 1;
+    /* Kludge to pretend every nonempty file ends with a newline. */
+    if (bufbytes && buffer->end[-1] != '\n')
+    {
+      read_count = 1;
+      *buffer->end = '\n';
+    }
   }
   buffer->end_of_line=(buffer->start_of_line=buffer->buffer)+bufbytes;
   buffer->end+=read_count;
@@ -186,7 +199,7 @@ static uint fill_buffer(LINE_BUFFER *buf
 
 
 
-char *intern_read_line(LINE_BUFFER *buffer,ulong *out_length)
+char *intern_read_line(LINE_BUFFER *buffer, ulong *out_length, bool *truncated)
 {
   char *pos;
   uint length;
@@ -200,14 +213,23 @@ char *intern_read_line(LINE_BUFFER *buff
       pos++;
     if (pos == buffer->end)
     {
-      if ((uint) (pos - buffer->start_of_line) < buffer->max_size)
+      /*
+        fill_buffer() can return 0 either on EOF in which case we abort
+        or when the internal buffer has hit the size limit. In the latter case
+        return what we have read so far and signal string truncation.
+      */
+      if (!(length=fill_buffer(buffer)) || length == (uint) -1)
       {
-	if (!(length=fill_buffer(buffer)) || length == (uint) -1)
-	  DBUG_RETURN(0);
-	continue;
+        if (buffer->eof)
+          DBUG_RETURN(0);
       }
+      else
+        continue;
       pos--;					/* break line here */
+      *truncated= 1;
     }
+    else
+      *truncated= 0;
     buffer->end_of_line=pos+1;
     *out_length=(ulong) (pos + 1 - buffer->eof - buffer->start_of_line);
     DBUG_RETURN(buffer->start_of_line);

=== modified file 'mysql-test/r/mysql.result'
--- a/mysql-test/r/mysql.result	2009-02-24 13:06:28 +0000
+++ b/mysql-test/r/mysql.result	2009-03-18 08:18:24 +0000
@@ -192,4 +192,15 @@ delimiter
 1
 1
 1
+set @old_max_allowed_packet = @@global.max_allowed_packet;
+set @@global.max_allowed_packet = 2 * 1024 * 1024 + 1024;
+set @@max_allowed_packet = @@global.max_allowed_packet;
+CREATE TABLE t1(data LONGBLOB);
+INSERT INTO t1 SELECT REPEAT('1', 2*1024*1024);
+SELECT LENGTH(data) FROM t1;
+LENGTH(data)
+2097152
+DROP TABLE t1;
+set @@global.max_allowed_packet = @old_max_allowed_packet;
+set @@max_allowed_packet = @@global.max_allowed_packet;
 End of 5.0 tests

=== modified file 'mysql-test/t/mysql.test'
--- a/mysql-test/t/mysql.test	2009-02-24 13:06:28 +0000
+++ b/mysql-test/t/mysql.test	2009-03-18 08:18:24 +0000
@@ -331,4 +331,31 @@ EOF
 
 remove_file $MYSQLTEST_VARDIR/tmp/bug31060.sql;
 
+#  
+# Bug #41486: extra character appears in BLOB for every ~40Mb after   
+#             mysqldump/import   
+#  
+  
+# Have to change the global variable as the mysql client will use 
+# a separate session 
+set @old_max_allowed_packet = @@global.max_allowed_packet; 
+# 2 MB blob length + some space for the rest of INSERT query
+set @@global.max_allowed_packet = 2 * 1024 * 1024 + 1024; 
+set @@max_allowed_packet = @@global.max_allowed_packet;
+
+CREATE TABLE t1(data LONGBLOB);  
+INSERT INTO t1 SELECT REPEAT('1', 2*1024*1024);  
+  
+--exec $MYSQL_DUMP test t1 >$MYSQLTEST_VARDIR/tmp/bug41486.sql  
+# Check that the mysql client does not insert extra newlines when loading  
+# strings longer than client's max_allowed_packet  
+--exec $MYSQL --max_allowed_packet=1M test < $MYSQLTEST_VARDIR/tmp/bug41486.sql 2>&1  
+SELECT LENGTH(data) FROM t1;  
+  
+remove_file $MYSQLTEST_VARDIR/tmp/bug41486.sql; 
+DROP TABLE t1;  
+
+set @@global.max_allowed_packet = @old_max_allowed_packet; 
+set @@max_allowed_packet = @@global.max_allowed_packet;
+ 
 --echo End of 5.0 tests

Thread
bzr commit into mysql-5.0-bugteam branch (Alexey.Kopytov:2764)Bug#41486Alexey Kopytov18 Mar