Merge of fix for Bug#58165.

6 files changed, 107 insertions, 26 deletions

diff --git a/mysql-test/r/func_str.result b/mysql-test/r/func_str.result
index 41fe9c096b0..81fe2413725 100644
--- a/mysql-test/r/func_str.result
+++ b/mysql-test/r/func_str.result
@@ -2615,6 +2615,22 @@ CONVERT(('' IN (REVERSE(CAST(('') AS DECIMAL)), '')), CHAR(3))
 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
 Start of 5.4 tests
 SELECT format(12345678901234567890.123, 3);
diff --git a/mysql-test/t/func_str.test b/mysql-test/t/func_str.test
index aff99d7e303..9a9a8110a74 100644
--- a/mysql-test/t/func_str.test
+++ b/mysql-test/t/func_str.test
@@ -1370,6 +1370,17 @@ 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
 
 --echo Start of 5.4 tests
diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc
index 2a93bcd0153..65eb2bb527b 100644
--- a/sql/item_strfunc.cc
+++ b/sql/item_strfunc.cc
@@ -56,6 +56,9 @@ C_MODE_START
 #include "../mysys/my_static.h"			// For soundex_map
 C_MODE_END
 
+/**
+   @todo Remove this. It is not safe to use a shared String object.
+ */
 String my_empty_string("",default_charset_info);
 
 /*
@@ -642,7 +645,7 @@ String *Item_func_des_encrypt::val_str(String *str)
   if ((null_value= args[0]->null_value))
     return 0;                                   // ENCRYPT(NULL) == NULL
   if ((res_length=res->length()) == 0)
-    return &my_empty_string;
+    return make_empty_result();
   if (arg_count == 1)
   {
     /* Protect against someone doing FLUSH DES_KEY_FILE */
@@ -832,7 +835,7 @@ String *Item_func_concat_ws::val_str(String *str)
     }
 
   if (i ==  arg_count)
-    return &my_empty_string;
+    return make_empty_result();
 
   for (i++; i < arg_count ; i++)
   {
@@ -978,7 +981,7 @@ String *Item_func_reverse::val_str(String *str)
     return 0;
   /* An empty string is a special case as the string pointer may be null */
   if (!res->length())
-    return &my_empty_string;
+    return make_empty_result();
   if (tmp_value.alloced_length() < res->length() &&
       tmp_value.realloc(res->length()))
   {
@@ -1311,8 +1314,7 @@ String *Item_func_left::val_str(String *str)
 
   /* if "unsigned_flag" is set, we have a *huge* positive number. */
   if ((length <= 0) && (!args[1]->unsigned_flag))
-    return &my_empty_string;
-
+    return make_empty_result();
   if ((res->length() <= (ulonglong) length) ||
       (res->length() <= (char_pos= res->charpos((int) length))))
     return res;
@@ -1357,7 +1359,7 @@ String *Item_func_right::val_str(String *str)
 
   /* if "unsigned_flag" is set, we have a *huge* positive number. */
   if ((length <= 0) && (!args[1]->unsigned_flag))
-    return &my_empty_string; /* purecov: inspected */
+    return make_empty_result(); /* purecov: inspected */
 
   if (res->length() <= (ulonglong) length)
     return res; /* purecov: inspected */
@@ -1397,7 +1399,7 @@ String *Item_func_substr::val_str(String *str)
   /* Negative or zero length, will return empty string. */
   if ((arg_count == 3) && (length <= 0) && 
       (length == 0 || !args[2]->unsigned_flag))
-    return &my_empty_string;
+    return make_empty_result();
 
   /* Assumes that the maximum length of a String is < INT_MAX32. */
   /* Set here so that rest of code sees out-of-bound value as such. */
@@ -1408,12 +1410,12 @@ String *Item_func_substr::val_str(String *str)
   /* Assumes that the maximum length of a String is < INT_MAX32. */
   if ((!args[1]->unsigned_flag && (start < INT_MIN32 || start > INT_MAX32)) ||
       (args[1]->unsigned_flag && ((ulonglong) start > INT_MAX32)))
-    return &my_empty_string;
+    return make_empty_result();
 
   start= ((start < 0) ? res->numchars() + start : start - 1);
   start= res->charpos((int) start);
   if ((start < 0) || ((uint) start + 1 > res->length()))
-    return &my_empty_string;
+    return make_empty_result();
 
   length= res->charpos((int) length, (uint32) start);
   tmp_length= res->length() - start;
@@ -1476,7 +1478,7 @@ String *Item_func_substr_index::val_str(String *str)
   null_value=0;
   uint delimiter_length= delimiter->length();
   if (!res->length() || !delimiter_length || !count)
-    return &my_empty_string;		// Wrong parameters
+    return make_empty_result();		// Wrong parameters
 
   res->set_charset(collation.collation);
 
@@ -1826,7 +1828,7 @@ String *Item_func_password::val_str_ascii(String *str)
   if ((null_value=args[0]->null_value))
     return 0;
   if (res->length() == 0)
-    return &my_empty_string;
+    return make_empty_result();
   my_make_scrambled_password(tmp_value, res->ptr(), res->length());
   str->set(tmp_value, SCRAMBLED_PASSWORD_CHAR_LENGTH, &my_charset_latin1);
   return str;
@@ -1850,7 +1852,7 @@ String *Item_func_old_password::val_str_ascii(String *str)
   if ((null_value=args[0]->null_value))
     return 0;
   if (res->length() == 0)
-    return &my_empty_string;
+    return make_empty_result();
   my_make_scrambled_password_323(tmp_value, res->ptr(), res->length());
   str->set(tmp_value, SCRAMBLED_PASSWORD_CHAR_LENGTH_323, &my_charset_latin1);
   return str;
@@ -1878,8 +1880,7 @@ String *Item_func_encrypt::val_str(String *str)
   if ((null_value=args[0]->null_value))
     return 0;
   if (res->length() == 0)
-    return &my_empty_string;
-
+    return make_empty_result();
   if (arg_count == 1)
   {					// generate random salt
     time_t timestamp=current_thd->query_start();
@@ -2141,7 +2142,7 @@ String *Item_func_soundex::val_str(String *str)
   for ( ; ; ) /* Skip pre-space */
   {
     if ((rc= cs->cset->mb_wc(cs, &wc, (uchar*) from, (uchar*) end)) <= 0)
-      return &my_empty_string; /* EOL or invalid byte sequence */
+      return make_empty_result(); /* EOL or invalid byte sequence */
     
     if (rc == 1 && cs->ctype)
     {
@@ -2166,7 +2167,7 @@ String *Item_func_soundex::val_str(String *str)
         {
           /* Extra safety - should not really happen */
           DBUG_ASSERT(false);
-          return &my_empty_string;
+          return make_empty_result();
         }
         to+= rc;
         break;
@@ -2507,7 +2508,7 @@ String *Item_func_make_set::val_str(String *str)
 	  else
 	  {
 	    if (tmp_str.copy(*res))		// Don't use 'str'
-	      return &my_empty_string;
+              return make_empty_result();
 	    result= &tmp_str;
 	  }
 	}
@@ -2517,11 +2518,11 @@ String *Item_func_make_set::val_str(String *str)
 	  {					// Copy data to tmp_str
 	    if (tmp_str.alloc(result->length()+res->length()+1) ||
 		tmp_str.copy(*result))
-	      return &my_empty_string;
+              return make_empty_result();
 	    result= &tmp_str;
 	  }
 	  if (tmp_str.append(STRING_WITH_LEN(","), &my_charset_bin) || tmp_str.append(*res))
-	    return &my_empty_string;
+            return make_empty_result();
 	}
       }
     }
@@ -2666,7 +2667,7 @@ String *Item_func_repeat::val_str(String *str)
   null_value= 0;
 
   if (count <= 0 && (count == 0 || !args[1]->unsigned_flag))
-    return &my_empty_string;
+    return make_empty_result();
 
   /* Assumes that the maximum length of a String is < INT_MAX32. */
   /* Bounds check on count:  If this is triggered, we will error. */
@@ -2948,7 +2949,7 @@ String *Item_func_conv::val_str(String *str)
 
   ptr= longlong2str(dec, ans, to_base);
   if (str->copy(ans, (uint32) (ptr-ans), default_charset()))
-    return &my_empty_string;
+    return make_empty_result();
   return str;
 }
 
