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/10/27 22:37:07 UTC

[42/50] [abbrv] git commit: TS-2230 header_rewrite should support the same hook-management that header_filter does for remap rules.

TS-2230 header_rewrite should support the same hook-management that
        header_filter does for remap rules.

This allows for injecting hook / rules from remap.config into hooks
outside of the remap phase itself. This is particularly useful for
doing header filtering on responses from an origin, based on which
remap rule triggered. With this, a remap.config instance can do e.g.

cond %{READ_RESPONSE_HDR_HOOK}
cond %{STATUS} >199    [AND]
cond %{STATUS} <299
rm-header Cache-Control
add-header Cache-Control "max-age=3600"

(we should add a set-header, but that's a different bug).

TS-2227 This commit also allows for multiple configs per line

This allows a remap.config file to have multiple @pparam's with
configs (such that you can e.g. share one across mulitple rules).
This also works for the global config(s) in plugin.config.

I accidentally merged the two patches, hence just one commit.


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/9dcd00b8
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/9dcd00b8
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/9dcd00b8

Branch: refs/heads/5.0.x
Commit: 9dcd00b8f82ec0a2678ae11ae739f68d0a496191
Parents: a3b07f9
Author: Leif Hedstrom <zw...@apache.org>
Authored: Fri Oct 18 15:32:15 2013 -0600
Committer: Leif Hedstrom <zw...@apache.org>
Committed: Wed Oct 23 16:09:35 2013 -0600

----------------------------------------------------------------------
 CHANGES                                  |   6 +
 plugins/header_rewrite/README            |  22 ++++
 plugins/header_rewrite/header_rewrite.cc | 163 ++++++++++++++++----------
 plugins/header_rewrite/resources.h       |   2 +-
 4 files changed, 132 insertions(+), 61 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9dcd00b8/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 67976e1..c14aee9 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,12 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 4.1.0
 
+  *) [TS-2227] Allow for multiple config files for a header_rewrite plugin
+   invocation (be it in remap.config or plugin.config).
+
+  *) [TS-2230] header_rewrite should support the same hook-management that
+  header_filter does for remap rules. This allows per-remap rules that
+  triggers in hooks other than the remap phase.
 
   *) [TS-2228] Add a set-config operator for header_rewrite plugin.
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9dcd00b8/plugins/header_rewrite/README
----------------------------------------------------------------------
diff --git a/plugins/header_rewrite/README b/plugins/header_rewrite/README
index 4244e4a..6b5f535 100644
--- a/plugins/header_rewrite/README
+++ b/plugins/header_rewrite/README
@@ -7,6 +7,24 @@ this rolling asap.
 Note that currently only static string "values" are supported. We'll add
 advanced features to allow for expansions in a future release.
 
+Using the plugin
+----------------
+
+This plugin can be used as both a global plugin, enabled in plugin.config:
+
+  header_rewrite.so config_file_1.conf config_file_2.conf ...
+
+These are global rules, which would apply to all requests. Another option is
+to use per remap rules in remap.config
+
+  map http://a http://b @plugin=header_rewrite.so @pparam=rules1.conf ...
+
+
+In the second example, hooks which are not to be executed during the remap
+phase (the default) causes a transaction hook to be instantiated and used
+at a later time. This allows you to setup e.g. a rule that gets executed
+during the origin response header parsing, using READ_RESPONSE_HDR_HOOK.
+
 
 Operators
 ---------
@@ -78,6 +96,10 @@ each rule. This implies that a new hook condition starts a new rule as well.
   cond %{SEND_REQUEST_HDR_HOOK}
   cond %{SEND_RESPONSE_HDR_HOOK}
 
+For remap.config plugin instanations, the default hook is named
+REMAP_PSEUDO_HOOK. This can be useful if you are mixing other hooks in a
+configuration, but being the default it is also optional.
+
 
 Condition flags
 ---------------

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9dcd00b8/plugins/header_rewrite/header_rewrite.cc
----------------------------------------------------------------------
diff --git a/plugins/header_rewrite/header_rewrite.cc b/plugins/header_rewrite/header_rewrite.cc
index 9d3dde0..3bcb21d 100644
--- a/plugins/header_rewrite/header_rewrite.cc
+++ b/plugins/header_rewrite/header_rewrite.cc
@@ -15,11 +15,6 @@
   See the License for the specific language governing permissions and
   limitations under the License.
 */
-//////////////////////////////////////////////////////////////////////////////////////////////
-// header_rewrite: YTS plugin to do header rewrites
-// --------------
-//
-//
 #include <fstream>
 #include <string>
 #include <boost/algorithm/string.hpp>
