You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2013/11/21 19:42:32 UTC

[24/44] git commit: TS-2361: improve regex_remap plugin loading semantics

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/5.0.x
Commit: 4626716fd390efdab12bae35a25abf41cf88c937
Parents: 6bfba8b
Author: James Peach <jp...@apache.org>
Authored: Fri Nov 15 22:05:31 2013 -0800
Committer: James Peach <jp...@apache.org>
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;
   }