Repository: trafficserver
Updated Branches:
  refs/heads/master eb0f161ea -> 56fbfdd29


TS-2650: Redirect handling enhancements in ATS


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

Branch: refs/heads/master
Commit: 56fbfdd292e7e94a30bb596a11a2f2db4d56c52f
Parents: eb0f161
Author: Sudheer Vinukonda <[email protected]>
Authored: Thu Apr 17 16:46:27 2014 -0700
Committer: Bryan Call <[email protected]>
Committed: Thu Apr 17 16:52:49 2014 -0700

----------------------------------------------------------------------
 mgmt/RecordsConfig.cc      |   2 +
 proxy/http/HttpConfig.cc   |   8 +++
 proxy/http/HttpConfig.h    |   2 +
 proxy/http/HttpSM.cc       | 154 +++++++++++++++++++++++++++++++++++++---
 proxy/http/HttpSM.h        |   1 +
 proxy/http/HttpTransact.cc |  22 ++++++
 6 files changed, 181 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/56fbfdd2/mgmt/RecordsConfig.cc
----------------------------------------------------------------------
diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc
index 1516d1a..b0a14d7 100644
--- a/mgmt/RecordsConfig.cc
+++ b/mgmt/RecordsConfig.cc
@@ -156,6 +156,8 @@ RecordElement RecordsConfig[] = {
   ,
   {RECT_CONFIG, "proxy.config.http.number_of_redirections", RECD_INT, "1", 
RECU_DYNAMIC, RR_NULL, RECC_NULL, NULL, RECA_NULL}
   ,
+  {RECT_CONFIG, "proxy.config.http.redirect_host_no_port", RECD_INT, "0", 
RECU_DYNAMIC, RR_NULL, RECC_NULL, NULL, RECA_NULL}
+  ,
   {RECT_CONFIG, "proxy.config.http.post_copy_size", RECD_INT, "2048", 
RECU_NULL, RR_NULL, RECC_NULL, NULL, RECA_NULL}
   ,
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/56fbfdd2/proxy/http/HttpConfig.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpConfig.cc b/proxy/http/HttpConfig.cc
index ff487b6..3f2f268 100644
--- a/proxy/http/HttpConfig.cc
+++ b/proxy/http/HttpConfig.cc
@@ -1482,9 +1482,11 @@ HttpConfig::startup()
   //# 1. redirection_enabled: if set to 1, redirection is enabled.
   //# 2. number_of_redirections: The maximum number of redirections YTS permits
   //# 3. post_copy_size: The maximum POST data size YTS permits to copy
+  //# 4. redirection_host_no_port: do not include default port in host header 
during redirection
   //#
   
//##############################################################################
   HttpEstablishStaticConfigByte(c.redirection_enabled, 
"proxy.config.http.redirection_enabled");
+  HttpEstablishStaticConfigByte(c.redirection_host_no_port, 
"proxy.config.http.redirect_host_no_port");
   HttpEstablishStaticConfigLongLong(c.number_of_redirections, 
"proxy.config.http.number_of_redirections");
   HttpEstablishStaticConfigLongLong(c.post_copy_size, 
"proxy.config.http.post_copy_size");
 
@@ -1722,16 +1724,22 @@ params->push_method_enabled = 
INT_TO_BOOL(m_master.push_method_enabled);
   //# 1. redirection_enabled: if set to 1, redirection is enabled.
   //# 2. number_of_redirections: The maximum number of redirections YTS permits
   //# 3. post_copy_size: The maximum POST data size YTS permits to copy
+  //# 4. redirection_host_no_port: do not include default port in host header 
during redirection
   //#
   
//##############################################################################
 
   params->redirection_enabled = INT_TO_BOOL(m_master.redirection_enabled);
+  params->redirection_host_no_port = 
INT_TO_BOOL(m_master.redirection_host_no_port);
   params->number_of_redirections = m_master.number_of_redirections;
   params->post_copy_size = m_master.post_copy_size;
 
   m_id = configProcessor.set(m_id, params);
 
 #undef INT_TO_BOOL
+// Redirection debug statements
+  Debug("http_init", "proxy.config.http.redirection_enabled = %d", 
params->redirection_enabled);
+  Debug("http_init", "proxy.config.http.redirection_host_no_port = %d", 
params->redirection_host_no_port);
+  Debug("http_init", "proxy.config.http.number_of_redirections = %" PRId64"", 
params->number_of_redirections);
 }
 
 ////////////////////////////////////////////////////////////////

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/56fbfdd2/proxy/http/HttpConfig.h
----------------------------------------------------------------------
diff --git a/proxy/http/HttpConfig.h b/proxy/http/HttpConfig.h
index 031272d..1a6a26e 100644
--- a/proxy/http/HttpConfig.h
+++ b/proxy/http/HttpConfig.h
@@ -780,6 +780,7 @@ public:
   
