You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by os...@apache.org on 2014/08/06 12:22:25 UTC
git commit: TS-2988: ats_speed: bail out when gurl->IsWebValid() !=
true
Repository: trafficserver
Updated Branches:
refs/heads/master d9aba01de -> 8abc04d13
TS-2988: ats_speed: bail out when gurl->IsWebValid() != true
It seems that sometimes a url is received which can't be parsed as
we valid by GoogleUrl(). Just ignore those requests, log them, and
don't ASSERT on those.
This commit also addresses another check failure I encountered in
5.x, seemingly caused by different behaviour compared to 4.x (which
I used previously).
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/8abc04d1
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/8abc04d1
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/8abc04d1
Branch: refs/heads/master
Commit: 8abc04d1383b18536f3950388829d22b4d884e8b
Parents: d9aba01
Author: Otto van der Schaaf <os...@we-amp.com>
Authored: Tue Aug 5 18:46:02 2014 +0200
Committer: Otto van der Schaaf <os...@we-amp.com>
Committed: Tue Aug 5 18:46:02 2014 +0200
----------------------------------------------------------------------
.../ats_speed/ats_beacon_intercept.cc | 10 +++-
plugins/experimental/ats_speed/ats_speed.cc | 62 +++++++++++---------
2 files changed, 41 insertions(+), 31 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/8abc04d1/plugins/experimental/ats_speed/ats_beacon_intercept.cc
----------------------------------------------------------------------
diff --git a/plugins/experimental/ats_speed/ats_beacon_intercept.cc b/plugins/experimental/ats_speed/ats_beacon_intercept.cc
index 175e218..1e97d8f 100644
--- a/plugins/experimental/ats_speed/ats_beacon_intercept.cc
+++ b/plugins/experimental/ats_speed/ats_beacon_intercept.cc
@@ -213,6 +213,12 @@ handleRead(InterceptCtx *cont_data, bool &read_complete) {
static bool
processRequest(InterceptCtx *cont_data) {
+ // OS: Looks like on 5.x we sometimes receive read complete / EOS events twice,
+ // which needs looking into. Probably this intercept is doing something it shouldn't
+ if (cont_data->output.buffer) {
+ TSDebug("ats_speed", "Received read complete / EOS twice?!");
+ return true;
+ }
string reply_header("HTTP/1.1 204 No Content\r\n");
int body_size = static_cast<int>(cont_data->body.size());
if (cont_data->req_content_len != body_size) {
@@ -246,7 +252,7 @@ processRequest(InterceptCtx *cont_data) {
net_instaweb::RequestContextPtr(system_request_context))) {
TSError("Beacon handling failure!");
} else {
- //TSDebug(DEBUG_TAG, "Beacon post data processed OK: [%s]", beacon_data.c_str());
+ TSDebug(DEBUG_TAG, "Beacon post data processed OK: [%s]", beacon_data.c_str());
}
cont_data->setupWrite();
@@ -329,7 +335,7 @@ txn_intercept(TSCont contp, TSEvent event, void *edata) {
}
delete cont_data;
TSContDestroy(contp);
- }
+ }
return 1;
}
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/8abc04d1/plugins/experimental/ats_speed/ats_speed.cc
----------------------------------------------------------------------
diff --git a/plugins/experimental/ats_speed/ats_speed.cc b/plugins/experimental/ats_speed/ats_speed.cc
index afefa37..8cd96cb 100644
--- a/plugins/experimental/ats_speed/ats_speed.cc
+++ b/plugins/experimental/ats_speed/ats_speed.cc
@@ -716,40 +716,43 @@ handle_read_request_header(TSHttpTxn txnp) {
ctx->url_string = new GoogleString(url, url_length);
ctx->gurl = new GoogleUrl(*(ctx->url_string));
- CHECK(ctx->gurl->IsWebValid()) << "Invalid URL!";
- const char * method;
- int method_len;
- method = TSHttpHdrMethodGet(reqp, hdr_loc, &method_len);
- bool head_or_get = method == TS_HTTP_METHOD_GET || method == TS_HTTP_METHOD_HEAD;
- ctx->request_method = method;
- GoogleString user_agent = get_header(reqp, hdr_loc, "User-Agent");
- ctx->user_agent = new GoogleString(user_agent);
- ctx->server_context = ats_process_context->server_context();
- if (user_agent.find(kModPagespeedSubrequestUserAgent) != user_agent.npos) {
- ctx->mps_user_agent = true;
- }
- if (ats_process_context->server_context()->IsPagespeedResource(gurl)) {
- if (head_or_get && !ctx->mps_user_agent) {
+ if (!ctx->gurl->IsWebValid()) {
+ TSDebug("ats-speed", "URL != WebValid(): %s", ctx->url_string->c_str());
+ } else {
+ const char * method;
+ int method_len;
+ method = TSHttpHdrMethodGet(reqp, hdr_loc, &method_len);
+ bool head_or_get = method == TS_HTTP_METHOD_GET || method == TS_HTTP_METHOD_HEAD;
+ ctx->request_method = method;
+ GoogleString user_agent = get_header(reqp, hdr_loc, "User-Agent");
+ ctx->user_agent = new GoogleString(user_agent);
+ ctx->server_context = ats_process_context->server_context();
+ if (user_agent.find(kModPagespeedSubrequestUserAgent) != user_agent.npos) {
+ ctx->mps_user_agent = true;
+ }
+ if (ats_process_context->server_context()->IsPagespeedResource(gurl)) {
+ if (head_or_get && !ctx->mps_user_agent) {
+ ctx->resource_request = true;
+ TSHttpTxnArgSet(txnp, TXN_INDEX_OWNED_ARG, &TXN_INDEX_OWNED_ARG_UNSET);
+ }
+ } else if (ctx->gurl->PathSansQuery() == "/pagespeed_message"
+ || ctx->gurl->PathSansQuery() == "/pagespeed_statistics"
+ || ctx->gurl->PathSansQuery() == "/pagespeed_global_statistics"
+ || ctx->gurl->PathSansQuery() == "/pagespeed_console"
+ || ctx->gurl->PathSansLeaf() == "/ats_speed_static/"
+ || ctx->gurl->PathSansQuery() == "/robots.txt"
+ ) {
ctx->resource_request = true;
TSHttpTxnArgSet(txnp, TXN_INDEX_OWNED_ARG, &TXN_INDEX_OWNED_ARG_UNSET);
}
- } else if (ctx->gurl->PathSansQuery() == "/pagespeed_message"
- || ctx->gurl->PathSansQuery() == "/pagespeed_statistics"
- || ctx->gurl->PathSansQuery() == "/pagespeed_global_statistics"
- || ctx->gurl->PathSansQuery() == "/pagespeed_console"
- || ctx->gurl->PathSansLeaf() == "/ats_speed_static/"
- || ctx->gurl->PathSansQuery() == "/robots.txt"
- ) {
- ctx->resource_request = true;
+ else if (StringCaseEqual(gurl.PathSansQuery() ,"/ats_speed_beacon")) {
+ ctx->beacon_request = true;
TSHttpTxnArgSet(txnp, TXN_INDEX_OWNED_ARG, &TXN_INDEX_OWNED_ARG_UNSET);
- }
- else if (StringCaseEqual(gurl.PathSansQuery() ,"/ats_speed_beacon")) {
- ctx->beacon_request = true;
- TSHttpTxnArgSet(txnp, TXN_INDEX_OWNED_ARG, &TXN_INDEX_OWNED_ARG_UNSET);
- hook_beacon_intercept(txnp);
+ hook_beacon_intercept(txnp);
+ }
}
TSfree((void*)url);
- }
+ } // gurl->IsWebValid() == true
TSHandleMLocRelease(reqp, TS_NULL_MLOC, hdr_loc);
} else {
DCHECK(false) << "Could not get client request header\n";
@@ -868,7 +871,8 @@ transform_plugin(TSCont contp, TSEvent event, void *edata)
TSHandleMLocRelease(response_header_buf, TS_NULL_MLOC, response_header_loc);
}
}
- bool ok = !(ctx->resource_request || ctx->beacon_request || ctx->mps_user_agent);
+ bool ok = ctx->gurl->IsWebValid() &&
+ !(ctx->resource_request || ctx->beacon_request || ctx->mps_user_agent);
if (!ok) {
TSHttpTxnReenable(txn, TS_EVENT_HTTP_CONTINUE);
return 0;