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:21 UTC

[1/6] git commit: TS-2691 Fix how we count redirect retries

Repository: trafficserver
Updated Branches:
  refs/heads/master 2ab1bb6ac -> 4b6c8e022


TS-2691 Fix how we count redirect retries

TS-2691 Also eliminate api_enable_redirection which seems superfluous


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

Branch: refs/heads/master
Commit: dfc2a8f8dba53cd0ab541753ac192eb06bba2fa4
Parents: 2ab1bb6
Author: Leif Hedstrom <zw...@apache.org>
Authored: Wed Apr 2 10:21:57 2014 -0600
Committer: Leif Hedstrom <zw...@apache.org>
Committed: Thu Apr 10 06:50:20 2014 -0600

----------------------------------------------------------------------
 proxy/InkAPI.cc           | 33 +++++++++++++++------------------
 proxy/http/HttpCacheSM.cc |  4 ++--
 proxy/http/HttpConfig.h   |  2 +-
 proxy/http/HttpSM.cc      | 22 +++++-----------------
 proxy/http/HttpSM.h       |  1 -
 proxy/http/HttpTunnel.cc  |  3 +--
 6 files changed, 24 insertions(+), 41 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/dfc2a8f8/proxy/InkAPI.cc
----------------------------------------------------------------------
diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc
index 241ab45..68d31d0 100644
--- a/proxy/InkAPI.cc
+++ b/proxy/InkAPI.cc
@@ -4938,17 +4938,6 @@ TSHttpTxnSecondUrlTryLock(TSHttpTxn txnp)
 }
 
 TSReturnCode
-TSHttpTxnFollowRedirect(TSHttpTxn txnp, int on)
-{
-  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
-
-  HttpSM *sm = (HttpSM *) txnp;
-
-  sm->api_enable_redirection = (on ? true : false);
-  return TS_SUCCESS;
-}
-
-TSReturnCode
 TSHttpTxnRedirectRequest(TSHttpTxn txnp, TSMBuffer bufp, TSMLoc url_loc)
 {
   sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
@@ -7116,6 +7105,20 @@ TSCacheHttpInfoSizeSet(TSCacheHttpInfo infop, int64_t size)
   info->object_size_set(size);
 }
 
+// This API tells the core to follow normal (301/302) redirects using the
+// standard Location: URL. This does not need to be called if you set an
+// explicit URL using TSRedirectUrlSet().
+TSReturnCode
+TSHttpTxnFollowRedirect(TSHttpTxn txnp, int on)
+{
+  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
+
+  HttpSM *sm = (HttpSM *) txnp;
+
+  sm->enable_redirection = (on ? true : false);
+  return TS_SUCCESS;
+}
+
 // this function should be called at TS_EVENT_HTTP_READ_RESPONSE_HDR
 void
 TSRedirectUrlSet(TSHttpTxn txnp, const char* url, const int url_len)
@@ -7134,13 +7137,7 @@ TSRedirectUrlSet(TSHttpTxn txnp, const char* url, const int url_len)
   sm->redirect_url = (char*)ats_malloc(url_len + 1);
   ink_strlcpy(sm->redirect_url, (char*)url, url_len + 1);
   sm->redirect_url_len = url_len;
