List:Commits« Previous MessageNext Message »
From:Ingo Struewing Date:January 6 2010 5:55pm
Subject:bzr commit into mysql-backup-backport branch (ingo.struewing:3035)
Bug#43747 Bug#43767 WL#5101
View as plain text  
#At file:///home2/mydev/bzrroot/mysql-5.6-backup-backport-ms09-2/ based on revid:ingo.struewing@stripped

 3035 Ingo Struewing	2010-01-06
      WL#5101 - MySQL Backup back port
      Merged revid:ingo.struewing@stripped
      Merged revid:ingo.struewing@stripped
        Bug#43747 - BACKUP with backref path succeeds on invalid backupdir
        Bug#43767 - Wrong path if backupdir is symlink and backref path used
        
        A backupdir variable value like 'existent_file.txt' and a backup image
        file name like '../backup_file.bak' allowed for a successful BACKUP and
        RESTORE (Bug #43747). It had been decided that we do not want to allow
        BACKUP or RESTORE to succeed, if backupdir is set to an invalid path
        (compare Bug 43536 - BACKUP with absolute path fails on invalid
        backupdir).
        
        A backupdir variable value like 'symlink_to_dir1_dir2' and a backup image
        file name like '../backup_file.bak' located the file in the wrong
        directory. If "symlink_to_dir1_dir2 -> dir1/dir2", the used file was
        just "backup_file.bak". Cleanup of "../" was done based on the symlink
        itself, not based on its target (dir2). Normal file system operations
        interpret a path of "symlink_to_dir1_dir2/../backup_file.bak" as
        "dir1/backup_file.bak".
        
        The main problem was that MySQL did not have a path handling function,
        which cleans a path while respecting symbolic links, works even if the
        path name references a non-existent file system object, and works on
        Windows too. For more details see the explanation in the bug report
        #43747.
        
        Another problem was that the backupdir variable was checked for an
        existent and writable file system object only. It was not required that
        this object needs to be a directory.
        
        The main part of the patch is the new function safe_cleanup_cat_path()
        in mf_pack.c.
        
        Another important new function is test_if_directory() in my_lib.c. It
        allows to verify that backupdir is not just a writable path, but also a
        directory.
        
        Yet another new function is cut_print_path() in mf_format.c. It supports
        the creation of good error messages. Many error message templates have
        limited space for path names, e.g. %.64s. Often the last part of a path
        name is more informative for error messages than its begin. This
        function returns a cut path with a prepended "... ", if the path is
        longer than the length limit.
        
        Yet another new function is is_usable_directory() in set_var.cc. It
        supports checking of variable values when they are set.
        
        To avoid frequent use of strlen() I added length variables to the
        seldom or never changed variables home_dir, mysql_real_data_home, and
        opt_secure_backup_file_priv.
        
        Finally I used these functions in the backup kernel to build an
        absolute, clean path for the backupdir to check it for being a writable
        directory and to prepend it to a relative backup image file path.
        
        Additionally I used the functions on opt_secure_backup_file_priv before
        comparing it against the resulting backup image file path.
        
        To have a good test coverage for the new non-trivial function
        safe_cleanup_cat_path(), I made a unittest for it.
      ---
        Bug#43767 - Wrong path if backupdir is symlink and backref path used
        
        Addendum fix to Bug#43747 (BACKUP with backref path succeeds
        on invalid backupdir) and Bug#43767.
        
        The former fix checked every path element for being a symlink.
        Only then it was prevented to let ../ remove this path element.
        
        But the correct solution is to prevent ../ from removing all
        path elements except of real (non-symlinked) directories.
        
        A path like dir/non-existent-file.txt/../existent-file.txt must not
        be cleaned up to dir/existent-file.txt. Normal (operating system)
        file operations don't do that either.
        
        ".." is a valid path element with its back-referencing semantics
        in an existent directory only.
        
        However, on Windows, we need to do the clean-ups irrespectively
        of the file system object. Windows system services do the clean-up
        even after path elements that refer to non-existent objects.
        In the above example, dir/existent-file.txt is used.
        
        safe_cleanup_cat_path(), its unittest, and the regression tests
        do now respect the platform difference.
      
      original changeset: 2599.146.1
      original changeset: 2599.148.1
     @ include/my_sys.h
        WL#5101 - MySQL Backup back port
        Removed a MY_FLAG value clash.
     @ libmysql/CMakeLists.txt
        WL#5101 - MySQL Backup back port
        Added mysys as include directory. Required for my_rnd.c.
     @ unittest/mysys/CMakeLists.txt
        WL#5101 - MySQL Backup back port
            Bug#43747 - BACKUP with backref path succeeds on invalid backupdir
            Bug#43767 - Wrong path if backupdir is symlink and backref path used
            Added safe_cleanup_cat_path-t.
     @ unittest/mysys/Makefile.am
        WL#5101 - MySQL Backup back port
            Bug#43747 - BACKUP with backref path succeeds on invalid backupdir
            Bug#43767 - Wrong path if backupdir is symlink and backref path used
            Added safe_cleanup_cat_path-t.
     @ unittest/mysys/safe_cleanup_cat_path-t.c
        WL#5101 - MySQL Backup back port
            Bug#43747 - BACKUP with backref path succeeds on invalid backupdir
            Bug#43767 - Wrong path if backupdir is symlink and backref path used
            New unit test for safe_cleanup_cat_path().

    added:
      unittest/mysys/safe_cleanup_cat_path-t.c
    modified:
      include/my_sys.h
      libmysql/CMakeLists.txt
      unittest/mysys/CMakeLists.txt
      unittest/mysys/Makefile.am
