--------------------- PatchSet 3877 Date: 2006/10/25 04:58:05 Author: rousskov Branch: squid3-icap Tag: (none) Log: - When selecting a matching ICAP class before performing asynchronous ACL checks, ignore ICAP services that do not want to see the HTTP request URL. Only services that are "up" (have fresh OPTIONS response and such) can be ignored in such a manner. Classes with matching down services are always accepted as candidates. - When selecting a matching service after performing asynchronous ACL checks, first try to find a service that is "up" and interested in the HTTP request URL. If such a service does not exist, use the first matching service. This algorithm avoids unreachable ICAP services, provided that there is a reachable service in the same ICAP class. To avoid unreachable ICAP services under any circumstances, we need to distinguish unreachable (and otherwise broken) services from services that have not been tried/queried yet. Members: src/ICAP/ICAPConfig.cc:1.1.2.2->1.1.2.3 src/ICAP/ICAPConfig.h:1.1.2.2->1.1.2.3 Index: squid3/src/ICAP/ICAPConfig.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/ICAPConfig.cc,v retrieving revision 1.1.2.2 retrieving revision 1.1.2.3 diff -u -r1.1.2.2 -r1.1.2.3 --- squid3/src/ICAP/ICAPConfig.cc 29 Sep 2006 23:27:15 -0000 1.1.2.2 +++ squid3/src/ICAP/ICAPConfig.cc 25 Oct 2006 04:58:05 -0000 1.1.2.3 @@ -1,6 +1,6 @@ /* - * $Id: ICAPConfig.cc,v 1.1.2.2 2006/09/29 23:27:15 dwsquid Exp $ + * $Id: ICAPConfig.cc,v 1.1.2.3 2006/10/25 04:58:05 rousskov Exp $ * * SQUID Web Proxy Cache http://www.squid-cache.org/ * ---------------------------------------------------------- @@ -152,31 +152,17 @@ for (ci = TheICAPConfig.classes.begin(); ci != TheICAPConfig.classes.end(); ++ci) { - ICAPClass *theClass = *ci; - - Vector::iterator si; - - for (si = theClass->services.begin(); si != theClass->services.end(); ++si) { - ICAPServiceRep *theService = si->getRaw(); - - if (method != theService->method) - continue; - - if (point != theService->point) - continue; - - debug(93,3)("ICAPAccessCheck::check: class '%s' has candidate service '%s'\n", theClass->key.buf(), theService->key.buf()); - - candidateClasses += theClass->key; - - /* - * Break here because we only need one matching service - * to justify ACL-checking a class. We might use other - * services belonging to the class if the first service - * is unavailable, etc. - */ - break; - + /* + * We only find the first matching service because we only need + * one matching service to justify ACL-checking a class. We might + * use other services belonging to the class if the first service + * turns out to be unusable for some reason. + */ + ICAPClass *c = *ci; + ICAPServiceRep::Pointer service = findFirstService(c, false); + if (service.getRaw()) { + debug(93,3)("ICAPAccessCheck::check: class '%s' has candidate service '%s'\n", c->key.buf(), service->key.buf()); + candidateClasses += c->key; } } @@ -277,23 +263,55 @@ return; } - Vector::iterator i; + // Try to find an "up" service to use first. This usually suceeds. + ICAPServiceRep::Pointer service = findFirstService(theClass, true); - for (i = theClass->services.begin(); i != theClass->services.end(); ++i) { - ICAPServiceRep *theService = i->getRaw(); + // If all services are down, find any matching service + if (!service) + service = findFirstService(theClass, false); + + if (!service) + callback(NULL, callback_data); + else + callback(service, validated_cbdata); +} + +ICAPServiceRep::Pointer +ICAPAccessCheck::findFirstService(ICAPClass *c, bool mustBeUp) { + + debugs(93,7,HERE << "looking for the first matching " << + (mustBeUp ? "up " : "") << "service in class " << c->key); + + Vector::iterator si; + for (si = c->services.begin(); si != c->services.end(); ++si) { + ICAPServiceRep::Pointer service = *si; - if (method != theService->method) + if (method != service->method) continue; - if (point != theService->point) + if (point != service->point) continue; - callback(*i, validated_cbdata); + if (service->up()) { + // we cannot check wantsUrl() for service that is not "up" + if (!service->wantsUrl(req->urlpath)) + continue; + } else + if (mustBeUp) { + // the caller asked for an "up" service + continue; + } - return; + debugs(93,5,HERE << "found first matching " << + (mustBeUp ? "up " : "") << "service in class " << c->key << + ": " << service->key); + + return service; } - callback(NULL, callback_data); + debugs(93,5,HERE << "found no matching " << + (mustBeUp ? "up " : "") << "services in class " << c->key); + return ICAPServiceRep::Pointer(); } // ================================================================================ // Index: squid3/src/ICAP/ICAPConfig.h =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/ICAPConfig.h,v retrieving revision 1.1.2.2 retrieving revision 1.1.2.3 diff -u -r1.1.2.2 -r1.1.2.3 --- squid3/src/ICAP/ICAPConfig.h 29 Sep 2006 23:27:15 -0000 1.1.2.2 +++ squid3/src/ICAP/ICAPConfig.h 25 Oct 2006 04:58:05 -0000 1.1.2.3 @@ -1,6 +1,6 @@ /* - * $Id: ICAPConfig.h,v 1.1.2.2 2006/09/29 23:27:15 dwsquid Exp $ + * $Id: ICAPConfig.h,v 1.1.2.3 2006/10/25 04:58:05 rousskov Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -78,6 +78,7 @@ Vector candidateClasses; String matchedClass; void do_callback(); + ICAPServiceRep::Pointer findFirstService(ICAPClass *c, bool mustBeUp); public: void check();