trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jpe...@apache.org
Subject [trafficserver] branch master updated: TS-4420: Fix TSHttpTxnParentProxySet crash on parent failure.
Date Tue, 10 May 2016 19:46:18 GMT
This is an automated email from the ASF dual-hosted git repository.

jpeach pushed a commit to branch master
in repository https://git-dual.apache.org/repos/asf/trafficserver.git

The following commit(s) were added to refs/heads/master by this push:
       new  4d61d7b   TS-4420: Fix TSHttpTxnParentProxySet crash on parent failure.
4d61d7b is described below

commit 4d61d7b234e1f674408ae902d3adbe81e5ba5a2b
Author: James Peach <jpeach@apache.org>
AuthorDate: Mon May 9 16:15:42 2016 -0700

    TS-4420: Fix TSHttpTxnParentProxySet crash on parent failure.
    
    Fix NULL pointer deref when marking API parents down. Since the API
    parent caches are fixed, there's no place to store up/down status.
    Add an API test to exercise API parent failure.
    
    This closes #623.
---
 proxy/InkAPITest.cc     | 182 +++++++++++++++++++++++++++++++++++++-----------
 proxy/InkAPITestTool.cc |  29 ++++++--
 proxy/ParentSelection.h |  26 ++++---
 3 files changed, 184 insertions(+), 53 deletions(-)

diff --git a/proxy/InkAPITest.cc b/proxy/InkAPITest.cc
index 82cd9bb..5e68bda 100644
--- a/proxy/InkAPITest.cc
+++ b/proxy/InkAPITest.cc
@@ -5894,16 +5894,109 @@ EXCLUSIVE_REGRESSION_TEST(SDK_API_HttpSsn)(RegressionTest *test,
int /* atype AT
 }
 
 struct ParentTest {
+  ParentTest(RegressionTest *test, int *pstatus)
+  {
+    ink_zero(*this);
+    this->regtest = test;
+    this->pstatus = pstatus;
+    this->magic = MAGIC_ALIVE;
+
+    /* If parent proxy routing is not enabled, enable it for the life of the test. */
+    RecGetRecordBool("proxy.config.http.parent_proxy_routing_enable", &this->parent_proxy_routing_enable);
+    if (!this->parent_proxy_routing_enable) {
+      rprintf(this->regtest, "enabling proxy.config.http.parent_proxy_routing_enable");
+      RecSetRecordInt("proxy.config.http.parent_proxy_routing_enable", 1, REC_SOURCE_EXPLICIT);
+    }
+
+    RecSetRecordInt("proxy.config.http.parent_proxy_routing_enable", 1, REC_SOURCE_EXPLICIT);
+  }
+
+  ~ParentTest()
+  {
+    synclient_txn_delete(this->browser);
+    synserver_delete(this->os);
+    this->os = NULL;
+    this->magic = MAGIC_DEAD;
+  }
+
   RegressionTest *regtest;
   int *pstatus;
+
+  const char *testcase;
   SocketServer *os;
   ClientTxn *browser;
+  TSEventFunc handler;
 
   RecBool parent_proxy_routing_enable;
   unsigned int magic;
 };
 
 static int
