HTTP Compliance: 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 4cd07fdaade048dfe719d14e9851d1babae40c3b..feefc7cd85349b8a3cf47d6d6ef8b1fcede754fe 100644 (file)
--- a/src/HttpRequest.cc
+++ b/src/HttpRequest.cc
@@ -91,7 +91,6 @@ HttpRequest::init()
     ims = -1;
     imslen = 0;
     lastmod = -1;
-    max_forwards = -1;
     client_addr.SetEmpty();
 #if USE_SQUID_EUI
     client_eui48.clear();
@@ -205,7 +204,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 4c72c6d26696756cf6b510d9669a9709610b33fc..7d34c93f4653a62b8702cd31acf0923bc5988044 100644 (file)
--- a/src/HttpRequest.h
+++ b/src/HttpRequest.h
@@ -177,8 +177,6 @@ public:
 
     int imslen;
 
-    int64_t max_forwards;
-
     Ip::Address client_addr;
 
 #if FOLLOW_X_FORWARDED_FOR

diff --git a/src/client_side.cc b/src/client_side.cc
index 421697e1eac890df4fac88249c4fec660898fa6c..7fed8fc19c21a3a1ce4d62c9c7b90fad75078e79 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -2510,10 +2510,8 @@ clientProcessRequest(ConnStateData *conn, HttpParser *hp, ClientSocketContext *c
         unsupportedTe = te.size() && te != "identity";
     } // else implied identity coding
 
-    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);
     if (!urlCheckRequest(request) || mustReplyToOptions || unsupportedTe) {
         clientStreamNode *node = context->getClientReplyContext();
         clientReplyContext *repContext = dynamic_cast<clientReplyContext *>(node->data.getRaw());

diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc
index ee04056c0b3393c5caad03ee43900e5d565150a4..793d5b828c22d2c63aef63893cf3e6ed53791264 100644 (file)
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -997,7 +997,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;
@@ -1693,7 +1692,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 7c06675911361422038f9bc677cd9a0edf35810e..db1ecb321509b207de29a8309a0dc9a56a9883ba 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;