---------------------
PatchSet 11528 
Date: 2007/07/15 09:52:17
Author: hno
Branch: SQUID_2_6
Tag: (none) 
Log:
MFC: Deal better with forwarding loops

There was still cases where forwarding loops would end up in very bad
situations. This patch makes Squid reject requests with a double loop
hard, and also to consider looping requests uncachable to avoid problems
with collapsed forwarding.

Merged changes:
2007/07/15 05:41:19	hno	+25 -33	Deal better with forwarding loops

Members: 
	src/HttpHeaderTools.c:1.37->1.37.2.1 
	src/client_side.c:1.693.2.12->1.693.2.13 
	src/protos.h:1.520.2.2->1.520.2.3 
	src/structs.h:1.507.2.6->1.507.2.7 

Index: squid/src/HttpHeaderTools.c
===================================================================
RCS file: /cvsroot/squid/squid/src/HttpHeaderTools.c,v
retrieving revision 1.37
retrieving revision 1.37.2.1
diff -u -r1.37 -r1.37.2.1
--- squid/src/HttpHeaderTools.c	26 Jul 2006 20:09:33 -0000	1.37
+++ squid/src/HttpHeaderTools.c	15 Jul 2007 09:52:17 -0000	1.37.2.1
@@ -1,6 +1,6 @@
 
 /*
- * $Id: HttpHeaderTools.c,v 1.37 2006/07/26 20:09:33 hno Exp $
+ * $Id: HttpHeaderTools.c,v 1.37.2.1 2007/07/15 09:52:17 hno Exp $
  *
  * DEBUG: section 66    HTTP Header Tools
  * AUTHOR: Alex Rousskov
@@ -190,31 +190,17 @@
     return 0;
 }
 
-/* returns true iff "s" is a substring of a member of the list */
+/* returns true iff "s" is a substring of a member of the list, >1 if more than once */
 int
-strListIsSubstr(const String * list, const char *s, char del)
+strIsSubstr(const String * list, const char *s)
 {
-    assert(list && del);
-    return strStr(*list, s) != 0;
-
-    /*
-     * Note: the original code with a loop is broken because it uses strstr()
-     * instead of strnstr(). If 's' contains a 'del', strListIsSubstr() may
-     * return true when it should not. If 's' does not contain a 'del', the
-     * implementaion is equavalent to strstr()! Thus, we replace the loop with
-     * strstr() above until strnstr() is available.
-     */
-
-#ifdef BROKEN_CODE
-    const char *pos = NULL;
-    const char *item;
     assert(list && s);
-    while (strListGetItem(list, del, &item, NULL, &pos)) {
-	if (strstr(item, s))
-	    return 1;
-    }
-    return 0;
-#endif
+    const char *p = strStr(*list, s);
+    if (!p)
+	return 0;
+    if (strstr(p + 1, s) != NULL)
+	return 2;
+    return 1;
 }
 
 /* appends an item to the list */
Index: squid/src/client_side.c
===================================================================
RCS file: /cvsroot/squid/squid/src/client_side.c,v
retrieving revision 1.693.2.12
retrieving revision 1.693.2.13
diff -u -r1.693.2.12 -r1.693.2.13
--- squid/src/client_side.c	20 Mar 2007 21:26:34 -0000	1.693.2.12
+++ squid/src/client_side.c	15 Jul 2007 09:52:17 -0000	1.693.2.13
@@ -1,6 +1,6 @@
 
 /*
- * $Id: client_side.c,v 1.693.2.12 2007/03/20 21:26:34 hno Exp $
+ * $Id: client_side.c,v 1.693.2.13 2007/07/15 09:52:17 hno Exp $
  *
  * DEBUG: section 33    Client-side Routines
  * AUTHOR: Duane Wessels
@@ -1520,16 +1520,19 @@
 	}
     }
     if (httpHeaderHas(req_hdr, HDR_VIA)) {
-	String s = httpHeaderGetList(req_hdr, HDR_VIA);
 	/*
 	 * ThisCache cannot be a member of Via header, "1.0 ThisCache" can.
 	 * Note ThisCache2 has a space prepended to the hostname so we don't
 	 * accidentally match super-domains.
 	 */
