Return-Path: Delivered-To: apmail-httpd-dev-archive@www.apache.org Received: (qmail 13762 invoked from network); 1 Aug 2004 17:29:25 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (209.237.227.199) by minotaur-2.apache.org with SMTP; 1 Aug 2004 17:29:25 -0000 Received: (qmail 937 invoked by uid 500); 1 Aug 2004 17:29:17 -0000 Delivered-To: apmail-httpd-dev-archive@httpd.apache.org Received: (qmail 849 invoked by uid 500); 1 Aug 2004 17:29:16 -0000 Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm Precedence: bulk Reply-To: dev@httpd.apache.org list-help: list-unsubscribe: list-post: Delivered-To: mailing list dev@httpd.apache.org Received: (qmail 836 invoked by uid 99); 1 Aug 2004 17:29:16 -0000 X-ASF-Spam-Status: No, hits=0.0 required=10.0 tests= X-Spam-Check-By: apache.org Received: from [128.195.24.168] (HELO scotch.ics.uci.edu) (128.195.24.168) by apache.org (qpsmtpd/0.27.1) with ESMTP; Sun, 01 Aug 2004 10:29:15 -0700 Received: from [127.0.0.1] (scotch.ics.uci.edu [128.195.24.168]) (authenticated bits=0) by scotch.ics.uci.edu (8.12.6/8.12.6) with ESMTP id i71HT6fX026841 (version=TLSv1/SSLv3 cipher=EDH-RSA-DES-CBC3-SHA bits=168 verify=NO) for ; Sun, 1 Aug 2004 10:29:10 -0700 (PDT) Date: Sun, 01 Aug 2004 10:29:11 -0700 From: Justin Erenkrantz To: dev@httpd.apache.org Subject: [PATCH] mod_cache fixes: #4 Message-ID: In-Reply-To: <410D0B83.9040100@wstoddard.com> References: <40B25DBF8F84D316162D79F8@[10.0.1.74]> <410D0B83.9040100@wstoddard.com> X-Mailer: Mulberry/3.1.5 (Mac OS X) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline X-Spam-Status: No, score=-2.0 required=5.0 tests=RCS_FORMAT_PATCH autolearn=failed version=3.0.0-pre1-r21475 X-Spam-Checker-Version: SpamAssassin 3.0.0-pre1-r21475 (2004-06-19) on scotch.ics.uci.edu X-Virus-Checked: Checked X-Spam-Rating: minotaur-2.apache.org 1.6.2 0/1000/N --On Sunday, August 1, 2004 11:25 AM -0400 Bill Stoddard wrote: > Too many changes in one patch. Break this up into multiple consumable in 15 > minute patches and I'll review them. (This is probably the largest and most complicated one. At the bottom, I've also pasted the current quick_handler function as I have it in my tree.) * modules/experimental/mod_cache.c: Rewrite quick_handler. Index: modules/experimental/mod_cache.c =================================================================== RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_cache.c,v retrieving revision 1.83 diff -u -r1.83 mod_cache.c --- modules/experimental/mod_cache.c 25 May 2004 18:01:02 -0000 1.83 +++ modules/experimental/mod_cache.c 1 Aug 2004 08:24:52 -0000 @@ -48,24 +48,30 @@ static int cache_url_handler(request_rec *r, int lookup) { apr_status_t rv; - const char *cc_in, *pragma, *auth; - apr_uri_t uri = r->parsed_uri; - char *url = r->unparsed_uri; + const char *pragma, *auth; + apr_uri_t uri; + char *url; apr_size_t urllen; - char *path = uri.path; + char *path; const char *types; - cache_info *info = NULL; + cache_info *info; cache_request_rec *cache; cache_server_conf *conf; + apr_bucket_brigade *out; - conf = (cache_server_conf *) ap_get_module_config(r->server->module_config, - &cache_module); - - /* we don't handle anything but GET */ + /* Delay initialization until we know we are handling a GET */ if (r->method_number != M_GET) { return DECLINED; } + uri = r->parsed_uri; + url = r->unparsed_uri; + path = uri.path; + info = NULL; + + conf = (cache_server_conf *) ap_get_module_config(r->server->module_config, + &cache_module); + /* * Which cache module (if any) should handle this request? */ @@ -73,19 +79,8 @@ return DECLINED; } - urllen = strlen(url); - if (urllen > MAX_URL_LENGTH) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, - "cache: URL exceeds length threshold: %s", url); - return DECLINED; - } - /* DECLINE urls ending in / ??? EGP: why? */ - if (url[urllen-1] == '/') { - return DECLINED; - } - /* make space for the per request config */ - cache = (cache_request_rec *) ap_get_module_config(r->request_config, + cache = (cache_request_rec *) ap_get_module_config(r->request_config, &cache_module); if (!cache) { cache = apr_pcalloc(r->pool, sizeof(cache_request_rec)); @@ -137,171 +132,106 @@ /* * Try to serve this request from the cache. * - * If no existing cache file - * add cache_in filter - * If stale cache file - * If conditional request - * add cache_in filter - * If non-conditional request - * fudge response into a conditional - * add cache_conditional filter - * If fresh cache file - * clear filter stack - * add cache_out filter + * If no existing cache file (DECLINED) + * add cache_save filter + * If cached file (OK) + * If fresh cache file + * clear filter stack + * add cache_out filter + * return OK + * If stale cache file + * add cache_conditional filter (which updates cache) */ rv = cache_select_url(r, cache->types, url); - if (DECLINED == rv) { - if (!lookup) { - /* no existing cache file */ - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, - "cache: no cache - add cache_in filter and DECLINE"); - /* add cache_in filter to cache this request */ - ap_add_output_filter_handle(cache_in_filter_handle, NULL, r, - r->connection); + if (rv != OK) { + if (rv == DECLINED) { + if (!lookup) { + /* add cache_save filter to cache this request */ + ap_add_output_filter_handle(cache_save_filter_handle, NULL, r, + r->connection); + } + } + else { + /* error */ + ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server, + "cache: error returned while checking for cached " + "file by %s cache", cache->type); } return DECLINED; } - else if (OK == rv) { - /* RFC2616 13.2 - Check cache object expiration */ - cache->fresh = ap_cache_check_freshness(cache, r); - if (cache->fresh) { - /* fresh data available */ - apr_bucket_brigade *out; - conn_rec *c = r->connection; - if (lookup) { - return OK; - } + /* We have located a suitable cache file now. */ + /* RFC2616 13.2 - Check cache object expiration */ + cache->fresh = ap_cache_check_freshness(cache, r); + + /* What we have in our cache isn't fresh. */ + if (!cache->fresh) { + /* If our stale cached response was conditional... */ + if (!lookup && ap_cache_request_is_conditional(r)) { info = &(cache->handle->cache_obj->info); - if (info && info->lastmod) { - ap_update_mtime(r, info->lastmod); + /* fudge response into a conditional */ + if (info && info->etag) { + /* if we have a cached etag */ + apr_table_set(r->headers_in, "If-None-Match", info->etag); } - - rv = ap_meets_conditions(r); - if (rv != OK) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, - "cache: fresh cache - returning status %d", rv); - return rv; + else if (info && info->lastmods) { + /* if we have a cached IMS */ + apr_table_set(r->headers_in, "If-Modified-Since", + info->lastmods); } + } - /* - * Not a conditionl request. Serve up the content - */ - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, - "cache: fresh cache - add cache_out filter and " - "handle request"); + /* Add cache_conditional_filter to see if we can salvage + * later. + */ + ap_add_output_filter_handle(cache_conditional_filter_handle, + NULL, r, r->connection); + return DECLINED; + } - /* We are in the quick handler hook, which means that no output - * filters have been set. So lets run the insert_filter hook. - */ - ap_run_insert_filter(r); - ap_add_output_filter_handle(cache_out_filter_handle, NULL, - r, r->connection); - - /* kick off the filter stack */ - out = apr_brigade_create(r->pool, c->bucket_alloc); - if (APR_SUCCESS - != (rv = ap_pass_brigade(r->output_filters, out))) { - ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server, - "cache: error returned while trying to return %s " - "cached data", - cache->type); - return rv; - } - return OK; - } - else { - if (!r->err_headers_out) { - r->err_headers_out = apr_table_make(r->pool, 3); - } - /* stale data available */ - if (lookup) { - return DECLINED; - } + /* fresh data available */ - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, - "cache: stale cache - test conditional"); - /* if conditional request */ - if (ap_cache_request_is_conditional(r)) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, - r->server, - "cache: conditional - add cache_in filter and " - "DECLINE"); - /* Why not add CACHE_CONDITIONAL? */ - ap_add_output_filter_handle(cache_in_filter_handle, NULL, - r, r->connection); + info = &(cache->handle->cache_obj->info); - return DECLINED; - } - /* else if non-conditional request */ - else { - /* Temporarily hack this to work the way it had been. Its broken, - * but its broken the way it was before. I'm working on figuring - * out why the filter add in the conditional filter doesn't work. pjr - * - * info = &(cache->handle->cache_obj->info); - * - * Uncomment the above when the code in cache_conditional_filter_handle - * is properly fixed... pjr - */ - - /* fudge response into a conditional */ - if (info && info->etag) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, - r->server, - "cache: nonconditional - fudge conditional " - "by etag"); - /* if we have a cached etag */ - apr_table_set(r->headers_in, "If-None-Match", info->etag); - } - else if (info && info->lastmods) { - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, - r->server, - "cache: nonconditional - fudge conditional " - "by lastmod"); - /* if we have a cached IMS */ - apr_table_set(r->headers_in, - "If-Modified-Since", - info->lastmods); - } - else { - /* something else - pretend there was no cache */ - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, - r->server, - "cache: nonconditional - no cached " - "etag/lastmods - add cache_in and DECLINE"); - - ap_add_output_filter_handle(cache_in_filter_handle, NULL, - r, r->connection); - - return DECLINED; - } - /* add cache_conditional filter */ - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, - r->server, - "cache: nonconditional - add cache_conditional " - "and DECLINE"); - ap_add_output_filter_handle(cache_conditional_filter_handle, - NULL, - r, - r->connection); + if (info && info->lastmod) { + ap_update_mtime(r, info->lastmod); + } - return DECLINED; - } - } + rv = ap_meets_conditions(r); + if (rv != OK) { + /* Return cached status. */ + return rv; } - else { - /* error */ - ap_log_error(APLOG_MARK, APLOG_ERR, rv, - r->server, - "cache: error returned while checking for cached file by " - "%s cache", + + /* If we're a lookup, we can exit now instead of serving the content. */ + if (lookup) { + return OK; + } + + /* Serve up the content */ + + /* We are in the quick handler hook, which means that no output + * filters have been set. So lets run the insert_filter hook. + */ + ap_run_insert_filter(r); + ap_add_output_filter_handle(cache_out_filter_handle, NULL, + r, r->connection); + + /* kick off the filter stack */ + out = apr_brigade_create(r->pool, r->connection->bucket_alloc); + rv = ap_pass_brigade(r->output_filters, out); + if (rv != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server, + "cache: error returned while trying to return %s " + "cached data", cache->type); - return DECLINED; + return rv; } + + return OK; } /* ---new function--- static int cache_url_handler(request_rec *r, int lookup) { apr_status_t rv; const char *pragma, *auth; apr_uri_t uri; char *url; apr_size_t urllen; char *path; const char *types; cache_info *info; cache_request_rec *cache; cache_server_conf *conf; apr_bucket_brigade *out; /* Delay initialization until we know we are handling a GET */ if (r->method_number != M_GET) { return DECLINED; } uri = r->parsed_uri; url = r->unparsed_uri; path = uri.path; info = NULL; conf = (cache_server_conf *) ap_get_module_config(r->server->module_config, &cache_module); /* * Which cache module (if any) should handle this request? */ if (!(types = ap_cache_get_cachetype(r, conf, path))) { return DECLINED; } /* make space for the per request config */ cache = (cache_request_rec *) ap_get_module_config(r->request_config, &cache_module); if (!cache) { cache = apr_pcalloc(r->pool, sizeof(cache_request_rec)); ap_set_module_config(r->request_config, &cache_module, cache); } /* save away the type */ cache->types = types; /* * Are we allowed to serve cached info at all? */ /* find certain cache controlling headers */ pragma = apr_table_get(r->headers_in, "Pragma"); auth = apr_table_get(r->headers_in, "Authorization"); /* first things first - does the request allow us to return * cached information at all? If not, just decline the request. * * Note that there is a big difference between not being allowed * to cache a request (no-store) and not being allowed to return * a cached request without revalidation (max-age=0). * * Caching is forbidden under the following circumstances: * * - RFC2616 14.9.2 Cache-Control: no-store * - Pragma: no-cache * - Any requests requiring authorization. */ if (conf->ignorecachecontrol == 1 && auth == NULL) { ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "incoming request is asking for a uncached version of " "%s, but we know better and are ignoring it", url); } else { if (ap_cache_liststr(NULL, pragma, "no-cache", NULL) || auth != NULL) { /* delete the previously cached file */ cache_remove_url(r, cache->types, url); ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, "cache: no-cache or authorization forbids caching " "of %s", url); return DECLINED; } } /* * Try to serve this request from the cache. * * If no existing cache file (DECLINED) * add cache_save filter * If cached file (OK) * If fresh cache file * clear filter stack * add cache_out filter * return OK * If stale cache file * add cache_conditional filter (which updates cache) */ rv = cache_select_url(r, cache->types, url); if (rv != OK) { if (rv == DECLINED) { if (!lookup) { /* add cache_save filter to cache this request */ ap_add_output_filter_handle(cache_save_filter_handle, NULL, r, r->connection); } } else { /* error */ ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server, "cache: error returned while checking for cached " "file by %s cache", cache->type); } return DECLINED; } /* We have located a suitable cache file now. */ /* RFC2616 13.2 - Check cache object expiration */ cache->fresh = ap_cache_check_freshness(cache, r); /* What we have in our cache isn't fresh. */ if (!cache->fresh) { /* If our stale cached response was conditional... */ if (!lookup && ap_cache_request_is_conditional(r)) { info = &(cache->handle->cache_obj->info); /* fudge response into a conditional */ if (info && info->etag) { /* if we have a cached etag */ apr_table_set(r->headers_in, "If-None-Match", info->etag); } else if (info && info->lastmods) { /* if we have a cached IMS */ apr_table_set(r->headers_in, "If-Modified-Since", info->lastmods); } } /* Add cache_conditional_filter to see if we can salvage * later. */ ap_add_output_filter_handle(cache_conditional_filter_handle, NULL, r, r->connection); return DECLINED; } /* fresh data available */ info = &(cache->handle->cache_obj->info); if (info && info->lastmod) { ap_update_mtime(r, info->lastmod); } rv = ap_meets_conditions(r); if (rv != OK) { /* Return cached status. */ return rv; } /* If we're a lookup, we can exit now instead of serving the content. */ if (lookup) { return OK; } /* Serve up the content */ /* We are in the quick handler hook, which means that no output * filters have been set. So lets run the insert_filter hook. */ ap_run_insert_filter(r); ap_add_output_filter_handle(cache_out_filter_handle, NULL, r, r->connection); /* kick off the filter stack */ out = apr_brigade_create(r->pool, r->connection->bucket_alloc); rv = ap_pass_brigade(r->output_filters, out); if (rv != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_ERR, rv, r->server, "cache: error returned while trying to return %s " "cached data", cache->type); return rv; } return OK; }