+parent_proxy_success(TSCont contp, TSEvent event, void *edata)
+{
+  ParentTest *ptest = (ParentTest *)TSContDataGet(contp);
+  TSHttpTxn txnp = (TSHttpTxn)edata;
+
+  int expected;
+  int received;
+
+  switch (event) {
+  case TS_EVENT_HTTP_SEND_RESPONSE_HDR:
+    expected = get_request_id(txnp);
+    received = get_response_id(txnp);
+
+    if (expected != received) {
+      *(ptest->pstatus) = REGRESSION_TEST_FAILED;
+      SDK_RPRINT(ptest->regtest, "TSHttpTxnParentProxySet", "TestCase", TC_FAIL, "Expected
response ID %d, received %d", expected,
+                 received);
+    } else {
+      *(ptest->pstatus) = REGRESSION_TEST_PASSED;
+      SDK_RPRINT(ptest->regtest, "TSHttpTxnParentProxySet", "TestCase", TC_PASS, "Received
expected response ID %d", expected);
+    }
+    TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
+    return TS_EVENT_NONE;
+
+  default:
+    SDK_RPRINT(ptest->regtest, "TSHttpTxnParentProxySet", ptest->testcase, TC_FAIL,
"Unexpected event %d", event);
+    return TS_EVENT_ERROR;
+  }
+}
+
+static int
+parent_proxy_fail(TSCont contp, TSEvent event, void *edata)
+{
+  ParentTest *ptest = (ParentTest *)TSContDataGet(contp);
+  TSHttpTxn txnp = (TSHttpTxn)edata;
+
+  TSMBuffer mbuf;
+  TSMLoc hdr;
+  TSHttpStatus expected = TS_HTTP_STATUS_BAD_GATEWAY;
+  TSHttpStatus received;
+
+  switch (event) {
+  case TS_EVENT_HTTP_SEND_RESPONSE_HDR:
+    ink_release_assert(TSHttpTxnClientRespGet(txnp, &mbuf, &hdr) == TS_SUCCESS);
+    received = TSHttpHdrStatusGet(mbuf, hdr);
+
+    if (expected != received) {
+      *(ptest->pstatus) = REGRESSION_TEST_FAILED;
+      SDK_RPRINT(ptest->regtest, "TSHttpTxnParentProxySet", "TestCase", TC_FAIL, "Expected
response status %d, received %d",
+                 expected, received);
+    } else {
+      *(ptest->pstatus) = REGRESSION_TEST_PASSED;
+      SDK_RPRINT(ptest->regtest, "TSHttpTxnParentProxySet", "TestCase", TC_PASS, "Received
expected response status %d", expected);
+    }
+
+    TSHandleMLocRelease(mbuf, TS_NULL_MLOC, hdr);
+    return TS_EVENT_NONE;
+
+  default:
+    SDK_RPRINT(ptest->regtest, "TSHttpTxnParentProxySet", ptest->testcase, TC_FAIL,
"Unexpected event %d", event);
+    return TS_EVENT_ERROR;
+  }
+}
+
+static int
 parent_proxy_handler(TSCont contp, TSEvent event, void *edata)
 {
   ParentTest *ptest = NULL;
@@ -5927,22 +6020,6 @@ parent_proxy_handler(TSCont contp, TSEvent event, void *edata)
     TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
     break;
 
-  case TS_EVENT_HTTP_SEND_RESPONSE_HDR: {
-    int expected = get_request_id(txnp);
-    int received = get_response_id(txnp);
-
-    if (expected != received) {
-      *(ptest->pstatus) = REGRESSION_TEST_FAILED;
-      SDK_RPRINT(ptest->regtest, "TSHttpTxnParentProxySet", "TestCase", TC_FAIL, "Expected
response ID %d, received %d", expected,
-                 received);
-    } else {
-      *(ptest->pstatus) = REGRESSION_TEST_PASSED;
-      SDK_RPRINT(ptest->regtest, "TSHttpTxnParentProxySet", "TestCase", TC_PASS, "Received
expected response ID %d", expected);
-    }
-    TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
-    break;
-  }
-
   case TS_EVENT_TIMEOUT:
     if (*(ptest->pstatus) == REGRESSION_TEST_INPROGRESS) {
       // If we are still in progress, reschedule.
@@ -5953,12 +6030,7 @@ parent_proxy_handler(TSCont contp, TSEvent event, void *edata)
       RecSetRecordInt("proxy.config.http.parent_proxy_routing_enable", ptest->parent_proxy_routing_enable,
REC_SOURCE_EXPLICIT);
 
       TSContDataSet(contp, NULL);
-
-      synclient_txn_delete(ptest->browser);
-      synserver_delete(ptest->os);
-      ptest->os = NULL;
-      ptest->magic = MAGIC_DEAD;
-      TSfree(ptest);
+      delete ptest;
     }
     break;
 
@@ -5966,52 +6038,80 @@ parent_proxy_handler(TSCont contp, TSEvent event, void *edata)
     // We expected to pass or fail reading the response header. At this point we must have
failed.
     if (*(ptest->pstatus) == REGRESSION_TEST_INPROGRESS) {
       *(ptest->pstatus) = REGRESSION_TEST_FAILED;
-      SDK_RPRINT(ptest->regtest, "TSHttpTxnParentProxySet", "TestCase", TC_FAIL, "Failed
on txn close");
+      SDK_RPRINT(ptest->regtest, "TSHttpTxnParentProxySet", ptest->testcase, TC_FAIL,
"Failed on txn close");
     }
     TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
     break;
 
   default:
-    *(ptest->pstatus) = REGRESSION_TEST_FAILED;
-    SDK_RPRINT(ptest->regtest, "TSHttpTxnParentProxySet", "TestCase", TC_FAIL, "Unexpected
event %d", event);
-    break;
+    if (ptest->handler(contp, event, edata) == TS_ERROR) {
+      *(ptest->pstatus) = REGRESSION_TEST_FAILED;
+
+      TSContDataSet(contp, NULL);
+      delete ptest;
+    }
   }
 
-  return 0;
+  return TS_EVENT_NONE;
 }
 
-EXCLUSIVE_REGRESSION_TEST(SDK_API_HttpParentProxySet)(RegressionTest *test, int level, int
*pstatus)
+EXCLUSIVE_REGRESSION_TEST(SDK_API_HttpParentProxySet_Fail)(RegressionTest *test, int level,
int *pstatus)
 {
   *pstatus = REGRESSION_TEST_INPROGRESS;
 
   TSCont cont = TSContCreate(parent_proxy_handler, TSMutexCreate());
   if (cont == NULL) {
-    SDK_RPRINT(test, "TSHttpTxnParentProxySet", "TestCase", TC_FAIL, "Unable to create continuation");
+    SDK_RPRINT(test, "TSHttpTxnParentProxySet", "FailCase", TC_FAIL, "Unable to create continuation");
     *pstatus = REGRESSION_TEST_FAILED;
     return;
   }
 
-  ParentTest *ptest = (ParentTest *)TSmalloc(sizeof(SocketTest));
-  ink_zero(*ptest);
+  ParentTest *ptest = new ParentTest(test, pstatus);
 
-  ptest->regtest = test;
-  ptest->pstatus = pstatus;
-  ptest->magic = MAGIC_ALIVE;
+  ptest->testcase = "FailCase";
+  ptest->handler = parent_proxy_fail;
   TSContDataSet(cont, ptest);
 
-  /* If parent proxy routing is not enabled, enable it for the life of the test. */
-  RecGetRecordBool("proxy.config.http.parent_proxy_routing_enable", &ptest->parent_proxy_routing_enable);
-  if (!ptest->parent_proxy_routing_enable) {
-    rprintf(test, "enabling proxy.config.http.parent_proxy_routing_enable");
-    RecSetRecordInt("proxy.config.http.parent_proxy_routing_enable", 1, REC_SOURCE_EXPLICIT);
+  /* Hook read request headers, since that is the earliest reasonable place to set the parent
proxy. */
+  TSHttpHookAdd(TS_HTTP_READ_REQUEST_HDR_HOOK, cont);
+
+  /* Create a new synthetic server */
+  ptest->os = synserver_create(SYNSERVER_LISTEN_PORT, TSContCreate(synserver_vc_refuse,
TSMutexCreate()));
+  synserver_start(ptest->os);
+
+  /* Create a client transaction */
+  ptest->browser = synclient_txn_create();
+
+  // HTTP_REQUEST_FORMAT10 is a hostname, so we will need to set the parent to the synserver
to get a response.
+  char *request = generate_request(10);
+  synclient_txn_send_request(ptest->browser, request);
+  TSfree(request);
+
+  TSContSchedule(cont, 25, TS_THREAD_POOL_DEFAULT);
+}
+
+EXCLUSIVE_REGRESSION_TEST(SDK_API_HttpParentProxySet_Success)(RegressionTest *test, int level,
int *pstatus)
+{
+  *pstatus = REGRESSION_TEST_INPROGRESS;
+
+  TSCont cont = TSContCreate(parent_proxy_handler, TSMutexCreate());
+  if (cont == NULL) {
+    SDK_RPRINT(test, "TSHttpTxnParentProxySet", "SuccessCase", TC_FAIL, "Unable to create
continuation");
+    *pstatus = REGRESSION_TEST_FAILED;
+    return;
   }
 
-  RecSetRecordInt("proxy.config.http.parent_proxy_routing_enable", 1, REC_SOURCE_EXPLICIT);
+  ParentTest *ptest = new ParentTest(test, pstatus);
+
+  ptest->testcase = "SuccessCase";
+  ptest->handler = parent_proxy_success;
+  TSContDataSet(cont, ptest);
+
   /* Hook read request headers, since that is the earliest reasonable place to set the parent
proxy. */
   TSHttpHookAdd(TS_HTTP_READ_REQUEST_HDR_HOOK, cont);
 
   /* Create a new synthetic server */
-  ptest->os = synserver_create(SYNSERVER_LISTEN_PORT);
+  ptest->os = synserver_create(SYNSERVER_LISTEN_PORT, TSContCreate(synserver_vc_accept,
TSMutexCreate()));
   synserver_start(ptest->os);
 
   /* Create a client transaction */
diff --git a/proxy/InkAPITestTool.cc b/proxy/InkAPITestTool.cc
index 1ebac13..4974887 100644
--- a/proxy/InkAPITestTool.cc
+++ b/proxy/InkAPITestTool.cc
@@ -154,7 +154,8 @@ SocketServer *synserver_create(int port);
 static int synserver_start(SocketServer *s);
 static int synserver_stop(SocketServer *s);
 static int synserver_delete(SocketServer *s);
-static int synserver_accept_handler(TSCont contp, TSEvent event, void *data);
+static int synserver_vc_accept(TSCont contp, TSEvent event, void *data);
+static int synserver_vc_refuse(TSCont contp, TSEvent event, void *data);
 static int synserver_txn_close(TSCont contp);
 static int synserver_txn_write_response(TSCont contp);
 static int synserver_txn_write_response_handler(TSCont contp, TSEvent event, void *data);
@@ -761,7 +762,7 @@ synclient_txn_main_handler(TSCont contp, TSEvent event, void *data)
 //////////////////////////////////////////////////////////////////////////////
 
 SocketServer *
-synserver_create(int port)
+synserver_create(int port, TSCont cont)
 {
   if (port != SYNSERVER_DUMMY_PORT) {
     TSAssert(port > 0);
@@ -772,11 +773,17 @@ synserver_create(int port)
   s->magic = MAGIC_ALIVE;
   s->accept_port = port;
   s->accept_action = NULL;
-  s->accept_cont = TSContCreate(synserver_accept_handler, TSMutexCreate());
+  s->accept_cont = cont;
   TSContDataSet(s->accept_cont, s);
   return s;
 }
 
+SocketServer *
+synserver_create(int port)
+{
+  return synserver_create(port, TSContCreate(synserver_vc_accept, TSMutexCreate()));
+}
+
 static int
 synserver_start(SocketServer *s)
 {
@@ -828,7 +835,21 @@ synserver_delete(SocketServer *s)
 }
 
 static int
-synserver_accept_handler(TSCont contp, TSEvent event, void *data)
+synserver_vc_refuse(TSCont contp, TSEvent event, void *data)
+{
+  TSAssert((event == TS_EVENT_NET_ACCEPT) || (event == TS_EVENT_NET_ACCEPT_FAILED));
+
+  SocketServer *s = (SocketServer *)TSContDataGet(contp);
+  TSAssert(s->magic == MAGIC_ALIVE);
+
+  TSDebug(SDBG_TAG, "NET_ACCEPT");
+
+  TSVConnClose((TSVConn)data);
+  return TS_EVENT_IMMEDIATE;
+}
+
+static int
+synserver_vc_accept(TSCont contp, TSEvent event, void *data)
 {
   TSAssert((event == TS_EVENT_NET_ACCEPT) || (event == TS_EVENT_NET_ACCEPT_FAILED));
 
diff --git a/proxy/ParentSelection.h b/proxy/ParentSelection.h
index cb520b8..b5155e6 100644
--- a/proxy/ParentSelection.h
+++ b/proxy/ParentSelection.h
@@ -331,29 +331,39 @@ public:
   void
   selectParent(bool firstCall, ParentResult *result, RequestData *rdata)
   {
-    ink_release_assert(result->rec->selection_strategy != NULL);
-    return result->rec->selection_strategy->selectParent(&policy, firstCall,
result, rdata);
+    if (!result->is_api_result()) {
+      ink_release_assert(result->rec->selection_strategy != NULL);
+      return result->rec->selection_strategy->selectParent(&policy, firstCall,
result, rdata);
+    }
   }
 
   void
   markParentDown(ParentResult *result)
   {
-    ink_release_assert(result->rec->selection_strategy != NULL);
-    result->rec->selection_strategy->markParentDown(&policy, result);
+    if (!result->is_api_result()) {
+      ink_release_assert(result->rec->selection_strategy != NULL);
+      result->rec->selection_strategy->markParentDown(&policy, result);
+    }
   }
 
   uint32_t
   numParents(ParentResult *result)
   {
-    ink_release_assert(result->rec->selection_strategy != NULL);
-    return result->rec->selection_strategy->numParents(result);
+    if (result->is_api_result()) {
+      return 1;
+    } else {
+      ink_release_assert(result->rec->selection_strategy != NULL);
+      return result->rec->selection_strategy->numParents(result);
+    }
   }
 
   void
   markParentUp(ParentResult *result)
   {
-    ink_release_assert(result != NULL);
-    result->rec->selection_strategy->markParentUp(result);
+    if (!result->is_api_result()) {
+      ink_release_assert(result != NULL);
+      result->rec->selection_strategy->markParentUp(result);
+    }
   }
 
   P_table *parent_table;

-- 
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <commits@trafficserver.apache.org>'].

Mime
View raw message