Added some checking that LEX_CSTRING is \0 terminated

- When adding LEX_CSTRING to String, we are now checking that string is \0 terminated (as normally LEX_CSTRING should be usable for printf(). In the cases when one wants to avoid the checking one can use String->append(ptr, length) instead of just String->append(LEX_CSTRING*)
MariaDB · Jan 30, 2018 · 486c86d · 486c86d
1 parent f55dc7f
commit 486c86d
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 19 deletions.
diff --git a/sql/handler.cc b/sql/handler.cc
@@ -7346,8 +7346,8 @@ bool Vers_parse_info::fix_alter_info(THD *thd, Alter_info *alter_info,
       {
         String tmp;
         tmp.append("DROP COLUMN ");
-        tmp.append(done_start ? table->vers_end_field()->field_name
-                              : table->vers_start_field()->field_name);
+        tmp.append(done_start ? &table->vers_end_field()->field_name
+                              : &table->vers_start_field()->field_name);
         my_error(ER_MISSING, MYF(0), table_name, tmp.c_ptr());
         return true;
       }

diff --git a/sql/log_event.cc b/sql/log_event.cc
@@ -322,7 +322,8 @@ class Write_on_release_cache
       bool res;
       if (copy_event_cache_to_string_and_reinit(m_cache, &tmp_str))
         return 1;
-      res= m_ev->output_buf.append(&tmp_str) != 0;
+      /* use 2 argument append as tmp_str is not \0 terminated */
+      res= m_ev->output_buf.append(tmp_str.str, tmp_str.length);
       my_free(tmp_str.str);
       return res ? res : 0;
     }
@@ -11783,16 +11784,16 @@ bool Rows_log_event::print_helper(FILE *file,
       LEX_STRING tmp_str;
       if (copy_event_cache_to_string_and_reinit(head, &tmp_str))
         return 1;
-      output_buf.append(&tmp_str);
+      output_buf.append(tmp_str.str, tmp_str.length);  // Not \0 terminated
       my_free(tmp_str.str);
       if (copy_event_cache_to_string_and_reinit(body, &tmp_str))
         return 1;
-      output_buf.append(&tmp_str);
+      output_buf.append(tmp_str.str, tmp_str.length);
       my_free(tmp_str.str);
 #ifdef WHEN_FLASHBACK_REVIEW_READY
       if (copy_event_cache_to_string_and_reinit(sql, &tmp_str))
         return 1;
-      output_buf.append(&tmp_str);
+      output_buf.append(tmp_str.str, tmp_str.length);
       my_free(tmp_str.str);
 #endif
     }

diff --git a/sql/sp.cc b/sql/sp.cc
@@ -2328,7 +2328,7 @@ Sp_handler::show_create_sp(THD *thd, String *buf,
     buf->append(STRING_WITH_LEN("    DETERMINISTIC\n"));
   append_suid(buf, chistics.suid);
   append_comment(buf, chistics.comment);
-  buf->append(&body);
+  buf->append(body.str, body.length);           // Not \0 terminated
   thd->variables.sql_mode= old_sql_mode;
   return false;
 }

diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc
@@ -7217,7 +7217,7 @@ void append_row_to_str(String &str, const uchar *row, TABLE *table)
   {
     Field *field= *field_ptr;
     str.append(" ");
-    str.append(field->field_name);
+    str.append(&field->field_name);
     str.append(":");
     field_unpack(&str, field, rec, 0, false);
   }

diff --git a/sql/sql_string.h b/sql/sql_string.h
@@ -477,19 +477,18 @@ class String
   bool append(const char *s);
   bool append(const LEX_STRING *ls)
   {
-    DBUG_ASSERT(ls->length < UINT_MAX32);
+    DBUG_ASSERT(ls->length < UINT_MAX32 &&
+                ((ls->length == 0 && !ls->str) ||
+                 ls->length == strlen(ls->str)));
     return append(ls->str, (uint32) ls->length);
   }
   bool append(const LEX_CSTRING *ls)
   {
-    DBUG_ASSERT(ls->length < UINT_MAX32);
+    DBUG_ASSERT(ls->length < UINT_MAX32 &&
+                ((ls->length == 0 && !ls->str) ||
+                 ls->length == strlen(ls->str)));
     return append(ls->str, (uint32) ls->length);
   }
-  bool append(const LEX_CSTRING &ls)
-  {
-    DBUG_ASSERT(ls.length < UINT_MAX32);
-    return append(ls.str, (uint32) ls.length);
-  }
   bool append(const char *s, size_t size);
   bool append(const char *s, uint arg_length, CHARSET_INFO *cs);
   bool append_ulonglong(ulonglong val);
@@ -582,7 +581,9 @@ class String
   }
   void q_append(const LEX_CSTRING *ls)
   {
-    DBUG_ASSERT(ls->length < UINT_MAX32);
+    DBUG_ASSERT(ls->length < UINT_MAX32 &&
+                ((ls->length == 0 && !ls->str) ||
+                 ls->length == strlen(ls->str)));
     q_append(ls->str, (uint32) ls->length);
   }
 
@@ -595,9 +596,12 @@ class String
   {
     qs_append(str, (uint32)strlen(str));
   }
-  void qs_append(const LEX_CSTRING *str)
+  void qs_append(const LEX_CSTRING *ls)
   {
-    qs_append(str->str, str->length);
+    DBUG_ASSERT(ls->length < UINT_MAX32 &&
+                ((ls->length == 0 && !ls->str) ||
+                 ls->length == strlen(ls->str)));
+    qs_append(ls->str, ls->length);
   }
   void qs_append(const char *str, uint32 len);
   void qs_append_hex(const char *str, uint32 len);

diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc
@@ -693,7 +693,13 @@ static void build_trig_stmt_query(THD *thd, TABLE_LIST *tables,
 
   /* Create statement for storing trigger (without trigger order) */
   if (lex->trg_chistics.ordering_clause == TRG_ORDER_NONE)
-    trigger_def->append(&stmt_definition);
+  {
+    /*
+      Not that here stmt_definition doesn't end with a \0, which is
+      normally expected from a LEX_CSTRING
+    */
+    trigger_def->append(stmt_definition.str, stmt_definition.length);
+  }
   else
   {
     /* Copy data before FOLLOWS/PRECEDES trigger_name */