This is only *PART OF* the commit.

Connection code was changed in 3.2 so complete patch results in
squid segfaulting....

based on revno: 13991
revision-id: rousskov@measurement-factory.com-20160219231541-syrgnvl1av8bbn8d
parent: rousskov@measurement-factory.com-20160218041533-8tmtd45c3nky2gyy
committer: Alex Rousskov <rousskov@measurement-factory.com>
branch nick: 3.5
timestamp: Fri 2016-02-19 16:15:41 -0700
message:
  Throw instead of asserting on some String overflows.
  
  Note that Client-caught exceptions result in HTTP 500 (Internal Server
  Error) responses with X-Squid-Error set to "ERR_CANNOT_FORWARD 0".
  
  Also avoid stuck Client jobs on exceptions. 
  
  Also unified String size limit checks.
  
  Essentially trunk r14552, which has a detailed commit message.


Index: squid-3.1.23/src/SquidString.h
===================================================================
--- squid-3.1.23.orig/src/SquidString.h
+++ squid-3.1.23/src/SquidString.h
@@ -153,6 +153,13 @@ public:
     _SQUID_INLINE_ int caseCmp (char const *, size_type count) const;
     _SQUID_INLINE_ int caseCmp (String const &) const;
 
+    /// Whether creating a totalLen-character string is safe (i.e., unlikely to assert).
+    /// Optional extras can be used for overflow-safe length addition.
+    /// Implementation has to add 1 because many String allocation methods do.
+    static bool CanGrowTo(size_type totalLen, const size_type extras = 0) { return SafeAdd(totalLen, extras) && SafeAdd(totalLen, 1); }
+    /// whether appending growthLen characters is safe (i.e., unlikely to assert)
+    bool canGrowBy(const size_type growthLen) const { return CanGrowTo(size(), growthLen); }
+
     String substr(size_type from, size_type to) const;
 
     _SQUID_INLINE_ void cut(size_type newLength);
@@ -170,10 +177,20 @@ private:
     _SQUID_INLINE_ bool nilCmp(bool, bool, int &) const;
 
     /* never reference these directly! */
-    size_type size_; /* buffer size; 64K limit */
+    size_type size_; /* buffer size; limited by SizeMax_ */
 
     size_type len_;  /* current length  */
 
+    static const size_type SizeMax_ = 65535; ///< 64K limit protects some fixed-size buffers
+    /// returns true after increasing the first argument by extra if the sum does not exceed SizeMax_
+    static bool SafeAdd(size_type &base, size_type extra) {
+        if (extra <= SizeMax_ && base <= SizeMax_ - extra) {
+            base += extra;
+            return true;
+        }
+        return false;
+    }
+
     char *buf_;
 
     _SQUID_INLINE_ void set(char const *loc, char const ch);
Index: squid-3.1.23/src/HttpHeaderTools.cc
===================================================================
--- squid-3.1.23.orig/src/HttpHeaderTools.cc
+++ squid-3.1.23/src/HttpHeaderTools.cc
@@ -38,6 +38,7 @@
 #include "HttpHdrContRange.h"
 #include "acl/FilledChecklist.h"
 #include "MemBuf.h"
+#include "TextException.h"
 
 static void httpHeaderPutStrvf(HttpHeader * hdr, http_hdr_type id, const char *fmt, va_list vargs);
 
@@ -205,16 +206,19 @@ void
 strListAdd(String * str, const char *item, char del)
 {
     assert(str && item);
+    const String::size_type itemSize = strlen(item);
 
     if (str->size()) {
         char buf[3];
         buf[0] = del;
         buf[1] = ' ';
         buf[2] = '\0';
+        Must(str->canGrowBy(2));
         str->append(buf, 2);
     }
 
-    str->append(item, strlen(item));
+    Must(str->canGrowBy(itemSize));
+    str->append(item, itemSize);
 }
 
 /**
Index: squid-3.1.23/src/String.cc
===================================================================
--- squid-3.1.23.orig/src/String.cc
+++ squid-3.1.23/src/String.cc
@@ -63,7 +63,7 @@ void
 String::setBuffer(char *aBuf, String::size_type aSize)
 {
     assert(undefined());
-    assert(aSize < 65536);
+    assert(aSize < SizeMax_);
     buf_ = aBuf;
     size_ = aSize;
 }
@@ -194,7 +194,7 @@ String::append(const char *str, int len)
     } else {
         // Create a temporary string and absorb it later.
         String snew;
-        assert(len_ + len < 65536); // otherwise snew.len_ overflows below
+        assert(canGrowBy(len)); // otherwise snew.len_ overflows below
         snew.len_ = len_ + len;
         snew.allocBuffer(snew.len_ + 1);
 
Index: squid-3.1.23/src/Server.cc
===================================================================
--- squid-3.1.23.orig/src/Server.cc
+++ squid-3.1.23/src/Server.cc
@@ -58,7 +58,8 @@ ServerStateData::ServerStateData(FwdStat
         , adaptationAccessCheckPending(false)
         , startedAdaptation(false)
 #endif
-        ,theVirginReply(NULL),
+        ,doneWithFwd(NULL),
+        theVirginReply(NULL),
         theFinalReply(NULL)
 {
     fwd = theFwdState;
@@ -236,6 +237,7 @@ ServerStateData::completeForwarding()
 {
     debugs(11,5, HERE << "completing forwarding for "  << fwd);
     assert(fwd != NULL);
+    doneWithFwd = "completeForwarding()";
     fwd->complete();
 }
 
Index: squid-3.1.23/src/Server.h
===================================================================
--- squid-3.1.23.orig/src/Server.h
+++ squid-3.1.23/src/Server.h
@@ -198,6 +198,10 @@ protected:
     bool startedAdaptation;
 #endif
 
+    /// whether we should not be talking to FwdState; XXX: clear fwd instead
+    /// points to a string literal which is used only for debugging
+    const char *doneWithFwd;
+
 private:
     void quitIfAllDone();            /**< successful termination */
     void sendBodyIsTooLargeError();
Index: squid-3.1.23/src/ftp.cc
===================================================================
--- squid-3.1.23.orig/src/ftp.cc
+++ squid-3.1.23/src/ftp.cc
@@ -429,6 +429,7 @@ void
 FtpStateData::ctrlClosed(const CommCloseCbParams &io)
 {
     ctrl.clear();
+    doneWithFwd = "ctrlClosed()"; // assume FwdState is monitoring too
     deleteThis("FtpStateData::ctrlClosed");
 }
 
Index: squid-3.1.23/src/http.cc
===================================================================
--- squid-3.1.23.orig/src/http.cc
+++ squid-3.1.23/src/http.cc
@@ -181,6 +181,7 @@ void
 HttpStateData::httpStateConnClosed(const CommCloseCbParams &params)
 {
     debugs(11, 5, "httpStateFree: FD " << params.fd << ", httpState=" << params.data);
+    doneWithFwd = "httpStateConnClosed()"; // assume FwdState is monitoring too
     deleteThis("HttpStateData::httpStateConnClosed");
 }
 
@@ -1565,7 +1566,8 @@ HttpStateData::httpBuildRequestHeader(Ht
 
         String strFwd = hdr_in->getList(HDR_X_FORWARDED_FOR);
 
-        if (strFwd.size() > 65536/2) {
+        // if we cannot double strFwd size, then it grew past 50% of the limit
+        if (!strFwd.canGrowBy(strFwd.size())) {
             // There is probably a forwarding loop with Via detection disabled.
             // If we do nothing, String will assert on overflow soon.
             // TODO: Terminate all transactions with huge XFF?