=== modified file 'include/my_sys.h'
--- a/include/my_sys.h	2009-12-30 17:12:16 +0000
+++ b/include/my_sys.h	2010-01-06 17:55:08 +0000
@@ -84,12 +84,12 @@ extern int NEAR my_errno;		/* Last error
 #define MY_DONT_OVERWRITE_FILE 1024	/* my_copy: Don't overwrite file */
 #define MY_THREADSAFE 2048      /* my_seek(): lock fd mutex */
 #define MY_SYNC       4096      /* my_copy(): sync dst file */
+#define MY_RETURN_SYMLINK 8192  /* my_stat(); Use lstat(2), don't follow link */
 
 #define MY_CHECK_ERROR	1	/* Params to my_end; Check open-close */
 #define MY_GIVE_INFO	2	/* Give time info about process*/
 #define MY_DONT_FREE_DBUG 4     /* Do not call DBUG_END() in my_end() */
 
-#define MY_RETURN_SYMLINK 4096  /* my_stat(); Use lstat(2), don't follow link */
 #define MY_REMOVE_NONE    0     /* Params for modify_defaults_file */
 #define MY_REMOVE_OPTION  1
 #define MY_REMOVE_SECTION 2

=== modified file 'libmysql/CMakeLists.txt'
--- a/libmysql/CMakeLists.txt	2010-01-05 18:06:34 +0000
+++ b/libmysql/CMakeLists.txt	2010-01-06 17:55:08 +0000
@@ -25,7 +25,8 @@ INCLUDE_DIRECTORIES(${CMAKE_SOURCE_DIR}/
                     ${CMAKE_SOURCE_DIR}/libmysql
                     ${CMAKE_SOURCE_DIR}/regex
                     ${CMAKE_SOURCE_DIR}/sql
-                    ${CMAKE_SOURCE_DIR}/strings)
+                    ${CMAKE_SOURCE_DIR}/strings
+                    ${CMAKE_SOURCE_DIR}/mysys)
 
 # We include the source file listing instead of referencing the
 # libraries. At least with CMake 2.4 and Visual Studio 2005 a static

