Updated Branches: refs/heads/master 6bfba8bc7 -> 4626716fd
TS-2361: improve regex_remap plugin loading semantics The location of the regex_remap configuration is not well-defined if it is a relative path. Either it will be found in the current directory (which is undefined), or a hard-coded global directory will be used. This change always uses the absolute path if an absolute path is provided. If a relative path is provided, then we load that relative to the configured Traffic Server configuration directory. Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/4626716f Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/4626716f Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/4626716f Branch: refs/heads/master Commit: 4626716fd390efdab12bae35a25abf41cf88c937 Parents: 6bfba8b Author: James Peach <[email protected]> Authored: Fri Nov 15 22:05:31 2013 -0800 Committer: James Peach <[email protected]> Committed: Mon Nov 18 14:41:21 2013 -0800 ---------------------------------------------------------------------- CHANGES | 2 + plugins/regex_remap/regex_remap.cc | 160 +++++++++++++++++--------------- 2 files changed, 89 insertions(+), 73 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4626716f/CHANGES ---------------------------------------------------------------------- diff --git a/CHANGES b/CHANGES index 8d47730..5bb14b4 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,8 @@ Changes with Apache Traffic Server 4.2.0 + *) [TS-2361] Load regex_remap configuration relative to the configuration directory. + *) [TS-2359] Make install over existing installation can fail. *) [TS-2350] Enhancements to traffic_top http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4626716f/plugins/regex_remap/regex_remap.cc ---------------------------------------------------------------------- diff --git a/plugins/regex_remap/regex_remap.cc b/plugins/regex_remap/regex_remap.cc index 496fd30..a85475e 100644 --- a/plugins/regex_remap/regex_remap.cc +++ b/plugins/regex_remap/regex_remap.cc @@ -54,9 +54,6 @@ static const char* PLUGIN_NAME = "regex_remap"; static const int OVECCOUNT = 30; // We support $0 - $9 x2 ints, and this needs to be 1.5x that static const int MAX_SUBS = 32; // No more than 32 substitution variables in the subst string -// TODO: This should be "autoconf'ed" or something ... -#define DEFAULT_PATH "/usr/local/etc/regex_remap/" - // Substitutions other than regex matches enum ExtraSubstitutions { SUB_HOST = 11, @@ -600,8 +597,13 @@ TSRemapNewInstance(int argc, char* argv[], void** ih, char* /* errbuf ATS_UNUSED return TS_ERROR; } + if (argc < 3) { + TSError("%s missing configuration file", PLUGIN_NAME); + return TS_ERROR; + } + // Really simple (e.g. basic) config parser - for (int i=2; i < argc; ++i) { + for (int i = 3; i < argc; ++i) { if (strncmp(argv[i], "profile", 7) == 0) { ri->profile = true; } else if (strncmp(argv[i], "no-profile", 10) == 0) { @@ -619,92 +621,104 @@ TSRemapNewInstance(int argc, char* argv[], void** ih, char* /* errbuf ATS_UNUSED } else if (strncmp(argv[i], "no-matrix-parameters", 18) == 0) { ri->matrix_params = false; } else { - if (0 != access(argv[2], R_OK)) { - ri->filename = DEFAULT_PATH; - ri->filename += argv[2]; - } else { - ri->filename = argv[2]; - } + TSError("%s: invalid option '%s'", PLUGIN_NAME, argv[i]); + } + } - f.open((ri->filename).c_str(), std::ios::in); - if (!f.is_open()) { // Try with the default path instead - TSError("unable to open %s", (ri->filename).c_str()); - return TS_ERROR; - } - TSDebug(PLUGIN_NAME, "loading regular expression maps from %s", (ri->filename).c_str()); + if (*argv[2] == '/') { + // Absolute path, just use it. + ri->filename = argv[2]; + } else { + // Relative path. Make it relative to the configuration directory. + ri->filename = TSConfigDirGet(); + ri->filename += "/"; + ri->filename += argv[2]; + } - while (!f.eof()) { - std::string line, regex, subst, options; - std::string::size_type pos1, pos2; + if (0 != access(ri->filename.c_str(), R_OK)) { + TSError("%s: failed to access %s: %s", PLUGIN_NAME, ri->filename.c_str(), strerror(errno)); + return TS_ERROR; + } + + f.open((ri->filename).c_str(), std::ios::in); + if (!f.is_open()) { + TSError("%s: unable to open %s", PLUGIN_NAME, (ri->filename).c_str()); + return TS_ERROR; + } + TSDebug(PLUGIN_NAME, "loading regular expression maps from %s", (ri->filename).c_str()); - getline(f, line); - ++lineno; - if (line.empty()) - continue; - pos1 = line.find_first_not_of(" \t\n"); - if (line[pos1] == '#') - continue; // Skip comment lines + while (!f.eof()) { + std::string line, regex, subst, options; + std::string::size_type pos1, pos2; + getline(f, line); + ++lineno; + if (line.empty()) + continue; + pos1 = line.find_first_not_of(" \t\n"); + if (line[pos1] == '#') + continue; // Skip comment lines + + if (pos1 != std::string::npos) { + pos2 = line.find_first_of(" \t\n", pos1); + if (pos2 != std::string::npos) { + regex = line.substr(pos1, pos2-pos1); + pos1 = line.find_first_not_of(" \t\n#", pos2); if (pos1 != std::string::npos) { pos2 = line.find_first_of(" \t\n", pos1); - if (pos2 != std::string::npos) { - regex = line.substr(pos1, pos2-pos1); - pos1 = line.find_first_not_of(" \t\n#", pos2); - if (pos1 != std::string::npos) { - pos2 = line.find_first_of(" \t\n", pos1); - if (pos2 == std::string::npos) - pos2 = line.length(); - subst = line.substr(pos1, pos2-pos1); - pos1 = line.find_first_not_of(" \t\n#", pos2); - if (pos1 != std::string::npos) { - pos2 = line.find_first_of("\n#", pos1); - if (pos2 == std::string::npos) - pos2 = line.length(); - options = line.substr(pos1, pos2-pos1); - } - } + if (pos2 == std::string::npos) + pos2 = line.length(); + subst = line.substr(pos1, pos2-pos1); + pos1 = line.find_first_not_of(" \t\n#", pos2); + if (pos1 != std::string::npos) { + pos2 = line.find_first_of("\n#", pos1); + if (pos2 == std::string::npos) + pos2 = line.length(); + options = line.substr(pos1, pos2-pos1); } } + } + } - if (regex.empty()) { - // No regex found on this line - TSError("no regexp found in %s: line %d", (ri->filename).c_str(), lineno); - continue; - } - if (subst.empty() && options.empty()) { - // No substitution found on this line (and no options) - TSError("no substitution string found in %s: line %d", (ri->filename).c_str(), lineno); - continue; - } + if (regex.empty()) { + // No regex found on this line + TSError("no regexp found in %s: line %d", (ri->filename).c_str(), lineno); + continue; + } + if (subst.empty() && options.empty()) { + // No substitution found on this line (and no options) + TSError("no substitution string found in %s: line %d", (ri->filename).c_str(), lineno); + continue; + } - // Got a regex and substitution string - RemapRegex* cur = new RemapRegex(regex, subst, options); + // Got a regex and substitution string + RemapRegex* cur = new RemapRegex(regex, subst, options); - if (cur == NULL) { - TSError("can't create a new regex remap rule"); - continue; - } + if (cur == NULL) { + TSError("%s: can't create a new regex remap rule", PLUGIN_NAME); + continue; + } - if (cur->compile(&error, &erroffset) < 0) { - TSError("PCRE failed in %s (line %d) at offset %d: %s", (ri->filename).c_str(), lineno, erroffset, error); - delete(cur); - } else { - TSDebug(PLUGIN_NAME, "added regex=%s with substitution=%s and options `%s'", - regex.c_str(), subst.c_str(), options.c_str()); - cur->set_order(++count); - if (ri->first == NULL) - ri->first = cur; - else - ri->last->set_next(cur); - ri->last = cur; - } - } + if (cur->compile(&error, &erroffset) < 0) { + TSError("%s: PCRE failed in %s (line %d) at offset %d: %s", + PLUGIN_NAME, (ri->filename).c_str(), lineno, erroffset, error); + delete(cur); + } else { + TSDebug(PLUGIN_NAME, "added regex=%s with substitution=%s and options `%s'", + regex.c_str(), subst.c_str(), options.c_str()); + cur->set_order(++count); + if (ri->first == NULL) + ri->first = cur; + else + ri->last->set_next(cur); + ri->last = cur; } + } // Make sure we got something... if (ri->first == NULL) { - TSError("Got no regular expressions from the maps"); + TSError("%s: no regular expressions from the maps", PLUGIN_NAME); return TS_ERROR; }
