List:Commits« Previous MessageNext Message »
From:Chuck Bell Date:February 26 2009 10:19pm
Subject:bzr commit into mysql-6.0-backup branch (charles.bell:2788) Bug#42685
Bug#42695
View as plain text  
#At file:///C:/source/bzr/mysql-6.0-bug-42695/ based on revid:ingo.struewing@stripped

 2788 Chuck Bell	2009-02-26
      BUG#42695 : memory leak when setting backupdir
      BUG#42685 : valgrind errors setting backup_progress_log_file
      
      This patch adds code to detect when users attempt to set a path 
      longer than FN_REFLEN. 
      
      For BUG#42695, the patch also frees memory used when a successful 
      update to the backupdir variable is completed.
      modified:
        mysql-test/suite/backup/r/backup_backupdir.result
        mysql-test/suite/backup/r/backup_logs.result
        mysql-test/suite/backup/t/backup_backupdir.test
        mysql-test/suite/backup/t/backup_logs.test
        sql/set_var.cc
        sql/share/errmsg.txt

per-file messages:
  mysql-test/suite/backup/r/backup_backupdir.result
    Corrected result file.
  mysql-test/suite/backup/r/backup_logs.result
    Corrected result file.
  mysql-test/suite/backup/t/backup_backupdir.test
    Added test cases to check for exceeding maximum path length.
  mysql-test/suite/backup/t/backup_logs.test
    Added test cases to check for exceeding maximum path length.
  sql/set_var.cc
    Added code to ensure users do not attempt to assign a path longer
    than FN_REFLEN.
    
    Added code to free the memory used on update for backupdir.
    
    Refactored update method for backupdir to make variables easier
    to read and understand their use.
  sql/share/errmsg.txt
    New error message for exceeding maximum path length.
=== modified file 'mysql-test/suite/backup/r/backup_backupdir.result'
--- a/mysql-test/suite/backup/r/backup_backupdir.result	2009-02-24 20:57:21 +0000
+++ b/mysql-test/suite/backup/r/backup_backupdir.result	2009-02-26 22:19:09 +0000
@@ -50,11 +50,32 @@ Try a backup to an invalid hard path.
 *BACKUP DATABASE bup_backupdir TO '$MYSQLTEST_VARDIR/not/there/either/bup_backupdir6.bak';
 ERROR HY000: Can't create/write to file 'MYSQLTEST_VARDIR/not/there/either/bup_backupdir6.bak' (Errcode: #)
 