=== modified file 'unittest/mysys/CMakeLists.txt'
--- a/unittest/mysys/CMakeLists.txt	2009-11-24 23:36:31 +0000
+++ b/unittest/mysys/CMakeLists.txt	2010-01-06 17:55:08 +0000
@@ -30,6 +30,9 @@ TARGET_LINK_LIBRARIES(my_atomic-t mytap 
 ADD_EXECUTABLE(lf-t lf-t.c)
 TARGET_LINK_LIBRARIES(lf-t mytap mysys dbug strings)
 
+ADD_EXECUTABLE(safe_cleanup_cat_path-t safe_cleanup_cat_path-t.c)
+TARGET_LINK_LIBRARIES(safe_cleanup_cat_path-t mytap mysys dbug strings)
+
 ADD_EXECUTABLE(my_rdtsc-t my_rdtsc-t.c)
 TARGET_LINK_LIBRARIES(my_rdtsc-t mytap mysys dbug strings)
 

=== modified file 'unittest/mysys/Makefile.am'
--- a/unittest/mysys/Makefile.am	2009-11-24 23:36:31 +0000
+++ b/unittest/mysys/Makefile.am	2010-01-06 17:55:08 +0000
@@ -23,7 +23,8 @@ LDADD 		= $(top_builddir)/unittest/mytap
 		  $(top_builddir)/dbug/libdbug.a \
 		  $(top_builddir)/strings/libmystrings.a
 
-noinst_PROGRAMS  = bitmap-t base64-t lf-t my_rdtsc-t my_vsnprintf-t
+noinst_PROGRAMS  = bitmap-t base64-t lf-t my_rdtsc-t my_vsnprintf-t \
+		   safe_cleanup_cat_path-t
 
 if NEED_THREAD
 # my_atomic-t is used to check thread functions, so it is safe to 

=== added file 'unittest/mysys/safe_cleanup_cat_path-t.c'
--- a/unittest/mysys/safe_cleanup_cat_path-t.c	1970-01-01 00:00:00 +0000
+++ b/unittest/mysys/safe_cleanup_cat_path-t.c	2010-01-06 17:55:08 +0000
@@ -0,0 +1,295 @@
+/* Copyright (C) 2009 Sun Microsystems, Inc.
+
+   This program is free software; you can redistribute it and/or
+   modify it under the terms of the GNU General Public License as
+   published by the Free Software Foundation; version 2 of the License.
+
+   This program is distributed in the hope that it will be useful, but
+   WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software
+   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA */
+
+#include <my_global.h>
+#include <my_sys.h>
+#include <tap.h>
+#include <string.h>
+#if defined(__WIN__)
+#include <direct.h>
+#define chdir _chdir
+#define rmdir _rmdir
+#endif
+
+#define DST_SIZE 80
+
+static char   *concat_buf= NULL;
+static size_t concat_size= 0;
+
+
+/**
+  Concatenate two strings.
+
+  @param[in]    str1            string 1
+  @param[in]    str2            string 2
+
+  @return       string in allocated storage
+*/
+
+static char*
+concat_str(const char *str1, const char *str2)
+{
+  size_t len1;
+  size_t len2;
+  size_t sum;
+
+  len1= strlen(str1);
+  len2= strlen(str2);
+  sum= len1 + len2 + 1; /* Include terminating '\0'. */
+
+  if (sum > concat_size)
+  {
+    concat_buf= my_realloc(concat_buf, sum, MYF(MY_FAE | MY_ALLOW_ZERO_PTR));
+    if (!concat_buf)
+      exit(3); /* purecov: inspected */
+    concat_size= sum;
+  }
+  memcpy(concat_buf, str1, len1);
+  memcpy(concat_buf + len1, str2, len2 + 1); /* Include terminating '\0'. */
+  return concat_buf;
+}
+
+
+/**
+  Do a test for a path name.
+
+  @param[in]    path            input path name
+  @param[out]   expect          expected output path name
+*/
+
+static void
+check_result(const char *path1, const char *path2,
+             const char *path3, const char *result, const char *expect)
+{
+  const char    *exp_src;
+  char          *exp_dst;
+  char          *exp_end;
+  char          exp[DST_SIZE];
+
+  /* Convert FN_LIBCHAR to slash for comparable results. */
+  for (exp_src= expect, exp_dst= exp, exp_end= exp + DST_SIZE - 1;
+       *exp_src && (exp_dst < exp_end);
+       exp_src++, exp_dst++)
+  {
+    if (*exp_src == '/')
+      *exp_dst= FN_LIBCHAR;
+    else
+      *exp_dst= *exp_src;
+  }
+  *exp_dst= '\0';
+
+  /* Check and print result. */
+  ok(!strcmp(result, exp),
+     "input: '%s' '%s' '%s'  result: '%s'  expected: '%s'",
+     path1 ? path1 : "[NullS]",
+     path2 ? path2 : "[NullS]",
+     path3 ? path3 : "[NullS]",
+     result, exp);
+}
+
+
+/**
+  Do a test for a path name.
+
+  @param[in]    path1           input path name
+  @param[in]    path2           input path name, optional NullS
+  @param[in]    path3           input path name, optional NullS
+  @param[out]   expect          expected output path name
+*/
+
+static void
+do_multi_test(const char *path1, const char *path2,
+              const char *path3, const char *expect)
+{
+  size_t dst_len;
+  char   dst[DST_SIZE];
+
+  /* Cleanup the path. That's the core test. */
+  dst_len= safe_cleanup_cat_path(dst, DST_SIZE, path1, path2, path3, NullS);
+
+  /* If path too long, dst_len == 0 but dst has partial result. Clear it. */
+  if (!dst_len)
+    dst[0]= '\0';
+
+  /* Check and print result. */
+  check_result(path1, path2, path3, dst, expect);
+}
+
+
+#define do_test(path, expect)   do_multi_test(path, NullS, NullS, expect)
+
+
+int
+main(void)
+{
+  MY_INIT("safe_cleanup_cat_path-t");
+
+  /* Set HOME for ~/ test. Trailing slash is for coverage test. */
+  putenv((char*) "HOME=/home/unittest/");
+
+  /* Create a directory with unique name and change to it. */
+  if (my_mkdir("safe_cleanup_cat_path_dir", 0777, MYF(MY_WME)))
+    goto end; /* purecov: inspected */
+  if (my_setwd("safe_cleanup_cat_path_dir", MYF(MY_WME)))
+    goto end1; /* purecov: inspected */
+
+  /* Create some test directories. */
+  if (my_mkdir("dir1", 0777, MYF(MY_WME)) ||
+      my_mkdir("dir1/dir2", 0777, MYF(MY_WME)) ||
+      my_mkdir("dir1/dir2/dir3", 0777, MYF(MY_WME)))
+    goto end2;
+
+  /* Use "file" for a non-existent file. */
+
+  plan(65);
+
+  do_test("", "");
+
+  do_test("file", "file");
+  do_test("file.file", "file.file");
+  do_test("file.file.file.file.file.file.file.file.file.file.file",
+          "file.file.file.file.file.file.file.file.file.file.file");
+
+  do_test("/", "/");
+  do_test("///", "/");
+  do_test("/file", "/file");
+#ifdef FN_NETWORK_DRIVES
+  do_test("//file///", "//file");
+  do_test("///file///", "//file");
+#else
+  do_test("//file///", "/file");
+  do_test("///file///", "/file");
+#endif
+
+  do_test("dir1/file", "dir1/file");
+  do_test("dir1///file", "dir1/file");
+  do_test("dir1/dir2/", "dir1/dir2");
+  do_test("dir1/dir2///", "dir1/dir2");
+
+  do_test("file/file", "file/file");
+  do_test("file///file", "file/file");
+  do_test("file/file/", "file/file");
+  do_test("file/file///", "file/file");
+
+  do_test(".", ".");
+  do_test("./", ".");
+  do_test("./.", ".");
+  do_test("././", ".");
+  do_test("./dir1", "dir1");
+  do_test("./file", "file");
+  do_test("././dir1", "dir1");
+  do_test("././file", "file");
+
+  do_test("dir1/./file", "dir1/file");
+  do_test("dir1/././file", "dir1/file");
+  do_test("dir1/.", "dir1");
+  do_test("dir1/./.", "dir1");
+
+#if defined(__WIN__)
+  /* Windows does path cleanup differently. */
+  do_test("file/./file", "file/file");
+  do_test("file/././file", "file/file");
+  do_test("file/.", "file");
+  do_test("file/./.", "file");
+#else
+  do_test("file/./file", "file/./file");
+  do_test("file/././file", "file/././file");
+  do_test("file/.", "file/.");
+  do_test("file/./.", "file/./.");
+#endif
+
+  do_test("..", "..");
+  do_test("../..", "../..");
+  do_test("../../", "../..");
+  do_test("../../file", "../../file");
+  do_test("/../..", "/");
+  do_test("/../file", "/file");
+  do_test("/../../file", "/file");
+  do_test("/../../", "/");
+
+  do_test("dir1/../file", "file");
+  do_test("dir1/dir2/dir3/../../file", "dir1/file");
+  do_test("dir1/dir2/..", "dir1");
+  do_test("dir1/..", ".");
+
+#if defined(__WIN__)
+  /* Windows does path cleanup differently. */
+  do_test("file/../file", "file");
+  do_test("dir1/dir2/file/../../file", "dir1/file");
+  do_test("file/file/..", "file");
+  do_test("file/..", ".");
+  do_test("dir1/../dir2/non-existent-file.txt/../existent-file.txt",
+          "dir2/existent-file.txt");
+  do_test("dir1/../dir2/non-existent-file.txt/./.././existent-file.txt",
+          "dir2/existent-file.txt");
+#else
+  do_test("file/../file", "file/../file");
+  do_test("dir1/dir2/file/../../file", "dir1/dir2/file/../../file");
+  do_test("file/file/..", "file/file/..");
+  do_test("file/..", "file/..");
+  do_test("dir1/../dir2/non-existent-file.txt/../existent-file.txt",
+          "dir2/non-existent-file.txt/../existent-file.txt");
+  do_test("dir1/../dir2/non-existent-file.txt/./.././existent-file.txt",
+          "dir2/non-existent-file.txt/./.././existent-file.txt");
+#endif
+
+  do_test("~/file", concat_str(home_dir, "/file"));
+  do_test("dir1/~/file", "dir1/~/file");
+  /* Exceed DST_SIZE. Result should be empty string. */
+  do_test("file.file.file.file.file.file.file.file.file.file.file"
+          "file.file.file.file.file.file.file.file.file.file.file"
+          "file.file.file.file.file.file.file.file.file.file.file",
+          "");
+
+#ifdef HAVE_READLINK
+  {
+    int rc;
+    (void) my_delete("symlink1", MYF(0));
+    rc= my_symlink("dir1/dir2", "symlink1", MYF(MY_WME));
+    do_test("symlink1/../../dir1/././dir2/../../file",
+            "symlink1/../../file");
+    (void) my_delete("symlink1", MYF(0));
+  }
+#else
+  {
+    do_test("dummy_test_to_please_the_test_counter",
+            "dummy_test_to_please_the_test_counter");
+  }
+#endif
+  do_test("dir1/dir2/../../dir1/././dir2/../../file",
+          "file");
+
+  do_multi_test(NullS,   NullS,     NullS,  "");
+  do_multi_test("",      NullS,     NullS,  "");
+  do_multi_test("",      "",        NullS,  "");
+  do_multi_test("",      "/file",   NullS,  "/file");
+  do_multi_test("dir1/", "/dir2",   NullS,  "dir1/dir2");
+  do_multi_test("dir1",  "dir2",    NullS,  "dir1/dir2");
+  do_multi_test("dir1",  "",        "dir2", "dir1/dir2");
+  do_multi_test("dir1/", "../file", NullS,  "file");
+  do_multi_test("",      "file",    NullS,  "file");
+
+end2:
+  my_free(concat_buf, MYF(MY_ALLOW_ZERO_PTR));
+  (void) rmdir("dir1/dir2/dir3");
+  (void) rmdir("dir1/dir2");
+  (void) rmdir("dir1");
+  (void) chdir("..");
+end1:
+  (void) rmdir("safe_cleanup_cat_path_dir");
+end:
+  my_end(MY_CHECK_ERROR);
+  return exit_status();
+}


Attachment: [text/bzr-bundle] bzr/ingo.struewing@sun.com-20100106175508-vxg3pgyk2pnpcfnt.bundle
Thread
bzr commit into mysql-backup-backport branch (ingo.struewing:3035)Bug#43747 Bug#43767 WL#5101Ingo Struewing6 Jan