Commit 0f41a01f authored by Jacob Champion's avatar Jacob Champion
Browse files

Merge r1783842 from trunk: 

mod_cache: Fix a regression in 2.4.25 for the forward proxy case by
computing and using the same entity key according to when the cache
checks, loads and saves the request.  PR 60577.

Submitted by: ylavic


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1788511 13f79535-47bb-0310-9956-ffa450edef68
parent a3951ed2

CHANGES

+5 −0
Original line number Diff line number Diff line
@@ -2,6 +2,11 @@ 


Changes with Apache 2.4.26



  *) mod_cache: Fix a regression in 2.4.25 for the forward proxy case by

     computing and using the same entity key according to when the cache

     checks, loads and saves the request.

     PR 60577.  [Yann Ylavic]

  

  *) mod_proxy_hcheck: Ensure thread-safety when concurrent healthchecks are

     in use (ProxyHCTPsize > 0).  PR 60071.  [Yann Ylavic, Jim Jagielski]

STATUS

+0 −7
Original line number Diff line number Diff line
@@ -118,13 +118,6 @@ RELEASE SHOWSTOPPERS: 
PATCHES ACCEPTED TO BACKPORT FROM TRUNK:

  [ start all new proposals below, under PATCHES PROPOSED. ]



  *) mod_cache: Fix a regression in 2.4.25 for the forward proxy case by

     computing and using the same entity key according to when the cache

     checks, loads and saves the request.  PR 60577.

     trunk patch: http://svn.apache.org/r1783842

     2.4.x patch: trunk works (modulo CHANGES)

     +1: ylavic, jim, covener





PATCHES PROPOSED TO BACKPORT FROM TRUNK:

  [ New proposals should be added at the end of the list ]

modules/cache/cache_storage.c

+45 −42
Original line number Diff line number Diff line
@@ -427,7 +427,7 @@ int cache_select(cache_request_rec *cache, request_rec *r) 
}



static apr_status_t cache_canonicalise_key(request_rec *r, apr_pool_t* p,

                                           const char *uri, const char *query,

                                           const char *path, const char *query,

                                           apr_uri_t *parsed_uri,

                                           const char **key)