@@ -31,26 +26,41 @@
 #include "ruleset.h"
 #include "resources.h"
 
-// "Defines"
+// Debugs
 const char* PLUGIN_NAME = "header_rewrite";
 const char* PLUGIN_NAME_DBG = "header_rewrite_dbg";
 
 static const char* DEFAULT_CONF_PATH = "/usr/local/etc/header_rewrite/";
 
+// Simple wrapper around a configuration file / set. This is useful such that
+// we can reuse most of the code for both global and per-remap rule sets.
+struct RulesConfig
+{
+  RulesConfig()
+  {
+    memset(rules, 0, sizeof(rules));
+    memset(resids, 0, sizeof(resids));
+  }
+
+  ~RulesConfig()
+  {
+    for (int i=TS_HTTP_READ_REQUEST_HDR_HOOK; i<TS_HTTP_LAST_HOOK; ++i)
+      delete rules[i];
+  }
 
-// Global holding the rulesets and resource IDs
-static RuleSet* all_rules[TS_HTTP_LAST_HOOK+1];
-static ResourceIDs all_resids[TS_HTTP_LAST_HOOK+1];
+  RuleSet* rules[TS_HTTP_LAST_HOOK+1];
+  ResourceIDs resids[TS_HTTP_LAST_HOOK+1];
+};
 
 // Helper function to add a rule to the rulesets
 static bool
