--------------------- PatchSet 1858 Date: 2005/09/21 20:33:37 Author: dwsquid Branch: squid3-icap Tag: (none) Log: A semi-significant rearrangement of the way that client_side does "callout" operations. In order to fit ICAP into the mix we added a "doCallouts" function that manages the sequence of callout operations, such as non-blocking ACLs, redirectors, and ICAP. Members: src/client_side.cc:1.74.2.9->1.74.2.10 src/client_side_request.cc:1.34.4.6->1.34.4.7 src/client_side_request.h:1.17.12.3->1.17.12.4 Index: squid3/src/client_side.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/client_side.cc,v retrieving revision 1.74.2.9 retrieving revision 1.74.2.10 diff -u -r1.74.2.9 -r1.74.2.10 --- squid3/src/client_side.cc 19 Sep 2005 18:00:09 -0000 1.74.2.9 +++ squid3/src/client_side.cc 21 Sep 2005 20:33:37 -0000 1.74.2.10 @@ -1,6 +1,6 @@ /* - * $Id: client_side.cc,v 1.74.2.9 2005/09/19 18:00:09 dwsquid Exp $ + * $Id: client_side.cc,v 1.74.2.10 2005/09/21 20:33:37 dwsquid Exp $ * * DEBUG: section 33 Client-side Routines * AUTHOR: Duane Wessels @@ -70,6 +70,7 @@ #include "ACLChecklist.h" #include "ConnectionDetail.h" #include "client_side_reply.h" +#include "ClientRequestContext.h" #include "MemBuf.h" #if LINGERING_CLOSE @@ -2306,7 +2307,9 @@ if (http->request->method == METHOD_CONNECT) context->mayUseConnection(true); - clientAccessCheck(http); + http->calloutContext = new ClientRequestContext(http); + + http->doCallouts(); } static void Index: squid3/src/client_side_request.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/client_side_request.cc,v retrieving revision 1.34.4.6 retrieving revision 1.34.4.7 diff -u -r1.34.4.6 -r1.34.4.7 --- squid3/src/client_side_request.cc 19 Sep 2005 20:51:42 -0000 1.34.4.6 +++ squid3/src/client_side_request.cc 21 Sep 2005 20:33:38 -0000 1.34.4.7 @@ -1,6 +1,6 @@ /* - * $Id: client_side_request.cc,v 1.34.4.6 2005/09/19 20:51:42 dwsquid Exp $ + * $Id: client_side_request.cc,v 1.34.4.7 2005/09/21 20:33:38 dwsquid Exp $ * * DEBUG: section 85 Client-side Request Routines * AUTHOR: Robert Collins (Originally Duane Wessels in client_side.c) @@ -85,12 +85,12 @@ /* Local functions */ /* other */ -static void clientAccessCheckDone(int, void *); +static void clientAccessCheckDoneWrapper(int, void *); static int clientCachable(ClientHttpRequest * http); static int clientHierarchical(ClientHttpRequest * http); static void clientInterpretRequestHeaders(ClientHttpRequest * http); -static void clientRedirectStart(ClientRequestContext *context); -static RH clientRedirectDone; +static RH clientRedirectDoneWrapper; +static PF checkNoCacheDoneWrapper; extern "C" CSR clientGetMoreData; extern "C" CSS clientReplyStatus; extern "C" CSD clientReplyDetach; @@ -98,26 +98,23 @@ ClientRequestContext::~ClientRequestContext() { - if (http) - cbdataReferenceDone(http); + /* + * Note: ClientRequestContext is only deleted by its parent, + * ClientHttpRequest. That can only happen after ClientRequestContext + * calls cbdataReferenceDone(http) + */ + assert(NULL == http); if (acl_checklist) delete acl_checklist; - -#if ICAP_CLIENT - - if (icap) - delete icap; - -#endif } -ClientRequestContext::ClientRequestContext() : acl_checklist (NULL), redirect_state (REDIRECT_NONE), http(NULL) -{} - -ClientRequestContext::ClientRequestContext(ClientHttpRequest *newHttp) : acl_checklist (NULL), redirect_state (REDIRECT_NONE), http(cbdataReference(newHttp)) +ClientRequestContext::ClientRequestContext(ClientHttpRequest *anHttp) : http(anHttp), acl_checklist (NULL), redirect_state (REDIRECT_NONE) { - assert (newHttp != NULL); + (void) cbdataReference(http); + http_access_done = 0; + redirect_done = 0; + no_cache_done = 0; } CBDATA_CLASS_INIT(ClientHttpRequest); @@ -243,6 +240,17 @@ freeResources(); +#if ICAP_CLIENT + + if (icap) { + delete icap; + cbdataReferenceDone(icap); + } + +#endif + if (calloutContext) + delete calloutContext; + /* moving to the next connection is handled by the context free */ dlinkDelete(&active, &ClientActiveRequests); } @@ -329,35 +337,58 @@ http->request = requestLink(request); /* optional - skip the access check ? */ - clientAccessCheck(http); + http->calloutContext = new ClientRequestContext(http); + + http->calloutContext->http_access_done = 0; + + http->calloutContext->redirect_done = 1; + + http->calloutContext->no_cache_done = 1; + + http->doCallouts(); return 0; } +bool +ClientRequestContext::httpStateIsValid() +{ + ClientHttpRequest *http_ = http; + + if (cbdataReferenceValid(http_)) + return true; + + http = NULL; + + cbdataReferenceDone(http_); + + return false; +} + /* This is the entry point for external users of the client_side routines */ void -clientAccessCheck(ClientHttpRequest *http) +ClientRequestContext::clientAccessCheck() { - ClientRequestContext *context = new ClientRequestContext(http); - context->acl_checklist = + acl_checklist = clientAclChecklistCreate(Config.accessList.http, http); - context->acl_checklist->nonBlockingCheck(clientAccessCheckDone, context); + acl_checklist->nonBlockingCheck(clientAccessCheckDoneWrapper, this); } void -clientAccessCheckDone(int answer, void *data) +clientAccessCheckDoneWrapper(int answer, void *data) { - ClientRequestContext *context = (ClientRequestContext *)data; - - context->acl_checklist = NULL; - ClientHttpRequest *http_ = context->http; + ClientRequestContext *calloutContext = (ClientRequestContext *) data; - if (!cbdataReferenceValid (http_)) { - delete context; + if (!calloutContext->httpStateIsValid()) return; - } - ClientHttpRequest *http = context->http; + calloutContext->clientAccessCheckDone(answer); +} + +void +ClientRequestContext::clientAccessCheckDone(int answer) +{ + acl_checklist = NULL; err_type page_id; http_status status; debug(85, 2) ("The request %s %s is %s, because it matched '%s'\n", @@ -373,7 +404,6 @@ if (answer != ACCESS_ALLOWED) { /* Send an error */ - delete context; debug(85, 5) ("Access Denied: %s\n", http->uri); debug(85, 5) ("AclMatchedName = %s\n", AclMatchedName ? AclMatchedName : ""); @@ -424,20 +454,7 @@ http->uri = xstrdup(urlCanonical(http->request)); -#if ICAP_CLIENT - - if (0 == context->doIcap()) { - context->icap->startReqMod(context, http->request); - context->icap->doneSending(); - return; - } - -#endif - assert(context->redirect_state == REDIRECT_NONE); - - context->redirect_state = REDIRECT_PENDING; - - clientRedirectStart(context); + http->doCallouts(); } static void @@ -448,27 +465,21 @@ context->acl_checklist = NULL; if (answer == ACCESS_ALLOWED) - redirectStart(http, clientRedirectDone, context); + redirectStart(http, clientRedirectDoneWrapper, context); else - clientRedirectDone(context, NULL); + context->clientRedirectDone(NULL); } -static void -clientRedirectStart(ClientRequestContext *context) +void +ClientRequestContext::clientRedirectStart() { - ClientHttpRequest *http = context->http; debug(33, 5) ("clientRedirectStart: '%s'\n", http->uri); - if (Config.Program.redirect == NULL) { - clientRedirectDone(context, NULL); - return; - } - if (Config.accessList.redirector) { - context->acl_checklist = clientAclChecklistCreate(Config.accessList.redirector, http); - context->acl_checklist->nonBlockingCheck(clientRedirectAccessCheckDone, context); + acl_checklist = clientAclChecklistCreate(Config.accessList.redirector, http); + acl_checklist->nonBlockingCheck(clientRedirectAccessCheckDone, this); } else - redirectStart(http, clientRedirectDone, context); + redirectStart(http, clientRedirectDoneWrapper, this); } static int @@ -530,7 +541,7 @@ /* * This is incorrect: authenticating requests can be sent via a hierarchy - * (they can even be cached if the correct headers are set on the reply + * (they can even be cached if the correct headers are set on the reply) */ if (request->flags.auth) return 0; @@ -727,23 +738,25 @@ } void -clientRedirectDone(void *data, char *result) +clientRedirectDoneWrapper(void *data, char *result) { - ClientRequestContext *context = (ClientRequestContext *)data; - ClientHttpRequest *http_ = context->http; + ClientRequestContext *calloutContext = (ClientRequestContext *)data; - if (!cbdataReferenceValid (http_)) { - delete context; + if (!calloutContext->httpStateIsValid()) return; - } - ClientHttpRequest *http = context->http; + calloutContext->clientRedirectDone(result); +} + +void +ClientRequestContext::clientRedirectDone(char *result) +{ HttpRequest *new_request = NULL; HttpRequest *old_request = http->request; debug(85, 5) ("clientRedirectDone: '%s' result=%s\n", http->uri, result ? result : "NULL"); - assert(context->redirect_state == REDIRECT_PENDING); - context->redirect_state = REDIRECT_DONE; + assert(redirect_state == REDIRECT_PENDING); + redirect_state = REDIRECT_DONE; if (result) { http_status status = (http_status) atoi(result); @@ -811,50 +824,33 @@ assert(http->uri); - context->checkNoCache(); + http->doCallouts(); } void ClientRequestContext::checkNoCache() { - if (Config.accessList.noCache && http->request->flags.cachable) { - acl_checklist = - clientAclChecklistCreate(Config.accessList.noCache, http); - acl_checklist->nonBlockingCheck(CheckNoCacheDone, cbdataReference(this)); - } else { - CheckNoCacheDone(http->request->flags.cachable, cbdataReference(this)); - } + acl_checklist = clientAclChecklistCreate(Config.accessList.noCache, http); + acl_checklist->nonBlockingCheck(checkNoCacheDoneWrapper, cbdataReference(this)); } -void -ClientRequestContext::CheckNoCacheDone(int answer, void *data) +static void +checkNoCacheDoneWrapper(int answer, void *data) { - void *temp; -#ifndef PURIFY + ClientRequestContext *calloutContext = (ClientRequestContext *) data; - bool valid = -#endif - cbdataReferenceValidDone(data, &temp); - /* acl NB calls cannot invalidate cbdata in the normal course of things */ - assert (valid); - ClientRequestContext *context = (ClientRequestContext *)temp; - context->checkNoCacheDone(answer); + if (!calloutContext->httpStateIsValid()) + return; + + calloutContext->checkNoCacheDone(answer); } void ClientRequestContext::checkNoCacheDone(int answer) { acl_checklist = NULL; - ClientHttpRequest *http_ = http; - - if (!cbdataReferenceValid (http_)) { - delete this; - return; - } - - delete this; - http_->request->flags.cachable = answer; - http_->processRequest(); + http->request->flags.cachable = answer; + http->doCallouts(); } /* @@ -946,6 +942,87 @@ storeLockObject(loggingEntry_); } +/* + * doCallouts() - This function controls the order of "callout" + * executions, including non-blocking access control checks, the + * redirector, and ICAP. Previously, these callouts were chained + * together such that "clientAccessCheckDone()" would call + * "clientRedirectStart()" and so on. + * + * The ClientRequestContext (aka calloutContext) class holds certain + * state data for the callout/callback operations. Previously + * ClientHttpRequest would sort of hand off control to ClientRequestContext + * for a short time. ClientRequestContext would then delete itself + * and pass control back to ClientHttpRequest when all callouts + * were finished. + * + * This caused some problems for ICAP because we want to make the + * ICAP callout after checking ACLs, but before checking the no_cache + * list. We can't stuff the ICAP state into the ClientRequestContext + * class because we still need the ICAP state after ClientRequestContext + * goes away. + * + * Note that ClientRequestContext is created before the first call + * to doCallouts(). + * + * If one of the callouts notices that ClientHttpRequest is no + * longer valid, it should call cbdataReferenceDone() so that + * ClientHttpRequest's reference count goes to zero and it will get + * deleted. ClientHttpRequest will then delete ClientRequestContext. + * + * Note that we set the _done flags here before actually starting + * the callout. This is strictly for convenience. + */ + +void +ClientHttpRequest::doCallouts() +{ + assert(calloutContext); + + if (!calloutContext->http_access_done) { + calloutContext->http_access_done = 1; + calloutContext->clientAccessCheck(); + return; + } + +#if ICAP_CLIENT + if (!calloutContext->icap_reqmod_done) { + calloutContext->icap_reqmod_done = 1; + + if (0 == doIcap()) { + icap->startReqMod(this, request); + icap->doneSending(); + return; + } + } + +#endif + if (!calloutContext->redirect_done) { + calloutContext->redirect_done = 1; + assert(calloutContext->redirect_state == REDIRECT_NONE); + + if (Config.Program.redirect) { + calloutContext->redirect_state = REDIRECT_PENDING; + calloutContext->clientRedirectStart(); + return; + } + } + + if (!calloutContext->no_cache_done) { + calloutContext->no_cache_done = 1; + + if (Config.accessList.noCache && request->flags.cachable) { + calloutContext->checkNoCache(); + return; + } + } + + cbdataReferenceDone(calloutContext->http); + delete calloutContext; + calloutContext = NULL; + processRequest(); +} + #ifndef _USE_INLINE_ #include "client_side_request.cci" #endif @@ -957,9 +1034,9 @@ * or take other action. */ int -ClientRequestContext::doIcap() +ClientHttpRequest::doIcap() { - debug(33,3)("ClientRequestContext::doIcap() called\n"); + debug(85,3)("ClientHttpRequest::doIcap() called\n"); assert(NULL == icap); icap = new ICAPClientSideHook; (void) cbdataReference(icap); @@ -970,90 +1047,82 @@ * Called by ICAPAnchor when it has space available for us. */ void -ClientRequestContext::icapSpaceAvailable() +ClientHttpRequest::icapSpaceAvailable() { - debug(33,3)("ClientRequestContext::icapSpaceAvailable() called\n"); + debug(85,3)("ClientHttpRequest::icapSpaceAvailable() called\n"); } void -ClientRequestContext::takeAdaptedHeaders(HttpMsg *msg) +ClientHttpRequest::takeAdaptedHeaders(HttpMsg *msg) { - debug(33,3)("ClientRequestContext::takeAdaptedHeaders() called\n"); + debug(85,3)("ClientHttpRequest::takeAdaptedHeaders() called\n"); + assert(cbdataReferenceValid(this)); // indicates bug - if (!cbdataReferenceValid (http)) { - debug(33,0)("WARNING: deleting this from ClientRequestContext::takeAdaptedHeaders, expect bugs\n"); - delete this; - return; - } - - HttpRequest *request = dynamic_cast(msg); - assert(request); + HttpRequest *new_req = dynamic_cast(msg); + assert(new_req); /* * Replace the old request with the new request. First, * Move the "body_connection" over, then unlink old and * link new to the http state. */ - request->body_connection = http->request->body_connection; - http->request->body_connection = NULL; - requestUnlink(http->request); - http->request = requestLink(request); + new_req->body_connection = request->body_connection; + request->body_connection = NULL; + requestUnlink(request); + request = requestLink(new_req); /* * Store the new URI for logging */ - xfree(http->uri); - http->uri = xstrdup(urlCanonical(request)); - setLogUri(http, urlCanonicalClean(request)); + xfree(uri); + uri = xstrdup(urlCanonical(request)); + setLogUri(this, urlCanonicalClean(request)); assert(request->method); - clientInterpretRequestHeaders(http); + clientInterpretRequestHeaders(this); #if HEADERS_LOG headersLog(0, 1, request->method, request); #endif - delete icap; - icap = NULL; - checkNoCache(); - debug(33,3)("ClientRequestContext::takeAdaptedHeaders() finished\n"); + doCallouts(); + + debug(85,3)("ClientHttpRequest::takeAdaptedHeaders() finished\n"); } void -ClientRequestContext::takeAdaptedBody(MemBuf *buf) +ClientHttpRequest::takeAdaptedBody(MemBuf *buf) { - debug(33,0)("ClientRequestContext::takeAdaptedBody() called\n"); + debug(85,3)("ClientHttpRequest::takeAdaptedBody() called\n"); } void -ClientRequestContext::doneAdapting() +ClientHttpRequest::doneAdapting() { - debug(33,3)("ClientRequestContext::doneAdapting() called\n"); + debug(85,3)("ClientHttpRequest::doneAdapting() called\n"); } void -ClientRequestContext::abortAdapting() +ClientHttpRequest::abortAdapting() { - debug(33,3)("ClientRequestContext::abortAdapting() called\n"); + debug(85,3)("ClientHttpRequest::abortAdapting() called\n"); - if ((NULL == http->storeEntry()) || http->storeEntry()->isEmpty()) { - debug(33,1)("WARNING: ICAP REQMOD callout failed, proceeding with original request\n"); - delete icap; - icap = NULL; - checkNoCache(); + if ((NULL == storeEntry()) || storeEntry()->isEmpty()) { + debug(85,3)("WARNING: ICAP REQMOD callout failed, proceeding with original request\n"); + doCallouts(); #if ICAP_HARD_ERROR - clientStreamNode *node = (clientStreamNode *)http->client_stream.tail->prev->data; + clientStreamNode *node = (clientStreamNode *)client_stream.tail->prev->data; clientReplyContext *repContext = dynamic_cast(node->data.getRaw()); assert (repContext); // Note if this code is ever used, clientBuildError() should be modified to // accept an errno arg repContext->setReplyToError(ERR_ICAP_FAILURE, HTTP_INTERNAL_SERVER_ERROR, - http->request->method, NULL, - http->getConn().getRaw() != NULL ? &http->getConn()->peer.sin_addr : &no_addr, http->request, - NULL, http->getConn().getRaw() != NULL - && http->getConn()->auth_user_request ? http->getConn()-> - auth_user_request : http->request->auth_user_request, errno); - node = (clientStreamNode *)http->client_stream.tail->data; - clientStreamRead(node, http, node->readBuffer); + request->method, NULL, + getConn().getRaw() != NULL ? &getConn()->peer.sin_addr : &no_addr, request, + NULL, getConn().getRaw() != NULL + && getConn()->auth_user_request ? getConn()-> + auth_user_request : request->auth_user_request, errno); + node = (clientStreamNode *)client_stream.tail->data; + clientStreamRead(node, this, node->readBuffer); #endif return; Index: squid3/src/client_side_request.h =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/client_side_request.h,v retrieving revision 1.17.12.3 retrieving revision 1.17.12.4 diff -u -r1.17.12.3 -r1.17.12.4 --- squid3/src/client_side_request.h 19 Sep 2005 18:00:09 -0000 1.17.12.3 +++ squid3/src/client_side_request.h 21 Sep 2005 20:33:38 -0000 1.17.12.4 @@ -1,6 +1,6 @@ /* - * $Id: client_side_request.h,v 1.17.12.3 2005/09/19 18:00:09 dwsquid Exp $ + * $Id: client_side_request.h,v 1.17.12.4 2005/09/21 20:33:38 dwsquid Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -53,6 +53,8 @@ class ConnStateData; +class ClientRequestContext; + class ClientHttpRequest { @@ -139,12 +141,27 @@ void maxReplyBodySize(ssize_t size); bool isReplyBodyTooLarge(ssize_t len) const; + ClientRequestContext *calloutContext; + void doCallouts(); + private: CBDATA_CLASS(ClientHttpRequest); ssize_t maxReplyBodySize_; StoreEntry *entry_; StoreEntry *loggingEntry_; ConnStateData::Pointer conn_; + +#if ICAP_CLIENT + +public: + ICAPClientSideHook *icap; + int doIcap(); + void icapSpaceAvailable(); + void takeAdaptedHeaders(HttpMsg *); + void takeAdaptedBody(MemBuf *); + void doneAdapting(); + void abortAdapting(); +#endif }; /* client http based routines */