Commit 945728d0 authored by Graham Leggett's avatar Graham Leggett
Browse files

Try to correctly follow RFC 2616 13.3 on validating stale cache

responses by teaching mod_cache's cache_select_url and
cache_save_filter how to deal with this corner case.
PR:
Obtained from:
Submitted by:	jerenkrantz
Reviewed by:	stoddard, jerenkrantz, minfrin, jim


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/APACHE_2_0_BRANCH@105439 13f79535-47bb-0310-9956-ffa450edef68
parent c6146faf
Loading
Loading
Loading
Loading
+3 −0
Original line number Diff line number Diff line
Changes with Apache 2.0.53
  *) mod_cache: Try to correctly follow RFC 2616 13.3 on validating stale
     cache responses.  [Justin Erenkrantz]
  *) mod_rewrite: Handle per-location rules when r->filename is unset.
     Previously this would segfault or simply not match as expected,
     depending on the platform.  [Jeff Trawick]
+1 −12
Original line number Diff line number Diff line
APACHE 2.0 STATUS:                                              -*-text-*-
Last modified at [$Date: 2004/10/13 17:54:43 $]
Last modified at [$Date: 2004/10/13 18:12:20 $]

Release:

@@ -115,17 +115,6 @@ PATCHES TO BACKPORT FROM 2.1
       jorton replies: it does indeed, hang on...
       +1: jorton

    *) Try to correctly follow RFC 2616 13.3 on validating stale cache
       responses by teaching mod_cache's cache_select_url and
       cache_save_filter how to deal with this corner case.
        modules/experimental/cache_storage.c?r1=1.39&r2=1.40
        modules/experimental/cache_util.c?r1=1.34&r2=1.35
        modules/experimental/mod_cache.c?r1=1.91&r2=1.92
        modules/experimental/mod_cache.h?r1=1.50&r2=1.51
        modules/experimental/mod_disk_cache.c?r1=1.64&r2=1.65
        modules/experimental/mod_mem_cache.c?r1=1.117&r2=1.118
       +1: stoddard, jerenkrantz, minfrin, jim

    *) mod_rewrite:Fix query string handling for proxied URLs. PR 14518.
       (2.0 + 1.3)
         modules/mappers/mod_rewrite.c: r1.259
+23 −3
Original line number Diff line number Diff line
@@ -245,10 +245,32 @@ int cache_select_url(request_rec *r, char *url)
                }
            }

            cache->provider = list->provider;
            cache->provider_name = list->provider_name;

            /* Is our cached response fresh enough? */
            fresh = ap_cache_check_freshness(h, r);
            if (!fresh) {
                list->provider->remove_entity(h);
                cache_info *info = &(h->cache_obj->info);

                /* Make response into a conditional */
                /* FIXME: What if the request is already conditional? */
                if (info && info->etag) {
                    /* if we have a cached etag */
                    cache->stale_headers = apr_table_copy(r->pool,
                                                          r->headers_in);
                    apr_table_set(r->headers_in, "If-None-Match", info->etag);
                    cache->stale_handle = h;
                }
                else if (info && info->lastmods) {
                    /* if we have a cached Last-Modified header */
                    cache->stale_headers = apr_table_copy(r->pool,
                                                          r->headers_in);
                    apr_table_set(r->headers_in, "If-Modified-Since",
                                  info->lastmods);
                    cache->stale_handle = h;
                }

                return DECLINED;
            }

@@ -258,8 +280,6 @@ int cache_select_url(request_rec *r, char *url)
            r->filename = apr_pstrdup(r->pool, h->cache_obj->info.filename);
            accept_headers(h, r);

            cache->provider = list->provider;
            cache->provider_name = list->provider_name;
            cache->handle = h;
            return OK;
        }
+5 −5
Original line number Diff line number Diff line
@@ -22,12 +22,12 @@
/* -------------------------------------------------------------- */

