From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Sun, 26 Jul 2015 18:26:52 +0000 (-0600)
Subject: Fix ICAP transactions that read a lot of data
X-Git-Tag: merge-candidate-3-v1~29^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2b42f3fd7cc5c5b562a068eab22874de94813a9f;p=thirdparty%2Fsquid.git

Fix ICAP transactions that read a lot of data
by ensuring the read buffer has space [unless it is really full].

Trunk r13995 (Parser-NG: Convert the ICAP read buffer to an SBuf)
broke ICAP transactions that read a lot of data because the new
SBuf::consume() method often does not free buffer space, unlike the
old MemBuf::consume(). Affected transactions failed with mayReadMore()
exceptions because their readBuf.spaceSize() was zero while they
needed to read more data.

Any append,parse,consume;append,parse,consume;... user of SBuf cannot
rely on SBuf::spaceSize() to be meaningful because even consuming the
entire SBuf contents may leave spaceSize() at zero! Instead such code
has to use SBuf::length() to keep buffer from growing too big and
SBuf::rawSpace(1) to ensure some space is available for reading when
the buffer is not too big.
---

diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc
index 8fb8f11099..8cb90777f6 100644
--- a/src/adaptation/icap/ModXact.cc
+++ b/src/adaptation/icap/ModXact.cc
@@ -557,10 +557,10 @@ void Adaptation::Icap::ModXact::readMore()
         return;
     }
 
-    if (readBuf.spaceSize())
+    if (readBuf.length() < SQUID_TCP_SO_RCVBUF)
         scheduleRead();
     else
-        debugs(93,3,HERE << "nothing to do because !readBuf.spaceSize()");
+        debugs(93,3,HERE << "cannot read with a full buffer");
 }
 
 // comm module read a portion of the ICAP response for us
diff --git a/src/adaptation/icap/Xaction.cc b/src/adaptation/icap/Xaction.cc
index e731ad2af6..c2df952634 100644
--- a/src/adaptation/icap/Xaction.cc
+++ b/src/adaptation/icap/Xaction.cc
@@ -456,6 +456,11 @@ void Adaptation::Icap::Xaction::noteCommRead(const CommIoCbParams &io)
 
     // TODO: tune this better to expected message sizes
     readBuf.reserveCapacity(SQUID_TCP_SO_RCVBUF);
+    // we are not asked to grow beyond the allowed maximum
+    Must(readBuf.length() < SQUID_TCP_SO_RCVBUF);
+    // now we can ensure that there is space to read new data,
+    // even if readBuf.spaceSize() currently returns zero.
+    readBuf.rawSpace(1);
 
     CommIoCbParams rd(this); // will be expanded with ReadNow results
     rd.conn = io.conn;
@@ -534,7 +539,7 @@ bool Adaptation::Icap::Xaction::parseHttpMsg(HttpMsg *msg)
 bool Adaptation::Icap::Xaction::mayReadMore() const
 {
     return !doneReading() && // will read more data
-           readBuf.spaceSize();  // have space for more data
+           readBuf.length() < SQUID_TCP_SO_RCVBUF;  // have space for more data
 }
 
 bool Adaptation::Icap::Xaction::doneReading() const