trafficserver-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Peach <jpe...@apache.org>
Subject Re: [2/6] git commit: TS-2690 Make the escalate plugin support URL and Host redirects
Date Thu, 10 Apr 2014 16:55:01 GMT
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 <zwoop@apache.org>
> Authored: Wed Apr 9 13:29:23 2014 -0600
> Committer: Leif Hedstrom <zwoop@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;
>   }
> 


Mime
View raw message