-	if (strListIsSubstr(&s, ThisCache2, ',')) {
+	String s = httpHeaderGetList(req_hdr, HDR_VIA);
+	int n = strIsSubstr(&s, ThisCache2);
+	if (n) {
 	    debugObj(33, 1, "WARNING: Forwarding loop detected for:\n",
 		request, (ObjPackMethod) & httpRequestPackDebug);
 	    request->flags.loopdetect = 1;
+	    if (n > 1)
+		request->flags.loopdetect_twice = 1;
 	}
 #if FORW_VIA_DB
 	fvdbCountVia(strBuf(s));
@@ -1615,6 +1618,8 @@
 {
     request_t *req = http->request;
     method_t method = req->method;
+    if (req->flags.loopdetect)
+	return 0;
     if (req->protocol == PROTO_HTTP)
 	return httpCachable(method);
     /* FTP is always cachable */
@@ -3415,11 +3420,11 @@
 	return;
     }
     /*
-     * Deny loops when running in accelerator/transproxy mode.
+     * Deny double loops
      */
-    if (r->flags.loopdetect && (http->flags.accel || http->flags.transparent)) {
-	http->al.http.code = HTTP_FORBIDDEN;
-	err = errorCon(ERR_ACCESS_DENIED, HTTP_FORBIDDEN, http->orig_request);
+    if (r->flags.loopdetect_twice) {
+	http->al.http.code = HTTP_GATEWAY_TIMEOUT;
+	err = errorCon(ERR_CANNOT_FORWARD, HTTP_GATEWAY_TIMEOUT, http->orig_request);
 	http->log_type = LOG_TCP_DENIED;
 	http->entry = clientCreateStoreEntry(http, r->method, null_request_flags);
 	errorAppendEntry(http->entry, err);
Index: squid/src/protos.h
===================================================================
RCS file: /cvsroot/squid/squid/src/protos.h,v
retrieving revision 1.520.2.2
retrieving revision 1.520.2.3
diff -u -r1.520.2.2 -r1.520.2.3
--- squid/src/protos.h	22 Jun 2007 12:07:36 -0000	1.520.2.2
+++ squid/src/protos.h	15 Jul 2007 09:52:17 -0000	1.520.2.3
@@ -1,6 +1,6 @@
 
 /*
- * $Id: protos.h,v 1.520.2.2 2007/06/22 12:07:36 adrian Exp $
+ * $Id: protos.h,v 1.520.2.3 2007/07/15 09:52:17 hno Exp $
  *
  *
  * SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -402,7 +402,7 @@
 extern void strListAdd(String * str, const char *item, char del);
 extern void strListAddUnique(String * str, const char *item, char del);
 extern int strListIsMember(const String * str, const char *item, char del);
-extern int strListIsSubstr(const String * list, const char *s, char del);
+extern int strIsSubstr(const String * list, const char *s);
 extern int strListGetItem(const String * str, char del, const char **item, int *ilen, const char **pos);
 extern const char *getStringPrefix(const char *str, const char *end);
 extern int httpHeaderParseInt(const char *start, int *val);
Index: squid/src/structs.h
===================================================================
RCS file: /cvsroot/squid/squid/src/structs.h,v
retrieving revision 1.507.2.6
retrieving revision 1.507.2.7
diff -u -r1.507.2.6 -r1.507.2.7
--- squid/src/structs.h	23 Jun 2007 22:50:18 -0000	1.507.2.6
+++ squid/src/structs.h	15 Jul 2007 09:52:18 -0000	1.507.2.7
@@ -1,6 +1,6 @@
 
 /*
- * $Id: structs.h,v 1.507.2.6 2007/06/23 22:50:18 hno Exp $
+ * $Id: structs.h,v 1.507.2.7 2007/07/15 09:52:18 hno Exp $
  *
  *
  * SQUID Web Proxy Cache          http://www.squid-cache.org/
@@ -1815,6 +1815,7 @@
     unsigned int cachable:1;
     unsigned int hierarchical:1;
     unsigned int loopdetect:1;
+    unsigned int loopdetect_twice:1;
     unsigned int proxy_keepalive:1;
     unsigned int proxying:1;	/* this should be killed, also in httpstateflags */
     unsigned int refresh:1;