Limit X-Forwarded-For growth. X-Forwarded-For growth leads to String size limit assertions and probably other problems. To make growth-associated problems visible during forwarding loops, the loop breaking code must be disabled (no Via) or not applicable (direct forwarding) and request_header_max_size has to be raised or disabled. The X-Forwarded-For header may also grow too large for reasons unrelated to forwarding loops. Ported from 3p0-plus (r8940..8941). This change also prevents most cases of pointless computation of the original X-Forwarded-For value list. That computation can be quite expensive. === modified file 'src/http.cc' --- src/http.cc 2009-07-11 00:16:42 +0000 +++ src/http.cc 2009-07-11 01:07:15 +0000 @@ -1464,7 +1464,6 @@ LOCAL_ARRAY(char, ntoabuf, MAX_IPSTRLEN); const HttpHeader *hdr_in = &orig_request->header; const HttpHeaderEntry *e = NULL; - String strFwd; HttpHeaderPos pos = HttpHeaderInitPos; assert (hdr_out->owner == hoRequest); @@ -1514,24 +1513,37 @@ } #endif - strFwd = hdr_in->getList(HDR_X_FORWARDED_FOR); - /** \pre Handle X-Forwarded-For */ if (strcmp(opt_forwarded_for, "delete") != 0) { + + String strFwd = hdr_in->getList(HDR_X_FORWARDED_FOR); + + if (strFwd.size() > 65536/2) { + // There is probably a forwarding loop with Via detection disabled. + // If we do nothing, String will assert on overflow soon. + + // XXX: the Right Thing is to catch this at a higher level and + // terminate this looping transaction or it may loop forever. + strFwd = "unknowns"; + + static int warnedCount = 0; + if (warnedCount++ < 100) { + const char *url = entry ? entry->url() : urlCanonical(orig_request); + debugs(11, 1, "Warning: Undetected forwarding loop for " << url); + } + } + if (strcmp(opt_forwarded_for, "on") == 0) { /** If set to ON - append client IP or 'unknown'. */ - strFwd = hdr_in->getList(HDR_X_FORWARDED_FOR); if ( orig_request->client_addr.IsNoAddr() ) strListAdd(&strFwd, "unknown", ','); else strListAdd(&strFwd, orig_request->client_addr.NtoA(ntoabuf, MAX_IPSTRLEN), ','); } else if (strcmp(opt_forwarded_for, "off") == 0) { /** If set to OFF - append 'unknown'. */ - strFwd = hdr_in->getList(HDR_X_FORWARDED_FOR); strListAdd(&strFwd, "unknown", ','); } else if (strcmp(opt_forwarded_for, "transparent") == 0) { /** If set to TRANSPARENT - pass through unchanged. */ - strFwd = hdr_in->getList(HDR_X_FORWARDED_FOR); } else if (strcmp(opt_forwarded_for, "truncate") == 0) { /** If set to TRUNCATE - drop existing list and replace with client IP or 'unknown'. */ if ( orig_request->client_addr.IsNoAddr() ) @@ -1543,7 +1555,6 @@ hdr_out->putStr(HDR_X_FORWARDED_FOR, strFwd.termedBuf()); } /** If set to DELETE - do not copy through. */ - strFwd.clean(); /* append Host if not there already */ if (!hdr_out->has(HDR_HOST)) {