You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2016/05/10 19:46:18 UTC
[trafficserver] branch master updated: TS-4420: Fix
TSHttpTxnParentProxySet crash on parent failure.
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 <jp...@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" <co...@trafficserver.apache.org>'].