/* return true if the request is conditional */
CACHE_DECLARE(int) ap_cache_request_is_conditional(request_rec *r)
CACHE_DECLARE(int) ap_cache_request_is_conditional(apr_table_t *table)
{
    if (apr_table_get(r->headers_in, "If-Match") ||
        apr_table_get(r->headers_in, "If-None-Match") ||
        apr_table_get(r->headers_in, "If-Modified-Since") ||
        apr_table_get(r->headers_in, "If-Unmodified-Since")) {
    if (apr_table_get(table, "If-Match") ||
        apr_table_get(table, "If-None-Match") ||
        apr_table_get(table, "If-Modified-Since") ||
        apr_table_get(table, "If-Unmodified-Since")) {
        return 1;
    }
    return 0;
+55 −23
Original line number Diff line number Diff line
@@ -219,7 +219,7 @@ static int cache_out_filter(ap_filter_t *f, apr_bucket_brigade *bb)
    ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, r->server,
                 "cache: running CACHE_OUT filter");

    /* recall_body() was called in cache_select_url() */
    /* recall_headers() was called in cache_select_url() */
    cache->provider->recall_body(cache->handle, r->pool, bb);

    /* This filter is done once it has served up its content */
@@ -290,6 +290,12 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
     * This section passes the brigades into the cache modules, but only
     * if the setup section (see below) is complete.
     */
    if (cache->block_response) {
        /* We've already sent down the response and EOS.  So, ignore
         * whatever comes now.
         */
        return APR_SUCCESS;
    }

    /* have we already run the cachability check and set up the
     * cached file handle? 
@@ -312,7 +318,6 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
     * parameters, and decides whether this URL should be cached at
     * all. This section is* run before the above section.
     */
    info = apr_pcalloc(r->pool, sizeof(cache_info));

    /* read expiry date; if a bad date, then leave it so the client can
     * read it 
@@ -384,7 +389,8 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
         */
        reason = "Query string present but no expires header";
    }
    else if (r->status == HTTP_NOT_MODIFIED && (NULL == cache->handle)) {
    else if (r->status == HTTP_NOT_MODIFIED &&
             !cache->handle && !cache->stale_handle) {
        /* if the server said 304 Not Modified but we have no cache
         * file - pass this untouched to the user agent, it's not for us.
         */
@@ -510,33 +516,34 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
     * - cache->handle == NULL. In this case there is no previously
     * cached entity anywhere on the system. We must create a brand
     * new entity and store the response in it.
     * - cache->handle != NULL. In this case there is a stale
     * - cache->stale_handle != NULL. In this case there is a stale
     * entity in the system which needs to be replaced by new
     * content (unless the result was 304 Not Modified, which means
     * the cached entity is actually fresh, and we should update
     * the headers).
     */
    /* no cache handle, create a new entity */
    if (!cache->handle) {
        rv = cache_create_entity(r, url, size);
    }
    /* pre-existing cache handle and 304, make entity fresh */
    else if (r->status == HTTP_NOT_MODIFIED) {
        /* update headers: TODO */

        /* remove this filter ??? */

        /* XXX is this right?  we must set rv to something other than OK 
         * in this path
         */
        rv = HTTP_NOT_MODIFIED;
    /* Did we have a stale cache entry that really is stale? */
    if (cache->stale_handle) {
        if (r->status == HTTP_NOT_MODIFIED) {
            /* Oh, hey.  It isn't that stale!  Yay! */
            cache->handle = cache->stale_handle;
            info = &cache->handle->cache_obj->info;
        }
    /* pre-existing cache handle and new entity, replace entity
     * with this one
     */
        else {
        cache->provider->remove_entity(cache->handle);
            /* Oh, well.  Toss it. */
            cache->provider->remove_entity(cache->stale_handle);
            /* Treat the request as if it wasn't conditional. */
            cache->stale_handle = NULL;
        }
    }

    /* no cache handle, create a new entity */
    if (!cache->handle) {
        rv = cache_create_entity(r, url, size);
        info = apr_pcalloc(r->pool, sizeof(cache_info));
        /* We only set info->status upon the initial creation. */
        info->status = r->status;
    }

    if (rv != OK) {
@@ -647,6 +654,31 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
     * Write away header information to cache.
     */
    rv = cache->provider->store_headers(cache->handle, r, info);

    /* Did we actually find an entity before, but it wasn't really stale? */
    if (rv == APR_SUCCESS && cache->stale_handle) {
        apr_bucket_brigade *bb;
        apr_bucket *bkt;

        bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);

        /* Were we initially a conditional request? */
        if (ap_cache_request_is_conditional(cache->stale_headers)) {
            /* FIXME: Should we now go and make sure it's really not
             * modified since what the user thought?
             */
            bkt = apr_bucket_eos_create(bb->bucket_alloc);
            APR_BRIGADE_INSERT_TAIL(bb, bkt);
        }
        else {
            r->status = info->status;
            cache->provider->recall_body(cache->handle, r->pool, bb);
        }

        cache->block_response = 1;
        return ap_pass_brigade(f->next, bb);
    }

    if (rv == APR_SUCCESS) {
        rv = cache->provider->store_body(cache->handle, r, in);
    }
Loading