From: Amos Jeffries <squid3@treenet.co.nz>
Date: Sat, 15 Nov 2014 15:04:27 +0000 (-0800)
Subject: Simplify payload tracking
X-Git-Tag: merge-candidate-3-v1~240^2~11
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=8e1007804f7a66a794979213e9a78714514cbcc2;p=thirdparty%2Fsquid.git

Simplify payload tracking

There is no need to account for message headers and total reply size
when tracking payloads for excess-data detection.

Simply start tracking payload after the message header block and detect
if the tracked amount ever exceeds expected.

Also make the payload tracking variables private.
---

diff --git a/src/http.cc b/src/http.cc
index 8e46c8e206..02181084e4 100644
--- a/src/http.cc
+++ b/src/http.cc
@@ -84,9 +84,13 @@ static void copyOneHeaderFromClientsideRequestToUpstreamRequest(const HttpHeader
 //Declared in HttpHeaderTools.cc
 void httpHdrAdd(HttpHeader *heads, HttpRequest *request, const AccessLogEntryPointer &al, HeaderWithAclList &headers_add);
 
-HttpStateData::HttpStateData(FwdState *theFwdState) : AsyncJob("HttpStateData"), Client(theFwdState),
-        lastChunk(0), header_bytes_read(0), reply_bytes_read(0),
-        body_bytes_truncated(0), httpChunkDecoder(NULL)
+HttpStateData::HttpStateData(FwdState *theFwdState) :
+        AsyncJob("HttpStateData"),
+        Client(theFwdState),
+        lastChunk(0),
+        httpChunkDecoder(NULL),
+        payloadSeen(0),
+        payloadTruncated(0)
 {
     debugs(11,5,HERE << "HttpStateData " << this << " created");
     ignoreCacheControl = false;
@@ -743,9 +747,10 @@ HttpStateData::processReplyHeader()
     debugs(11, 2, "HTTP Server RESPONSE:\n---------\n" <<
            hp->messageProtocol() << " " << hp->messageStatus() << " " << hp->reasonPhrase() << "\n" <<
            hp->mimeHeader() <<
-           "\n----------");
+           "----------");
 
-    header_bytes_read = hp->messageHeaderSize();
+    // reset payload tracking to begin after message headers
+    payloadSeen = inBuf.length();
 
     HttpReply *newrep = new HttpReply;
     // XXX: performance regression, c_str() reallocates.
@@ -848,12 +853,7 @@ void
 HttpStateData::proceedAfter1xx()
 {
     Must(flags.handling1xx);
-
-    debugs(11, 2, HERE << "consuming " << header_bytes_read <<
-           " header and " << reply_bytes_read << " body bytes read after 1xx");
-    header_bytes_read = 0;
-    reply_bytes_read = 0;
-
+    debugs(11, 2, "continuing with " << payloadSeen << " bytes in buffer after 1xx");
     CallJobHere(11, 3, this, HttpStateData, HttpStateData::processReply);
 }
 
@@ -1109,16 +1109,12 @@ HttpStateData::persistentConnStatus() const
     /** \par
      * If the body size is known, we must wait until we've gotten all of it. */
     if (clen > 0) {
-        // old technique:
-        // if (entry->mem_obj->endOffset() < vrep->content_length + vrep->hdr_sz)
-        const int64_t body_bytes_read = reply_bytes_read - header_bytes_read;
-        debugs(11,5, "persistentConnStatus: body_bytes_read=" <<
-               body_bytes_read << " content_length=" << vrep->content_length);
+        debugs(11,5, "payloadSeen=" << payloadSeen << " content_length=" << vrep->content_length);
 
-        if (body_bytes_read < vrep->content_length)
+        if (payloadSeen < vrep->content_length)
             return INCOMPLETE_MSG;
 
-        if (body_bytes_truncated > 0) // already read more than needed
+        if (payloadTruncated > 0) // already read more than needed
             return COMPLETE_NONPERSISTENT_MSG; // disable pconns
     }
 
@@ -1194,7 +1190,7 @@ HttpStateData::readReply(const CommIoCbParams &io)
 
     case Comm::OK:
     {
-        reply_bytes_read += rd.size;
+        payloadSeen += rd.size;
 #if USE_DELAY_POOLS
         DelayId delayId = entry->mem_obj->mostBytesAllowed();
         delayId.bytesIn(rd.size);
@@ -1353,18 +1349,17 @@ HttpStateData::truncateVirginBody()
     if (!vrep->expectingBody(request->method, clen) || clen < 0)
         return; // no body or a body of unknown size, including chunked
 
-    const int64_t body_bytes_read = reply_bytes_read - header_bytes_read;
-    if (body_bytes_read - body_bytes_truncated <= clen)
+    if (payloadSeen - payloadTruncated <= clen)
         return; // we did not read too much or already took care of the extras
 
-    if (const int64_t extras = body_bytes_read - body_bytes_truncated - clen) {
+    if (const int64_t extras = payloadSeen - payloadTruncated - clen) {
         // server sent more that the advertised content length
-        debugs(11,5, HERE << "body_bytes_read=" << body_bytes_read <<
+        debugs(11, 5, "payloadSeen=" << payloadSeen <<
                " clen=" << clen << '/' << vrep->content_length <<
-               " body_bytes_truncated=" << body_bytes_truncated << '+' << extras);
+               " trucated=" << payloadTruncated << '+' << extras);
 
         inBuf.chop(0, inBuf.length() - extras);
-        body_bytes_truncated += extras;
+        payloadTruncated += extras;
     }
 }
 
diff --git a/src/http.h b/src/http.h
index 362ac2fa21..44bee18445 100644
--- a/src/http.h
+++ b/src/http.h
@@ -47,9 +47,6 @@ public:
     int lastChunk;		/* reached last chunk of a chunk-encoded reply */
     HttpStateFlags flags;
     size_t read_sz;
-    int header_bytes_read;	// to find end of response,
-    int64_t reply_bytes_read;	// without relying on StoreEntry
-    int body_bytes_truncated; // positive when we read more than we wanted
     SBuf inBuf;                ///< I/O buffer for receiving server responses
     bool ignoreCacheControl;
     bool surrogateNoStore;
@@ -113,6 +110,11 @@ private:
     /// Parser being used at present to parse the HTTP/ICY server response.
     Http1::ResponseParserPointer hp;
     ChunkedCodingParser *httpChunkDecoder;
+
+    /// amount of message payload/body received so far.
+    int64_t payloadSeen;
+    /// positive when we read more than we wanted
+    int64_t payloadTruncated;
 };
 
 int httpCachable(const HttpRequestMethod&);