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 2014/04/10 14:54:22 UTC

[2/6] git commit: TS-2690 Make the escalate plugin support URL and Host redirects

TS-2690 Make the escalate plugin support URL and Host redirects

TS-2690 Support multiple status codes for each rule, e.g. 403,404:


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

Branch: refs/heads/master
Commit: 4b6c8e022de1b20371c334eec77d2ebd7c733ee2
Parents: a7b7d5f
Author: Leif Hedstrom <zw...@apache.org>
Authored: Wed Apr 9 13:29:23 2014 -0600
Committer: Leif Hedstrom <zw...@apache.org>
Committed: Thu Apr 10 06:50:21 2014 -0600

----------------------------------------------------------------------
 plugins/experimental/escalate/escalate.cc | 172 +++++++++++++++----------
 proxy/InkAPI.cc                           |   2 +-
 2 files changed, 108 insertions(+), 66 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4b6c8e02/plugins/experimental/escalate/escalate.cc
----------------------------------------------------------------------
diff --git a/plugins/experimental/escalate/escalate.cc b/plugins/experimental/escalate/escalate.cc
index 2f71041..6768a70 100644
--- a/plugins/experimental/escalate/escalate.cc
+++ b/plugins/experimental/escalate/escalate.cc
@@ -1,6 +1,6 @@
 /** @file
 
-  Escalation plugin.
+  This plugin allows retrying requests against different destinations.
 
   @section license License
 
@@ -20,7 +20,6 @@
   See the License for the specific language governing permissions and
   limitations under the License.
  */
-
 #include <ts/ts.h>
 #include <ts/remap.h>
 #include <ts/experimental.h>
@@ -32,94 +31,123 @@
 #include <map>
 
 
-// Constants
+// Constants and some declarations
 const char PLUGIN_NAME[] = "escalate";
-
-
 static int EscalateResponse(TSCont, TSEvent, void *);
 
+
+//////////////////////////////////////////////////////////////////////////////////////////
+// Hold information about the escalation / retry states for a remap rule.
+//
 struct EscalationState
 {
-  typedef std::map<unsigned, std::string> hostmap_type;
+  enum RetryType {
+    RETRY_URL,
+    RETRY_HOST
+  };
+
+  struct RetryInfo
+  {
+    RetryType type;
+    std::string target;
+  };
+
+  typedef std::map<unsigned, RetryInfo*> StatusMapType;
 
   EscalationState()
   {
-    handler = TSContCreate(EscalateResponse, NULL);
-    TSContDataSet(handler, this);
+    cont = TSContCreate(EscalateResponse, NULL);
+    TSContDataSet(cont, this);
   }
 
   ~EscalationState()
   {
-    TSContDestroy(handler);
+    for (StatusMapType::iterator iter = status_map.begin(); iter != status_map.end(); ++iter) {
+      delete(iter->second);
+    }
+    TSContDestroy(cont);
   }
 
-  TSCont       handler;
-  hostmap_type hostmap;
+  TSCont cont;
+  StatusMapType status_map;
 };
 
 