+Attempt to set the backupdir to a really long string.
+set global max_allowed_packet=1024*100;
+
+Now attempt to set the backupdir to 512 characters.
+
+SET @@global.backupdir = repeat('a',####);
+Warnings:
+Warning	####	The path specified for the system variable backupdir cannot be accessed or is invalid. ref: 
+
+Now attempt to set the backupdir to 513 characters.
+
+SET @@global.backupdir = repeat('a',513);
+ERROR HY000: The path specified for backupdir is too long.
+
+Now attempt to set the backupdir to a really long string.
+
+SET @@global.backupdir = repeat('a',99*1024);
+ERROR HY000: The path specified for backupdir is too long.
+
 Attempt to set the backupdir to something invalid.
+
 SET @@global.backupdir = 'This_is_really_stupid/not/there/at/all';
 Warnings:
-Warning	1733	The path specified for the system variable backupdir cannot be accessed or is invalid. ref: This_is_really_stupid/not/there/at/all
-Warning	1733	The path specified for the system variable backupdir cannot be accessed or is invalid. ref: This_is_really_stupid/not/there/at/all
+Warning	####	The path specified for the system variable backupdir cannot be accessed or is invalid. ref: This_is_really_stupid/not/there/at/all
+Warning	####	The path specified for the system variable backupdir cannot be accessed or is invalid. ref: This_is_really_stupid/not/there/at/all
+set global max_allowed_packet=DEFAULT;
 Cleanup
 Reset backupdir 
 SET @@global.backupdir = @@global.datadir;

=== modified file 'mysql-test/suite/backup/r/backup_logs.result'
--- a/mysql-test/suite/backup/r/backup_logs.result	2009-02-20 16:40:19 +0000
+++ b/mysql-test/suite/backup/r/backup_logs.result	2009-02-26 22:19:09 +0000
@@ -362,6 +362,41 @@ The backup id for this command should be
 BACKUP DATABASE backup_logs to 'backup_logs_orig.bak';
 backup_id
 505
+
+Attempt to set the backup log paths to a really long string.
+
+SET global max_allowed_packet=1024*100;
+
+Now attempt to set the backup_progress_log_file to 512 characters.
+
+SET @@global.backup_progress_log_file = repeat('a',512);
+
+Now attempt to set the backup_progress_log_file to 513 characters.
+
+SET @@global.backup_progress_log_file = repeat('a',513);
+ERROR HY000: The path specified for backup_progress_log_file is too long.
+
+Now attempt to set the backup_progress_log_file to a really long string.
+
+SET @@global.backup_progress_log_file = repeat('a',99*1024);
+ERROR HY000: The path specified for backup_progress_log_file is too long.
+SET @@global.backup_progress_log_file = DEFAULT;
+
+Now attempt to set the backup_history_log_file to 512 characters.
+
+SET @@global.backup_history_log_file = repeat('a',512);
+
+Now attempt to set the backup_history_log_file to 513 characters.
+
+SET @@global.backup_history_log_file = repeat('a',513);
+ERROR HY000: The path specified for backup_history_log_file is too long.
+
+Now attempt to set the backup_history_log_file to a really long string.
+
+SET @@global.backup_history_log_file = repeat('a',99*1024);
+ERROR HY000: The path specified for backup_history_log_file is too long.
+SET @@global.backup_history_log_file = DEFAULT;
+SET global max_allowed_packet=DEFAULT;
 SET @@global.log_backup_output = 'TABLE';
 DROP USER 'tom'@'localhost';
 SET DEBUG_SYNC= 'reset';

=== modified file 'mysql-test/suite/backup/t/backup_backupdir.test'
--- a/mysql-test/suite/backup/t/backup_backupdir.test	2009-02-24 20:57:21 +0000
+++ b/mysql-test/suite/backup/t/backup_backupdir.test	2009-02-26 22:19:09 +0000
@@ -103,9 +103,38 @@ BACKUP DATABASE bup_backupdir TO '../not
 --enable_query_log
 
 --echo
+--echo Attempt to set the backupdir to a really long string.
+
+set global max_allowed_packet=1024*100;
+
+--echo
+--echo Now attempt to set the backupdir to 512 characters.
+--echo
+# Mask out the error value on the warning.
+--replace_regex /[0000-9999]+/####/
+SET @@global.backupdir = repeat('a',512);
+
+--echo
+--echo Now attempt to set the backupdir to 513 characters.
+--echo
+--error ER_PATH_LENGTH
+SET @@global.backupdir = repeat('a',513);
+
+--echo
+--echo Now attempt to set the backupdir to a really long string.
+--echo
+--error ER_PATH_LENGTH
+SET @@global.backupdir = repeat('a',99*1024);
+
+--echo
 --echo Attempt to set the backupdir to something invalid.
+--echo
+# Mask out the error value on the warning.
+--replace_regex /[0000-9999]+/####/
 SET @@global.backupdir = 'This_is_really_stupid/not/there/at/all';
 
+set global max_allowed_packet=DEFAULT;
+
 --echo Cleanup
 
 --echo Reset backupdir 

=== modified file 'mysql-test/suite/backup/t/backup_logs.test'
--- a/mysql-test/suite/backup/t/backup_logs.test	2009-02-24 20:57:21 +0000
+++ b/mysql-test/suite/backup/t/backup_logs.test	2009-02-26 22:19:09 +0000
@@ -467,6 +467,50 @@ BACKUP DATABASE backup_logs to 'backup_l
 --echo The backup id for this command should be 505.
 BACKUP DATABASE backup_logs to 'backup_logs_orig.bak';
 
+--echo
+--echo Attempt to set the backup log paths to a really long string.
+--echo
+
+SET global max_allowed_packet=1024*100;
+
+--echo
+--echo Now attempt to set the backup_progress_log_file to 512 characters.
+--echo
+SET @@global.backup_progress_log_file = repeat('a',512);
+
+--echo
+--echo Now attempt to set the backup_progress_log_file to 513 characters.
+--echo
+--error ER_PATH_LENGTH
+SET @@global.backup_progress_log_file = repeat('a',513);
+
+--echo
+--echo Now attempt to set the backup_progress_log_file to a really long string.
+--echo
+--error ER_PATH_LENGTH
+SET @@global.backup_progress_log_file = repeat('a',99*1024);
+SET @@global.backup_progress_log_file = DEFAULT;
+
+--echo
+--echo Now attempt to set the backup_history_log_file to 512 characters.
+--echo
+SET @@global.backup_history_log_file = repeat('a',512);
+
+--echo
+--echo Now attempt to set the backup_history_log_file to 513 characters.
+--echo
+--error ER_PATH_LENGTH
+SET @@global.backup_history_log_file = repeat('a',513);
+
+--echo
+--echo Now attempt to set the backup_history_log_file to a really long string.
+--echo
+--error ER_PATH_LENGTH
+SET @@global.backup_history_log_file = repeat('a',99*1024);
+SET @@global.backup_history_log_file = DEFAULT;
+
+SET global max_allowed_packet=DEFAULT;
+
 #
 # Cleanup.
 #

=== modified file 'sql/set_var.cc'
--- a/sql/set_var.cc	2009-02-20 16:40:19 +0000
+++ b/sql/set_var.cc	2009-02-26 22:19:09 +0000
@@ -2528,6 +2528,16 @@ static int  sys_check_log_path(THD *thd,
   if (!(res= var->value->val_str(&str)))
     goto err;
 
+  /*
+    Check maximum string length and error if too long.
+    Do not set the value.
+  */
+  if (res->length() > FN_REFLEN)
+  {
+    my_error(ER_PATH_LENGTH, MYF(0), var->var->name);
+    return 1;
+  }
+
   log_file_str= res->c_ptr();
   bzero(&f_stat, sizeof(MY_STAT));
 
@@ -2982,7 +2992,7 @@ err:
 static bool sys_update_backupdir(THD *thd, set_var * var)
 {
   char buff[FN_REFLEN];
-  char *res= 0, *old_value= NULL;
+  char *new_value= 0, *copied_value= NULL;
   bool result= 0;
   uint str_length;
   String str(buff, sizeof(buff), system_charset_info);
@@ -2993,16 +3003,26 @@ static bool sys_update_backupdir(THD *th
 
     if (!(strres= var->value->val_str(&str)))
       goto err;
-    old_value= strres->c_ptr();
+    copied_value= strres->c_ptr();
     str_length= strres->length();
   }
   else
   {
-    old_value= make_default_backupdir(buff);
-    str_length= strlen(old_value);
+    copied_value= make_default_backupdir(buff);
+    str_length= strlen(copied_value);
   }
 
-  if (!(res= my_strndup(old_value, str_length, MYF(MY_FAE+MY_WME))))
+  /*
+    Check maximum string length and error if too long.
+    Do not set the value.
+  */
+  if (str_length > FN_REFLEN)
+  {
+    my_error(ER_PATH_LENGTH, MYF(0), var->var->name);
+    return 1;
+  }
+
+  if (!(new_value= my_strndup(copied_value, str_length, MYF(MY_FAE+MY_WME))))
   {
     result= 1;
     goto err;
@@ -3010,8 +3030,8 @@ static bool sys_update_backupdir(THD *th
 
   pthread_mutex_lock(&LOCK_global_system_variables);
   logger.lock_exclusive();
-  old_value= sys_var_backupdir.value;
-  sys_var_backupdir.value= res;
+  my_free(sys_var_backupdir.value, MYF(MY_ALLOW_ZERO_PTR));
+  sys_var_backupdir.value= new_value;
   sys_var_backupdir.value_length= str_length;
   logger.unlock();
   pthread_mutex_unlock(&LOCK_global_system_variables);

=== modified file 'sql/share/errmsg.txt'
--- a/sql/share/errmsg.txt	2009-02-12 17:56:03 +0000
+++ b/sql/share/errmsg.txt	2009-02-26 22:19:09 +0000
@@ -6463,4 +6463,5 @@ ER_OPERATION_ABORTED
   eng "Operation aborted"
 ER_OPERATION_ABORTED_CORRUPTED
   eng "Operation aborted - data might be corrupted"
-
+ER_PATH_LENGTH
+  eng "The path specified for %.64s is too long."

Thread
bzr commit into mysql-6.0-backup branch (charles.bell:2788) Bug#42685Bug#42695Chuck Bell26 Feb