-  // have to turn redirection on for this transaction if user wants to redirect to another URL
-  if (sm->enable_redirection == false) {
-    sm->enable_redirection = true;
-    // max-out "redirection_tries" to avoid the regular redirection being turned on in
-    // this transaction improperly. This variable doesn't affect the custom-redirection
-    sm->redirection_tries = HttpConfig::m_master.number_of_redirections;
-  }
+  sm->enable_redirection = true;
 }
 
 const char*

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/dfc2a8f8/proxy/http/HttpCacheSM.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpCacheSM.cc b/proxy/http/HttpCacheSM.cc
index 7f82dd5..a684f00 100644
--- a/proxy/http/HttpCacheSM.cc
+++ b/proxy/http/HttpCacheSM.cc
@@ -292,8 +292,8 @@ HttpCacheSM::open_write(URL * url, HTTPHdr * request, CacheHTTPInfo * old_info,
   //  a new write (could happen on a very busy document
   //  that must be revalidated every time)
   // Changed by YTS Team, yamsat Plugin
-  if (open_write_tries > master_sm->redirection_tries && open_write_tries >
-      master_sm->t_state.http_config_param->max_cache_open_write_retries) {
+  if (open_write_tries > master_sm->redirection_tries &&
+      open_write_tries > master_sm->t_state.http_config_param->max_cache_open_write_retries) {
     master_sm->handleEvent(CACHE_EVENT_OPEN_WRITE_FAILED, (void *) -ECACHE_DOC_BUSY);
     return ACTION_RESULT_DONE;
   }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/dfc2a8f8/proxy/http/HttpConfig.h
----------------------------------------------------------------------
diff --git a/proxy/http/HttpConfig.h b/proxy/http/HttpConfig.h
index 064f4c6..343148f 100644
--- a/proxy/http/HttpConfig.h
+++ b/proxy/http/HttpConfig.h
@@ -929,7 +929,7 @@ HttpConfigParams::HttpConfigParams()
     errors_log_error_pages(1),
     enable_http_info(0),
     cluster_time_delta(0),
-    redirection_enabled(1),
+    redirection_enabled(0),
     number_of_redirections(1),
     post_copy_size(2048),
     ignore_accept_mismatch(0),

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/dfc2a8f8/proxy/http/HttpSM.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 36135be..fc4f9bc 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -302,8 +302,8 @@ static int next_sm_id = 0;
 HttpSM::HttpSM()
   : Continuation(NULL), proto_stack(1u << TS_PROTO_HTTP), sm_id(-1), magic(HTTP_SM_MAGIC_DEAD),
     //YTS Team, yamsat Plugin
-    enable_redirection(false), api_enable_redirection(true), redirect_url(NULL), redirect_url_len(0), redirection_tries(0), transfered_bytes(0),
-    post_failed(false), debug_on(false),
+    enable_redirection(false), redirect_url(NULL), redirect_url_len(0), redirection_tries(0),
+    transfered_bytes(0), post_failed(false), debug_on(false),
     plugin_tunnel_type(HTTP_NO_PLUGIN_TUNNEL),
     plugin_tunnel(NULL), reentrancy_count(0),
     history_pos(0), tunnel(), ua_entry(NULL),
@@ -373,12 +373,7 @@ HttpSM::init()
   milestones.sm_start = ink_get_hrtime();
 
   magic = HTTP_SM_MAGIC_ALIVE;
-
   sm_id = 0;
-  enable_redirection = false;
-  api_enable_redirection = true;
-  redirect_url = NULL;
-  redirect_url_len = 0;
 
   // Unique state machine identifier.
   //  changed next_sm_id from int64_t to int because
@@ -1837,11 +1832,9 @@ HttpSM::state_read_server_response_header(int event, void *data)
     t_state.transact_return_point = HttpTransact::HandleResponse;
     t_state.api_next_action = HttpTransact::SM_ACTION_API_READ_RESPONSE_HDR;
 
-    // YTS Team, yamsat Plugin
-    // Incrementing redirection_tries according to config parameter
     // if exceeded limit deallocate postdata buffers and disable redirection
-    if (enable_redirection && (redirection_tries <= HttpConfig::m_master.number_of_redirections)) {
-      redirection_tries++;
+    if (enable_redirection && (redirection_tries < HttpConfig::m_master.number_of_redirections)) {
+      ++redirection_tries;
     } else {
       tunnel.deallocate_redirect_postdata_buffers();
       enable_redirection = false;
@@ -7267,12 +7260,7 @@ void
 HttpSM::do_redirect()
 {
   DebugSM("http_redirect", "[HttpSM::do_redirect]");
-  if (enable_redirection == false || redirection_tries > (HttpConfig::m_master.number_of_redirections)) {
-    tunnel.deallocate_redirect_postdata_buffers();
-    return;
-  }
-
-  if (api_enable_redirection == false) {
+  if (!enable_redirection || redirection_tries >= HttpConfig::m_master.number_of_redirections) {
     tunnel.deallocate_redirect_postdata_buffers();
     return;
   }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/dfc2a8f8/proxy/http/HttpSM.h
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.h b/proxy/http/HttpSM.h
index 0a0a23e..0a7b116 100644
--- a/proxy/http/HttpSM.h
+++ b/proxy/http/HttpSM.h
@@ -270,7 +270,6 @@ public:
 
   //YTS Team, yamsat Plugin
   bool enable_redirection;      //To check if redirection is enabled
-  bool api_enable_redirection;  //To check if redirection is enabled
   char *redirect_url;           //url for force redirect (provide users a functionality to redirect to another url when needed)
   int redirect_url_len;
   int redirection_tries;        //To monitor number of redirections

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/dfc2a8f8/proxy/http/HttpTunnel.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc
index 748d397..3e3baf2 100644
--- a/proxy/http/HttpTunnel.cc
+++ b/proxy/http/HttpTunnel.cc
@@ -903,8 +903,7 @@ HttpTunnel::producer_run(HttpTunnelProducer * p)
   //YTS Team, yamsat Plugin
   // Allocate and copy partial POST data to buffers. Check for the various parameters
   // including the maximum configured post data size
-  if (p->alive && sm->t_state.method == HTTP_WKSIDX_POST && sm->enable_redirection
-      && sm->redirection_tries == 0 && (p->vc_type == HT_HTTP_CLIENT)) {
+  if (p->alive && sm->t_state.method == HTTP_WKSIDX_POST && sm->enable_redirection && (p->vc_type == HT_HTTP_CLIENT)) {
     Debug("http_redirect", "[HttpTunnel::producer_run] client post: %" PRId64" max size: %" PRId64"",
           p->buffer_start->read_avail(), HttpConfig::m_master.post_copy_size);
 


[6/6] git commit: TS-2693 Modify string ownership for TSRedirectUrlSet()

Posted by zw...@apache.org.
TS-2693 Modify string ownership for TSRedirectUrlSet()


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

Branch: refs/heads/master
Commit: 6cec1a2092a40ed5794a7b8a65c1e1d63fc228c2
Parents: d3e2d2c
Author: Leif Hedstrom <zw...@apache.org>
Authored: Fri Apr 4 11:20:34 2014 -0600
Committer: Leif Hedstrom <zw...@apache.org>
Committed: Thu Apr 10 06:50:21 2014 -0600

----------------------------------------------------------------------
 plugins/experimental/custom_redirect/custom_redirect.cc | 6 ++++--
 plugins/experimental/escalate/escalate.cc               | 4 +---
 proxy/InkAPI.cc                                         | 7 +++----
 proxy/api/ts/ts.h                                       | 5 +----
 proxy/http/HttpSM.cc                                    | 4 ++--
 proxy/http/HttpSM.h                                     | 6 +++---
 proxy/http/HttpTransact.cc                              | 1 +
 7 files changed, 15 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/6cec1a20/plugins/experimental/custom_redirect/custom_redirect.cc
----------------------------------------------------------------------
diff --git a/plugins/experimental/custom_redirect/custom_redirect.cc b/plugins/experimental/custom_redirect/custom_redirect.cc
index 5ccf8ef..075f712 100644
--- a/plugins/experimental/custom_redirect/custom_redirect.cc
+++ b/plugins/experimental/custom_redirect/custom_redirect.cc
@@ -71,9 +71,11 @@ handle_response (TSHttpTxn txnp, TSCont /* contp ATS_UNUSED */)
                         redirect_url_str = TSMimeHdrFieldValueStringGet (resp_bufp, resp_loc, redirect_url_loc, -1, &redirect_url_length);
                         if (redirect_url_str) {
                             if (redirect_url_length > 0) {
-                              TSRedirectUrlSet(txnp, redirect_url_str, redirect_url_length);
+                              char* url = (char*)TSmalloc(redirect_url_length+1);
+
+                              TSstrlcpy(url, redirect_url_str, redirect_url_length + 1);
+                              TSRedirectUrlSet(txnp, url, redirect_url_length);
                             }
-                            //TSHandleStringRelease(resp_bufp, redirect_url_loc, redirect_url_str);
                         }
                         TSHandleMLocRelease (resp_bufp, resp_loc, redirect_url_loc);
                     }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/6cec1a20/plugins/experimental/escalate/escalate.cc
----------------------------------------------------------------------
diff --git a/plugins/experimental/escalate/escalate.cc b/plugins/experimental/escalate/escalate.cc
index d013b27..d57003a 100644
--- a/plugins/experimental/escalate/escalate.cc
+++ b/plugins/experimental/escalate/escalate.cc
@@ -95,9 +95,7 @@ EscalateResponse(TSCont cont, TSEvent event, void* edata)
           url_str = TSUrlStringGet(request, url, &url_len);
 
           TSDebug(PLUGIN_NAME, "Setting new URL to %.*s", url_len, url_str);
-          TSRedirectUrlSet(txn, url_str, url_len);
-
-          TSfree(static_cast<void*>(url_str));
+          TSRedirectUrlSet(txn, url_str, url_len); // Transfers ownership
         }
         // Release the response MLoc
         TSHandleMLocRelease(request, TS_NULL_MLOC, req_hdr);

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/6cec1a20/proxy/InkAPI.cc
----------------------------------------------------------------------
diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc
index 68d31d0..e4a0b9c 100644
--- a/proxy/InkAPI.cc
+++ b/proxy/InkAPI.cc
@@ -7129,13 +7129,12 @@ TSRedirectUrlSet(TSHttpTxn txnp, const char* url, const int url_len)
   HttpSM *sm = (HttpSM*) txnp;
 
   if (sm->redirect_url != NULL) {
-    ats_free(sm->redirect_url);
+    ats_free((void*)sm->redirect_url);
     sm->redirect_url = NULL;
     sm->redirect_url_len = 0;
   }
 
-  sm->redirect_url = (char*)ats_malloc(url_len + 1);
-  ink_strlcpy(sm->redirect_url, (char*)url, url_len + 1);
+  sm->redirect_url = url;
   sm->redirect_url_len = url_len;
   sm->enable_redirection = true;
 }
@@ -7148,7 +7147,7 @@ TSRedirectUrlGet(TSHttpTxn txnp, int *url_len_ptr)
   HttpSM *sm = (HttpSM*)txnp;
 
   *url_len_ptr = sm->redirect_url_len;
-  return (const char*)sm->redirect_url;
+  return sm->redirect_url;
 }
 
 char*

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/6cec1a20/proxy/api/ts/ts.h
----------------------------------------------------------------------
diff --git a/proxy/api/ts/ts.h b/proxy/api/ts/ts.h
index 367317b..99098da 100644
--- a/proxy/api/ts/ts.h
+++ b/proxy/api/ts/ts.h
@@ -1456,7 +1456,6 @@ extern "C"
    */
   tsapi void TSHttpTxnReenable(TSHttpTxn txnp, TSEvent event);
   tsapi TSReturnCode TSHttpCacheReenable(TSCacheTxn txnp, const TSEvent event, const void* data, const uint64_t size);
-  tsapi TSReturnCode TSHttpTxnFollowRedirect(TSHttpTxn txnp, int on);
 
   tsapi void TSHttpTxnArgSet(TSHttpTxn txnp, int arg_idx, void* arg);
   tsapi void* TSHttpTxnArgGet(TSHttpTxn txnp, int arg_idx);
@@ -2228,9 +2227,7 @@ extern "C"
 
   tsapi TSReturnCode TSHttpTxnConfigFind(const char* name, int length, TSOverridableConfigKey* conf, TSRecordDataType* type);
 
-  /*
-    It's unclear if these actually function properly still.
-  */
+  tsapi TSReturnCode TSHttpTxnFollowRedirect(TSHttpTxn txnp, int on);
   tsapi void TSRedirectUrlSet(TSHttpTxn txnp, const char* url, const int url_len);
   tsapi const char* TSRedirectUrlGet(TSHttpTxn txnp, int* url_len_ptr);
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/6cec1a20/proxy/http/HttpSM.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index fc4f9bc..f1fafff 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -6457,7 +6457,7 @@ HttpSM::kill_this()
     HTTP_SM_SET_DEFAULT_HANDLER(NULL);
 
     if (redirect_url != NULL) {
-      ats_free(redirect_url);
+      ats_free((void*)redirect_url);
       redirect_url = NULL;
       redirect_url_len = 0;
     }
@@ -7295,7 +7295,7 @@ HttpSM::do_redirect()
 
       if (redirect_url != NULL) {
         redirect_request(redirect_url, redirect_url_len);
-        ats_free(redirect_url);
+        ats_free((void*)redirect_url);
         redirect_url = NULL;
         redirect_url_len = 0;
         HTTP_INCREMENT_DYN_STAT(http_total_x_redirect_stat);

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/6cec1a20/proxy/http/HttpSM.h
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.h b/proxy/http/HttpSM.h
index 0a7b116..a04742d 100644
--- a/proxy/http/HttpSM.h
+++ b/proxy/http/HttpSM.h
@@ -270,12 +270,12 @@ public:
 
   //YTS Team, yamsat Plugin
   bool enable_redirection;      //To check if redirection is enabled
-  char *redirect_url;           //url for force redirect (provide users a functionality to redirect to another url when needed)
+  const char *redirect_url;     //url for force redirect (provide users a functionality to redirect to another url when needed)
   int redirect_url_len;
   int redirection_tries;        //To monitor number of redirections
-  int64_t transfered_bytes;         //Added to calculate POST data
+  int64_t transfered_bytes;     //Added to calculate POST data
   bool post_failed;             //Added to identify post failure
-  bool debug_on;              //Transaction specific debug flag
+  bool debug_on;               //Transaction specific debug flag
 
   // Tunneling request to plugin
   HttpPluginTunnel_t plugin_tunnel_type;

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/6cec1a20/proxy/http/HttpTransact.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index c5ee223..aa671fa 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -900,6 +900,7 @@ HttpTransact::EndRemapRequest(State* s)
 
       char *redirect_url = s->http_config_param->reverse_proxy_no_host_redirect;
       int redirect_url_len = s->http_config_param->reverse_proxy_no_host_redirect_len;
+
       SET_VIA_STRING(VIA_DETAIL_TUNNEL, VIA_DETAIL_TUNNEL_NO_FORWARD);
       if (redirect_url) {       /* there is a redirect url */
         build_error_response(s, HTTP_STATUS_MOVED_TEMPORARILY, "Redirect For Explanation", "request#no_host",


[5/6] git commit: TS-2707 Migrate TSRedirectUrlSet/Get to TSHttpTxnRedirectUrlSet/Get

Posted by zw...@apache.org.
TS-2707 Migrate TSRedirectUrlSet/Get to TSHttpTxnRedirectUrlSet/Get

1. We leave TSRedirectUrlSet() / Get() as is (as far as functionality
goes). This makes this proposal both API and binary compatible with v4.2.x.

2. We mark them as Deprecated, removing the old API in 6.0.0.

3. We add TSHttpTxnRedirectSet() / Get() , with the new string ownership as
described above. This naming convention is much better aligned with the rest
of our APIs.

I left the URL string lengths as int type for now, we’ll discuss at the Summit
about TS-2514, changing our usage of “int” to more appropriate size_t etc.


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

Branch: refs/heads/master
Commit: a7b7d5f98cfc94ba95ea572cadc5d14f8ecdf153
Parents: 98c5819
Author: Leif Hedstrom <zw...@apache.org>
Authored: Tue Apr 8 21:11:48 2014 -0600
Committer: Leif Hedstrom <zw...@apache.org>
Committed: Thu Apr 10 06:50:21 2014 -0600

----------------------------------------------------------------------
 .../custom_redirect/custom_redirect.cc          |  2 +-
 plugins/experimental/escalate/escalate.cc       |  4 +-
 proxy/InkAPI.cc                                 | 40 ++++++++++++++++++--
 proxy/api/ts/ts.h                               | 11 ++++--
 4 files changed, 48 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/a7b7d5f9/plugins/experimental/custom_redirect/custom_redirect.cc
----------------------------------------------------------------------
diff --git a/plugins/experimental/custom_redirect/custom_redirect.cc b/plugins/experimental/custom_redirect/custom_redirect.cc
index 075f712..5af9a83 100644
--- a/plugins/experimental/custom_redirect/custom_redirect.cc
+++ b/plugins/experimental/custom_redirect/custom_redirect.cc
@@ -74,7 +74,7 @@ handle_response (TSHttpTxn txnp, TSCont /* contp ATS_UNUSED */)
                               char* url = (char*)TSmalloc(redirect_url_length+1);
 
                               TSstrlcpy(url, redirect_url_str, redirect_url_length + 1);
-                              TSRedirectUrlSet(txnp, url, redirect_url_length);
+                              TSHttpTxnRedirectUrlSet(txnp, url, redirect_url_length);
                             }
                         }
                         TSHandleMLocRelease (resp_bufp, resp_loc, redirect_url_loc);

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/a7b7d5f9/plugins/experimental/escalate/escalate.cc
----------------------------------------------------------------------
diff --git a/plugins/experimental/escalate/escalate.cc b/plugins/experimental/escalate/escalate.cc
index 05a8234..2f71041 100644
--- a/plugins/experimental/escalate/escalate.cc
+++ b/plugins/experimental/escalate/escalate.cc
@@ -70,7 +70,7 @@ EscalateResponse(TSCont cont, TSEvent event, void* edata)
 
   // First, we need the server response ...
   if (TS_SUCCESS == TSHttpTxnServerRespGet(txn, &response, &resp_hdr)) {
-    int tries = TSRedirectRetriesGet(txn);
+    int tries = TSHttpTxnRedirectRetries(txn);
 
     TSDebug(PLUGIN_NAME, "This is try %d", tries);
     if (0 == tries) { // ToDo: Future support for more than one retry-URL
@@ -98,7 +98,7 @@ EscalateResponse(TSCont cont, TSEvent event, void* edata)
             url_str = TSUrlStringGet(request, url, &url_len);
 
             TSDebug(PLUGIN_NAME, "Setting new URL to %.*s", url_len, url_str);
-            TSRedirectUrlSet(txn, url_str, url_len); // Transfers ownership
+            TSHttpTxnRedirectUrlSet(txn, url_str, url_len); // Transfers ownership
           }
           // Release the request MLoc
         TSHandleMLocRelease(request, TS_NULL_MLOC, req_hdr);

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/a7b7d5f9/proxy/InkAPI.cc
----------------------------------------------------------------------
diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc
index 92c37fc..cc4c4dd 100644
--- a/proxy/InkAPI.cc
+++ b/proxy/InkAPI.cc
@@ -7121,7 +7121,7 @@ TSHttpTxnFollowRedirect(TSHttpTxn txnp, int on)
 
 // this function should be called at TS_EVENT_HTTP_READ_RESPONSE_HDR
 void
-TSRedirectUrlSet(TSHttpTxn txnp, const char* url, const int url_len)
+TSHttpTxnRedirectUrlSet(TSHttpTxn txnp, const char* url, const int url_len)
 {
   sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
   sdk_assert(sdk_sanity_check_null_ptr((void*)url) == TS_SUCCESS);
@@ -7139,8 +7139,35 @@ TSRedirectUrlSet(TSHttpTxn txnp, const char* url, const int url_len)
   sm->enable_redirection = true;
 }
 
+// Deprecated, remove for v6.0.0
+void
+TSRedirectUrlSet(TSHttpTxn txnp, const char* url, const int url_len)
+{
+  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
+  sdk_assert(sdk_sanity_check_null_ptr((void*)url) == TS_SUCCESS);
+
+  HttpSM *sm = (HttpSM*) txnp;
+
+  if (sm->redirect_url != NULL) {
+    ats_free(sm->redirect_url);
+    sm->redirect_url = NULL;
+    sm->redirect_url_len = 0;
+  }
+
+  sm->redirect_url = (char*)ats_malloc(url_len + 1);
+  ink_strlcpy(sm->redirect_url, url, url_len + 1);
+  sm->redirect_url_len = url_len;
+  // have to turn redirection on for this transaction if user wants to redirect to another URL
+  if (sm->enable_redirection == false) {
+    sm->enable_redirection = true;
+    // max-out "redirection_tries" to avoid the regular redirection being turned on in
+    // this transaction improperly. This variable doesn't affect the custom-redirection
+    sm->redirection_tries = HttpConfig::m_master.number_of_redirections;
+  }
+}
+
 const char*
-TSRedirectUrlGet(TSHttpTxn txnp, int *url_len_ptr)
+TSHttpTxnRedirectUrlGet(TSHttpTxn txnp, int *url_len_ptr)
 {
   sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
 
@@ -7150,8 +7177,15 @@ TSRedirectUrlGet(TSHttpTxn txnp, int *url_len_ptr)
   return sm->redirect_url;
 }
 
+// Deprecated, remove for v6.0.0
+const char*
+TSRedirectUrlGet(TSHttpTxn txnp, int *url_len_ptr)
+{
+  return TSHttpTxnRedirectUrlGet(txnp, url_len_ptr);
+}
+
 int
-TSRedirectRetriesGet(TSHttpTxn txnp)
+TSHttpTxnRedirectRetries(TSHttpTxn txnp)
 {
   sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/a7b7d5f9/proxy/api/ts/ts.h
----------------------------------------------------------------------
diff --git a/proxy/api/ts/ts.h b/proxy/api/ts/ts.h
index eaafda1..8279155 100644
--- a/proxy/api/ts/ts.h
+++ b/proxy/api/ts/ts.h
@@ -2237,6 +2237,7 @@ extern "C"
      @return @c TS_SUCCESS if it succeeded
   */
   tsapi TSReturnCode TSHttpTxnFollowRedirect(TSHttpTxn txnp, int on);
+
   /**
      This is a generalization of the TSHttpTxnFollowRedirect(), but gives finer
      control over the behavior. Instead of using the Location: header for the new
@@ -2251,7 +2252,9 @@ extern "C"
      @param url  a heap allocated string with the URL
      @param url_len the length of the URL
   */
-  tsapi void TSRedirectUrlSet(TSHttpTxn txnp, const char* url, const int url_len);
+  tsapi void TSHttpTxnRedirectUrlSet(TSHttpTxn txnp, const char* url, const int url_len);
+  tsapi TS_DEPRECATED void TSRedirectUrlSet(TSHttpTxn txnp, const char* url, const int url_len);
+
   /**
      Return the current (if set) redirection URL string. This is still owned by the
      core, and must not be free'd.
@@ -2261,7 +2264,9 @@ extern "C"
 
      @return the url string
   */
-  tsapi const char* TSRedirectUrlGet(TSHttpTxn txnp, int* url_len_ptr);
+  tsapi const char* TSHttpTxnRedirectUrlGet(TSHttpTxn txnp, int* url_len_ptr);
+  tsapi TS_DEPRECATED const char* TSRedirectUrlGet(TSHttpTxn txnp, int* url_len_ptr);
+
   /**
      Return the number of redirection retries we have done. This starts off
      at zero, and can be used to select different URLs based on which attempt this
@@ -2272,7 +2277,7 @@ extern "C"
 
      @return the redirect try count
   */
-  tsapi int TSRedirectRetriesGet(TSHttpTxn txnp);
+  tsapi int TSHttpTxnRedirectRetries(TSHttpTxn txnp);
 
   /* Get current HTTP connection stats */
   tsapi int TSHttpCurrentClientConnectionsGet(void);


[3/6] git commit: TS-2692 Add an API to Get the current redirection retry count

Posted by zw...@apache.org.
TS-2692 Add an API to Get the current redirection retry count


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

Branch: refs/heads/master
Commit: 98c581919958096b7b8a1bfff036bdeff12b7d30
Parents: 6cec1a2
Author: Leif Hedstrom <zw...@apache.org>
Authored: Fri Apr 4 11:55:02 2014 -0600
Committer: Leif Hedstrom <zw...@apache.org>
Committed: Thu Apr 10 06:50:21 2014 -0600

----------------------------------------------------------------------
 plugins/experimental/escalate/escalate.cc | 49 ++++++++++++++------------
 proxy/InkAPI.cc                           | 12 ++++++-
 proxy/api/ts/ts.h                         | 43 ++++++++++++++++++++++
 proxy/http/HttpSM.h                       |  2 +-
 4 files changed, 81 insertions(+), 25 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/98c58191/plugins/experimental/escalate/escalate.cc
----------------------------------------------------------------------
diff --git a/plugins/experimental/escalate/escalate.cc b/plugins/experimental/escalate/escalate.cc
index d57003a..05a8234 100644
--- a/plugins/experimental/escalate/escalate.cc
+++ b/plugins/experimental/escalate/escalate.cc
@@ -66,42 +66,45 @@ EscalateResponse(TSCont cont, TSEvent event, void* edata)
   TSMBuffer        response;
   TSMLoc           resp_hdr;
 
-  TSDebug(PLUGIN_NAME, "Recieved event %d", (int)event);
   TSReleaseAssert(event == TS_EVENT_HTTP_READ_RESPONSE_HDR);
 
   // First, we need the server response ...
   if (TS_SUCCESS == TSHttpTxnServerRespGet(txn, &response, &resp_hdr)) {
-    // Next, the respose status ...
-    TSHttpStatus status = TSHttpHdrStatusGet(response, resp_hdr);
+    int tries = TSRedirectRetriesGet(txn);
 
-    // 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);
+    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 (entry != es->hostmap.end()) {
-      TSMBuffer request;
-      TSMLoc    req_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);
 
-      TSDebug(PLUGIN_NAME, "found an escalation entry for HTTP status %u", (unsigned)status);
-      if (TS_SUCCESS == TSHttpTxnClientReqGet(txn, &request, &req_hdr)) {
-        TSMLoc url;
+      if (entry != es->hostmap.end()) {
+        TSMBuffer request;
+        TSMLoc    req_hdr;
 
-        if (TS_SUCCESS == TSHttpHdrUrlGet(request, req_hdr, &url)) {
-          char* url_str;
-          int url_len;
+        TSDebug(PLUGIN_NAME, "Found an entry for HTTP status %u", (unsigned)status);
+        if (TS_SUCCESS == TSHttpTxnClientReqGet(txn, &request, &req_hdr)) {
+          TSMLoc url;
 
-          // 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);
+          if (TS_SUCCESS == TSHttpHdrUrlGet(request, req_hdr, &url)) {
+            char* url_str;
+            int url_len;
 
-          TSDebug(PLUGIN_NAME, "Setting new URL to %.*s", url_len, url_str);
-          TSRedirectUrlSet(txn, url_str, url_len); // Transfers ownership
-        }
-        // Release the response MLoc
+            // 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);
+            TSRedirectUrlSet(txn, url_str, url_len); // Transfers ownership
+          }
+          // Release the request MLoc
         TSHandleMLocRelease(request, TS_NULL_MLOC, req_hdr);
+        }
       }
     }
-
     // Release the response MLoc
     TSHandleMLocRelease(response, TS_NULL_MLOC, resp_hdr);
   }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/98c58191/proxy/InkAPI.cc
----------------------------------------------------------------------
diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc
index e4a0b9c..92c37fc 100644
--- a/proxy/InkAPI.cc
+++ b/proxy/InkAPI.cc
@@ -7134,7 +7134,7 @@ TSRedirectUrlSet(TSHttpTxn txnp, const char* url, const int url_len)
     sm->redirect_url_len = 0;
   }
 
-  sm->redirect_url = url;
+  sm->redirect_url = (char*)url;
   sm->redirect_url_len = url_len;
   sm->enable_redirection = true;
 }
@@ -7150,6 +7150,16 @@ TSRedirectUrlGet(TSHttpTxn txnp, int *url_len_ptr)
   return sm->redirect_url;
 }
 
+int
+TSRedirectRetriesGet(TSHttpTxn txnp)
+{
+  sdk_assert(sdk_sanity_check_txn(txnp) == TS_SUCCESS);
+
+  HttpSM *sm = (HttpSM*)txnp;
+
+  return sm->redirection_tries;
+}
+
 char*
 TSFetchRespGet(TSHttpTxn txnp, int *length)
 {

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/98c58191/proxy/api/ts/ts.h
----------------------------------------------------------------------
diff --git a/proxy/api/ts/ts.h b/proxy/api/ts/ts.h
index 99098da..eaafda1 100644
--- a/proxy/api/ts/ts.h
+++ b/proxy/api/ts/ts.h
@@ -2227,9 +2227,52 @@ extern "C"
 
   tsapi TSReturnCode TSHttpTxnConfigFind(const char* name, int length, TSOverridableConfigKey* conf, TSRecordDataType* type);
 
+  /**
+     This API informs the core to try to follow redirections (e.g. 301 responses.
+     The new URL would be provided in the standard Location header.
+
+     @param txnp the transaction pointer
+     @param on   turn this on or off (0 or 1)
+
+     @return @c TS_SUCCESS if it succeeded
+  */
   tsapi TSReturnCode TSHttpTxnFollowRedirect(TSHttpTxn txnp, int on);
+  /**
+     This is a generalization of the TSHttpTxnFollowRedirect(), but gives finer
+     control over the behavior. Instead of using the Location: header for the new
+     destination, this API takes the new URL as a parameter. Calling this API
+     transfers the ownership of the URL from the plugin to the core, so you must
+     make sure it is heap allocated, and that you do not free it.
+
+     Calling this API implicitly also enables the "Follow Redirect" feature, so
+     there is no rason to call TSHttpTxnFollowRedirect() as well.
+
+     @param txnp the transaction pointer
+     @param url  a heap allocated string with the URL
+     @param url_len the length of the URL
+  */
   tsapi void TSRedirectUrlSet(TSHttpTxn txnp, const char* url, const int url_len);
+  /**
+     Return the current (if set) redirection URL string. This is still owned by the
+     core, and must not be free'd.
+
+     @param txnp the transaction pointer
+     @param url_len_ptr a pointer to where the URL length is to be stored
+
+     @return the url string
+  */
   tsapi const char* TSRedirectUrlGet(TSHttpTxn txnp, int* url_len_ptr);
+  /**
+     Return the number of redirection retries we have done. This starts off
+     at zero, and can be used to select different URLs based on which attempt this
+     is. This can be useful for example when providing a list of URLs to try, and
+     do so in order until one succeeds.
+
+     @param txnp the transaction pointer
+
+     @return the redirect try count
+  */
+  tsapi int TSRedirectRetriesGet(TSHttpTxn txnp);
 
   /* Get current HTTP connection stats */
   tsapi int TSHttpCurrentClientConnectionsGet(void);

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/98c58191/proxy/http/HttpSM.h
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.h b/proxy/http/HttpSM.h
index a04742d..f0117c0 100644
--- a/proxy/http/HttpSM.h
+++ b/proxy/http/HttpSM.h
@@ -270,7 +270,7 @@ public:
 
   //YTS Team, yamsat Plugin
   bool enable_redirection;      //To check if redirection is enabled
-  const char *redirect_url;     //url for force redirect (provide users a functionality to redirect to another url when needed)
+  char *redirect_url;     //url for force redirect (provide users a functionality to redirect to another url when needed)
   int redirect_url_len;
   int redirection_tries;        //To monitor number of redirections
   int64_t transfered_bytes;     //Added to calculate POST data


[4/6] git commit: TS-2690 Fixes for escalation plugin

Posted by zw...@apache.org.
TS-2690 Fixes for escalation plugin

This fixes the following:

1. Change to use the API for following redirects.
2. Make it only replace the "host" portions of a URL
3. Cleanup.


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

Branch: refs/heads/master
Commit: d3e2d2ce5ddf9f9eb07ece751b79ef810e291bd1
Parents: dfc2a8f
Author: Leif Hedstrom <zw...@apache.org>
Authored: Wed Apr 2 10:03:31 2014 -0600
Committer: Leif Hedstrom <zw...@apache.org>
Committed: Thu Apr 10 06:50:21 2014 -0600

----------------------------------------------------------------------
 plugins/experimental/escalate/escalate.cc | 140 +++++++++++++------------
 1 file changed, 75 insertions(+), 65 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d3e2d2ce/plugins/experimental/escalate/escalate.cc
----------------------------------------------------------------------
diff --git a/plugins/experimental/escalate/escalate.cc b/plugins/experimental/escalate/escalate.cc
index 7181111..d013b27 100644
--- a/plugins/experimental/escalate/escalate.cc
+++ b/plugins/experimental/escalate/escalate.cc
@@ -28,64 +28,84 @@
 #include <getopt.h>
 #include <string.h>
 #include <string>
-#include <sstream>
 #include <iterator>
 #include <map>
 
+
+// Constants
+const char PLUGIN_NAME[] = "escalate";
+
+
+static int EscalateResponse(TSCont, TSEvent, void *);
+
 struct EscalationState
 {
-  typedef std::map<unsigned, TSMLoc> urlmap_type;
+  typedef std::map<unsigned, std::string> hostmap_type;
 
-  EscalationState() {
-    this->mbuf = TSMBufferCreate();
+  EscalationState()
+  {
+    handler = TSContCreate(EscalateResponse, NULL);
+    TSContDataSet(handler, this);
   }
 
-  ~EscalationState() {
-    TSMBufferDestroy(this->mbuf);
+  ~EscalationState()
+  {
+    TSContDestroy(handler);
   }
 
-  TSCont      handler;
-  urlmap_type urlmap;
-  TSMBuffer   mbuf;
+  TSCont       handler;
+  hostmap_type hostmap;
 };
 
-static unsigned
-toint(const std::string& str)
-{
-  std::istringstream istr(str);
-  unsigned val;
-
-  istr >> val;
-  return val;
-}
 
 static int
-EscalateResponse(TSCont cont, TSEvent event, void * edata)
+EscalateResponse(TSCont cont, TSEvent event, void* edata)
 {
-  EscalationState * es = (EscalationState *)TSContDataGet(cont);
-  TSHttpTxn         txn = (TSHttpTxn)edata;
-  TSMBuffer         buffer;
-  TSMLoc            hdr;
-  TSHttpStatus      status;
+  EscalationState* es = static_cast<EscalationState*>(TSContDataGet(cont));
+  TSHttpTxn        txn = (TSHttpTxn)edata;
+  TSMBuffer        response;
+  TSMLoc           resp_hdr;
 
-  TSDebug("escalate", "hit escalation hook with event %d", (int)event);
+  TSDebug(PLUGIN_NAME, "Recieved event %d", (int)event);
   TSReleaseAssert(event == TS_EVENT_HTTP_READ_RESPONSE_HDR);
 
   // First, we need the server response ...
-  TSReleaseAssert(
-    TSHttpTxnServerRespGet(txn, &buffer, &hdr) == TS_SUCCESS
-  );
-
-  // Next, the respose status ...
-  status = TSHttpHdrStatusGet(buffer, hdr);
-
-  // If we have an escalation URL for this response code, set the redirection URL and force it
-  // to be followed.
-  EscalationState::urlmap_type::iterator entry = es->urlmap.find((unsigned)status);
-  if (entry != es->urlmap.end()) {
-    TSDebug("escalate", "found an escalation entry for HTTP status %u", (unsigned)status);
-    TSHttpTxnRedirectRequest(txn, es->mbuf, entry->second);
-    TSHttpTxnFollowRedirect(txn, 1 /* on */);
+  if (TS_SUCCESS == TSHttpTxnServerRespGet(txn, &response, &resp_hdr)) {
+    // Next, the respose 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 escalation 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);
+          TSRedirectUrlSet(txn, url_str, url_len);
+
+          TSfree(static_cast<void*>(url_str));
+        }
+        // Release the response MLoc
+        TSHandleMLocRelease(request, TS_NULL_MLOC, req_hdr);
+      }
+    }
+
+    // Release the response MLoc
+    TSHandleMLocRelease(response, TS_NULL_MLOC, resp_hdr);
   }
 
   // Set the transaction free ...
@@ -94,52 +114,42 @@ EscalateResponse(TSCont cont, TSEvent event, void * edata)
 }
 
 TSReturnCode
-TSRemapInit(TSRemapInterface * /* api */, char * /* errbuf */, int /* bufsz */)
+TSRemapInit(TSRemapInterface* /* api */, char* /* errbuf */, int /* bufsz */)
 {
   return TS_SUCCESS;
 }
 
 TSReturnCode
-TSRemapNewInstance(int argc, char * argv[], void ** instance, char * errbuf, int errbuf_size)
+TSRemapNewInstance(int argc, char* argv[], void** instance, char* errbuf, int errbuf_size)
 {
-  EscalationState * es((EscalationState *)instance);
-
-  es = new EscalationState();
-  es->handler = TSContCreate(EscalateResponse, NULL);
-  TSContDataSet(es->handler, es);
+  EscalationState* es = new EscalationState();
 
   // 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;
-    TSMLoc    url;
-    char *    sep;
+    unsigned status;
+    char*    sep;
 
-    // Each token should be a status code then a URL, separated by '='.
-    sep = strchr(argv[i], '=');
+    // 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]);
       goto fail;
     }
 
-    status = toint(std::string(argv[i], std::distance(argv[i], sep)));
+    *sep = '\0';
+    status = strtol(argv[i], NULL, 10);
+
     if (status < 100 || status > 599) {
       snprintf(errbuf, errbuf_size, "invalid status code: %.*s", (int)std::distance(argv[i], sep), argv[i]);
       goto fail;
     }
 
-    TSReleaseAssert(TSUrlCreate(es->mbuf, &url) == TS_SUCCESS);
-
-    ++sep; // Skip over the '='.
-
-    TSDebug("escalate", "escalating HTTP status %u to %s", status, sep);
-    if (TSUrlParse(es->mbuf, url, (const char **)&sep, argv[i] + strlen(argv[i])) != TS_PARSE_DONE) {
-      snprintf(errbuf, errbuf_size, "invalid target URL: %s", sep);
-      goto fail;
-    }
+    ++sep; // Skip over the ':'
 
     // OK, we have a valid status/URL pair.
-    es->urlmap[status] = url;
+    TSDebug(PLUGIN_NAME, "Redirect of HTTP status %u to %s", status, sep);
+    es->hostmap[status] = sep;
   }
 
   *instance = es;
@@ -151,15 +161,15 @@ fail:
 }
 
 void
-TSRemapDeleteInstance(void * instance)
+TSRemapDeleteInstance(void* instance)
 {
   delete (EscalationState *)instance;
 }
 
 TSRemapStatus
-TSRemapDoRemap(void * instance, TSHttpTxn txn, TSRemapRequestInfo * /* rri */)
+TSRemapDoRemap(void* instance, TSHttpTxn txn, TSRemapRequestInfo* /* rri */)
 {
-  EscalationState * es((EscalationState *)instance);
+  EscalationState* es = static_cast<EscalationState *>(instance);
 
   TSHttpTxnHookAdd(txn, TS_HTTP_READ_RESPONSE_HDR_HOOK, es->handler);
   return TSREMAP_NO_REMAP;


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;
>   }
> 


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

Posted by zw...@apache.org.
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;
   }