From 466224b316f2a06ebee2cfeb7760a4f841f22fe4 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 11 Nov 2015 16:07:52 +0200 Subject: [PATCH] Moved common monitor code to externcmd.c File existence and permission checks are now done in externcmd_can_execute --- server/core/externcmd.c | 81 +++++++++++++++++++++++--- server/include/externcmd.h | 1 + server/modules/monitor/galeramon.c | 35 +++-------- server/modules/monitor/mmmon.c | 34 +++-------- server/modules/monitor/mysql_mon.c | 38 ++++-------- server/modules/monitor/ndbclustermon.c | 32 +++------- 6 files changed, 113 insertions(+), 108 deletions(-) diff --git a/server/core/externcmd.c b/server/core/externcmd.c index 820821e3c..c8bf92a8f 100644 --- a/server/core/externcmd.c +++ b/server/core/externcmd.c @@ -93,11 +93,11 @@ EXTERNCMD* externcmd_allocate(char* argstr) { if (access(cmd->argv[0], F_OK) != 0) { - skygw_log_write(LE, "Error: Cannot find file: %s", cmd->argv[0]); + skygw_log_write(LE, "Cannot find file: %s", cmd->argv[0]); } else { - skygw_log_write(LE, "Error: Cannot execute file '%s'. Missing " + skygw_log_write(LE, "Cannot execute file '%s'. Missing " "execution permissions.", cmd->argv[0]); } externcmd_free(cmd); @@ -145,23 +145,88 @@ int externcmd_execute(EXTERNCMD* cmd) if(pid < 0) { char errbuf[STRERROR_BUFLEN]; - skygw_log_write(LOGFILE_ERROR,"Error: Failed to execute command '%s', fork failed: [%d] %s", - cmd->argv[0],errno,strerror_r(errno, errbuf, sizeof(errbuf))); + skygw_log_write(LOGFILE_ERROR, "Failed to execute command '%s', fork failed: [%d] %s", + cmd->argv[0], errno, strerror_r(errno, errbuf, sizeof(errbuf))); rval = -1; } else if(pid == 0) { /** Child process, execute command */ execvp(cmd->argv[0],cmd->argv); - _exit(1); + _exit(1); } else { - cmd->child = pid; - cmd->n_exec++; - LOGIF(LD,skygw_log_write(LD,"[monitor_exec_cmd] Forked child process %d : %s.",pid,cmd)); + cmd->child = pid; + cmd->n_exec++; + LOGIF(LD, skygw_log_write(LD, "[monitor_exec_cmd] Forked child process %d : %s.", pid, cmd)); + } + return rval; +} + +/** + * Get the name of the command being executed. + * + * This copies the command being executed into a new string. + * @param str Command string, optionally with arguments + * @return Command part of the string if arguments were defined + */ +char* get_command(const char* str) +{ + char* rval = NULL; + const char* start = str; + + while (*start && isspace(*start)) + { + start++; + } + + const char* end = start; + + while (*end && !isspace(*end)) + { + end++; + } + + size_t len = end - start; + + if (len > 0 && (rval = malloc(len + 1))) + { + memcpy(rval, start, len); + rval[len] = '\0'; } return rval; } +/** + * Check if a command can be executed. + * + * Checks if the file being executed exists and if the current user has execution + * permissions on the file. + * @param argstr Command to check. Can contain arguments for the command. + * @return True if the file was found and the use has execution permissions to it. + */ +bool externcmd_can_execute(const char* argstr) +{ + bool rval = false; + char *command = get_command(argstr); + + if (command) + { + if (access(command, X_OK) == 0) + { + rval = true; + } + else if (access(command, F_OK) == 0) + { + skygw_log_write(LE, "The executable cannot be executed: %s", command); + } + else + { + skygw_log_write(LE, "The executable cannot be found: %s", command); + } + free(command); + } + return rval; +} diff --git a/server/include/externcmd.h b/server/include/externcmd.h index e1214c178..ab74e8ff7 100644 --- a/server/include/externcmd.h +++ b/server/include/externcmd.h @@ -18,4 +18,5 @@ typedef struct extern_cmd_t{ EXTERNCMD* externcmd_allocate(char* argstr); void externcmd_free(EXTERNCMD* cmd); int externcmd_execute(EXTERNCMD* cmd); +bool externcmd_can_execute(char* argstr); #endif diff --git a/server/modules/monitor/galeramon.c b/server/modules/monitor/galeramon.c index 4b78f30e2..4df951430 100644 --- a/server/modules/monitor/galeramon.c +++ b/server/modules/monitor/galeramon.c @@ -152,32 +152,15 @@ startMonitor(void *arg,void* opt) handle->use_priority = config_truth_value(params->value); else if(!strcmp(params->name,"script")) { - if(handle->script) - { - free(handle->script); - handle->script = NULL; - } - - if(access(params->value,X_OK) == 0) - { - handle->script = strdup(params->value); - } - else - { - script_error = true; - if(access(params->value,F_OK) == 0) - { - skygw_log_write(LE, - "Error: The file cannot be executed: %s", - params->value); - } - else - { - skygw_log_write(LE, - "Error: The file cannot be found: %s", - params->value); - } - } + if (externcmd_can_execute(params->value)) + { + free(handle->script); + handle->script = strdup(params->value); + } + else + { + script_error = true; + } } else if(!strcmp(params->name,"events")) { diff --git a/server/modules/monitor/mmmon.c b/server/modules/monitor/mmmon.c index 9abccf153..7718f1435 100644 --- a/server/modules/monitor/mmmon.c +++ b/server/modules/monitor/mmmon.c @@ -135,31 +135,15 @@ startMonitor(void *arg,void* opt) } else if(!strcmp(params->name,"script")) { - if(handle->script) - { - free(handle->script); - } - if(access(params->value,X_OK) == 0) - { - handle->script = strdup(params->value); - } - else - { - script_error = true; - if(access(params->value,F_OK) == 0) - { - skygw_log_write(LE, - "Error: The file cannot be executed: %s", - params->value); - } - else - { - skygw_log_write(LE, - "Error: The file cannot be found: %s", - params->value); - } - handle->script = NULL; - } + if (externcmd_can_execute(params->value)) + { + free(handle->script); + handle->script = strdup(params->value); + } + else + { + script_error = true; + } } else if(!strcmp(params->name,"events")) { diff --git a/server/modules/monitor/mysql_mon.c b/server/modules/monitor/mysql_mon.c index 6a817c505..c45c7604f 100644 --- a/server/modules/monitor/mysql_mon.c +++ b/server/modules/monitor/mysql_mon.c @@ -166,32 +166,18 @@ startMonitor(void *arg, void* opt) handle->detectStaleMaster = config_truth_value(params->value); else if(!strcmp(params->name,"detect_replication_lag")) handle->replicationHeartbeat = config_truth_value(params->value); - else if(!strcmp(params->name,"script")) - { - if(handle->script) - free(handle->script); - if(access(params->value,X_OK) == 0) - { - handle->script = strdup(params->value); - } - else - { - script_error = true; - if(access(params->value,F_OK) == 0) - { - skygw_log_write(LE, - "Error: The file cannot be executed: %s", - params->value); - } - else - { - skygw_log_write(LE, - "Error: The file cannot be found: %s", - params->value); - } - handle->script = NULL; - } - } + else if (!strcmp(params->name, "script")) + { + if (externcmd_can_execute(params->value)) + { + free(handle->script); + handle->script = strdup(params->value); + } + else + { + script_error = true; + } + } else if(!strcmp(params->name,"events")) { if(mon_parse_event_string((bool*)&handle->events,sizeof(handle->events),params->value) != 0) diff --git a/server/modules/monitor/ndbclustermon.c b/server/modules/monitor/ndbclustermon.c index 02883c925..aabccf60c 100644 --- a/server/modules/monitor/ndbclustermon.c +++ b/server/modules/monitor/ndbclustermon.c @@ -127,29 +127,15 @@ startMonitor(void *arg,void* opt) { if(!strcmp(params->name,"script")) { - if(handle->script) - free(handle->script); - if(access(params->value,X_OK) == 0) - { - handle->script = strdup(params->value); - } - else - { - script_error = true; - if(access(params->value,F_OK) == 0) - { - skygw_log_write(LE, - "Error: The file cannot be executed: %s", - params->value); - } - else - { - skygw_log_write(LE, - "Error: The file cannot be found: %s", - params->value); - } - handle->script = NULL; - } + if (externcmd_can_execute(params->value)) + { + free(handle->script); + handle->script = strdup(params->value); + } + else + { + script_error = true; + } } else if(!strcmp(params->name,"events")) {