Author: Alex Rousskov <rousskov@measurement-factory.com>

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

diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc
index 2c1f038564028160818bd7f7938eda971a6f8ae1..b0615b9849a221e16ed4275f4b7013a75b583055 100644 (file)
--- 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 f2834b61eb3462dc6b71cbfea1be1a5160807606..ed5ea1db7de295b28431f54b933be0bc9ec484ee 100644 (file)
--- 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 a7d6c61f6b6293fd844f3f2e28c7e4624d48cede..9546aed09e04978bf89ae775eb862c504b0e0985 100644 (file)
--- 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 37823a9e6db155f312aaa6de88345ad502fa061d..e7a9957b907c32a802243b61b70f85ff9b6a1cc9 100644 (file)
--- 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 1b801152fa04283cfd294928fedfeb182c7a0e98..2f69f9a8360a0858f1f83594f5a1bd125bdecce9 100644 (file)
--- 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;