From 2b64ddfdac6a714d8689d735534175758c45eff1 Mon Sep 17 00:00:00 2001 From: Amos Jeffries Date: Sun, 19 Dec 2010 23:12:57 -0700 Subject: [PATCH] Author: Alex Rousskov HTTP/1.1: do not forward TRACE with Max-Forwards: 0 after REQMOD Before the change, Max-Forwards request value was cached in HttpRequest::max_forwards member. It was set once in clientProcessRequest() function. This works fine as long as no request adaptation is performed. Otherwise original HTTP request may be replaced with adopted one in ClientHttpRequest::noteAdaptationAnswer() method and max_forwards value is lost. This change removes HttpRequest::max_forwards member and gets the value directly from HttpHeader when needed. This adds another string-to-int conversion for TRACE and OPTIONS requests, but those are rare, and we save a little in the other, far more common cases by removing the HttpRequest::max_forwards member. Removed assertion from clientReplyContext::traceReply() since it is called from a single place and the condition is checked right before the call. Co-Advisors test cases: test_case/rfc2616/maxForwardsZero-TRACE-asterisk test_case/rfc2616/maxForwardsZero-TRACE-absolute --- src/HttpRequest.cc | 2 -- src/HttpRequest.h | 2 -- src/client_side.cc | 6 ++---- src/client_side_reply.cc | 3 +-- src/url.cc | 2 +- 5 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc index 2c1f038564..b0615b9849 100644 --- a/src/HttpRequest.cc +++ b/src/HttpRequest.cc @@ -89,7 +89,6 @@ HttpRequest::init() ims = -1; imslen = 0; lastmod = -1; - max_forwards = -1; client_addr.SetEmpty(); my_addr.SetEmpty(); body_pipe = NULL; @@ -197,7 +196,6 @@ HttpRequest::clone() const // range handled in hdrCacheInit() copy->ims = ims; copy->imslen = imslen; - copy->max_forwards = max_forwards; copy->hier = hier; // Is it safe to copy? Should we? copy->errType = errType; diff --git a/src/HttpRequest.h b/src/HttpRequest.h index f2834b61eb..ed5ea1db7d 100644 --- a/src/HttpRequest.h +++ b/src/HttpRequest.h @@ -163,8 +163,6 @@ public: int imslen; - int64_t max_forwards; - IpAddress client_addr; #if FOLLOW_X_FORWARDED_FOR diff --git a/src/client_side.cc b/src/client_side.cc index a7d6c61f6b..9546aed09e 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -2434,10 +2434,8 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c } else conn->cleanDechunkingRequest(); - if (method == METHOD_TRACE || method == METHOD_OPTIONS) - request->max_forwards = request->header.getInt64(HDR_MAX_FORWARDS); - - mustReplyToOptions = (method == METHOD_OPTIONS) && (request->max_forwards == 0); + mustReplyToOptions = (method == METHOD_OPTIONS) && + (request->header.getInt64(HDR_MAX_FORWARDS) == 0); unsupportedTe = tePresent && !deChunked; if (!urlCheckRequest(request) || mustReplyToOptions || unsupportedTe) { clientStreamNode *node = context->getClientReplyContext(); diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc index 37823a9e6d..e7a9957b90 100644 --- a/src/client_side_reply.cc +++ b/src/client_side_reply.cc @@ -947,7 +947,6 @@ clientReplyContext::traceReply(clientStreamNode * node) { clientStreamNode *nextNode = (clientStreamNode *)node->node.next->data; StoreIOBuffer localTempBuffer; - assert(http->request->max_forwards == 0); createStoreEntry(http->request->method, request_flags()); localTempBuffer.offset = nextNode->readBuffer.offset + headers_sz; localTempBuffer.length = nextNode->readBuffer.length; @@ -1625,7 +1624,7 @@ clientGetMoreData(clientStreamNode * aNode, ClientHttpRequest * http) // OPTIONS with Max-Forwards:0 handled in clientProcessRequest() if (context->http->request->method == METHOD_TRACE) { - if (context->http->request->max_forwards == 0) { + if (context->http->request->header.getInt64(HDR_MAX_FORWARDS) == 0) { context->traceReply(aNode); return; } diff --git a/src/url.cc b/src/url.cc index 1b801152fa..2f69f9a836 100644 --- a/src/url.cc +++ b/src/url.cc @@ -799,7 +799,7 @@ urlCheckRequest(const HttpRequest * r) // we support OPTIONS and TRACE directed at us (with a 501 reply, for now) // we also support forwarding OPTIONS and TRACE, except for the *-URI ones if (r->method == METHOD_OPTIONS || r->method == METHOD_TRACE) - return (r->max_forwards == 0 || r->urlpath != "*"); + return (r->header.getInt64(HDR_MAX_FORWARDS) == 0 || r->urlpath != "*"); if (r->method == METHOD_PURGE) return 1; -- 2.47.2