Commit fe6e7c89 authored by Justin Erenkrantz's avatar Justin Erenkrantz
Browse files

Cleanup structures in mod_cache and friends to remove unused or unnecessary

fields.  Also resolves a number of latent bugs due to the wrong fields being
accessed.


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@152679 13f79535-47bb-0310-9956-ffa450edef68
parent 28f64e45
Loading
Loading
Loading
Loading
+16 −22
Changes for modules/cache/cache_storage.c: 16 added lines, 22 removed lines.
Original line number Diff line number Diff line
@@ -140,10 +140,7 @@ static void accept_headers(cache_handle_t *h, request_rec *r)
    apr_table_unset(r->err_headers_out, "Set-Cookie");
    apr_table_unset(h->resp_hdrs, "Set-Cookie");

    apr_table_overlap(r->headers_out, h->resp_hdrs,
                      APR_OVERLAP_TABLES_SET);
    apr_table_overlap(r->err_headers_out, h->resp_err_hdrs,
                      APR_OVERLAP_TABLES_SET);
    apr_table_overlap(r->headers_out, h->resp_hdrs, APR_OVERLAP_TABLES_SET);
    if (!apr_is_empty_table(cookie_table)) {
        r->err_headers_out = apr_table_overlay(r->pool, r->err_headers_out,
                                               cookie_table);
@@ -209,10 +206,7 @@ int cache_select_url(request_rec *r, char *url)
             *
             * RFC2616 13.6 and 14.44 describe the Vary mechanism.
             */
            if ((varyhdr = apr_table_get(h->resp_err_hdrs, "Vary")) == NULL) {
                varyhdr = apr_table_get(h->resp_hdrs, "Vary");
            }
            vary = apr_pstrdup(r->pool, varyhdr);
            vary = apr_pstrdup(r->pool, apr_table_get(h->resp_hdrs, "Vary"));
            while (vary && *vary) {
                char *name = vary;
                const char *h1, *h2;
@@ -252,23 +246,25 @@ int cache_select_url(request_rec *r, char *url)
            /* Is our cached response fresh enough? */
            fresh = ap_cache_check_freshness(h, r);
            if (!fresh) {
                cache_info *info = &(h->cache_obj->info);
                const char *etag, *lastmod;

                /* 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;
                etag = apr_table_get(h->resp_hdrs, "ETag");
                if (!etag) {
                    lastmod = apr_table_get(h->resp_hdrs, "Last-Modified");
                }
                else if (info && info->lastmods) {
                    /* if we have a cached Last-Modified header */
                if (etag || lastmod) {
                    /* if we have a cached etag or Last-Modified */
                    cache->stale_headers = apr_table_copy(r->pool,
                                                          r->headers_in);
                    if (etag) {
                        apr_table_set(r->headers_in, "If-None-Match", etag);
                    }
                    else if (lastmod) {
                        apr_table_set(r->headers_in, "If-Modified-Since",
                                  info->lastmods);
                                      lastmod);
                    }
                    cache->stale_handle = h;
                }

@@ -276,9 +272,6 @@ int cache_select_url(request_rec *r, char *url)
            }

            /* Okay, this response looks okay.  Merge in our stuff and go. */
            apr_table_setn(r->headers_out, "Content-Type",
                           ap_make_content_type(r, h->content_type));
            r->filename = apr_pstrdup(r->pool, h->cache_obj->info.filename);
            accept_headers(h, r);

            cache->handle = h;
@@ -298,7 +291,8 @@ int cache_select_url(request_rec *r, char *url)
    return DECLINED;
}

