You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by bn...@apache.org on 2020/07/20 14:59:07 UTC
[trafficserver] branch master updated: slice: clean up of created
502 response header (#6919)
This is an automated email from the ASF dual-hosted git repository.
bnolsen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 9d23ab6 slice: clean up of created 502 response header (#6919)
9d23ab6 is described below
commit 9d23ab6932102cb7741d256791c7c29fe436b138
Author: Brian Olsen <bn...@gmail.com>
AuthorDate: Mon Jul 20 08:58:54 2020 -0600
slice: clean up of created 502 response header (#6919)
Change header debug to use TSHttpHdrPrint.
Co-authored-by: Brian Olsen <bo...@ipcdn-cache-58.cdnlab.comcast.net>
---
plugins/experimental/slice/Data.h | 9 ++--
plugins/experimental/slice/HttpHeader.cc | 84 +++++++----------------------
plugins/experimental/slice/Stage.h | 9 ----
plugins/experimental/slice/client.cc | 8 ---
plugins/experimental/slice/response.cc | 12 +++--
plugins/experimental/slice/response.h | 2 +-
plugins/experimental/slice/server.cc | 23 ++------
plugins/experimental/slice/slice.cc | 92 +++-----------------------------
plugins/experimental/slice/slice.h | 43 +--------------
plugins/experimental/slice/util.cc | 4 +-
10 files changed, 48 insertions(+), 238 deletions(-)
diff --git a/plugins/experimental/slice/Data.h b/plugins/experimental/slice/Data.h
index e29ad84..f8cdd64 100644
--- a/plugins/experimental/slice/Data.h
+++ b/plugins/experimental/slice/Data.h
@@ -48,6 +48,9 @@ struct Data {
sockaddr_storage m_client_ip;
+ // transaction pointer
+ TSHttpTxn m_txnp{nullptr};
+
// for pristine/effective url coming in
TSMBuffer m_urlbuf{nullptr};
TSMLoc m_urlloc{nullptr};
@@ -96,16 +99,10 @@ struct Data {
m_hostname[0] = '\0';
m_etag[0] = '\0';
m_lastmodified[0] = '\0';
-#if defined(COLLECT_STATS)
- TSStatIntIncrement(stats::DataCreate, 1);
-#endif
}
~Data()
{
-#if defined(COLLECT_STATS)
- TSStatIntIncrement(stats::DataDestroy, 1);
-#endif
if (nullptr != m_urlbuf) {
if (nullptr != m_urlloc) {
TSHandleMLocRelease(m_urlbuf, TS_NULL_MLOC, m_urlloc);
diff --git a/plugins/experimental/slice/HttpHeader.cc b/plugins/experimental/slice/HttpHeader.cc
index ba47b56..d98d112 100644
--- a/plugins/experimental/slice/HttpHeader.cc
+++ b/plugins/experimental/slice/HttpHeader.cc
@@ -290,77 +290,33 @@ HttpHeader::toString() const
{
std::string res;
- if (!isValid()) {
- return "<null>";
- }
-
- TSHttpType const htype(type());
-
- switch (htype) {
- case TS_HTTP_TYPE_REQUEST: {
- res.append(method());
+ if (isValid()) {
+ TSIOBuffer const iobufp = TSIOBufferCreate();
+ TSHttpHdrPrint(m_buffer, m_lochdr, iobufp);
+ TSIOBufferReader const reader = TSIOBufferReaderAlloc(iobufp);
+
+ if (nullptr != reader) {
+ TSIOBufferBlock block = TSIOBufferReaderStart(reader);
+ bool done = false;
+ while (!done && nullptr != block) {
+ int64_t avail = 0;
+ char const *blockptr = TSIOBufferBlockReadStart(block, reader, &avail);
+ if (0 < avail) {
+ res.append(blockptr, avail);
+ }
+ block = TSIOBufferBlockNext(block);
+ }
- int urllen = 0;
- char *const urlstr = urlString(&urllen);
- if (nullptr != urlstr) {
- res.append(" ");
- res.append(urlstr, urllen);
- TSfree(urlstr);
- } else {
- res.append(" UnknownURL");
+ TSIOBufferReaderFree(reader);
}
- res.append(" HTTP/unparsed");
- } break;
-
- case TS_HTTP_TYPE_RESPONSE: {
- char bufstr[1024];
- /*
- int const version = TSHttpHdrVersionGet(m_buffer, m_lochdr);
- snprintf(bufstr, 1023, "%d ", version);
- res.append(bufstr);
- */
- res.append("HTTP/unparsed");
-
- int const status = TSHttpHdrStatusGet(m_buffer, m_lochdr);
- snprintf(bufstr, 1023, " %d ", status);
- res.append(bufstr);
-
- int reasonlen = 0;
- char const *const hreason = reason(&reasonlen);
-
- res.append(hreason, reasonlen);
- } break;
-
- default:
- case TS_HTTP_TYPE_UNKNOWN:
- res.append("UNKNOWN");
- break;
+ TSIOBufferDestroy(iobufp);
}
- res.append("\r\n");
-
- int const numhdrs = TSMimeHdrFieldsCount(m_buffer, m_lochdr);
-
- for (int indexhdr = 0; indexhdr < numhdrs; ++indexhdr) {
- TSMLoc const locfield = TSMimeHdrFieldGet(m_buffer, m_lochdr, indexhdr);
-
- int keylen = 0;
- char const *const keystr = TSMimeHdrFieldNameGet(m_buffer, m_lochdr, locfield, &keylen);
-
- res.append(keystr, keylen);
- res.append(": ");
- int vallen = 0;
- char const *const valstr = TSMimeHdrFieldValueStringGet(m_buffer, m_lochdr, locfield, -1, &vallen);
-
- res.append(valstr, vallen);
- res.append("\r\n");
-
- TSHandleMLocRelease(m_buffer, m_lochdr, locfield);
+ if (res.empty()) {
+ res = "<null>";
}
- res.append("\r\n");
-
return res;
}
diff --git a/plugins/experimental/slice/Stage.h b/plugins/experimental/slice/Stage.h
index f677d64..4efa3f4 100644
--- a/plugins/experimental/slice/Stage.h
+++ b/plugins/experimental/slice/Stage.h
@@ -34,9 +34,6 @@ struct Channel {
{
if (nullptr != m_reader) {
TSIOBufferReaderFree(m_reader);
-#if defined(COLLECT_STATS)
- TSStatIntDecrement(stats::Reader, 1);
-#endif
}
if (nullptr != m_iobuf) {
TSIOBufferDestroy(m_iobuf);
@@ -65,9 +62,6 @@ struct Channel {
if (nullptr == m_iobuf) {
m_iobuf = TSIOBufferCreate();
m_reader = TSIOBufferReaderAlloc(m_iobuf);
-#if defined(COLLECT_STATS)
- TSStatIntIncrement(stats::Reader, 1);
-#endif
} else {
int64_t const drained = drainReader();
if (0 < drained) {
@@ -85,9 +79,6 @@ struct Channel {
if (nullptr == m_iobuf) {
m_iobuf = TSIOBufferCreate();
m_reader = TSIOBufferReaderAlloc(m_iobuf);
-#if defined(COLLECT_STATS)
- TSStatIntIncrement(stats::Reader, 1);
-#endif
} else {
int64_t const drained = drainReader();
if (0 < drained) {
diff --git a/plugins/experimental/slice/client.cc b/plugins/experimental/slice/client.cc
index 2179ba8..d5cbd52 100644
--- a/plugins/experimental/slice/client.cc
+++ b/plugins/experimental/slice/client.cc
@@ -27,9 +27,6 @@
bool
handle_client_req(TSCont contp, TSEvent event, Data *const data)
{
-#if defined(COLLECT_STATS)
- stats::StatsRAI const rai(stats::ClientTime);
-#endif
switch (event) {
case TS_EVENT_VCONN_READ_READY:
case TS_EVENT_VCONN_READ_COMPLETE: {
@@ -126,11 +123,6 @@ handle_client_req(TSCont contp, TSEvent event, Data *const data)
void
handle_client_resp(TSCont contp, TSEvent event, Data *const data)
{
-#if defined(COLLECT_STATS)
- TSStatIntIncrement(stats::Client, 1);
- stats::StatsRAI const rai(stats::ClientTime);
-#endif
-
switch (event) {
case TS_EVENT_VCONN_WRITE_READY: {
switch (data->m_blockstate) {
diff --git a/plugins/experimental/slice/response.cc b/plugins/experimental/slice/response.cc
index 410b150..53cf59e 100644
--- a/plugins/experimental/slice/response.cc
+++ b/plugins/experimental/slice/response.cc
@@ -48,8 +48,8 @@ bodyString416()
}
// Form a 502 response, preliminary
-std::string const &
-string502()
+std::string
+string502(int const httpver)
{
static std::string msg;
static std::mutex mutex;
@@ -68,10 +68,13 @@ string502()
bodystr.append("</body>\n");
bodystr.append("</html>\n");
+ char hverstr[64];
+ int const hlen =
+ snprintf(hverstr, sizeof(hverstr), "HTTP/%d.%d 502 Bad Gateway\r\n", TS_HTTP_MAJOR(httpver), TS_HTTP_MINOR(httpver));
+ msg.append(hverstr, hlen);
+
char clenstr[1024];
int const clen = snprintf(clenstr, sizeof(clenstr), "%lu", bodystr.size());
-
- msg.append("HTTP/1.1 502 Bad Gateway\r\n");
msg.append("Content-Length: ");
msg.append(clenstr, clen);
msg.append("\r\n");
@@ -87,6 +90,7 @@ void
form416HeaderAndBody(HttpHeader &header, int64_t const contentlen, std::string const &bodystr)
{
header.removeKey(TS_MIME_FIELD_LAST_MODIFIED, TS_MIME_LEN_LAST_MODIFIED);
+ header.removeKey(TS_MIME_FIELD_EXPIRES, TS_MIME_LEN_EXPIRES);
header.removeKey(TS_MIME_FIELD_ETAG, TS_MIME_LEN_ETAG);
header.removeKey(TS_MIME_FIELD_ACCEPT_RANGES, TS_MIME_LEN_ACCEPT_RANGES);
diff --git a/plugins/experimental/slice/response.h b/plugins/experimental/slice/response.h
index 925db65..c116d22 100644
--- a/plugins/experimental/slice/response.h
+++ b/plugins/experimental/slice/response.h
@@ -21,7 +21,7 @@
#include "HttpHeader.h"
#include <string>
-std::string const &string502();
+std::string string502(int const httpver);
std::string const &bodyString416();
diff --git a/plugins/experimental/slice/server.cc b/plugins/experimental/slice/server.cc
index 272ab7e..ac989c4 100644
--- a/plugins/experimental/slice/server.cc
+++ b/plugins/experimental/slice/server.cc
@@ -91,10 +91,6 @@ enum HeaderState {
HeaderState
handleFirstServerHeader(Data *const data, TSCont const contp)
{
-#if defined(COLLECT_STATS)
- stats::StatsRAI const rai(stats::FirstHeaderTime);
-#endif
-
HeaderState state = HeaderState::Good;
HttpHeader header(data->m_resp_hdrmgr.m_buffer, data->m_resp_hdrmgr.m_lochdr);
@@ -116,7 +112,7 @@ handleFirstServerHeader(Data *const data, TSCont const contp)
// Should run TSVIONSetBytes(output_io, hlen + bodybytes);
int64_t const hlen = TSHttpHdrLengthGet(header.m_buffer, header.m_lochdr);
int64_t const clen = contentLengthFrom(header);
- DEBUG_LOG("Passthru: header: %" PRId64 " body: %" PRId64, hlen, clen);
+ DEBUG_LOG("Passthru bytes: header: %" PRId64 " body: %" PRId64, hlen, clen);
if (clen != INT64_MAX) {
TSVIONBytesSet(output_vio, hlen + clen);
} else {
@@ -129,9 +125,9 @@ handleFirstServerHeader(Data *const data, TSCont const contp)
ContentRange const blockcr = contentRangeFrom(header);
- // 206 with bad content range?
+ // 206 with bad content range -- should NEVER happen.
if (!blockcr.isValid()) {
- static std::string const &msg502 = string502();
+ std::string const msg502 = string502(header.version());
TSVIONBytesSet(output_vio, msg502.size());
TSIOBufferWrite(output_buf, msg502.data(), msg502.size());
TSVIOReenable(output_vio);
@@ -343,10 +339,6 @@ logSliceError(char const *const message, Data const *const data, HttpHeader cons
bool
handleNextServerHeader(Data *const data, TSCont const contp)
{
-#if defined(COLLECT_STATS)
- stats::StatsRAI const rai(stats::NextHeaderTime);
-#endif
-
// block response header
HttpHeader header(data->m_resp_hdrmgr.m_buffer, data->m_resp_hdrmgr.m_lochdr);
if (TSIsDebugTagSet(PLUGIN_NAME)) {
@@ -490,11 +482,6 @@ handleNextServerHeader(Data *const data, TSCont const contp)
void
handle_server_resp(TSCont contp, TSEvent event, Data *const data)
{
-#if defined(COLLECT_STATS)
- TSStatIntIncrement(stats::Server, 1);
- stats::StatsRAI const rai(stats::ServerTime);
-#endif
-
switch (event) {
case TS_EVENT_VCONN_READ_READY: {
if (data->m_blockstate == BlockState::Passthru) {
@@ -512,11 +499,11 @@ handle_server_resp(TSCont contp, TSEvent event, Data *const data)
TSVIONDoneSet(input_vio, TSVIONDoneGet(input_vio) + consumed);
// the server response header didn't fit into the input buffer.
+ // wait for more data from upstream
if (TS_PARSE_CONT == res) {
return;
}
- // very first server response header
bool headerStat = false;
if (TS_PARSE_DONE == res) {
@@ -545,8 +532,6 @@ handle_server_resp(TSCont contp, TSEvent event, Data *const data)
}
data->m_server_block_header_parsed = true;
- } else if (TS_PARSE_CONT == res) {
- return;
}
// kill the upstream and allow dnstream to clean up
diff --git a/plugins/experimental/slice/slice.cc b/plugins/experimental/slice/slice.cc
index 60542fa..ff33650 100644
--- a/plugins/experimental/slice/slice.cc
+++ b/plugins/experimental/slice/slice.cc
@@ -27,25 +27,8 @@
#include "ts/ts.h"
#include <netinet/in.h>
-#include <cassert>
#include <mutex>
-#if defined(COLLECT_STATS)
-namespace stats
-{
-int DataCreate = -1;
-int DataDestroy = -1;
-int Reader = -1;
-int Server = -1;
-int Client = -1;
-int RequestTime = -1;
-int FirstHeaderTime = -1;
-int NextHeaderTime = -1;
-int ServerTime = -1;
-int ClientTime = -1;
-} // namespace stats
-#endif // COLLECT_STATS
-
namespace
{
Config globalConfig;
@@ -53,10 +36,6 @@ Config globalConfig;
bool
read_request(TSHttpTxn txnp, Config *const config)
{
-#if defined(COLLECT_STATS)
- stats::StatsRAI const rai(stats::RequestTime);
-#endif
-
DEBUG_LOG("slice read_request");
TxnHdrMgr hdrmgr;
hdrmgr.populateFrom(txnp, TSHttpTxnClientReqGet);
@@ -103,6 +82,8 @@ read_request(TSHttpTxn txnp, Config *const config)
TSAssert(nullptr != config);
Data *const data = new Data(config);
+ data->m_txnp = txnp;
+
// set up feedback connect
if (AF_INET == ip->sa_family) {
memcpy(&data->m_client_ip, ip, sizeof(sockaddr_in));
@@ -189,10 +170,15 @@ read_request(TSHttpTxn txnp, Config *const config)
// we'll intercept this GET and do it ourselves
TSMutex const mutex = TSContMutexGet(reinterpret_cast<TSCont>(txnp));
- // TSMutex const mutex = TSMutexCreate();
TSCont const icontp(TSContCreate(intercept_hook, mutex));
TSContDataSet(icontp, (void *)data);
+
+ // Skip remap and remap rule requirement
+ TSSkipRemappingSet(txnp, 1);
+
+ // Grab the transaction
TSHttpTxnIntercept(icontp, txnp);
+
return true;
} else {
DEBUG_LOG("slice passing GET request through to next plugin");
@@ -266,68 +252,6 @@ TSReturnCode
TSRemapInit(TSRemapInterface *api_info, char *errbug, int errbuf_size)
{
DEBUG_LOG("slice remap initializing.");
-
-#if defined(COLLECT_STATS)
- static bool init = false;
- static std::mutex mutex;
-
- std::lock_guard<std::mutex> lock(mutex);
-
- if (!init) {
- init = true;
-
- std::string const namedatacreate = std::string(PLUGIN_NAME) + ".DataCreate";
- stats::DataCreate = TSStatCreate(namedatacreate.c_str(), TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM);
-
- assert(0 <= stats::DataCreate);
-
- std::string const namedatadestroy = std::string(PLUGIN_NAME) + ".DataDestroy";
- stats::DataDestroy = TSStatCreate(namedatadestroy.c_str(), TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM);
-
- assert(0 <= stats::DataDestroy);
-
- std::string const namereader = std::string(PLUGIN_NAME) + ".Reader";
- stats::Reader = TSStatCreate(namereader.c_str(), TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM);
-
- assert(0 <= stats::Reader);
-
- std::string const nameserver = std::string(PLUGIN_NAME) + ".Server";
- stats::Server = TSStatCreate(nameserver.c_str(), TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM);
-
- assert(0 <= stats::Server);
-
- std::string const nameclient = std::string(PLUGIN_NAME) + ".Client";
- stats::Client = TSStatCreate(nameclient.c_str(), TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM);
-
- assert(0 <= stats::Client);
-
- std::string const namerequest = std::string(PLUGIN_NAME) + ".RequestTime";
- stats::RequestTime = TSStatCreate(namerequest.c_str(), TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM);
-
- assert(0 <= stats::RequestTime);
-
- std::string const namefirst = std::string(PLUGIN_NAME) + ".FirstHeaderTime";
- stats::FirstHeaderTime = TSStatCreate(namefirst.c_str(), TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM);
-
- assert(0 <= stats::FirstHeaderTime);
-
- std::string const namenext = std::string(PLUGIN_NAME) + ".NextHeaderTime";
- stats::NextHeaderTime = TSStatCreate(namenext.c_str(), TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM);
-
- assert(0 <= stats::NextHeaderTime);
-
- std::string const nameservertime = std::string(PLUGIN_NAME) + ".ServerTime";
- stats::ServerTime = TSStatCreate(nameservertime.c_str(), TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM);
-
- assert(0 <= stats::ServerTime);
-
- std::string const nameclienttime = std::string(PLUGIN_NAME) + ".ClientTime";
- stats::ClientTime = TSStatCreate(nameclienttime.c_str(), TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM);
-
- assert(0 <= stats::ClientTime);
- }
-#endif // COLLECT_STATS
-
return TS_SUCCESS;
}
diff --git a/plugins/experimental/slice/slice.h b/plugins/experimental/slice/slice.h
index fe3dba2..bd58818 100644
--- a/plugins/experimental/slice/slice.h
+++ b/plugins/experimental/slice/slice.h
@@ -32,23 +32,12 @@
#define PLUGIN_NAME "slice"
#endif
-#ifndef COLLECT_STATS
-#define COLLECT_STATS
-#endif
-
constexpr std::string_view X_CRR_IMS_HEADER = {"X-Crr-Ims"};
#if !defined(UNITTEST)
#define __FILENAME__ (strrchr(__FILE__, '/') ? strrchr(__FILE__, '/') + 1 : __FILE__)
-#define DEBUG_LOG(fmt, ...) \
- TSDebug(PLUGIN_NAME, "[%s:% 4d] %s(): " fmt, __FILENAME__, __LINE__, __func__, \
- ##__VA_ARGS__) /* \
- ; fprintf(stderr, "[%s:%04d]: " fmt "\n" \
- , __FILENAME__ \
- , __LINE__ \
- , ##__VA_ARGS__) \
- */
+#define DEBUG_LOG(fmt, ...) TSDebug(PLUGIN_NAME, "[%s:% 4d] %s(): " fmt, __FILENAME__, __LINE__, __func__, ##__VA_ARGS__)
#define ERROR_LOG(fmt, ...) \
TSError("[%s:% 4d] %s(): " fmt, __FILENAME__, __LINE__, __func__, ##__VA_ARGS__); \
@@ -60,33 +49,3 @@ constexpr std::string_view X_CRR_IMS_HEADER = {"X-Crr-Ims"};
#define ERROR_LOG(fmt, ...)
#endif
-
-#if defined(COLLECT_STATS)
-namespace stats
-{
-extern int DataCreate;
-extern int DataDestroy;
-extern int Reader;
-extern int Server;
-extern int Client;
-extern int RequestTime;
-extern int FirstHeaderTime;
-extern int NextHeaderTime;
-extern int ServerTime;
-extern int ClientTime;
-
-struct StatsRAI {
- int m_statid;
- TSHRTime m_timebeg;
-
- StatsRAI(int statid) : m_statid(statid), m_timebeg(TShrtime()) {}
-
- ~StatsRAI()
- {
- TSHRTime const timeend = TShrtime();
- TSStatIntIncrement(m_statid, timeend - m_timebeg);
- }
-};
-
-} // namespace stats
-#endif // COLLECT_STATS
diff --git a/plugins/experimental/slice/util.cc b/plugins/experimental/slice/util.cc
index cc78c2b..80f0868 100644
--- a/plugins/experimental/slice/util.cc
+++ b/plugins/experimental/slice/util.cc
@@ -25,6 +25,8 @@ void
shutdown(TSCont const contp, Data *const data)
{
DEBUG_LOG("shutting down transaction");
+ data->m_upstream.close();
+ data->m_dnstream.close();
TSContDataSet(contp, nullptr);
delete data;
TSContDestroy(contp);
@@ -34,9 +36,9 @@ void
abort(TSCont const contp, Data *const data)
{
DEBUG_LOG("aborting transaction");
- TSContDataSet(contp, nullptr);
data->m_upstream.abort();
data->m_dnstream.abort();
+ TSContDataSet(contp, nullptr);
delete data;
TSContDestroy(contp);
}