{
@@ -435,8 +435,8 @@ static apr_status_t cache_canonicalise_key(request_rec *r, apr_pool_t* p, 
    char *port_str, *hn, *lcs;

    const char *hostname, *scheme;

    int i;

    const char *path;

    char *querystring;

    const char *kpath;

    const char *kquery;



    if (*key) {

        /*
@@ -564,8 +564,8 @@ static apr_status_t cache_canonicalise_key(request_rec *r, apr_pool_t* p, 
     * Check if we need to ignore session identifiers in the URL and do so

     * if needed.

     */

    path = uri;

    querystring = apr_pstrdup(p, query ? query : parsed_uri->query);

    kpath = path;

    kquery = conf->ignorequerystring ? NULL : query;

    if (conf->ignore_session_id->nelts) {

        int i;

        char **identifier;
@@ -580,24 +580,23 @@ static apr_status_t cache_canonicalise_key(request_rec *r, apr_pool_t* p, 
             * Check that we have a parameter separator in the last segment

             * of the path and that the parameter matches our identifier

             */

            if ((param = ap_strrchr_c(path, ';'))

            if ((param = ap_strrchr_c(kpath, ';'))

                    && !strncmp(param + 1, *identifier, len)

                    && (*(param + len + 1) == '=')

                    && !ap_strchr_c(param + len + 2, '/')) {

                path = apr_pstrmemdup(p, path, param - path);

                kpath = apr_pstrmemdup(p, kpath, param - kpath);

                continue;

            }

            /*

             * Check if the identifier is in the query string and cut it out.

             */

            if (querystring && *querystring) {

            if (kquery && *kquery) {

                /*

                 * First check if the identifier is at the beginning of the

                 * query string and followed by a '='

                 */

                if (!strncmp(querystring, *identifier, len)

                        && (*(querystring + len) == '=')) {

                    param = querystring;

                if (!strncmp(kquery, *identifier, len) && kquery[len] == '=') {

                    param = kquery;

                }

                else {

                    char *complete;
@@ -607,7 +606,7 @@ static apr_status_t cache_canonicalise_key(request_rec *r, apr_pool_t* p, 
                     * identifier with a '&' and append a '='

                     */

                    complete = apr_pstrcat(p, "&", *identifier, "=", NULL);

                    param = ap_strstr_c(querystring, complete);

                    param = ap_strstr_c(kquery, complete);

                    /* If we found something we are sitting on the '&' */

                    if (param) {

                        param++;
@@ -615,18 +614,18 @@ static apr_status_t cache_canonicalise_key(request_rec *r, apr_pool_t* p, 
                }

                if (param) {

                    const char *amp;

                    char *dup = NULL;



                    if (querystring != param) {

                        querystring = apr_pstrndup(p, querystring,

                                param - querystring);

                    if (kquery != param) {

                        dup = apr_pstrmemdup(p, kquery, param - kquery);

                        kquery = dup;

                    }

                    else {

                        querystring = "";

                        kquery = "";

                    }



                    if ((amp = ap_strchr_c(param + len + 1, '&'))) {

                        querystring = apr_pstrcat(p, querystring, amp + 1,

                                NULL);

                        kquery = apr_pstrcat(p, kquery, amp + 1, NULL);

                    }

                    else {

                        /*
@@ -635,8 +634,8 @@ static apr_status_t cache_canonicalise_key(request_rec *r, apr_pool_t* p, 
                         * last one in the original query string. Hence we have

                         * a trailing '&' which needs to be removed.

                         */

                        if (*querystring) {

                            querystring[strlen(querystring) - 1] = '\0';

                        if (dup) {

                            dup[strlen(dup) - 1] = '\0';

                        }

                    }

                }
@@ -644,15 +643,11 @@ static apr_status_t cache_canonicalise_key(request_rec *r, apr_pool_t* p, 
        }

    }



    /* Key format is a URI, optionally without the query-string */

    if (conf->ignorequerystring) {

        *key = apr_pstrcat(p, scheme, "://", hostname, port_str, path, "?",

                NULL);

    }

    else {

        *key = apr_pstrcat(p, scheme, "://", hostname, port_str, path, "?",

                querystring, NULL);

    }

    /* Key format is a URI, optionally without the query-string (NULL

     * per above if conf->ignorequerystring)

     */

    *key = apr_pstrcat(p, scheme, "://", hostname, port_str,

                       kpath, "?", kquery, NULL);



    /*

     * Store the key in the request_config for the cache as r->parsed_uri
@@ -662,8 +657,8 @@ static apr_status_t cache_canonicalise_key(request_rec *r, apr_pool_t* p, 
     * resource in the cache under a key where it is never found by the quick

     * handler during following requests.

     */

    ap_log_rerror(

            APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, r, APLOGNO(00698) "cache: Key for entity %s?%s is %s", uri, parsed_uri->query, *key);

    ap_log_rerror(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, r, APLOGNO(00698)

                  "cache: Key for entity %s?%s is %s", path, query, *key);



    return APR_SUCCESS;

}
@@ -671,11 +666,17 @@ static apr_status_t cache_canonicalise_key(request_rec *r, apr_pool_t* p, 
apr_status_t cache_generate_key_default(request_rec *r, apr_pool_t* p,

                                        const char **key)

{

    /* We want the actual query-string, which may differ from

     * r->parsed_uri.query (immutable), so use "" (not NULL).

    /* In early processing (quick-handler, forward proxy), we want the initial

     * query-string from r->parsed_uri, since any change before CACHE_SAVE

     * shouldn't modify the key. Otherwise we want the actual query-string.

     */

    const char *args = r->args ? r->args : "";

    return cache_canonicalise_key(r, p, r->uri, args, &r->parsed_uri, key);

    const char *path = r->uri;

    const char *query = r->args;

    if (cache_use_early_url(r)) {

        path = r->parsed_uri.path;

        query = r->parsed_uri.query;

    }

    return cache_canonicalise_key(r, p, path, query, &r->parsed_uri, key);

}



/*
@@ -717,7 +718,8 @@ int cache_invalidate(cache_request_rec *cache, request_rec *r) 
    if (location) {

        if (apr_uri_parse(r->pool, location, &location_uri)

                || cache_canonicalise_key(r, r->pool,

                                          location, NULL,

                                          location_uri.path,

                                          location_uri.query,

                                          &location_uri, &location_key)

                || !(r->parsed_uri.hostname

                     && location_uri.hostname
@@ -732,7 +734,8 @@ int cache_invalidate(cache_request_rec *cache, request_rec *r) 
        if (apr_uri_parse(r->pool, content_location,

                          &content_location_uri)

                || cache_canonicalise_key(r, r->pool,

                                          content_location, NULL,

                                          content_location_uri.path,

                                          content_location_uri.query,

                                          &content_location_uri,

                                          &content_location_key)

                || !(r->parsed_uri.hostname

modules/cache/cache_util.c

+26 −7
Original line number Diff line number Diff line
@@ -31,10 +31,8 @@ extern module AP_MODULE_DECLARE_DATA cache_module; 
 * in "filter". All but the path comparisons are case-insensitive.

 */

static int uri_meets_conditions(const apr_uri_t *filter, const int pathlen,

                                request_rec *r)

                                const apr_uri_t *url, const char *path)

{

    const apr_uri_t *url = &r->parsed_uri;



    /* Scheme, hostname port and local part. The filter URI and the

     * URI we test may have the following shapes:

     *   /<path>
@@ -114,7 +112,7 @@ static int uri_meets_conditions(const apr_uri_t *filter, const int pathlen, 
    /* For HTTP caching purposes, an empty (NULL) path is equivalent to

     * a single "/" path. RFCs 3986/2396

     */

    if (!r->uri) {

    if (!path) {

        if (*filter->path == '/' && pathlen == 1) {

            return 1;

        }
@@ -126,7 +124,23 @@ static int uri_meets_conditions(const apr_uri_t *filter, const int pathlen, 
    /* Url has met all of the filter conditions so far, determine

     * if the paths match.

     */

    return !strncmp(filter->path, r->uri, pathlen);

    return !strncmp(filter->path, path, pathlen);

}



int cache_use_early_url(request_rec *r)

{

    cache_server_conf *conf;



    if (r->proxyreq == PROXYREQ_PROXY) {

        return 1;

    }



    conf = ap_get_module_config(r->server->module_config, &cache_module);

    if (conf->quick) {

        return 1;

    }



    return 0;

}



static cache_provider_list *get_provider(request_rec *r, struct cache_enable *ent,
@@ -172,6 +186,7 @@ cache_provider_list *cache_get_providers(request_rec *r, 
{

    cache_dir_conf *dconf = ap_get_module_config(r->per_dir_config, &cache_module);

    cache_provider_list *providers = NULL;

    const char *path;

    int i;



    /* per directory cache disable */
@@ -179,11 +194,14 @@ cache_provider_list *cache_get_providers(request_rec *r, 
        return NULL;

    }



    path = cache_use_early_url(r) ? r->parsed_uri.path : r->uri;



    /* global cache disable */

    for (i = 0; i < conf->cachedisable->nelts; i++) {

        struct cache_disable *ent =

                               (struct cache_disable *)conf->cachedisable->elts;

        if (uri_meets_conditions(&ent[i].url, ent[i].pathlen, r)) {

        if (uri_meets_conditions(&ent[i].url, ent[i].pathlen,

                                 &r->parsed_uri, path)) {

            /* Stop searching now. */

            return NULL;

        }
@@ -200,7 +218,8 @@ cache_provider_list *cache_get_providers(request_rec *r, 
    for (i = 0; i < conf->cacheenable->nelts; i++) {

        struct cache_enable *ent =

                                (struct cache_enable *)conf->cacheenable->elts;

        if (uri_meets_conditions(&ent[i].url, ent[i].pathlen, r)) {

        if (uri_meets_conditions(&ent[i].url, ent[i].pathlen,

                                 &r->parsed_uri, path)) {

            providers = get_provider(r, &ent[i], providers);

        }

    }

modules/cache/cache_util.h

+6 −0
Original line number Diff line number Diff line
@@ -327,6 +327,12 @@ char *cache_strqtok(char *str, const char *sep, char **last); 
 */

apr_table_t *cache_merge_headers_out(request_rec *r);



/**

 * Return whether to use request's path/query from early stage (r->parsed_uri)

 * or the current/rewritable ones (r->uri/r->args).

 */

int cache_use_early_url(request_rec *r);



#ifdef __cplusplus

}

#endif