diff --git a/server/core/externcmd.cc b/server/core/externcmd.cc index 2dec26b23..274c21e20 100644 --- a/server/core/externcmd.cc +++ b/server/core/externcmd.cc @@ -24,27 +24,22 @@ #include #include -/** - * Tokenize a string into arguments suitable for a `execvp` call. - * - * @param argstr Argument string - * @return Number of arguments parsed from the string - */ -int ExternalCmd::tokenize_arguments(const char* argstr) +using std::string; + +int ExternalCmd::tokenize_args(char* dest[], int dest_size) { - int i = 0; bool quoted = false; bool read = false; bool escaped = false; - char* ptr, * start; - char args[strlen(argstr) + 1]; char qc = 0; - strcpy(args, argstr); - start = args; - ptr = start; + char args[m_command_substituted.length() + 1]; + strcpy(args, m_command_substituted.c_str()); + char* start = args; + char* ptr = start; + int i = 0; - while (*ptr != '\0' && i < MAX_ARGS) + while (*ptr != '\0' && i < dest_size) { if (escaped) { @@ -59,7 +54,7 @@ int ExternalCmd::tokenize_arguments(const char* argstr) else if (quoted && !escaped && *ptr == qc) /** End of quoted string */ { *ptr = '\0'; - argv[i++] = MXS_STRDUP(start); + dest[i++] = MXS_STRDUP(start); read = false; quoted = false; } @@ -70,7 +65,7 @@ int ExternalCmd::tokenize_arguments(const char* argstr) *ptr = '\0'; if (read) /** New token */ { - argv[i++] = MXS_STRDUP(start); + dest[i++] = MXS_STRDUP(start); read = false; } } @@ -92,22 +87,21 @@ int ExternalCmd::tokenize_arguments(const char* argstr) } if (read) { - argv[i++] = MXS_STRDUP(start); + dest[i++] = MXS_STRDUP(start); } - - argv[i] = nullptr; return i; } ExternalCmd* ExternalCmd::create(const char* argstr, int timeout) { - auto cmd = new ExternalCmd(timeout); + auto cmd = new ExternalCmd(argstr, timeout); bool success = false; if (argstr && cmd) { - if (cmd->tokenize_arguments(argstr) > 0) + char* argvec[1] {}; // Parse just one argument for testing. + if (cmd->tokenize_args(argvec, 1) > 0) { - auto cmdname = cmd->argv[0]; + const char* cmdname = argvec[0]; if (access(cmdname, X_OK) != 0) { if (access(cmdname, F_OK) != 0) @@ -123,6 +117,7 @@ ExternalCmd* ExternalCmd::create(const char* argstr, int timeout) { success = true; } + MXS_FREE(argvec[0]); } else { @@ -138,19 +133,13 @@ ExternalCmd* ExternalCmd::create(const char* argstr, int timeout) return cmd; } -ExternalCmd::ExternalCmd(int timeout) - : m_timeout(timeout) +ExternalCmd::ExternalCmd(const std::string& script, int timeout) + : m_command(script) + , m_command_substituted(script) + , m_timeout(timeout) { } -ExternalCmd::~ExternalCmd() -{ - for (int i = 0; argv[i]; i++) - { - MXS_FREE(argv[i]); - } -} - static const char* skip_whitespace(const char* ptr) { while (*ptr && isspace(*ptr)) @@ -220,20 +209,21 @@ int ExternalCmd::externcmd_execute() { // Create a pipe where the command can print output int fd[2]; - if (pipe(fd) == -1) { MXS_ERROR("Failed to open pipe: [%d] %s", errno, mxs_strerror(errno)); return -1; } + // "execvp" takes its arguments as an array of tokens where the first element is the command. + char* argvec[MAX_ARGS + 1] {}; + tokenize_args(argvec, MAX_ARGS); + const char* cmdname = argvec[0]; + int rval = 0; - pid_t pid; - const char* cmdname = argv[0]; // The SIGCHLD handler must be disabled before child process is forked, // otherwise we'll get an error - pid = fork(); - + pid_t pid = fork(); if (pid < 0) { close(fd[0]); @@ -251,7 +241,7 @@ int ExternalCmd::externcmd_execute() dup2(fd[1], STDERR_FILENO); // Execute the command - execvp(argv[0], argv); + execvp(cmdname, argvec); // Close the write end of the pipe and exit close(fd[1]); @@ -359,52 +349,31 @@ int ExternalCmd::externcmd_execute() close(fd[0]); } + // Free the token array. + for (int i = 0; i < MAX_ARGS && argvec[i]; i++) + { + MXS_FREE(argvec[i]); + } return rval; } -bool ExternalCmd::substitute_arg(const char* match, const char* replace) +void ExternalCmd::substitute_arg(const std::string& match, const std::string& replace) { - int err; - bool rval = true; - size_t errpos; - pcre2_code* re = pcre2_compile((PCRE2_SPTR) match, PCRE2_ZERO_TERMINATED, 0, &err, &errpos, NULL); - if (re) + // The match may be in the subject multiple times. Find all locations. + string::size_type next_search_begin = 0; + while (next_search_begin < m_command_substituted.length()) { - for (int i = 0; argv[i] && rval; i++) + auto position = m_command_substituted.find(match, next_search_begin); + if (position == string::npos) { - size_t size_orig = strlen(argv[i]); - size_t size_replace = strlen(replace); - size_t size = MXS_MAX(size_orig, size_replace); - char* dest = (char*)MXS_MALLOC(size); - if (dest) - { - mxs_pcre2_result_t rc = mxs_pcre2_substitute(re, argv[i], replace, &dest, &size); - - switch (rc) - { - case MXS_PCRE2_ERROR: - MXS_FREE(dest); - rval = false; - break; - - case MXS_PCRE2_MATCH: - MXS_FREE(argv[i]); - argv[i] = dest; - break; - - case MXS_PCRE2_NOMATCH: - MXS_FREE(dest); - break; - } - } + next_search_begin = m_command_substituted.length(); + } + else + { + m_command_substituted.replace(position, match.length(), replace); + next_search_begin = position + replace.length(); } - pcre2_code_free(re); } - else - { - rval = false; - } - return rval; } /** @@ -449,13 +418,10 @@ static char* get_command(const char* str) bool ExternalCmd::externcmd_matches(const char* match) { - for (int i = 0; argv[i]; i++) - { - if (strstr(argv[i], match)) - { - return true; - } - } - - return false; + return m_command.find(match) != string::npos; } + +const char* ExternalCmd::substituted() const +{ + return m_command_substituted.c_str(); +} \ No newline at end of file diff --git a/server/core/internal/externcmd.hh b/server/core/internal/externcmd.hh index 91df13a26..df6a9c041 100644 --- a/server/core/internal/externcmd.hh +++ b/server/core/internal/externcmd.hh @@ -28,8 +28,6 @@ public: */ static ExternalCmd* create(const char* argstr, int timeout); - ~ExternalCmd(); - /** * Execute a command * @@ -44,10 +42,8 @@ public: * * @param match Match string * @param replace Replacement string - * - * @return True if replacement was successful, false on error */ - bool substitute_arg(const char* match, const char* replace); + void substitute_arg(const std::string& match, const std::string& replace); /** * Simple matching of string and command @@ -58,14 +54,18 @@ public: */ bool externcmd_matches(const char* match); + const char* substituted() const; + static const int MAX_ARGS {256}; - char* argv[MAX_ARGS + 1] {}; /**< Arguments. First element is the command. */ - private: - int m_timeout; /**< Command timeout in seconds */ + std::string m_command; /**< Original command */ + std::string m_command_substituted; /**< Command with substitutions */ - ExternalCmd(int timeout); - int tokenize_arguments(const char* argstr); + int m_timeout; /**< Command timeout in seconds */ + + ExternalCmd(const std::string& script, int timeout); + + int tokenize_args(char* dest[], int dest_size); }; diff --git a/server/core/monitor.cc b/server/core/monitor.cc index 96fe41982..fadd1c127 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -1060,7 +1060,7 @@ int Monitor::launch_command(MonitorServer* ptr, ExternalCmd* cmd) { char initiator[strlen(ptr->server->address) + 24]; // Extra space for port snprintf(initiator, sizeof(initiator), "[%s]:%d", ptr->server->address, ptr->server->port); - cmd->substitute_arg("[$]INITIATOR", initiator); + cmd->substitute_arg("$INITIATOR", initiator); } if (cmd->externcmd_matches("$PARENT")) @@ -1072,17 +1072,17 @@ int Monitor::launch_command(MonitorServer* ptr, ExternalCmd* cmd) { ss << "[" << parent->server->address << "]:" << parent->server->port; } - cmd->substitute_arg("[$]PARENT", ss.str().c_str()); + cmd->substitute_arg("$PARENT", ss.str().c_str()); } if (cmd->externcmd_matches("$CHILDREN")) { - cmd->substitute_arg("[$]CHILDREN", child_nodes(ptr).c_str()); + cmd->substitute_arg("$CHILDREN", child_nodes(ptr).c_str()); } if (cmd->externcmd_matches("$EVENT")) { - cmd->substitute_arg("[$]EVENT", ptr->get_event_name()); + cmd->substitute_arg("$EVENT", ptr->get_event_name()); } char nodelist[PATH_MAX + MON_ARG_MAX + 1] = {'\0'}; @@ -1091,37 +1091,37 @@ int Monitor::launch_command(MonitorServer* ptr, ExternalCmd* cmd) { // We provide the credentials for _all_ servers. append_node_names(nodelist, sizeof(nodelist), 0, CredentialsApproach::INCLUDE); - cmd->substitute_arg("[$]CREDENTIALS", nodelist); + cmd->substitute_arg("$CREDENTIALS", nodelist); } if (cmd->externcmd_matches("$NODELIST")) { append_node_names(nodelist, sizeof(nodelist), SERVER_RUNNING); - cmd->substitute_arg("[$]NODELIST", nodelist); + cmd->substitute_arg("$NODELIST", nodelist); } if (cmd->externcmd_matches("$LIST")) { append_node_names(nodelist, sizeof(nodelist), 0); - cmd->substitute_arg("[$]LIST", nodelist); + cmd->substitute_arg("$LIST", nodelist); } if (cmd->externcmd_matches("$MASTERLIST")) { append_node_names(nodelist, sizeof(nodelist), SERVER_MASTER); - cmd->substitute_arg("[$]MASTERLIST", nodelist); + cmd->substitute_arg("$MASTERLIST", nodelist); } if (cmd->externcmd_matches("$SLAVELIST")) { append_node_names(nodelist, sizeof(nodelist), SERVER_SLAVE); - cmd->substitute_arg("[$]SLAVELIST", nodelist); + cmd->substitute_arg("$SLAVELIST", nodelist); } if (cmd->externcmd_matches("$SYNCEDLIST")) { append_node_names(nodelist, sizeof(nodelist), SERVER_JOINED); - cmd->substitute_arg("[$]SYNCEDLIST", nodelist); + cmd->substitute_arg("$SYNCEDLIST", nodelist); } int rv = cmd->externcmd_execute(); @@ -1131,66 +1131,20 @@ int Monitor::launch_command(MonitorServer* ptr, ExternalCmd* cmd) if (rv == -1) { // Internal error - MXS_ERROR("Failed to execute script '%s' on server state change event '%s'", - cmd->argv[0], - ptr->get_event_name()); + MXS_ERROR("Failed to execute script on server state change event '%s'. Script was: '%s'", + ptr->get_event_name(), cmd->substituted()); } else { // Script returned a non-zero value - MXS_ERROR("Script '%s' returned %d on event '%s'", - cmd->argv[0], - rv, - ptr->get_event_name()); + MXS_ERROR("Script returned %d on event '%s'. Script was: '%s'", + rv, ptr->get_event_name(), cmd->substituted()); } } else { - mxb_assert(cmd->argv != NULL && cmd->argv[0] != NULL); - // Construct a string with the script + arguments - char* scriptStr = NULL; - int totalStrLen = 0; - bool memError = false; - for (int i = 0; cmd->argv[i]; i++) - { - totalStrLen += strlen(cmd->argv[i]) + 1; // +1 for space and one \0 - } - int spaceRemaining = totalStrLen; - if ((scriptStr = (char*)MXS_CALLOC(totalStrLen, sizeof(char))) != NULL) - { - char* currentPos = scriptStr; - // The script name should not begin with a space - int len = snprintf(currentPos, spaceRemaining, "%s", cmd->argv[0]); - currentPos += len; - spaceRemaining -= len; - - for (int i = 1; cmd->argv[i]; i++) - { - if ((cmd->argv[i])[0] == '\0') - { - continue; // Empty argument, print nothing - } - len = snprintf(currentPos, spaceRemaining, " %s", cmd->argv[i]); - currentPos += len; - spaceRemaining -= len; - } - mxb_assert(spaceRemaining > 0); - *currentPos = '\0'; - } - else - { - memError = true; - scriptStr = cmd->argv[0]; // print at least something - } - - MXS_NOTICE("Executed monitor script '%s' on event '%s'", - scriptStr, - ptr->get_event_name()); - - if (!memError) - { - MXS_FREE(scriptStr); - } + MXS_NOTICE("Executed monitor script on event '%s'. Script was: '%s'", + ptr->get_event_name(), cmd->substituted()); } return rv;