From 1942506b82897e498493cc3ec274a2b402c4d105 Mon Sep 17 00:00:00 2001 From: Shishir Jaiswal Date: Thu, 29 Oct 2015 13:35:32 +0530 Subject: DESCRIPTION =========== When doing an upgrade, you execute mysql_upgrade. If mysql_upgrade fails to connect or it connects with a user without the proper privileges, it will return the error: FATAL ERROR: Upgrade failed which is not very informative. ANALYSIS ======== In main() and check_version_match(), the condition for errors are clubbed together and throw the same error msg. The functions need to be splitted up and the corresponding error msgs have to be displayed. FIX === Splitted the functions and added the specific error msg. --- client/mysql_upgrade.c | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) (limited to 'client/mysql_upgrade.c') diff --git a/client/mysql_upgrade.c b/client/mysql_upgrade.c index 33f9f030c1e..fcbde2653e8 100644 --- a/client/mysql_upgrade.c +++ b/client/mysql_upgrade.c @@ -1,5 +1,5 @@ /* - Copyright (c) 2006, 2013, Oracle and/or its affiliates. All rights reserved. + Copyright (c) 2006, 2015, Oracle and/or its affiliates. All rights reserved. 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 @@ -862,10 +862,17 @@ static int check_version_match(void) if (init_dynamic_string(&ds_version, NULL, NAME_CHAR_LEN, NAME_CHAR_LEN)) die("Out of memory"); - if (run_query("show variables like 'version'", - &ds_version, FALSE) || - extract_variable_from_show(&ds_version, version_str)) + + if (run_query("show variables like 'version'", &ds_version, FALSE)) { + fprintf(stderr, "Error: Failed while fetching Server version! Could be" + " due to unauthorized access.\n"); + dynstr_free(&ds_version); + return 1; /* Query failed */ + } + if (extract_variable_from_show(&ds_version, version_str)) + { + fprintf(stderr, "Error: Failed while extracting Server version!\n"); dynstr_free(&ds_version); return 1; /* Query failed */ } @@ -955,15 +962,20 @@ int main(int argc, char **argv) /* Run "mysqlcheck" and "mysql_fix_privilege_tables.sql" */ - if ((!opt_systables_only && - (run_mysqlcheck_fixnames() || run_mysqlcheck_upgrade())) || - run_sql_fix_privilege_tables()) + if (!opt_systables_only) { - /* - The upgrade failed to complete in some way or another, - significant error message should have been printed to the screen - */ - die("Upgrade failed" ); + if (run_mysqlcheck_fixnames()) + die("Error during call to mysql_check for fixing the db/tables names."); + + if (run_mysqlcheck_upgrade()) + die("Error during call to mysql_check for upgrading the tables names."); + } + + if (run_sql_fix_privilege_tables()) + { + /* Specific error msg (if present) would be printed in the function call + * above */ + die("Upgrade failed"); } verbose("OK"); -- cgit v1.2.1 From c21b927145e2da7fe835bc84b2b2b57e9dc2c60a Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Tue, 8 Dec 2015 10:13:13 +0100 Subject: mysql_upgrade cleanup --- client/mysql_upgrade.c | 45 +++++++++++++++++---------------------------- 1 file changed, 17 insertions(+), 28 deletions(-) (limited to 'client/mysql_upgrade.c') diff --git a/client/mysql_upgrade.c b/client/mysql_upgrade.c index ee6845def68..916af8755a0 100644 --- a/client/mysql_upgrade.c +++ b/client/mysql_upgrade.c @@ -109,6 +109,7 @@ static struct my_option my_long_options[]= &opt_force, &opt_force, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0}, {"host", 'h', "Connect to host.", 0, 0, 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0}, +#define PASSWORD_OPT 12 {"password", 'p', "Password to use when connecting to server. If password is not given," " it's solicited on the tty.", &opt_password,&opt_password, @@ -146,6 +147,7 @@ static struct my_option my_long_options[]= "do not try to upgrade the data.", &opt_systables_only, &opt_systables_only, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0}, +#define USER_OPT (array_elements(my_long_options) - 6) {"user", 'u', "User for login if not current user.", &opt_user, &opt_user, 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0}, {"verbose", 'v', "Display more output about the process.", @@ -235,26 +237,15 @@ static void verbose(const char *fmt, ...) static void add_one_option(DYNAMIC_STRING* ds, const struct my_option *opt, - const char* argument) - + const char* arg) { - const char* eq= NullS; - const char* arg= NullS; - if (opt->arg_type != NO_ARG) + dynstr_append(ds, "--"); + dynstr_append(ds, opt->name); + if (arg) { - eq= "="; - switch (opt->var_type & GET_TYPE_MASK) { - case GET_STR: - arg= argument; - break; - case GET_BOOL: - arg= (*(my_bool *)opt->value) ? "1" : "0"; - break; - default: - die("internal error at %s: %d",__FILE__, __LINE__); - } + dynstr_append(ds, "="); + dynstr_append_os_quoted(ds, arg, NullS); } - dynstr_append_os_quoted(ds, "--", opt->name, eq, arg, NullS); dynstr_append(ds, " "); } @@ -288,7 +279,6 @@ get_one_option(int optid, const struct my_option *opt, case 'p': if (argument == disabled_my_option) argument= (char*) ""; /* Don't require password */ - tty_password= 1; add_option= FALSE; if (argument) { @@ -298,6 +288,8 @@ get_one_option(int optid, const struct my_option *opt, *argument++= 'x'; /* Destroy argument */ tty_password= 0; } + else + tty_password= 1; break; case 't': @@ -351,7 +343,7 @@ get_one_option(int optid, const struct my_option *opt, if (add_option) { /* - This is an option that is accpted by mysql_upgrade just so + This is an option that is accepted by mysql_upgrade just so it can be passed on to "mysql" and "mysqlcheck" Save it in the ds_args string */ @@ -415,11 +407,8 @@ static int run_tool(char *tool_path, DYNAMIC_STRING *ds_res, ...) while ((arg= va_arg(args, char *))) { - /* Options should be os quoted */ - if (strncmp(arg, "--", 2) == 0) - dynstr_append_os_quoted(&ds_cmdline, arg, NullS); - else - dynstr_append(&ds_cmdline, arg); + /* Options should already be os quoted */ + dynstr_append(&ds_cmdline, arg); dynstr_append(&ds_cmdline, " "); } @@ -1031,12 +1020,12 @@ int main(int argc, char **argv) { opt_password= get_tty_password(NullS); /* add password to defaults file */ - dynstr_append_os_quoted(&ds_args, "--password=", opt_password, NullS); - dynstr_append(&ds_args, " "); + add_one_option(&ds_args, &my_long_options[PASSWORD_OPT], opt_password); + DBUG_ASSERT(strcmp(my_long_options[PASSWORD_OPT].name, "password") == 0); } /* add user to defaults file */ - dynstr_append_os_quoted(&ds_args, "--user=", opt_user, NullS); - dynstr_append(&ds_args, " "); + add_one_option(&ds_args, &my_long_options[USER_OPT], opt_user); + DBUG_ASSERT(strcmp(my_long_options[USER_OPT].name, "user") == 0); /* Find mysql */ find_tool(mysql_path, IF_WIN("mysql.exe", "mysql"), self_name); -- cgit v1.2.1 From 50a796dcba2abe5f25c1e4cd8a69d7ea43343a8d Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Tue, 8 Dec 2015 10:16:41 +0100 Subject: MDEV-8825 mysql_upgrade leaks the admin password when it spawns a shell process to execute mysqlcheck don't put common arguments on the command-line, use a config file instead --- client/mysql_upgrade.c | 51 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 15 deletions(-) (limited to 'client/mysql_upgrade.c') diff --git a/client/mysql_upgrade.c b/client/mysql_upgrade.c index 916af8755a0..d8ee8f0d911 100644 --- a/client/mysql_upgrade.c +++ b/client/mysql_upgrade.c @@ -53,6 +53,8 @@ static DYNAMIC_STRING conn_args; static char *opt_password= 0; static char *opt_plugin_dir= 0, *opt_default_auth= 0; +static char *cnf_file_path= 0, defaults_file[FN_REFLEN + 32]; + static my_bool tty_password= 0; static char opt_tmpdir[FN_REFLEN] = ""; @@ -184,6 +186,8 @@ static void free_used_memory(void) dynstr_free(&ds_args); dynstr_free(&conn_args); + if (cnf_file_path) + my_delete(cnf_file_path, MYF(MY_WME)); } @@ -235,8 +239,8 @@ static void verbose(const char *fmt, ...) this way we pass the same arguments on to mysql and mysql_check */ -static void add_one_option(DYNAMIC_STRING* ds, - const struct my_option *opt, +static void add_one_option_cmd_line(DYNAMIC_STRING *ds, + const struct my_option *opt, const char* arg) { dynstr_append(ds, "--"); @@ -249,6 +253,18 @@ static void add_one_option(DYNAMIC_STRING* ds, dynstr_append(ds, " "); } +static void add_one_option_cnf_file(DYNAMIC_STRING *ds, + const struct my_option *opt, + const char* arg) +{ + dynstr_append(ds, opt->name); + if (arg) + { + dynstr_append(ds, "="); + dynstr_append_os_quoted(ds, arg, NullS); + } + dynstr_append(ds, "\n"); +} static my_bool get_one_option(int optid, const struct my_option *opt, @@ -283,7 +299,7 @@ get_one_option(int optid, const struct my_option *opt, if (argument) { /* Add password to ds_args before overwriting the arg with x's */ - add_one_option(&ds_args, opt, argument); + add_one_option_cnf_file(&ds_args, opt, argument); while (*argument) *argument++= 'x'; /* Destroy argument */ tty_password= 0; @@ -336,7 +352,7 @@ get_one_option(int optid, const struct my_option *opt, case OPT_SHARED_MEMORY_BASE_NAME: /* --shared-memory-base-name */ case OPT_PLUGIN_DIR: /* --plugin-dir */ case OPT_DEFAULT_AUTH: /* --default-auth */ - add_one_option(&conn_args, opt, argument); + add_one_option_cmd_line(&conn_args, opt, argument); break; } @@ -347,7 +363,7 @@ get_one_option(int optid, const struct my_option *opt, it can be passed on to "mysql" and "mysqlcheck" Save it in the ds_args string */ - add_one_option(&ds_args, opt, argument); + add_one_option_cnf_file(&ds_args, opt, argument); } return 0; } @@ -550,8 +566,7 @@ static int run_query(const char *query, DYNAMIC_STRING *ds_res, ret= run_tool(mysql_path, ds_res, - "--no-defaults", - ds_args.str, + defaults_file, "--database=mysql", "--batch", /* Turns off pager etc. */ force ? "--force": "--skip-force", @@ -740,8 +755,7 @@ static int run_mysqlcheck_upgrade(void) print_conn_args("mysqlcheck"); retch= run_tool(mysqlcheck_path, NULL, /* Send output from mysqlcheck directly to screen */ - "--no-defaults", - ds_args.str, + defaults_file, "--check-upgrade", "--all-databases", "--auto-repair", @@ -794,8 +808,7 @@ static int run_mysqlcheck_views(void) print_conn_args("mysqlcheck"); return run_tool(mysqlcheck_path, NULL, /* Send output from mysqlcheck directly to screen */ - "--no-defaults", - ds_args.str, + defaults_file, "--all-databases", "--repair", upgrade_views, "--skip-process-tables", @@ -819,8 +832,7 @@ static int run_mysqlcheck_fixnames(void) print_conn_args("mysqlcheck"); return run_tool(mysqlcheck_path, NULL, /* Send output from mysqlcheck directly to screen */ - "--no-defaults", - ds_args.str, + defaults_file, "--all-databases", "--fix-db-names", "--fix-table-names", @@ -1020,13 +1032,22 @@ int main(int argc, char **argv) { opt_password= get_tty_password(NullS); /* add password to defaults file */ - add_one_option(&ds_args, &my_long_options[PASSWORD_OPT], opt_password); + add_one_option_cnf_file(&ds_args, &my_long_options[PASSWORD_OPT], opt_password); DBUG_ASSERT(strcmp(my_long_options[PASSWORD_OPT].name, "password") == 0); } /* add user to defaults file */ - add_one_option(&ds_args, &my_long_options[USER_OPT], opt_user); + add_one_option_cnf_file(&ds_args, &my_long_options[USER_OPT], opt_user); DBUG_ASSERT(strcmp(my_long_options[USER_OPT].name, "user") == 0); + cnf_file_path= strmov(defaults_file, "--defaults-file="); + { + int fd= create_temp_file(cnf_file_path, opt_tmpdir[0] ? opt_tmpdir : NULL, + "mysql_upgrade-", O_CREAT | O_WRONLY, MYF(MY_FAE)); + my_write(fd, USTRING_WITH_LEN( "[client]\n"), MYF(MY_FAE)); + my_write(fd, (uchar*)ds_args.str, ds_args.length, MYF(MY_FAE)); + my_close(fd, MYF(0)); + } + /* Find mysql */ find_tool(mysql_path, IF_WIN("mysql.exe", "mysql"), self_name); -- cgit v1.2.1