Bug #19953349: MYSQL_CONFIG_EDITOR DOES NOT ESCAPE STRINGS

RB#25710

mysql_config_editor was not surrounding the values it was putting
into the special login file with double quotes. This means any
spaces or special symbols will break the value and will not be sent
down to the mysql command line tool when the login path is used.

Fixed by making sure mysql_config_editor puts these values into
double quotes and also that the double quote symbols are escaped.

Test case added.

Author: gkodinov

client/mysql_config_editor.cc

@@ -1,5 +1,5 @@
 /*
-   Copyright (c) 2012, 2020, Oracle and/or its affiliates.
+   Copyright (c) 2012, 2021, Oracle and/or its affiliates.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -515,24 +515,24 @@ static int set_command(void) {
   if (opt_user) /* --user */
   {
     dynstr_append(&path_buf, "\nuser = ");
-    dynstr_append(&path_buf, opt_user);
+    dynstr_append_quoted(&path_buf, "\"", 1, opt_user, NullS);
   }
 
   if (opt_password) /* --password */
   {
     dynstr_append(&path_buf, "\npassword = ");
-    dynstr_append(&path_buf, opt_password);
+    dynstr_append_quoted(&path_buf, "\"", 1, opt_password, NullS);
   }
 
   if (opt_host) /* --host */
   {
     dynstr_append(&path_buf, "\nhost = ");
-    dynstr_append(&path_buf, opt_host);
+    dynstr_append_quoted(&path_buf, "\"", 1, opt_host, NullS);
   }
 
   if (opt_socket) {
     dynstr_append(&path_buf, "\nsocket = ");
-    dynstr_append(&path_buf, opt_socket);
+    dynstr_append_quoted(&path_buf, "\"", 1, opt_socket, NullS);
   }
 
   if (opt_port) {

include/my_sys.h

@@ -1,4 +1,4 @@
-/* Copyright (c) 2000, 2020, Oracle and/or its affiliates.
+/* Copyright (c) 2000, 2021, Oracle and/or its affiliates.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -801,6 +801,8 @@ extern bool dynstr_append(DYNAMIC_STRING *str, const char *append);
 bool dynstr_append_mem(DYNAMIC_STRING *str, const char *append, size_t length);
 extern bool dynstr_append_os_quoted(DYNAMIC_STRING *str, const char *append,
                                     ...);
+extern bool dynstr_append_quoted(DYNAMIC_STRING *str, const char *quote_str,
+                                 const uint quote_len, const char *append, ...);
 extern bool dynstr_set(DYNAMIC_STRING *str, const char *init_str);
 extern bool dynstr_realloc(DYNAMIC_STRING *str, size_t additional_size);
 extern bool dynstr_trunc(DYNAMIC_STRING *str, size_t n);

mysql-test/r/mysql_config_editor.result

@@ -20,54 +20,54 @@
 #
 # Default path
 [client]
-user = test_user1
-host = localhost
+user = "test_user1"
+host = "localhost"
 
 #
 # test-login-path1
 [test-login-path1]
-user = test_user2
-host = 127.0.0.1
+user = "test_user2"
+host = "127.0.0.1"
 
 #
 # test-login-path2
 [test-login-path2]
-user = test_user3
-host = www.mysql.com
+user = "test_user3"
+host = "www.mysql.com"
 
 #
 # all the paths
 [client]
-user = test_user1
-host = localhost
+user = "test_user1"
+host = "localhost"
 [test-login-path1]
-user = test_user2
-host = 127.0.0.1
+user = "test_user2"
+host = "127.0.0.1"
 [test-login-path2]
-user = test_user3
-host = www.mysql.com
+user = "test_user3"
+host = "www.mysql.com"
 [test-login-path3]
-user = test_user4
-host = 127.0.0.1
-socket = /tmp/configtest.sock
+user = "test_user4"
+host = "127.0.0.1"
+socket = "/tmp/configtest.sock"
 port = 15000
 
 #
 # Overwrite existing paths, test-login-path2 & default
 #
 # all the paths again
 [test-login-path1]
-user = test_user2
-host = 127.0.0.1
+user = "test_user2"
+host = "127.0.0.1"
 [test-login-path3]
-user = test_user4
-host = 127.0.0.1
-socket = /tmp/configtest.sock
+user = "test_user4"
+host = "127.0.0.1"
+socket = "/tmp/configtest.sock"
 port = 15000
 [test-login-path2]
-user = test_user4
+user = "test_user4"
 [client]
-user = test_user5
+user = "test_user5"
 
 ################################################
 # Tests for mysql_config_editor's remove command
@@ -86,10 +86,10 @@ user = test_user5
 #
 # using all
 [test-login-path3]
-user = test_user4
-host = 127.0.0.1
+user = "test_user4"
+host = "127.0.0.1"
 [test-login-path2]
-user = test_user4
+user = "test_user4"
 
 ###############################################
 # Tests for mysql_config_editor's reset command
@@ -189,18 +189,18 @@ mysqld is alive
 
 # Printing all the paths..
 [client]
-user = test_user1
-host = localhost
+user = "test_user1"
+host = "localhost"
 [test-login-path1]
-user = test_user2
-host = 127.0.0.1
+user = "test_user2"
+host = "127.0.0.1"
 [test-login-path2]
-user = test_user3
-host = 127.0.0.1
-socket = /tmp/configtest.sock
+user = "test_user3"
+host = "127.0.0.1"
+socket = "/tmp/configtest.sock"
 port = 15000
 [client_suffix1]
-user = test_user3
+user = "test_user3"
 
 # Now trying to connect using 'test_user3'
 # Note : In this case options from login
@@ -327,3 +327,16 @@ mysqld is alive
 # Cleanup
 DROP USER 'test#user1'@'localhost';
 # End of 5.6 tests
+#
+# Bug #19953349: MYSQL_CONFIG_EDITOR DOES NOT ESCAPE STRINGS
+#
+CREATE USER 'test1 test1'@'localhost';
+test: must contain space
+[test11]
+user = "test1 test1"
+host = "127.0.0.1"
+# test: must not fail with a space in the name
+mysqld is alive
+# cleanup
+DROP USER 'test1 test1'@'localhost';
+# End of 8.0 tests

mysql-test/suite/interactive_utilities/r/mysql_config_editor.result

@@ -88,17 +88,17 @@ user_2@localhost	user_2@localhost
 #    .mylogin.cnf file before cleanup      #
 ############################################
 [login_path_1]
-user = user_1
+user = "user_1"
 password = *****
-host = localhost
+host = "localhost"
 [login_path_3]
-user = user_3
+user = "user_3"
 password = *****
-host = localhost
+host = "localhost"
 [login_path_2]
-user = user_2
+user = "user_2"
 password = *****
-host = localhost
+host = "localhost"
 
 #################################################
 #     empty .mylogin.cnf file after cleanup     #

mysql-test/t/mysql_config_editor.test

@@ -268,3 +268,22 @@ DROP USER 'test#user1'@'localhost';
 --remove_file $MYSQL_TEST_LOGIN_FILE
 
 --echo # End of 5.6 tests
+
+--echo #
+--echo # Bug #19953349: MYSQL_CONFIG_EDITOR DOES NOT ESCAPE STRINGS
+--echo #
+
+CREATE USER 'test1 test1'@'localhost';
+
+--exec $MYSQL_CONFIG_EDITOR set --login-path=test11 --user="test1 test1" --host=127.0.0.1 2>&1
+--echo test: must contain space
+--exec $MYSQL_CONFIG_EDITOR print --login-path=test11
+
+--echo # test: must not fail with a space in the name
+--exec $MYSQLADMIN --no-defaults --login-path=test11 --port=$MASTER_MYPORT ping 2>&1
+
+--echo # cleanup
+DROP USER 'test1 test1'@'localhost';
+--remove_file $MYSQL_TEST_LOGIN_FILE
+
+--echo # End of 8.0 tests

mysys/my_string.cc

@@ -1,4 +1,4 @@
-/* Copyright (c) 2000, 2020, Oracle and/or its affiliates.
+/* Copyright (c) 2000, 2021, Oracle and/or its affiliates.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License, version 2.0,
@@ -118,7 +118,7 @@ bool dynstr_trunc(DYNAMIC_STRING *str, size_t n) {
 }
 
 /*
-  Concatenates any number of strings, escapes any OS quote in the result then
+  Concatenates any number of strings, escapes any quote in the result then
   surround the whole affair in another set of quotes which is finally appended
   to specified DYNAMIC_STRING.  This function is especially useful when
   building strings to be executed with the system() function.
@@ -133,19 +133,12 @@ bool dynstr_trunc(DYNAMIC_STRING *str, size_t n) {
   @return True = Success.
 */
 
-bool dynstr_append_os_quoted(DYNAMIC_STRING *str, const char *append, ...) {
-#ifdef _WIN32
-  const char *quote_str = "\"";
-  const uint quote_len = 1;
-#else
-  const char *quote_str = "\'";
-  const uint quote_len = 1;
-#endif /* _WIN32 */
+static bool dynstr_append_quoted_inner(DYNAMIC_STRING *str,
+                                       const char *quote_str,
+                                       const uint quote_len, const char *append,
+                                       va_list dirty_text) {
   bool ret = true;
-  va_list dirty_text;
-
   ret &= dynstr_append_mem(str, quote_str, quote_len); /* Leading quote */
-  va_start(dirty_text, append);
   while (append != NullS) {
     const char *cur_pos = append;
     const char *next_pos = cur_pos;
@@ -166,6 +159,30 @@ bool dynstr_append_os_quoted(DYNAMIC_STRING *str, const char *append, ...) {
   return ret;
 }
 
+bool dynstr_append_os_quoted(DYNAMIC_STRING *str, const char *append, ...) {
+#ifdef _WIN32
+  const char *quote_str = "\"";
+  const uint quote_len = 1;
+#else
+  const char *quote_str = "\'";
+  const uint quote_len = 1;
+#endif /* _WIN32 */
+  va_list dirty_text;
+
+  va_start(dirty_text, append);
+  return dynstr_append_quoted_inner(str, quote_str, quote_len, append,
+                                    dirty_text);
+}
+
+bool dynstr_append_quoted(DYNAMIC_STRING *str, const char *quote_str,
+                          const uint quote_len, const char *append, ...) {
+  va_list dirty_text;
+
+  va_start(dirty_text, append);
+  return dynstr_append_quoted_inner(str, quote_str, quote_len, append,
+                                    dirty_text);
+}
+
 void dynstr_free(DYNAMIC_STRING *str) {
   my_free(str->str);
   str->str = nullptr;