bzr commit into mysql-5.1-bugteam branch (martin.hansson:3534) Bug#58165

[Martin Hansson]

#At file:///data0/martin/bzrroot/bug58165/5.1bt-lazy_copy/ based on revid:sergey.glukhov@stripped

 3534 Martin Hansson	2010-12-22
      Bug#58165: "my_empty_string" gets modified and causes LOAD DATA to fail and
      other crashes
      
      Some string manipulating SQL functions use a shared string buffer intended to
      contain an immutable empty string. This buffer was used by the SQL function
      SUBSTRING_INDEX() to return an empty string when one argument was of the wrong
      datatype. If the string was then modified by the sql function INSERT(),
      undefined behavior ensued.
      
      Fixed by allocating a new String buffer when SUBSTRING_INDEX() returns an
      empty string due to errors, and returning NULL should it fail.
      
      Relevant code has also been documented.

    modified:
      mysql-test/r/func_str.result
      mysql-test/t/func_str.test
      sql/item_strfunc.cc
      sql/sql_string.cc
=== modified file 'mysql-test/r/func_str.result'
--- a/mysql-test/r/func_str.result	2010-12-14 16:08:25 +0000
+++ b/mysql-test/r/func_str.result	2010-12-22 12:30:13 +0000
@@ -2612,4 +2612,20 @@ CONVERT(('' IN (REVERSE(CAST(('') AS DEC
 1
 Warnings:
 Warning	1292	Truncated incorrect DECIMAL value: ''
+# 
+# Bug#58165: "my_empty_string" gets modified and causes LOAD DATA to fail
+# and other crashes
+#
+CREATE TABLE t1 ( a TEXT );
+SELECT 'aaaaaaaaaaaaaa' INTO OUTFILE 'bug58165.txt';
+SELECT insert( substring_index( 'a', 'a', 'b' ), 1, 0, 'x' );
+insert( substring_index( 'a', 'a', 'b' ), 1, 0, 'x' )
+x
+Warnings:
+Warning	1292	Truncated incorrect INTEGER value: 'b'
+LOAD DATA INFILE 'bug58165.txt' INTO TABLE t1;
+SELECT * FROM t1;
+a
+aaaaaaaaaaaaaa
+DROP TABLE t1;
 End of 5.1 tests

=== modified file 'mysql-test/t/func_str.test'
--- a/mysql-test/t/func_str.test	2010-12-14 16:08:25 +0000
+++ b/mysql-test/t/func_str.test	2010-12-22 12:30:13 +0000
@@ -1369,4 +1369,15 @@ DROP TABLE t1;
 SELECT '1' IN ('1', SUBSTRING(-9223372036854775809, 1));
 SELECT CONVERT(('' IN (REVERSE(CAST(('') AS DECIMAL)), '')), CHAR(3));
 
+--echo # 
+--echo # Bug#58165: "my_empty_string" gets modified and causes LOAD DATA to fail
+--echo # and other crashes
+--echo #
+CREATE TABLE t1 ( a TEXT );
+SELECT 'aaaaaaaaaaaaaa' INTO OUTFILE 'bug58165.txt';
+SELECT insert( substring_index( 'a', 'a', 'b' ), 1, 0, 'x' );
+LOAD DATA INFILE 'bug58165.txt' INTO TABLE t1;
+SELECT * FROM t1;
+DROP TABLE t1;
+
 --echo End of 5.1 tests

=== modified file 'sql/item_strfunc.cc'
--- a/sql/item_strfunc.cc	2010-11-11 10:25:23 +0000
+++ b/sql/item_strfunc.cc	2010-12-22 12:30:13 +0000
@@ -1305,8 +1305,15 @@ String *Item_func_substr_index::val_str(
   null_value=0;
   uint delimiter_length= delimiter->length();
   if (!res->length() || !delimiter_length || !count)
-    return &my_empty_string;		// Wrong parameters
-
+  {
+    if (str->copy("", 0, default_charset_info))		// Wrong parameters
+    {
+      /* Out of memory */
+      null_value= true;
+      return NULL;
+    }
+    return str;
+  }
   res->set_charset(collation.collation);
 
 #ifdef USE_MB

=== modified file 'sql/sql_string.cc'
--- a/sql/sql_string.cc	2010-07-09 12:00:17 +0000
+++ b/sql/sql_string.cc	2010-12-22 12:30:13 +0000
@@ -58,11 +58,33 @@ bool String::real_alloc(uint32 arg_lengt
 }
 
 
-/*
-** Check that string is big enough. Set string[alloc_length] to 0
-** (for C functions)
-*/
+/**
+   Allocates a new buffer on the heap for this String.
+
+   - If the String's internal buffer is privately owned and heap allocated,
+     one of the following is performed.
+
+     - If the requested length is greater than what fits in the buffer, a new
+       buffer is allocated, data moved and the old buffer freed.
+
+     - If the requested length is less or equal to what fits in the buffer, a
+       null character is inserted at the appropriate position.
 
+   - If the String does not keep a private buffer on the heap, such a buffer
+     will be allocated and the string copied accoring to its length, as found
+     in String::length().
+ 
+   For C compatibility, the new string buffer is null terminated.
+
+   @param alloc_length The requested string size in characters, excluding any
+   null terminator.
+
+   @retval false Either the copy operation is complete or, if the size of the
+   new buffer is smaller than the currently allocated buffer (if one exists),
+   no allocation occured.
+
+   @retval true An error occured when attempting to allocate memory.
+*/
 bool String::realloc(uint32 alloc_length)
 {
   uint32 len=ALIGN_SIZE(alloc_length+1);
@@ -196,6 +218,17 @@ bool String::copy()
   return FALSE;
 }
 
+/**
+   Copies the internal buffer from str. If this String has a private heap
+   allocated buffer where new data does not fit, a new buffer is allocated
+   before copying and the old buffer freed. Character set information is also
+   copied.
+   
+   @param str The string whose internal buffer is to be copied.
+   
+   @retval false Success.
+   @retval true Memory allocation failed.
+*/
 bool String::copy(const String &str)
 {
   if (alloc(str.str_length))


Attachment: [text/bzr-bundle] bzr/[e-mail]