From 4b69156875672655fc7af9e86a37e15cdacba0ce Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Wed, 29 May 2019 13:00:09 +0300 Subject: [PATCH] Simplify external command script substitution The command script is now stored in string form. Substitution is performed using normal string methods instead of regular expressions, since all used substitutions are simple string replacements. Tokenization is performed after substitution. --- server/core/externcmd.cc | 136 +++++++++++------------------- server/core/internal/externcmd.hh | 20 ++--- server/core/monitor.cc | 78 ++++------------- 3 files changed, 77 insertions(+), 157 deletions(-) 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;