//##############################################################################
 
   MgmtByte redirection_enabled;
+  MgmtByte redirection_host_no_port;
   MgmtInt number_of_redirections;
   MgmtInt post_copy_size;
 
@@ -933,6 +934,7 @@ HttpConfigParams::HttpConfigParams()
     enable_http_info(0),
     cluster_time_delta(0),
     redirection_enabled(0),
+    redirection_host_no_port(0),
     number_of_redirections(1),
     post_copy_size(2048),
     ignore_accept_mismatch(0),

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/56fbfdd2/proxy/http/HttpSM.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 45dd36f..506ac6f 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -6990,7 +6990,8 @@ HttpSM::set_next_state()
         ink_assert((t_state.hdr_info.client_response.valid()? true : false) == 
true);
         t_state.api_next_action = 
HttpTransact::SM_ACTION_API_SEND_RESPONSE_HDR;
 
-        if (hooks_set) {
+        // check to see if we are going to handle the redirection from server 
response and if there is a plugin hook set
+        if (hooks_set && is_redirect_required() == false) {
           do_api_callout_internal();
         } else {
           do_redirect();
@@ -7025,7 +7026,9 @@ HttpSM::set_next_state()
 
         perform_cache_write_action();
         t_state.api_next_action = 
HttpTransact::SM_ACTION_API_SEND_RESPONSE_HDR;
-        if (hooks_set) {
+
+        // check to see if we are going to handle the redirection from server 
response and if there is a plugin hook set
+        if (hooks_set && is_redirect_required() == false) {
           do_api_callout_internal();
         } else {
           do_redirect();
@@ -7289,9 +7292,8 @@ HttpSM::do_redirect()
     return;
   }
 
-  HTTPStatus status = t_state.hdr_info.client_response.status_get();
   // if redirect_url is set by an user's plugin, yts will redirect to this url 
anyway.
-  if ((redirect_url != NULL) || (status == HTTP_STATUS_MOVED_TEMPORARILY) || 
(status == HTTP_STATUS_MOVED_PERMANENTLY)) {
+    if (is_redirect_required()) {
     if (redirect_url != NULL || 
t_state.hdr_info.client_response.field_find(MIME_FIELD_LOCATION, 
MIME_LEN_LOCATION)) {
       if (Log::transaction_logging_enabled() && 
t_state.api_info.logging_enabled) {
         LogAccessHttp accessor(this);
@@ -7359,6 +7361,10 @@ HttpSM::redirect_request(const char *redirect_url, const 
int redirect_len)
   if (!redirectUrl.valid()) {
     redirectUrl.create(NULL);
   }
+
+  // reset the path from previous redirects (if any)
+  t_state.redirect_info.redirect_url.path_set(NULL, 0);
+
   // redirectUrl.user_set(redirect_url, redirect_len);
   redirectUrl.parse(redirect_url, redirect_len);
 
@@ -7377,6 +7383,38 @@ HttpSM::redirect_request(const char *redirect_url, const 
int redirect_len)
     t_state.hdr_info.client_response.destroy();
   }
 
+
+  bool valid_origHost = true;
+  int origHost_len, origMethod_len;
+  char* tmpOrigHost = (char *) 
t_state.hdr_info.server_request.value_get(MIME_FIELD_HOST, MIME_LEN_HOST, 
&origHost_len);
+  char origHost[origHost_len + 1];
+
+  if (tmpOrigHost)
+    memcpy(origHost, tmpOrigHost, origHost_len);
+  else
+    valid_origHost = false;
+
+  origHost[origHost_len] = '\0';
+  int origPort = t_state.hdr_info.server_request.port_get();
+
+  char *tmpOrigMethod = (char *) 
t_state.hdr_info.server_request.method_get(&origMethod_len);
+  char origMethod[origMethod_len + 1];
+
+  if (tmpOrigMethod)
+    memcpy(origMethod, tmpOrigMethod, origMethod_len);
+  else
+    valid_origHost = false;
+
+  int scheme = t_state.next_hop_scheme;
+  int scheme_len = hdrtoken_index_to_length(scheme);
+  const char* next_hop_scheme = hdrtoken_index_to_wks(scheme);
+  char scheme_str[scheme_len+1];
+
+  if (next_hop_scheme)
+    memcpy(scheme_str, next_hop_scheme, scheme_len);
+  else
+    valid_origHost = false;
+
   t_state.hdr_info.server_request.destroy();
   // we want to close the server session
   t_state.api_release_server_session = true;
@@ -7387,6 +7425,9 @@ HttpSM::redirect_request(const char *redirect_url, const 
int redirect_len)
   t_state.next_action = HttpTransact::SM_ACTION_REDIRECT_READ;
   // we have a new OS and need to have DNS lookup the new OS
   t_state.dns_info.lookup_success = false;
+  t_state.force_dns = false;
+
+  bool noPortInHost = HttpConfig::m_master.redirection_host_no_port;
 
   // check to see if the client request passed a host header, if so copy the 
host and port from the redirect url and
   // make a new host header
@@ -7398,12 +7439,75 @@ HttpSM::redirect_request(const char *redirect_url, 
const int redirect_len)
       int port = clientUrl.port_get();
       char buf[host_len + 7];
 
-      memcpy(buf, host, host_len);
-      host_len += snprintf(buf + host_len, sizeof(buf) - host_len, ":%d", 
port);
+      int redirectSchemeLen;
+      const char* redirectScheme = clientUrl.scheme_get(&redirectSchemeLen);
+      if (redirectScheme == NULL) {
+        clientUrl.scheme_set(scheme_str, scheme_len);
+        DebugSM("http_redirect", "[HttpSM::redirect_request] hsot without 
scheme %s", buf);
+      }
+
+      if (noPortInHost) {
+        int redirectSchemeIdx = clientUrl.scheme_get_wksidx();
+
+        bool defaultPort = (((redirectSchemeIdx == URL_WKSIDX_HTTP) && (port 
== 80)) ||
+            ((redirectSchemeIdx == URL_WKSIDX_HTTPS) && (port == 443)));
+
+        if (!defaultPort) noPortInHost = false;
+      }
+
+      ink_strlcpy(buf, host, host_len + 1);
+
+      if (!noPortInHost) {
+        char port_buf[6]; // handle upto 5 digit port
+        buf[host_len++] = ':';
+
+        host_len += ink_small_itoa(port, port_buf, sizeof(port_buf));
+        ink_strlcat(buf, port_buf, sizeof(buf));
+      }
+
+      t_state.hdr_info.client_request.m_target_cached = false;
       t_state.hdr_info.client_request.value_set(MIME_FIELD_HOST, 
MIME_LEN_HOST, buf, host_len);
     } else {
-      // the client request didn't have a host, so remove it from the headers
-      t_state.hdr_info.client_request.field_delete(MIME_FIELD_HOST, 
MIME_LEN_HOST);
+      // the client request didn't have a host, so use the current origin host
+      if (valid_origHost)
+      {
+        // the client request didn't have a host, so use the current origin 
host
+        DebugSM("http_redirect", "[HttpSM::redirect_request] keeping client 
request host %s://%s", next_hop_scheme, origHost);
+        char* origHost1 = strtok(origHost, ":");
+        origHost_len = strlen(origHost1);
+        int origHostPort_len = origHost_len;
+        char buf[origHostPort_len + 7];
+        ink_strlcpy(buf, origHost1, origHost_len + 1);
+
+        if (noPortInHost) {
+          int redirectSchemeIdx = t_state.next_hop_scheme;
+
+          bool defaultPort = (((redirectSchemeIdx == URL_WKSIDX_HTTP) && 
(origPort == 80)) ||
+              ((redirectSchemeIdx == URL_WKSIDX_HTTPS) && (origPort == 443)));
+
+          if (!defaultPort) noPortInHost = false;
+        }
+
+        if (!noPortInHost) {
+          char port_buf[6]; // handle upto 5 digit port
+          buf[origHostPort_len++] = ':';
+          origHostPort_len += ink_small_itoa(origPort, port_buf, 
sizeof(port_buf));
+          ink_strlcat(buf, port_buf, sizeof(buf));
+        }
+
+        url_nuke_proxy_stuff(clientUrl.m_url_impl);
+        
url_nuke_proxy_stuff(t_state.hdr_info.client_request.m_url_cached.m_url_impl);
+        t_state.hdr_info.client_request.method_set(origMethod, origMethod_len);
+        if (noPortInHost)
+          t_state.hdr_info.client_request.value_set(MIME_FIELD_HOST, 
MIME_LEN_HOST, buf, origHost_len);
+        else
+          t_state.hdr_info.client_request.value_set(MIME_FIELD_HOST, 
MIME_LEN_HOST, buf, origHostPort_len);
+        t_state.hdr_info.client_request.m_target_cached = false;
+        clientUrl.scheme_set(scheme_str, scheme_len);
+      } else {
+        // the server request didn't have a host, so remove it from the headers
+        t_state.hdr_info.client_request.field_delete(MIME_FIELD_HOST, 
MIME_LEN_HOST);
+      }
     }
 
   }
@@ -7470,3 +7574,37 @@ HttpSM::is_private()
     }
     return res;
 }
+
+// check to see if redirection is enabled and less than max redirections tries 
or if a plugin enabled redirection
+inline bool
+HttpSM::is_redirect_required()
+{
+  bool redirect_required = (enable_redirection && (redirection_tries <= 
HttpConfig::m_master.number_of_redirections));
+
+  if (redirect_required == true) {
+    HTTPStatus status = t_state.hdr_info.client_response.status_get();
+    // check to see if the response from the orgin was a 301, 302, or 303
+    switch (status)
+    {
+    case HTTP_STATUS_MULTIPLE_CHOICES:     //300
+    case HTTP_STATUS_MOVED_PERMANENTLY:    //301
+    case HTTP_STATUS_MOVED_TEMPORARILY:    //302
+    case HTTP_STATUS_SEE_OTHER:            //303
+    case HTTP_STATUS_USE_PROXY:            //305
+    case HTTP_STATUS_TEMPORARY_REDIRECT:   //307
+      redirect_required = true;
+      break;
+    default:
+      redirect_required = false;
+      break;
+    }
+
+    // if redirect_url is set by an user's plugin, ats will redirect to this 
url anyway.
+    if (redirect_url != NULL) {
+      redirect_required = true;
+    } else {
+      redirect_required = false;
+    }
+  }
+  return redirect_required;
+}

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/56fbfdd2/proxy/http/HttpSM.h
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.h b/proxy/http/HttpSM.h
index f0117c0..9c1b58f 100644
--- a/proxy/http/HttpSM.h
+++ b/proxy/http/HttpSM.h
@@ -263,6 +263,7 @@ public:
   void add_history_entry(const char *fileline, int event, int reentrant);
   void add_cache_sm();
   bool is_private();
+  bool is_redirect_required();
 
   TSClientProtoStack proto_stack;
   int64_t sm_id;

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/56fbfdd2/proxy/http/HttpTransact.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 6b00e98..78cae79 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -1409,6 +1409,7 @@ HttpTransact::HandleRequest(State* s)
   // if the newly added varible doc_in_cache_skip_dns is not enabled
   if (s->dns_info.lookup_name[0] <= '9' &&
       s->dns_info.lookup_name[0] >= '0' &&
+      (!s->state_machine->enable_redirection || 
!s->redirect_info.redirect_in_process) &&
      // (s->state_machine->authAdapter.needs_rev_dns() ||
       ( host_rule_in_CacheControlTable() || 
s->parent_params->ParentTable->hostMatch)) {
     s->force_dns = 1;
@@ -3968,6 +3969,27 @@ 
HttpTransact::handle_forward_server_connection_open(State* s)
     // xing xing in the tunneling case, need to check when the cache_read_vc 
is closed, make sure the cache_read_vc is closed right away
   }
 
+  CacheVConnection* cw_vc = s->state_machine->get_cache_sm().cache_write_vc;
+
+  if (s->redirect_info.redirect_in_process && 
s->state_machine->enable_redirection) {
+    if (s->cache_info.action == CACHE_DO_NO_ACTION) {
+      switch (s->hdr_info.server_response.status_get())
+      {
+      case HTTP_STATUS_MULTIPLE_CHOICES:     //300
+      case HTTP_STATUS_MOVED_PERMANENTLY:    //301
+      case HTTP_STATUS_MOVED_TEMPORARILY:    //302
+      case HTTP_STATUS_SEE_OTHER:            //303
+      case HTTP_STATUS_USE_PROXY:            //305
+      case HTTP_STATUS_TEMPORARY_REDIRECT:   //307
+        break;
+      default:
+        DebugTxn("http_trans", "[hfsco] redirect in progress, non-3xx 
response, setting cache_do_write");
+        if (cw_vc) s->cache_info.action = CACHE_DO_WRITE;
+        break;
+      }
+    }
+  }
+
   switch (s->cache_info.action) {
   case CACHE_DO_WRITE:
     /* fall through */

Reply via email to