@@ -3115,7 +3116,7 @@ String *Item_func_hex::val_str_ascii(String *str)
       return 0;
     ptr= longlong2str(dec,ans,16);
     if (str->copy(ans,(uint32) (ptr-ans), &my_charset_numeric))
-      return &my_empty_string;			// End of memory
+      return make_empty_result();		// End of memory
     return str;
   }
 
diff --git a/sql/item_strfunc.h b/sql/item_strfunc.h
index d76e139883c..626926213e6 100644
--- a/sql/item_strfunc.h
+++ b/sql/item_strfunc.h
@@ -27,6 +27,16 @@ class MY_LOCALE;
 
 class Item_str_func :public Item_func
 {
+protected:
+  /**
+     Sets the result value of the function an empty string, using the current
+     character set. No memory is allocated.
+     @retval A pointer to the str_value member.
+   */
+  String *make_empty_result() {
+    str_value.set("", 0, collation.collation);
+    return &str_value; 
+  }
 public:
   Item_str_func() :Item_func() { decimals=NOT_FIXED_DEC; }
   Item_str_func(Item *a) :Item_func(a) {decimals=NOT_FIXED_DEC; }
diff --git a/sql/sql_string.cc b/sql/sql_string.cc
index 4b7dab243d2..d19b876a82f 100644
--- a/sql/sql_string.cc
+++ b/sql/sql_string.cc
@@ -51,11 +51,33 @@ bool String::real_alloc(uint32 length)
 }
 
 
-/*
-** 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);
@@ -128,6 +150,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))
diff --git a/sql/sql_string.h b/sql/sql_string.h
index 3642a96de35..dcd9975f399 100644
--- a/sql/sql_string.h
+++ b/sql/sql_string.h
@@ -148,6 +148,16 @@ public:
       Alloced_length=0;
     str_charset=str.str_charset;
   }
+
+
+  /**
+     Points the internal buffer to the supplied one. The old buffer is freed.
+     @param str Pointer to the new buffer.
+     @param arg_length Length of the new buffer in characters, excluding any 
+            null character.
+     @param cs Character set to use for interpreting string data.
+     @note The new buffer will not be null terminated.
+  */
   inline void set(char *str,uint32 arg_length, CHARSET_INFO *cs)
   {
     free();