apr_status_t cache_generate_key_default( request_rec *r, apr_pool_t*p, char**key ) 
apr_status_t cache_generate_key_default(request_rec *r, apr_pool_t* p,
                                        char**key)
{
    if (r->hostname) {
        *key = apr_pstrcat(p, r->hostname, r->uri, "?", r->args, NULL);
+8 −40
Changes for modules/cache/cache_util.c: 8 added lines, 40 removed lines.
Original line number Diff line number Diff line
@@ -127,7 +127,7 @@ CACHE_DECLARE(int) ap_cache_check_freshness(cache_handle_t *h,
    apr_int64_t age, maxage_req, maxage_cresp, maxage, smaxage, maxstale;
    apr_int64_t minfresh;
    int age_in_errhdr = 0;
    const char *cc_cresp, *cc_ceresp, *cc_req;
    const char *cc_cresp, *cc_req;
    const char *agestr = NULL;
    const char *expstr = NULL;
    char *val;
@@ -167,20 +167,12 @@ CACHE_DECLARE(int) ap_cache_check_freshness(cache_handle_t *h,
     * 
     */
    cc_cresp = apr_table_get(h->resp_hdrs, "Cache-Control");
    cc_ceresp = apr_table_get(h->resp_err_hdrs, "Cache-Control");
    cc_req = apr_table_get(h->req_hdrs, "Cache-Control");
    expstr = apr_table_get(h->resp_hdrs, "Expires");

    if ((agestr = apr_table_get(h->resp_hdrs, "Age"))) {
        age_c = apr_atoi64(agestr);
    }
    else if ((agestr = apr_table_get(h->resp_err_hdrs, "Age"))) {
        age_c = apr_atoi64(agestr);
        age_in_errhdr = 1;
    }

    if (!(expstr = apr_table_get(h->resp_err_hdrs, "Expires"))) {
        expstr = apr_table_get(h->resp_hdrs, "Expires");
    }

    /* calculate age of object */
    age = ap_cache_current_age(info, age_c, r->request_time);
@@ -189,9 +181,6 @@ CACHE_DECLARE(int) ap_cache_check_freshness(cache_handle_t *h,
    if (cc_cresp && ap_cache_liststr(r->pool, cc_cresp, "s-maxage", &val)) {
        smaxage = apr_atoi64(val);
    }
    else if (cc_ceresp && ap_cache_liststr(r->pool, cc_ceresp, "s-maxage", &val)) {
        smaxage = apr_atoi64(val);
    }
    else {
        smaxage = -1;
    }
@@ -208,9 +197,6 @@ CACHE_DECLARE(int) ap_cache_check_freshness(cache_handle_t *h,
    if (cc_cresp && ap_cache_liststr(r->pool, cc_cresp, "max-age", &val)) {
        maxage_cresp = apr_atoi64(val);
    }
    else if (cc_ceresp && ap_cache_liststr(r->pool, cc_ceresp, "max-age", &val)) {
        maxage_cresp = apr_atoi64(val);
    }
    else
    {
        maxage_cresp = -1;
@@ -219,10 +205,10 @@ CACHE_DECLARE(int) ap_cache_check_freshness(cache_handle_t *h,
    /*
     * if both maxage request and response, the smaller one takes priority
     */
    if (-1 == maxage_req) {
    if (maxage_req == -1) {
        maxage = maxage_cresp;
    }
    else if (-1 == maxage_cresp) {
    else if (maxage_cresp == -1) {
        maxage = maxage_req;
    }
    else {
@@ -251,12 +237,6 @@ CACHE_DECLARE(int) ap_cache_check_freshness(cache_handle_t *h,
                                       "must-revalidate", NULL)) ||
                     (cc_cresp &&
                      ap_cache_liststr(NULL, cc_cresp,
                                       "proxy-revalidate", NULL)) ||
                     (cc_ceresp &&
                      ap_cache_liststr(NULL, cc_ceresp,
                                       "must-revalidate", NULL)) ||
                     (cc_ceresp &&
                      ap_cache_liststr(NULL, cc_ceresp,
                                       "proxy-revalidate", NULL)))) {
        maxstale = 0;
    }
@@ -268,27 +248,13 @@ CACHE_DECLARE(int) ap_cache_check_freshness(cache_handle_t *h,
         (info->expire != APR_DATE_BAD) &&
         (age < (apr_time_sec(info->expire - info->date) + maxstale - minfresh)))) {
        const char *warn_head;
        apr_table_t *head_ptr;

        warn_head = apr_table_get(h->resp_hdrs, "Warning");
        if (warn_head != NULL) {
            head_ptr = h->resp_hdrs;
        }
        else {
            warn_head = apr_table_get(h->resp_err_hdrs, "Warning");
            head_ptr = h->resp_err_hdrs;
        }

        /* it's fresh darlings... */
        /* set age header on response */
        if (age_in_errhdr) {
            apr_table_set(h->resp_err_hdrs, "Age",
                          apr_psprintf(r->pool, "%lu", (unsigned long)age));
        }
        else {
        apr_table_set(h->resp_hdrs, "Age",
                      apr_psprintf(r->pool, "%lu", (unsigned long)age));
        }

        /* add warning if maxstale overrode freshness calculation */
        if (!(((smaxage != -1) && age < smaxage) ||
@@ -298,7 +264,8 @@ CACHE_DECLARE(int) ap_cache_check_freshness(cache_handle_t *h,
            /* make sure we don't stomp on a previous warning */
            if ((warn_head == NULL) ||
                ((warn_head != NULL) && (ap_strstr_c(warn_head, "110") == NULL))) {
                apr_table_merge(head_ptr, "Warning", "110 Response is stale");
                apr_table_merge(h->resp_hdrs, "Warning",
                                "110 Response is stale");
            }
        }
        /* 
@@ -315,7 +282,8 @@ CACHE_DECLARE(int) ap_cache_check_freshness(cache_handle_t *h,
             */
            if ((warn_head == NULL) ||
                ((warn_head != NULL) && (ap_strstr_c(warn_head, "113") == NULL))) {
                apr_table_merge(head_ptr, "Warning", "113 Heuristic expiration");
                apr_table_merge(h->resp_hdrs, "Warning",
                                "113 Heuristic expiration");
            }
        }
        return 1;    /* Cache object is fresh (enough) */
+2 −14
Changes for modules/cache/mod_cache.c: 2 added lines, 14 removed lines.
Original line number Diff line number Diff line
@@ -161,13 +161,6 @@ static int cache_url_handler(request_rec *r, int lookup)
        return DECLINED;
    }

    /* We have located a suitable cache file now. */
    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. */
@@ -356,7 +349,8 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
        lastmods = apr_table_get(r->headers_out, "Last-Modified");
    }
    if (lastmods != NULL) {
        if (APR_DATE_BAD == (lastmod = apr_date_parse_http(lastmods))) {
        lastmod = apr_date_parse_http(lastmods);
        if (lastmod == APR_DATE_BAD) {
            lastmods = NULL;
        }
    }
@@ -628,7 +622,6 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
                     "cache: Last modified is in the future, "
                     "replacing with now");
    }
    info->lastmod = lastmod;

    /* if no expiry date then
     *   if lastmod
@@ -655,11 +648,6 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
    }
    info->expire = exp;

    info->content_type = apr_pstrdup(r->pool, r->content_type);
    info->etag = apr_pstrdup(r->pool, etag);
    info->lastmods = apr_pstrdup(r->pool, lastmods);
    info->filename = apr_pstrdup(r->pool, r->filename);

    /*
     * Write away header information to cache.
     */
+3 −15
Changes for modules/cache/mod_cache.h: 3 added lines, 15 removed lines.
Original line number Diff line number Diff line
@@ -151,21 +151,10 @@ typedef struct {
typedef struct cache_info cache_info;
struct cache_info {
    int status;
    char *content_type;
    char *etag;
    char *lastmods;         /* last modified of cache entity */
    char *filename;
    apr_time_t date;
    apr_time_t lastmod;
    char lastmod_str[APR_RFC822_DATE_LEN];
    apr_time_t expire;
    apr_time_t request_time;
    apr_time_t response_time;
    apr_size_t len;
    apr_time_t ims;    /*  If-Modified_Since header value    */
    apr_time_t ius;    /*  If-UnModified_Since header value    */
    const char *im;         /* If-Match header value */
    const char *inm;         /* If-None-Match header value */
};

/* cache handle information */
@@ -181,7 +170,9 @@ struct cache_object {
    char *key;
    cache_object_t *next;
    cache_info info;
    void *vobj;         /* Opaque portion (specific to the cache implementation) of the cache object */
    /* Opaque portion (specific to the implementation) of the cache object */
    void *vobj;
    /* FIXME: These are only required for mod_mem_cache. */
    apr_size_t count;   /* Number of body bytes written to the cache so far */
    int complete;
    apr_uint32_t refcount;  /* refcount and bit flag to cleanup object */
@@ -192,9 +183,6 @@ struct cache_handle {
    cache_object_t *cache_obj;
    apr_table_t *req_hdrs;        /* cached request headers */
    apr_table_t *resp_hdrs;       /* cached response headers */
    apr_table_t *resp_err_hdrs;   /* cached response err headers */
    const char *content_type;     /* cached content type */
    int status;                   /* cached status */
};

#define CACHE_PROVIDER_GROUP "cache"
+2 −40
Changes for modules/cache/mod_disk_cache.c: 2 added lines, 40 removed lines.
Original line number Diff line number Diff line
@@ -249,9 +249,7 @@ static int file_cache_recall_mydata(apr_file_t *fd, cache_info *info,
 * Hook and mod_cache callback functions
 */
#define AP_TEMPFILE "/aptmpXXXXXX"
static int create_entity(cache_handle_t *h, request_rec *r,
                         const char *key,
                         apr_off_t len)
static int create_entity(cache_handle_t *h, request_rec *r, const char *key)
{
    disk_cache_conf *conf = ap_get_module_config(r->server->module_config,
                                                 &disk_cache_module);
@@ -262,20 +260,11 @@ static int create_entity(cache_handle_t *h, request_rec *r,
        return DECLINED;
    }

    /* If the Content-Length is still unknown, cache anyway */
    if (len != -1 && (len < conf->minfs || len > conf->maxfs)) {
        ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
                     "cache_disk: URL %s failed the size check", key);
        return DECLINED;
    }

    /* Allocate and initialize cache_object_t and disk_cache_object_t */
    h->cache_obj = obj = apr_pcalloc(r->pool, sizeof(*obj));
    obj->vobj = dobj = apr_pcalloc(r->pool, sizeof(*dobj));

    obj->key = apr_pstrdup(r->pool, key);
    /* XXX Bad Temporary Cast - see cache_object_t notes */
    obj->info.len = (apr_size_t) len;

    dobj->name = obj->key;
    dobj->datafile = data_file(r->pool, conf, dobj, key);
@@ -471,7 +460,6 @@ static apr_status_t recall_headers(cache_handle_t *h, request_rec *r)

    h->req_hdrs = apr_table_make(r->pool, 20);
    h->resp_hdrs = apr_table_make(r->pool, 20);
    h->resp_err_hdrs = apr_table_make(r->pool, 20);

    /* Call routine to read the header lines/status line */
    read_table(h, r, h->resp_hdrs, dobj->hfd);
@@ -479,10 +467,6 @@ static apr_status_t recall_headers(cache_handle_t *h, request_rec *r)

    apr_file_close(dobj->hfd);

    h->status = dobj->disk_info.status;
    h->content_type = apr_table_get(h->resp_hdrs, "Content-Type");
    h->cache_obj->info.etag = apr_table_get(h->resp_hdrs, "ETag");

    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
                 "disk_cache: Recalled headers for URL %s",  dobj->name);
    return APR_SUCCESS;
@@ -687,14 +671,6 @@ static apr_status_t store_body(cache_handle_t *h, request_rec *r,
     * sanity checks.
     */
    if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) {
        if (h->cache_obj->info.len <= 0) {
          /* If the target value of the content length is unknown
           * (h->cache_obj->info.len <= 0), check if connection has been
           * aborted by client to avoid caching incomplete request bodies.
           *
           * This can happen with large responses from slow backends like
           * Tomcat via mod_jk.
           */
        if (r->connection->aborted) {
            ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server,
                         "disk_cache: Discarding body for URL %s "
@@ -704,24 +680,10 @@ static apr_status_t store_body(cache_handle_t *h, request_rec *r,
            file_cache_errorcleanup(dobj, r);
            return APR_EGENERAL;
        }
          /* XXX Fixme: file_size isn't constrained by size_t. */
          h->cache_obj->info.len = dobj->file_size;
        }
        else if (h->cache_obj->info.len != dobj->file_size) {
          /* "Content-Length" and actual content disagree in size. Log that. */
          ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
                       "disk_cache: URL %s failed the size check (%lu != %lu)",
                       h->cache_obj->key,
                       (unsigned long)h->cache_obj->info.len,
                       (unsigned long)dobj->file_size);
          /* Remove the intermediate cache file and return non-APR_SUCCESS */
          file_cache_errorcleanup(dobj, r);
          return APR_EGENERAL;
        }
        if (dobj->file_size < conf->minfs) {
            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
                         "cache_disk: URL %s failed the size check (%lu<%lu)",
                     h->cache_obj->key, (unsigned long)dobj->file_size, (unsigned long)conf->minfs);
                         h->cache_obj->key, dobj->file_size, conf->minfs);
            /* Remove the intermediate cache file and return non-APR_SUCCESS */
            file_cache_errorcleanup(dobj, r);
            return APR_EGENERAL;
Loading