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 8fb8f1109973ae65c303a0c5a463e82474dcf4de..8cb90777f6abcb07e7ba38de81fb80466313df67 100644 (file)
--- 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 e731ad2af69503a9473c2df4e70cb28abf6fb904..c2df9526342abb25daf262386800a24266a4d70c 100644 (file)
--- 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