+//////////////////////////////////////////////////////////////////////////////////////////
+// Main continuation for the plugin, examining an origin response for a potential retry.
+//
 static int
 EscalateResponse(TSCont cont, TSEvent event, void* edata)
 {
+  TSHttpTxn txn = (TSHttpTxn)edata;
   EscalationState* es = static_cast<EscalationState*>(TSContDataGet(cont));
-  TSHttpTxn        txn = (TSHttpTxn)edata;
-  TSMBuffer        response;
-  TSMLoc           resp_hdr;
+  EscalationState::StatusMapType::iterator entry;
+  TSMBuffer mbuf;
+  TSMLoc hdrp, url;
+  TSHttpStatus status;
+  char* url_str = NULL;
+  int url_len, tries;
 
-  TSReleaseAssert(event == TS_EVENT_HTTP_READ_RESPONSE_HDR);
+  TSAssert(event == TS_EVENT_HTTP_READ_RESPONSE_HDR);
 
   // First, we need the server response ...
-  if (TS_SUCCESS == TSHttpTxnServerRespGet(txn, &response, &resp_hdr)) {
-    int tries = TSHttpTxnRedirectRetries(txn);
-
-    TSDebug(PLUGIN_NAME, "This is try %d", tries);
-    if (0 == tries) { // ToDo: Future support for more than one retry-URL
-      // Next, the response status ...
-      TSHttpStatus status = TSHttpHdrStatusGet(response, resp_hdr);
-
-      // If we have an escalation URL for this response code, set the redirection URL and force it
-      // to be followed.
-      EscalationState::hostmap_type::iterator entry = es->hostmap.find((unsigned)status);
-
-      if (entry != es->hostmap.end()) {
-        TSMBuffer request;
-        TSMLoc    req_hdr;
-
-        TSDebug(PLUGIN_NAME, "Found an entry for HTTP status %u", (unsigned)status);
-        if (TS_SUCCESS == TSHttpTxnClientReqGet(txn, &request, &req_hdr)) {
-          TSMLoc url;
-
-          if (TS_SUCCESS == TSHttpHdrUrlGet(request, req_hdr, &url)) {
-            char* url_str;
-            int url_len;
-
-            // Update the request URL with the new Host to try.
-            TSUrlHostSet(request, url, entry->second.c_str(), entry->second.size());
-            url_str = TSUrlStringGet(request, url, &url_len);
-
-            TSDebug(PLUGIN_NAME, "Setting new URL to %.*s", url_len, url_str);
-            TSHttpTxnRedirectUrlSet(txn, url_str, url_len); // Transfers ownership
-          }
-          // Release the request MLoc
-        TSHandleMLocRelease(request, TS_NULL_MLOC, req_hdr);
-        }
+  if (TS_SUCCESS != TSHttpTxnServerRespGet(txn, &mbuf, &hdrp)) {
+    goto no_action;
+  }
+
+  tries = TSHttpTxnRedirectRetries(txn);
+  if (0 != tries) { // ToDo: Future support for more than one retry-URL
+    goto no_action;
+  }
+  TSDebug(PLUGIN_NAME, "This is try %d, proceeding", tries);
+
+  // Next, the response status ...
+  status = TSHttpHdrStatusGet(mbuf, hdrp);
+  TSHandleMLocRelease(mbuf, TS_NULL_MLOC, hdrp);  // Don't need this any more
+
+  // See if we have an escalation retry config for this response code
+  entry  = es->status_map.find((unsigned)status);
+  if (entry == es->status_map.end()) {
+    goto no_action;
+  }
+
+  TSDebug(PLUGIN_NAME, "Found an entry for HTTP status %u", (unsigned)status);
+  if (EscalationState::RETRY_URL == entry->second->type) {
+    url_str = TSstrdup(entry->second->target.c_str());
+    url_len = entry->second->target.size();
+    TSDebug(PLUGIN_NAME, "Setting new URL to %.*s", url_len, url_str);
+  } else if (EscalationState::RETRY_HOST == entry->second->type) {
+    if (TS_SUCCESS == TSHttpTxnClientReqGet(txn, &mbuf, &hdrp)) {
+      if (TS_SUCCESS == TSHttpHdrUrlGet(mbuf, hdrp, &url)) {
+        // Update the request URL with the new Host to try.
+        TSUrlHostSet(mbuf, url, entry->second->target.c_str(), entry->second->target.size());
+        url_str = TSUrlStringGet(mbuf, url, &url_len);
+        TSDebug(PLUGIN_NAME, "Setting new Host: to %.*s", url_len, url_str);
       }
+      // Release the request MLoc
+      TSHandleMLocRelease(mbuf, TS_NULL_MLOC, hdrp);
     }
-    // Release the response MLoc
-    TSHandleMLocRelease(response, TS_NULL_MLOC, resp_hdr);
+  }
+
+  // Now update the Redirect URL, if set
+  if (url_str) {
+    TSHttpTxnRedirectUrlSet(txn, url_str, url_len); // Transfers ownership
   }
 
   // Set the transaction free ...
+ no_action:
   TSHttpTxnReenable(txn, TS_EVENT_HTTP_CONTINUE);
   return TS_EVENT_NONE;
 }
 
+
 TSReturnCode
 TSRemapInit(TSRemapInterface* /* api */, char* /* errbuf */, int /* bufsz */)
 {
   return TS_SUCCESS;
 }
 
+
 TSReturnCode
 TSRemapNewInstance(int argc, char* argv[], void** instance, char* errbuf, int errbuf_size)
 {
@@ -128,29 +156,41 @@ TSRemapNewInstance(int argc, char* argv[], void** instance, char* errbuf, int er
   // The first two arguments are the "from" and "to" URL string. We can just
   // skip those, since we only ever remap on the error path.
   for (int i = 2; i < argc; ++i) {
-    unsigned status;
-    char*    sep;
+    char *sep, *token, *save;
 
     // Each token should be a status code then a URL, separated by ':'.
     sep = strchr(argv[i], ':');
     if (sep == NULL) {
-      snprintf(errbuf, errbuf_size, "missing status code: %s", argv[i]);
+      snprintf(errbuf, errbuf_size, "malformed status:target config: %s", argv[i]);
       goto fail;
     }
 
     *sep = '\0';
-    status = strtol(argv[i], NULL, 10);
+    ++sep; // Skip over the ':' (which is now \0)
 
-    if (status < 100 || status > 599) {
-      snprintf(errbuf, errbuf_size, "invalid status code: %.*s", (int)std::distance(argv[i], sep), argv[i]);
-      goto fail;
+    // OK, we have a valid status/URL pair.
+    EscalationState::RetryInfo* info = new EscalationState::RetryInfo();
+
+    info->target = sep;
+    if (std::string::npos != info->target.find('/')) {
+      info->type = EscalationState::RETRY_URL;
+      TSDebug(PLUGIN_NAME, "Creating Redirect rule with URL = %s", sep);
+    } else {
+      info->type = EscalationState::RETRY_HOST;
+      TSDebug(PLUGIN_NAME, "Creating Redirect rule with Host = %s", sep);
     }
 
-    ++sep; // Skip over the ':'
+    for (token = strtok_r(argv[i], ",", &save); token; token = strtok_r(NULL, ",", &save)) {
+      unsigned status = strtol(token, NULL, 10);
 
-    // OK, we have a valid status/URL pair.
-    TSDebug(PLUGIN_NAME, "Redirect of HTTP status %u to %s", status, sep);
-    es->hostmap[status] = sep;
+      if (status < 100 || status > 599) {
+        snprintf(errbuf, errbuf_size, "invalid status code: %.*s", (int)std::distance(argv[i], sep), argv[i]);
+        goto fail;
+      }
+
+      TSDebug(PLUGIN_NAME, "      added status = %d to rule", status);
+      es->status_map[status] = info;
+    }
   }
 
   *instance = es;
@@ -161,17 +201,19 @@ fail:
   return TS_ERROR;
 }
 
+
 void
 TSRemapDeleteInstance(void* instance)
 {
-  delete (EscalationState *)instance;
+  delete static_cast<EscalationState*>(instance);
 }
 
+
 TSRemapStatus
 TSRemapDoRemap(void* instance, TSHttpTxn txn, TSRemapRequestInfo* /* rri */)
 {
-  EscalationState* es = static_cast<EscalationState *>(instance);
+  EscalationState* es = static_cast<EscalationState*>(instance);
 
-  TSHttpTxnHookAdd(txn, TS_HTTP_READ_RESPONSE_HDR_HOOK, es->handler);
+  TSHttpTxnHookAdd(txn, TS_HTTP_READ_RESPONSE_HDR_HOOK, es->cont);
   return TSREMAP_NO_REMAP;
 }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4b6c8e02/proxy/InkAPI.cc
----------------------------------------------------------------------
diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc
index cc4c4dd..2aa4cb5 100644
--- a/proxy/InkAPI.cc
+++ b/proxy/InkAPI.cc
@@ -7129,7 +7129,7 @@ TSHttpTxnRedirectUrlSet(TSHttpTxn txnp, const char* url, const int url_len)
   HttpSM *sm = (HttpSM*) txnp;
 
   if (sm->redirect_url != NULL) {
-    ats_free((void*)sm->redirect_url);
+    ats_free(sm->redirect_url);
     sm->redirect_url = NULL;
     sm->redirect_url_len = 0;
   }


Re: [2/6] git commit: TS-2690 Make the escalate plugin support URL and Host redirects

Posted by James Peach <jp...@apache.org>.
On Apr 10, 2014, at 5:54 AM, zwoop@apache.org wrote:

> TS-2690 Make the escalate plugin support URL and Host redirects
> 
> TS-2690 Support multiple status codes for each rule, e.g. 403,404:
> 
> 
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/4b6c8e02
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/4b6c8e02
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/4b6c8e02
> 
> Branch: refs/heads/master
> Commit: 4b6c8e022de1b20371c334eec77d2ebd7c733ee2
> Parents: a7b7d5f
> Author: Leif Hedstrom <zw...@apache.org>
> Authored: Wed Apr 9 13:29:23 2014 -0600
> Committer: Leif Hedstrom <zw...@apache.org>
> Committed: Thu Apr 10 06:50:21 2014 -0600
> 
> ----------------------------------------------------------------------
> plugins/experimental/escalate/escalate.cc | 172 +++++++++++++++----------
> proxy/InkAPI.cc                           |   2 +-
> 2 files changed, 108 insertions(+), 66 deletions(-)
> ----------------------------------------------------------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4b6c8e02/plugins/experimental/escalate/escalate.cc
> ----------------------------------------------------------------------
> diff --git a/plugins/experimental/escalate/escalate.cc b/plugins/experimental/escalate/escalate.cc
> index 2f71041..6768a70 100644
> --- a/plugins/experimental/escalate/escalate.cc
> +++ b/plugins/experimental/escalate/escalate.cc
> @@ -1,6 +1,6 @@
> /** @file
> 
> -  Escalation plugin.
> +  This plugin allows retrying requests against different destinations.
> 
>   @section license License
> 
> @@ -20,7 +20,6 @@
>   See the License for the specific language governing permissions and
>   limitations under the License.
>  */
> -
> #include <ts/ts.h>
> #include <ts/remap.h>
> #include <ts/experimental.h>
> @@ -32,94 +31,123 @@
> #include <map>
> 
> 
> -// Constants
> +// Constants and some declarations
> const char PLUGIN_NAME[] = "escalate";
> -
> -
> static int EscalateResponse(TSCont, TSEvent, void *);
> 
> +
> +//////////////////////////////////////////////////////////////////////////////////////////
> +// Hold information about the escalation / retry states for a remap rule.
> +//
> struct EscalationState
> {
> -  typedef std::map<unsigned, std::string> hostmap_type;
> +  enum RetryType {
> +    RETRY_URL,
> +    RETRY_HOST
> +  };
> +
> +  struct RetryInfo
> +  {
> +    RetryType type;
> +    std::string target;
> +  };
> +
> +  typedef std::map<unsigned, RetryInfo*> StatusMapType;

It would be simpler to map RetryInfo by value so you don't need to manually destroy it:
	typedef std::map<unsigned, RetryInfo> StatusMapType.

> 
>   EscalationState()
>   {
> -    handler = TSContCreate(EscalateResponse, NULL);
> -    TSContDataSet(handler, this);
> +    cont = TSContCreate(EscalateResponse, NULL);
> +    TSContDataSet(cont, this);
>   }
> 
>   ~EscalationState()
>   {
> -    TSContDestroy(handler);
> +    for (StatusMapType::iterator iter = status_map.begin(); iter != status_map.end(); ++iter) {
> +      delete(iter->second);
> +    }
> +    TSContDestroy(cont);
>   }
> 
> -  TSCont       handler;
> -  hostmap_type hostmap;
> +  TSCont cont;
> +  StatusMapType status_map;
> };
> 
> 
> +//////////////////////////////////////////////////////////////////////////////////////////
> +// Main continuation for the plugin, examining an origin response for a potential retry.
> +//
> static int
> EscalateResponse(TSCont cont, TSEvent event, void* edata)
> {
> +  TSHttpTxn txn = (TSHttpTxn)edata;
>   EscalationState* es = static_cast<EscalationState*>(TSContDataGet(cont));
> -  TSHttpTxn        txn = (TSHttpTxn)edata;
> -  TSMBuffer        response;
> -  TSMLoc           resp_hdr;
> +  EscalationState::StatusMapType::iterator entry;
> +  TSMBuffer mbuf;
> +  TSMLoc hdrp, url;
> +  TSHttpStatus status;
> +  char* url_str = NULL;
> +  int url_len, tries;
> 
> -  TSReleaseAssert(event == TS_EVENT_HTTP_READ_RESPONSE_HDR);
> +  TSAssert(event == TS_EVENT_HTTP_READ_RESPONSE_HDR);
> 
>   // First, we need the server response ...
> -  if (TS_SUCCESS == TSHttpTxnServerRespGet(txn, &response, &resp_hdr)) {
> -    int tries = TSHttpTxnRedirectRetries(txn);
> -
> -    TSDebug(PLUGIN_NAME, "This is try %d", tries);
> -    if (0 == tries) { // ToDo: Future support for more than one retry-URL
> -      // Next, the response status ...
> -      TSHttpStatus status = TSHttpHdrStatusGet(response, resp_hdr);
> -
> -      // If we have an escalation URL for this response code, set the redirection URL and force it
> -      // to be followed.
> -      EscalationState::hostmap_type::iterator entry = es->hostmap.find((unsigned)status);
> -
> -      if (entry != es->hostmap.end()) {
> -        TSMBuffer request;
> -        TSMLoc    req_hdr;
> -
> -        TSDebug(PLUGIN_NAME, "Found an entry for HTTP status %u", (unsigned)status);
> -        if (TS_SUCCESS == TSHttpTxnClientReqGet(txn, &request, &req_hdr)) {
> -          TSMLoc url;
> -
> -          if (TS_SUCCESS == TSHttpHdrUrlGet(request, req_hdr, &url)) {
> -            char* url_str;
> -            int url_len;
> -
> -            // Update the request URL with the new Host to try.
> -            TSUrlHostSet(request, url, entry->second.c_str(), entry->second.size());
> -            url_str = TSUrlStringGet(request, url, &url_len);
> -
> -            TSDebug(PLUGIN_NAME, "Setting new URL to %.*s", url_len, url_str);
> -            TSHttpTxnRedirectUrlSet(txn, url_str, url_len); // Transfers ownership
> -          }
> -          // Release the request MLoc
> -        TSHandleMLocRelease(request, TS_NULL_MLOC, req_hdr);
> -        }
> +  if (TS_SUCCESS != TSHttpTxnServerRespGet(txn, &mbuf, &hdrp)) {
> +    goto no_action;
> +  }
> +
> +  tries = TSHttpTxnRedirectRetries(txn);
> +  if (0 != tries) { // ToDo: Future support for more than one retry-URL
> +    goto no_action;
> +  }
> +  TSDebug(PLUGIN_NAME, "This is try %d, proceeding", tries);
> +
> +  // Next, the response status ...
> +  status = TSHttpHdrStatusGet(mbuf, hdrp);
> +  TSHandleMLocRelease(mbuf, TS_NULL_MLOC, hdrp);  // Don't need this any more
> +
> +  // See if we have an escalation retry config for this response code
> +  entry  = es->status_map.find((unsigned)status);
> +  if (entry == es->status_map.end()) {
> +    goto no_action;
> +  }
> +
> +  TSDebug(PLUGIN_NAME, "Found an entry for HTTP status %u", (unsigned)status);
> +  if (EscalationState::RETRY_URL == entry->second->type) {
> +    url_str = TSstrdup(entry->second->target.c_str());
> +    url_len = entry->second->target.size();
> +    TSDebug(PLUGIN_NAME, "Setting new URL to %.*s", url_len, url_str);
> +  } else if (EscalationState::RETRY_HOST == entry->second->type) {
> +    if (TS_SUCCESS == TSHttpTxnClientReqGet(txn, &mbuf, &hdrp)) {
> +      if (TS_SUCCESS == TSHttpHdrUrlGet(mbuf, hdrp, &url)) {
> +        // Update the request URL with the new Host to try.
> +        TSUrlHostSet(mbuf, url, entry->second->target.c_str(), entry->second->target.size());
> +        url_str = TSUrlStringGet(mbuf, url, &url_len);
> +        TSDebug(PLUGIN_NAME, "Setting new Host: to %.*s", url_len, url_str);
>       }
> +      // Release the request MLoc
> +      TSHandleMLocRelease(mbuf, TS_NULL_MLOC, hdrp);
>     }
> -    // Release the response MLoc
> -    TSHandleMLocRelease(response, TS_NULL_MLOC, resp_hdr);
> +  }
> +
> +  // Now update the Redirect URL, if set
> +  if (url_str) {
> +    TSHttpTxnRedirectUrlSet(txn, url_str, url_len); // Transfers ownership
>   }
> 
>   // Set the transaction free ...
> + no_action:
>   TSHttpTxnReenable(txn, TS_EVENT_HTTP_CONTINUE);
>   return TS_EVENT_NONE;
> }
> 
> +
> TSReturnCode
> TSRemapInit(TSRemapInterface* /* api */, char* /* errbuf */, int /* bufsz */)
> {
>   return TS_SUCCESS;
> }
> 
> +
> TSReturnCode
> TSRemapNewInstance(int argc, char* argv[], void** instance, char* errbuf, int errbuf_size)
> {
> @@ -128,29 +156,41 @@ TSRemapNewInstance(int argc, char* argv[], void** instance, char* errbuf, int er
>   // The first two arguments are the "from" and "to" URL string. We can just
>   // skip those, since we only ever remap on the error path.
>   for (int i = 2; i < argc; ++i) {
> -    unsigned status;
> -    char*    sep;
> +    char *sep, *token, *save;
> 
>     // Each token should be a status code then a URL, separated by ':'.
>     sep = strchr(argv[i], ':');
>     if (sep == NULL) {
> -      snprintf(errbuf, errbuf_size, "missing status code: %s", argv[i]);
> +      snprintf(errbuf, errbuf_size, "malformed status:target config: %s", argv[i]);
>       goto fail;
>     }
> 
>     *sep = '\0';
> -    status = strtol(argv[i], NULL, 10);
> +    ++sep; // Skip over the ':' (which is now \0)
> 
> -    if (status < 100 || status > 599) {
> -      snprintf(errbuf, errbuf_size, "invalid status code: %.*s", (int)std::distance(argv[i], sep), argv[i]);
> -      goto fail;
> +    // OK, we have a valid status/URL pair.
> +    EscalationState::RetryInfo* info = new EscalationState::RetryInfo();
> +
> +    info->target = sep;
> +    if (std::string::npos != info->target.find('/')) {
> +      info->type = EscalationState::RETRY_URL;
> +      TSDebug(PLUGIN_NAME, "Creating Redirect rule with URL = %s", sep);
> +    } else {
> +      info->type = EscalationState::RETRY_HOST;
> +      TSDebug(PLUGIN_NAME, "Creating Redirect rule with Host = %s", sep);
>     }
> 
> -    ++sep; // Skip over the ':'
> +    for (token = strtok_r(argv[i], ",", &save); token; token = strtok_r(NULL, ",", &save)) {

You should copy the argument before whacking it with strtok.

> +      unsigned status = strtol(token, NULL, 10);
> 
> -    // OK, we have a valid status/URL pair.
> -    TSDebug(PLUGIN_NAME, "Redirect of HTTP status %u to %s", status, sep);
> -    es->hostmap[status] = sep;
> +      if (status < 100 || status > 599) {
> +        snprintf(errbuf, errbuf_size, "invalid status code: %.*s", (int)std::distance(argv[i], sep), argv[i]);
> +        goto fail;
> +      }
> +
> +      TSDebug(PLUGIN_NAME, "      added status = %d to rule", status);
> +      es->status_map[status] = info;
> +    }
>   }
> 
>   *instance = es;
> @@ -161,17 +201,19 @@ fail:
>   return TS_ERROR;
> }
> 
> +
> void
> TSRemapDeleteInstance(void* instance)
> {
> -  delete (EscalationState *)instance;
> +  delete static_cast<EscalationState*>(instance);
> }
> 
> +
> TSRemapStatus
> TSRemapDoRemap(void* instance, TSHttpTxn txn, TSRemapRequestInfo* /* rri */)
> {
> -  EscalationState* es = static_cast<EscalationState *>(instance);
> +  EscalationState* es = static_cast<EscalationState*>(instance);
> 
> -  TSHttpTxnHookAdd(txn, TS_HTTP_READ_RESPONSE_HDR_HOOK, es->handler);
> +  TSHttpTxnHookAdd(txn, TS_HTTP_READ_RESPONSE_HDR_HOOK, es->cont);
>   return TSREMAP_NO_REMAP;
> }
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4b6c8e02/proxy/InkAPI.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc
> index cc4c4dd..2aa4cb5 100644
> --- a/proxy/InkAPI.cc
> +++ b/proxy/InkAPI.cc
> @@ -7129,7 +7129,7 @@ TSHttpTxnRedirectUrlSet(TSHttpTxn txnp, const char* url, const int url_len)
>   HttpSM *sm = (HttpSM*) txnp;
> 
>   if (sm->redirect_url != NULL) {

You don't need the NULL check here.

> -    ats_free((void*)sm->redirect_url);
> +    ats_free(sm->redirect_url);
>     sm->redirect_url = NULL;
>     sm->redirect_url_len = 0;
>   }
> 


Re: [2/6] git commit: TS-2690 Make the escalate plugin support URL and Host redirects

Posted by James Peach <jp...@apache.org>.
On Apr 10, 2014, at 5:54 AM, zwoop@apache.org wrote:

> TS-2690 Make the escalate plugin support URL and Host redirects
> 
> TS-2690 Support multiple status codes for each rule, e.g. 403,404:
> 
> 
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/4b6c8e02
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/4b6c8e02
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/4b6c8e02
> 
> Branch: refs/heads/master
> Commit: 4b6c8e022de1b20371c334eec77d2ebd7c733ee2
> Parents: a7b7d5f
> Author: Leif Hedstrom <zw...@apache.org>
> Authored: Wed Apr 9 13:29:23 2014 -0600
> Committer: Leif Hedstrom <zw...@apache.org>
> Committed: Thu Apr 10 06:50:21 2014 -0600
> 
> ----------------------------------------------------------------------
> plugins/experimental/escalate/escalate.cc | 172 +++++++++++++++----------
> proxy/InkAPI.cc                           |   2 +-
> 2 files changed, 108 insertions(+), 66 deletions(-)
> ----------------------------------------------------------------------
> 
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4b6c8e02/plugins/experimental/escalate/escalate.cc
> ----------------------------------------------------------------------
> diff --git a/plugins/experimental/escalate/escalate.cc b/plugins/experimental/escalate/escalate.cc
> index 2f71041..6768a70 100644
> --- a/plugins/experimental/escalate/escalate.cc
> +++ b/plugins/experimental/escalate/escalate.cc
> @@ -1,6 +1,6 @@
> /** @file
> 
> -  Escalation plugin.
> +  This plugin allows retrying requests against different destinations.
> 
>   @section license License
> 
> @@ -20,7 +20,6 @@
>   See the License for the specific language governing permissions and
>   limitations under the License.
>  */
> -
> #include <ts/ts.h>
> #include <ts/remap.h>
> #include <ts/experimental.h>
> @@ -32,94 +31,123 @@
> #include <map>
> 
> 
> -// Constants
> +// Constants and some declarations
> const char PLUGIN_NAME[] = "escalate";
> -
> -
> static int EscalateResponse(TSCont, TSEvent, void *);
> 
> +
> +//////////////////////////////////////////////////////////////////////////////////////////
> +// Hold information about the escalation / retry states for a remap rule.
> +//
> struct EscalationState
> {
> -  typedef std::map<unsigned, std::string> hostmap_type;
> +  enum RetryType {
> +    RETRY_URL,
> +    RETRY_HOST
> +  };
> +
> +  struct RetryInfo
> +  {
> +    RetryType type;
> +    std::string target;
> +  };
> +
> +  typedef std::map<unsigned, RetryInfo*> StatusMapType;

It would be simpler to map RetryInfo by value so you don't need to manually destroy it:
	typedef std::map<unsigned, RetryInfo> StatusMapType.

> 
>   EscalationState()
>   {
> -    handler = TSContCreate(EscalateResponse, NULL);
> -    TSContDataSet(handler, this);
> +    cont = TSContCreate(EscalateResponse, NULL);
> +    TSContDataSet(cont, this);
>   }
> 
>   ~EscalationState()
>   {
> -    TSContDestroy(handler);
> +    for (StatusMapType::iterator iter = status_map.begin(); iter != status_map.end(); ++iter) {
> +      delete(iter->second);
> +    }
> +    TSContDestroy(cont);
>   }
> 
> -  TSCont       handler;
> -  hostmap_type hostmap;
> +  TSCont cont;
> +  StatusMapType status_map;
> };
> 
> 
> +//////////////////////////////////////////////////////////////////////////////////////////
> +// Main continuation for the plugin, examining an origin response for a potential retry.
> +//
> static int
> EscalateResponse(TSCont cont, TSEvent event, void* edata)
> {
> +  TSHttpTxn txn = (TSHttpTxn)edata;
>   EscalationState* es = static_cast<EscalationState*>(TSContDataGet(cont));
> -  TSHttpTxn        txn = (TSHttpTxn)edata;
> -  TSMBuffer        response;
> -  TSMLoc           resp_hdr;
> +  EscalationState::StatusMapType::iterator entry;
> +  TSMBuffer mbuf;
> +  TSMLoc hdrp, url;
> +  TSHttpStatus status;
> +  char* url_str = NULL;
> +  int url_len, tries;
> 
> -  TSReleaseAssert(event == TS_EVENT_HTTP_READ_RESPONSE_HDR);
> +  TSAssert(event == TS_EVENT_HTTP_READ_RESPONSE_HDR);
> 
>   // First, we need the server response ...
> -  if (TS_SUCCESS == TSHttpTxnServerRespGet(txn, &response, &resp_hdr)) {
> -    int tries = TSHttpTxnRedirectRetries(txn);
> -
> -    TSDebug(PLUGIN_NAME, "This is try %d", tries);
> -    if (0 == tries) { // ToDo: Future support for more than one retry-URL
> -      // Next, the response status ...
> -      TSHttpStatus status = TSHttpHdrStatusGet(response, resp_hdr);
> -
> -      // If we have an escalation URL for this response code, set the redirection URL and force it
> -      // to be followed.
> -      EscalationState::hostmap_type::iterator entry = es->hostmap.find((unsigned)status);
> -
> -      if (entry != es->hostmap.end()) {
> -        TSMBuffer request;
> -        TSMLoc    req_hdr;
> -
> -        TSDebug(PLUGIN_NAME, "Found an entry for HTTP status %u", (unsigned)status);
> -        if (TS_SUCCESS == TSHttpTxnClientReqGet(txn, &request, &req_hdr)) {
> -          TSMLoc url;
> -
> -          if (TS_SUCCESS == TSHttpHdrUrlGet(request, req_hdr, &url)) {
> -            char* url_str;
> -            int url_len;
> -
> -            // Update the request URL with the new Host to try.
> -            TSUrlHostSet(request, url, entry->second.c_str(), entry->second.size());
> -            url_str = TSUrlStringGet(request, url, &url_len);
> -
> -            TSDebug(PLUGIN_NAME, "Setting new URL to %.*s", url_len, url_str);
> -            TSHttpTxnRedirectUrlSet(txn, url_str, url_len); // Transfers ownership
> -          }
> -          // Release the request MLoc
> -        TSHandleMLocRelease(request, TS_NULL_MLOC, req_hdr);
> -        }
> +  if (TS_SUCCESS != TSHttpTxnServerRespGet(txn, &mbuf, &hdrp)) {
> +    goto no_action;
> +  }
> +
> +  tries = TSHttpTxnRedirectRetries(txn);
> +  if (0 != tries) { // ToDo: Future support for more than one retry-URL
> +    goto no_action;
> +  }
> +  TSDebug(PLUGIN_NAME, "This is try %d, proceeding", tries);
> +
> +  // Next, the response status ...
> +  status = TSHttpHdrStatusGet(mbuf, hdrp);
> +  TSHandleMLocRelease(mbuf, TS_NULL_MLOC, hdrp);  // Don't need this any more
> +
> +  // See if we have an escalation retry config for this response code
> +  entry  = es->status_map.find((unsigned)status);
> +  if (entry == es->status_map.end()) {
> +    goto no_action;
> +  }
> +
> +  TSDebug(PLUGIN_NAME, "Found an entry for HTTP status %u", (unsigned)status);
> +  if (EscalationState::RETRY_URL == entry->second->type) {
> +    url_str = TSstrdup(entry->second->target.c_str());
> +    url_len = entry->second->target.size();
> +    TSDebug(PLUGIN_NAME, "Setting new URL to %.*s", url_len, url_str);
> +  } else if (EscalationState::RETRY_HOST == entry->second->type) {
> +    if (TS_SUCCESS == TSHttpTxnClientReqGet(txn, &mbuf, &hdrp)) {
> +      if (TS_SUCCESS == TSHttpHdrUrlGet(mbuf, hdrp, &url)) {
> +        // Update the request URL with the new Host to try.
> +        TSUrlHostSet(mbuf, url, entry->second->target.c_str(), entry->second->target.size());
> +        url_str = TSUrlStringGet(mbuf, url, &url_len);
> +        TSDebug(PLUGIN_NAME, "Setting new Host: to %.*s", url_len, url_str);
>       }
> +      // Release the request MLoc
> +      TSHandleMLocRelease(mbuf, TS_NULL_MLOC, hdrp);
>     }
> -    // Release the response MLoc
> -    TSHandleMLocRelease(response, TS_NULL_MLOC, resp_hdr);
> +  }
> +
> +  // Now update the Redirect URL, if set
> +  if (url_str) {
> +    TSHttpTxnRedirectUrlSet(txn, url_str, url_len); // Transfers ownership
>   }
> 
>   // Set the transaction free ...
> + no_action:
>   TSHttpTxnReenable(txn, TS_EVENT_HTTP_CONTINUE);
>   return TS_EVENT_NONE;
> }
> 
> +
> TSReturnCode
> TSRemapInit(TSRemapInterface* /* api */, char* /* errbuf */, int /* bufsz */)
> {
>   return TS_SUCCESS;
> }
> 
> +
> TSReturnCode
> TSRemapNewInstance(int argc, char* argv[], void** instance, char* errbuf, int errbuf_size)
> {
> @@ -128,29 +156,41 @@ TSRemapNewInstance(int argc, char* argv[], void** instance, char* errbuf, int er
>   // The first two arguments are the "from" and "to" URL string. We can just
>   // skip those, since we only ever remap on the error path.
>   for (int i = 2; i < argc; ++i) {
> -    unsigned status;
> -    char*    sep;
> +    char *sep, *token, *save;
> 
>     // Each token should be a status code then a URL, separated by ':'.
>     sep = strchr(argv[i], ':');
>     if (sep == NULL) {
> -      snprintf(errbuf, errbuf_size, "missing status code: %s", argv[i]);
> +      snprintf(errbuf, errbuf_size, "malformed status:target config: %s", argv[i]);
>       goto fail;
>     }
> 
>     *sep = '\0';
> -    status = strtol(argv[i], NULL, 10);
> +    ++sep; // Skip over the ':' (which is now \0)
> 
> -    if (status < 100 || status > 599) {
> -      snprintf(errbuf, errbuf_size, "invalid status code: %.*s", (int)std::distance(argv[i], sep), argv[i]);
> -      goto fail;
> +    // OK, we have a valid status/URL pair.
> +    EscalationState::RetryInfo* info = new EscalationState::RetryInfo();
> +
> +    info->target = sep;
> +    if (std::string::npos != info->target.find('/')) {
> +      info->type = EscalationState::RETRY_URL;
> +      TSDebug(PLUGIN_NAME, "Creating Redirect rule with URL = %s", sep);
> +    } else {
> +      info->type = EscalationState::RETRY_HOST;
> +      TSDebug(PLUGIN_NAME, "Creating Redirect rule with Host = %s", sep);
>     }
> 
> -    ++sep; // Skip over the ':'
> +    for (token = strtok_r(argv[i], ",", &save); token; token = strtok_r(NULL, ",", &save)) {

You should copy the argument before whacking it with strtok.

> +      unsigned status = strtol(token, NULL, 10);
> 
> -    // OK, we have a valid status/URL pair.
> -    TSDebug(PLUGIN_NAME, "Redirect of HTTP status %u to %s", status, sep);
> -    es->hostmap[status] = sep;
> +      if (status < 100 || status > 599) {
> +        snprintf(errbuf, errbuf_size, "invalid status code: %.*s", (int)std::distance(argv[i], sep), argv[i]);
> +        goto fail;
> +      }
> +
> +      TSDebug(PLUGIN_NAME, "      added status = %d to rule", status);
> +      es->status_map[status] = info;
> +    }
>   }
> 
>   *instance = es;
> @@ -161,17 +201,19 @@ fail:
>   return TS_ERROR;
> }
> 
> +
> void
> TSRemapDeleteInstance(void* instance)
> {
> -  delete (EscalationState *)instance;
> +  delete static_cast<EscalationState*>(instance);
> }
> 
> +
> TSRemapStatus
> TSRemapDoRemap(void* instance, TSHttpTxn txn, TSRemapRequestInfo* /* rri */)
> {
> -  EscalationState* es = static_cast<EscalationState *>(instance);
> +  EscalationState* es = static_cast<EscalationState*>(instance);
> 
> -  TSHttpTxnHookAdd(txn, TS_HTTP_READ_RESPONSE_HDR_HOOK, es->handler);
> +  TSHttpTxnHookAdd(txn, TS_HTTP_READ_RESPONSE_HDR_HOOK, es->cont);
>   return TSREMAP_NO_REMAP;
> }
> 
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/4b6c8e02/proxy/InkAPI.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc
> index cc4c4dd..2aa4cb5 100644
> --- a/proxy/InkAPI.cc
> +++ b/proxy/InkAPI.cc
> @@ -7129,7 +7129,7 @@ TSHttpTxnRedirectUrlSet(TSHttpTxn txnp, const char* url, const int url_len)
>   HttpSM *sm = (HttpSM*) txnp;
> 
>   if (sm->redirect_url != NULL) {

You don't need the NULL check here.

> -    ats_free((void*)sm->redirect_url);
> +    ats_free(sm->redirect_url);
>     sm->redirect_url = NULL;
>     sm->redirect_url_len = 0;
>   }
>