From: Daniel Earl Poirier Date: Mon, 5 Oct 2009 12:13:20 +0000 (+0000) Subject: Back out r818492 which prevented all caching of incomplete responses. X-Git-Tag: 2.3.3~200 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=29a4d8a4175eedd887272ca0cad6359229f736ca;p=apache Back out r818492 which prevented all caching of incomplete responses. Instead move the check to mod_disk_cache. This leaves cache implementations the flexibility to implement caching of incomplete responses. PR: 15866 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@821763 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 022168f641..b4b649cc7e 100644 --- a/CHANGES +++ b/CHANGES @@ -43,7 +43,7 @@ Changes with Apache 2.3.3 SSL_SERVER_I_DN back to the environment variables to be set by mod_ssl. [Peter Sylvester ] - *) mod_cache: don't cache incomplete responses, per RFC 2616, 13.8. + *) mod_disk_cache: don't cache incomplete responses, per RFC 2616, 13.8. PR15866. [Dan Poirier] *) ab: ab segfaults in verbose mode on https sites diff --git a/modules/cache/mod_cache.c b/modules/cache/mod_cache.c index 3df61c3873..c97b39217e 100644 --- a/modules/cache/mod_cache.c +++ b/modules/cache/mod_cache.c @@ -575,101 +575,6 @@ static int cache_out_filter(ap_filter_t *f, apr_bucket_brigade *bb) return ap_pass_brigade(f->next, bb); } -/* Find out the size of the cached response body. - Returns -1 if unable to do so. - */ -static apr_off_t get_cached_response_body_size(cache_request_rec *cache, request_rec *r) -{ - cache_handle_t *h = cache->handle; - apr_off_t size = -1; - apr_bucket_brigade *bb; - apr_status_t rv; - - /* There's not an API to ask the cache provider for the size - directly, so retrieve it and count the bytes. - - But it's not as inefficient as it might look. mod_disk_cache - will just create a file bucket and set its length to the file - size, and we'll access that length here without ever having to - read the cached file. - - If there's some other cache provider that has to read the whole - cached body to fill in the brigade, though, that would make - this rather expensive. - - XXX Add a standard cache provider API to get the size of the - cached data. - */ - - bb = apr_brigade_create(r->pool, r->connection->bucket_alloc); - rv = cache->provider->recall_body(h, r->pool, bb); - if (rv != APR_SUCCESS) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, - "cache: error recalling body"); - /* try to remove cache entry, it's probably messed up */ - cache->provider->remove_url(h, r->pool); - } - else { - int all_buckets_here = 0; - apr_bucket *e; - - size=0; - for (e = APR_BRIGADE_FIRST(bb); - e != APR_BRIGADE_SENTINEL(bb); - e = APR_BUCKET_NEXT(e)) { - if (APR_BUCKET_IS_EOS(e)) { - all_buckets_here=1; - break; - } - if (APR_BUCKET_IS_FLUSH(e)) { - continue; - } - if (e->length == (apr_size_t)-1) { - break; - } - size += e->length; - } - if (!all_buckets_here) { - size = -1; - } - } - apr_brigade_destroy(bb); - return size; -} - -/* Check that the response body we cached has the same length - * as the Content-Length header, if available. If not, cancel - * caching this response. - */ -static int validate_content_length(ap_filter_t *f, cache_request_rec *cache, request_rec *r) -{ - int rv = OK; - - if (cache->size != -1) { /* We have a content-length to check */ - apr_off_t actual_len = get_cached_response_body_size(cache, r); - if ((actual_len != -1) && (cache->size != actual_len)) { - ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, - "cache: Content-Length header was %"APR_OFF_T_FMT" " - "but response body size was %"APR_OFF_T_FMT - ", removing url from cache: %s", - cache->size, actual_len, - r->unparsed_uri - ); - ap_remove_output_filter(f); - rv = cache->provider->remove_url(cache->handle, r->pool); - if (rv != OK) { - /* Probably a mod_disk_cache cache area has been (re)mounted - * read-only, or that there is a permissions problem. - */ - ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, r->server, - "cache: attempt to remove url from cache unsuccessful."); - } - } - } - return rv; -} - - /* * CACHE_SAVE filter * --------------- @@ -760,11 +665,6 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in) } - /* Was this the final bucket? If yes, perform sanity checks. */ - if ((rv == APR_SUCCESS) && APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(in))) { - rv = validate_content_length(f, cache, r); - } - return ap_pass_brigade(f->next, in); } @@ -1283,11 +1183,6 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in) ap_cache_remove_lock(conf, r, cache->handle ? (char *)cache->handle->cache_obj->key : NULL, in); - /* Was this the final bucket? If yes, perform sanity checks. */ - if ((rv == APR_SUCCESS) && APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(in))) { - rv = validate_content_length(f, cache, r); - } - return ap_pass_brigade(f->next, in); } diff --git a/modules/cache/mod_disk_cache.c b/modules/cache/mod_disk_cache.c index 3c48769451..b3817496ee 100644 --- a/modules/cache/mod_disk_cache.c +++ b/modules/cache/mod_disk_cache.c @@ -1062,6 +1062,8 @@ static apr_status_t store_body(cache_handle_t *h, request_rec *r, * sanity checks. */ if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) { + const char *cl_header = apr_table_get(r->headers_out, "Content-Length"); + if (r->connection->aborted || r->no_cache) { ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server, "disk_cache: Discarding body for URL %s " @@ -1080,6 +1082,17 @@ static apr_status_t store_body(cache_handle_t *h, request_rec *r, file_cache_errorcleanup(dobj, r); return APR_EGENERAL; } + if (cl_header) { + apr_size_t cl = apr_atoi64(cl_header); + if ((errno == 0) && (dobj->file_size != cl)) { + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server, + "disk_cache: URL %s didn't receive complete response, not caching", + h->cache_obj->key); + /* Remove the intermediate cache file and return non-APR_SUCCESS */ + file_cache_errorcleanup(dobj, r); + return APR_EGENERAL; + } + } /* All checks were fine. Move tempfile to final destination */ /* Link to the perm file, and close the descriptor */