-add_rule(RuleSet* rule) {
+add_rule(RuleSet* rule, RulesConfig *conf) {
   if (rule && rule->has_operator()) {
     TSDebug(PLUGIN_NAME, "Adding rule to hook=%d\n", rule->get_hook());
-    if (NULL == all_rules[rule->get_hook()]) {
-      all_rules[rule->get_hook()] = rule;
+    if (NULL == conf->rules[rule->get_hook()]) {
+      conf->rules[rule->get_hook()] = rule;
     } else {
-      all_rules[rule->get_hook()]->append(rule);
+      conf->rules[rule->get_hook()]->append(rule);
     }
     return true;
   }
@@ -60,14 +70,13 @@ add_rule(RuleSet* rule) {
 
 
 ///////////////////////////////////////////////////////////////////////////////
-// Simple "config" parser, this modifies the global above. ToDo: What happens
-// on a "config" reload?
+// Config parser, use to parse both the global, and per-remap, configurations.
 //
 // Note that this isn't particularly efficient, but it's a startup time cost
 // anyways (or reload for remap.config), so not really in the critical path.
 //
 bool
-parse_config(const std::string fname, TSHttpHookID default_hook)
+parse_config(const std::string fname, TSHttpHookID default_hook, RulesConfig *conf)
 {
   RuleSet* rule = NULL;
   std::string filename = fname;
@@ -93,7 +102,7 @@ parse_config(const std::string fname, TSHttpHookID default_hook)
 
     getline(f, line);
     ++lineno; // ToDo: we should probably use this for error messages ...
-    TSDebug(PLUGIN_NAME, "Reading line: %d: %s", lineno, line.c_str());
+    TSDebug(PLUGIN_NAME_DBG, "Reading line: %d: %s", lineno, line.c_str());
 
     boost::trim(line);
     if (line.empty() || (line[0] == '#'))
@@ -104,7 +113,7 @@ parse_config(const std::string fname, TSHttpHookID default_hook)
       continue;
 
     // If we are at the beginning of a new condition, save away the previous rule (but only if it has operators).
-    if (p.is_cond() && add_rule(rule))
+    if (p.is_cond() && add_rule(rule, conf))
       rule = NULL;
 
     if (NULL == rule) {
@@ -128,6 +137,9 @@ parse_config(const std::string fname, TSHttpHookID default_hook)
       } else if (p.cond_op_is("SEND_RESPONSE_HDR_HOOK")) {
         rule->set_hook(TS_HTTP_SEND_RESPONSE_HDR_HOOK);
         continue;
+      } else if (p.cond_op_is("REMAP_PSEUDO_HOOK")) {
+        rule->set_hook(TS_REMAP_PSEUDO_HOOK);
+        continue;
       }
     }
 
@@ -139,12 +151,14 @@ parse_config(const std::string fname, TSHttpHookID default_hook)
   }
 
   // Add the last rule (possibly the only rule)
-  add_rule(rule);
+  add_rule(rule, conf);
 
   // Collect all resource IDs that we need
-  for (int i=TS_HTTP_READ_REQUEST_HDR_HOOK; i<TS_HTTP_LAST_HOOK; ++i)
-    if (all_rules[i])
-      all_resids[i] = all_rules[i]->get_all_resource_ids();
+  for (int i=TS_HTTP_READ_REQUEST_HDR_HOOK; i<TS_HTTP_LAST_HOOK; ++i) {
+    if (conf->rules[i]) {
+      conf->resids[i] = conf->rules[i]->get_all_resource_ids();
+    }
+  }
 
   return true;
 }
@@ -156,12 +170,9 @@ parse_config(const std::string fname, TSHttpHookID default_hook)
 static int
 cont_rewrite_headers(TSCont contp, TSEvent event, void *edata)
 {
-  TSDebug(PLUGIN_NAME, "plugin: %d", event);
-
-  TSHttpTxn txnp = (TSHttpTxn) edata;
-  Resources res(txnp, contp);
-  const RuleSet* rule = NULL;
+  TSHttpTxn txnp = static_cast<TSHttpTxn>(edata);
   TSHttpHookID hook = TS_HTTP_LAST_HOOK;
+  RulesConfig* conf = static_cast<RulesConfig*>(TSContDataGet(contp));
 
   // Get the resources necessary to process this event
   switch (event) {
@@ -187,10 +198,12 @@ cont_rewrite_headers(TSCont contp, TSEvent event, void *edata)
   }
 
   if (hook != TS_HTTP_LAST_HOOK) {
-    res.gather(all_resids[hook], hook);
-    rule = all_rules[hook];
+    const RuleSet* rule = conf->rules[hook];
+    Resources res(txnp, contp);
+
+    res.gather(conf->resids[hook], hook);
 
-    // Evaluation
+    // Evaluation of all rules. This code is sort of duplicate in DoRemap as well.
     while (rule) {
       if (rule->eval(res)) {
         OperModifiers rt = rule->exec(res);
@@ -223,29 +236,37 @@ TSPluginInit(int argc, const char *argv[])
   if (TS_SUCCESS != TSPluginRegister(TS_SDK_VERSION_3_0 , &info)) {
     TSError("%s: plugin registration failed.\n", PLUGIN_NAME);
   }
-
   TSDebug(PLUGIN_NAME, "number of arguments: %d", argc);
-  if (argc != 2) {
-    TSError("%s usage: %s <config-file> ... \n", PLUGIN_NAME, argv[0]);
-    assert(argc == 2);
-  }
 
-  // Initialize the globals
-  for (int i=TS_HTTP_READ_REQUEST_HDR_HOOK; i<TS_HTTP_LAST_HOOK; ++i) {
-    all_rules[i] = NULL;
-    all_resids[i] = RSRC_NONE;
+  // Parse the global config file(s). All rules are just appended
+  // to the "global" Rules configuration.
+  RulesConfig* conf = new RulesConfig;
+  bool got_config = false;
+
+  for (int i=1; i < argc; ++i) {
+    // Parse the config file(s). Note that multiple config files are
+    // just appended to the configurations.
+    if (!parse_config(argv[i], TS_HTTP_READ_RESPONSE_HDR_HOOK, conf)) {
+      TSError("header_rewrite: failed to parse configuration file %s", argv[argc]);
+    } else {
+      got_config = true;
+    }
   }
 
-  // Parse the config file
-  if (parse_config(argv[1], TS_HTTP_READ_RESPONSE_HDR_HOOK)) {
+  if (got_config) {
+    TSCont contp = TSContCreate(cont_rewrite_headers, NULL);
+    TSContDataSet(contp, conf);
+
     for (int i=TS_HTTP_READ_REQUEST_HDR_HOOK; i<TS_HTTP_LAST_HOOK; ++i) {
-      if (all_rules[i]) {
+      if (conf->rules[i]) {
         TSDebug(PLUGIN_NAME, "adding hook: %d", i);
-        TSHttpHookAdd(static_cast<TSHttpHookID>(i), TSContCreate(cont_rewrite_headers, NULL));
+        TSHttpHookAdd(static_cast<TSHttpHookID>(i), contp);
       }
     }
   } else {
+    // Didn't get anything, nuke it.
     TSError("%s: failed to parse configuration file", PLUGIN_NAME);
+    delete conf;
   }
 }
 
@@ -287,27 +308,27 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE
     return TS_ERROR;
   }
 
-  // TODO: this is a big ugly, we use a pseudo hook for parsing the config for a 
-  // remap instantiation.
-  all_rules[TS_REMAP_PSEUDO_HOOK] = NULL;
-  if (!parse_config(argv[2], TS_REMAP_PSEUDO_HOOK)) {
-    TSError("%s: Unable to create remap instance", PLUGIN_NAME);
-    return TS_ERROR;
+  RulesConfig* conf = new RulesConfig;
+
+  for (int i=2; i < argc; ++i) {
+    if (!parse_config(argv[i], TS_REMAP_PSEUDO_HOOK, conf)) {
+      TSError("%s: Unable to create remap instance", PLUGIN_NAME);
+      return TS_ERROR;
+    }
   }
 
-  *ih = all_rules[TS_REMAP_PSEUDO_HOOK];
-  all_rules[TS_REMAP_PSEUDO_HOOK] = NULL;
+  *ih = conf;
+  TSDebug(PLUGIN_NAME, "added header_rewrite remap rule set");
 
-  TSDebug(PLUGIN_NAME, "successfully initialize the header_rewrite plugin");
   return TS_SUCCESS;
 }
 
 void
 TSRemapDeleteInstance(void *ih)
 {
-  RuleSet* rule = static_cast<RuleSet*>(ih);
+  RulesConfig* conf = static_cast<RulesConfig*>(ih);
 
-  delete rule;
+  delete conf;
 }
 
 
@@ -323,17 +344,40 @@ TSRemapDoRemap(void *ih, TSHttpTxn rh, TSRemapRequestInfo *rri)
     TSDebug(PLUGIN_NAME, "No Rules configured, falling back to default");
     return rval;
   } else {
-    RuleSet* rule = (RuleSet*)ih;
-    Resources res((TSHttpTxn)rh, rri);
+    RulesConfig* conf = static_cast<RulesConfig*>(ih);
+
+    // ToDo: Would it be faster / better to register the global hook for all
+    // hooks, as we do in header_filter, regardless if there's a known config
+    // for that hook? For now, we assume it's cheaper / faster to create a
+    // TXN hook when necessary.
+
+    // Go through all hooks we support, and setup the txn hook(s) as necessary
+    for (int i=TS_HTTP_READ_REQUEST_HDR_HOOK; i<TS_HTTP_LAST_HOOK; ++i) {
+      TSCont contp = NULL;
+
+      if (conf->rules[i]) {
+        if (NULL == contp) {
+          contp = TSContCreate(cont_rewrite_headers, NULL);
+          TSContDataSet(contp, conf);
+        }
+        TSHttpTxnHookAdd(rh, static_cast<TSHttpHookID>(i), contp);
+        TSDebug(PLUGIN_NAME, "activated transaction hook via remap.config: new hook=%d", i);
+      }
+    }
+
+    // Now handle the remap specific rules for the "remap hook" (which is not a real hook).
+    // This avoids scheduling an additional continuation for a very common case.
+    RuleSet* rule = conf->rules[TS_REMAP_PSEUDO_HOOK];
+    Resources res(rh, rri);
 
-    // TODO: This might be suboptimal, do we always need the client request
-    // headers in a remap plugin?
+    // res.gather(conf->resids[TS_REMAP_PSEUDO_HOOK], TS_REMAP_PSEUDO_HOOK);
     res.gather(RSRC_CLIENT_REQUEST_HEADERS, TS_REMAP_PSEUDO_HOOK);
 
-    // Evaluation
+    // Evaluation. This code is duplicated sort of, should we merge with the continuation evaluator ?
     while (rule) {
       if (rule->eval(res)) {
         OperModifiers rt = rule->exec(res);
+
         if (res.changed_url == true)
           rval = TSREMAP_DID_REMAP;
 
@@ -343,7 +387,6 @@ TSRemapDoRemap(void *ih, TSHttpTxn rh, TSRemapRequestInfo *rri)
       }
       rule = rule->next;
     }
-
   }
 
   TSDebug(PLUGIN_NAME, "returing with status: %d", rval);

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9dcd00b8/plugins/header_rewrite/resources.h
----------------------------------------------------------------------
diff --git a/plugins/header_rewrite/resources.h b/plugins/header_rewrite/resources.h
index d2ce9f5..d63b4e7 100644
--- a/plugins/header_rewrite/resources.h
+++ b/plugins/header_rewrite/resources.h
@@ -57,7 +57,7 @@ public:
   Resources(TSHttpTxn txnptr, TSRemapRequestInfo *rri) :
     txnp(txnptr), contp(NULL),
     bufp(NULL), hdr_loc(NULL), client_bufp(NULL), client_hdr_loc(NULL), resp_status(TS_HTTP_STATUS_NONE),
-    _rri(rri), _ready(false)
+      _rri(rri), changed_url(false), _ready(false)
   {
     TSDebug(PLUGIN_NAME_DBG, "Calling CTOR for Resources (RemapAPI)");
     TSDebug(PLUGIN_NAME, "rri: %p", _rri);