On Fri, Aug 12, 2005 at 01:34:50PM -0400, Jim Jagielski wrote: > >Here's a more involved patch that gets the logic right. It's 6pm on a > >Friday for me, so I have only tested it a little, but thought I'd > >share > >for comment before the weekend. > > > > +1 on inspection... testing to be done over > the weekend :)
Of course :) I've run http local and proxy cases, and ftp proxy cases, as well as a few odd things now. With UseCanonicalName on, it does improve the hitrates. I've changed the patch a little (attached) but only some cosmetic comment changes, and I ditched the "local://" uri; Thinking about it, for a cache to be shared amongst protocols, things like the connection port would have to be faked anyway. So might aswell include the real serving protocol - makes much more sense to administrators. -- Colm MacCárthaigh Public Key: [EMAIL PROTECTED]
Index: modules/cache/cache_storage.c =================================================================== --- modules/cache/cache_storage.c (revision 232437) +++ modules/cache/cache_storage.c (working copy) @@ -320,12 +320,82 @@ 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); + char *port_str, *scheme, *hn; + const char * hostname; + int i; + + /* Use the canonical name to improve cache hit rate, but only if this is + * not a proxy request. + */ + if (!r->proxyreq) { + /* Use _default_ as the hostname if none present, as in mod_vhost */ + hostname = ap_get_server_name(r); + if (!hostname) { + hostname = "_default_"; + } } + else if(r->parsed_uri.hostname) { + /* Copy the parsed uri hostname */ + hn = apr_pcalloc(p, strlen(r->parsed_uri.hostname) + 1); + for (i = 0; r->parsed_uri.hostname[i]; i++) { + hn[i] = apr_tolower(r->parsed_uri.hostname[i]); + } + /* const work-around */ + hostname = hn; + } else { - *key = apr_pstrcat(p, r->uri, "?", r->args, NULL); + /* We are a proxied request, with no hostname. Unlikely + * to get very far - but just in case */ + hostname = "_default_"; } + + /* Copy the scheme, ensuring that it is lower case. If the parsed uri + * contains no string or if this is not a proxy request. + */ + if (r->proxyreq && r->parsed_uri.scheme) { + /* Copy the scheme */ + scheme = apr_pcalloc(p, strlen(r->parsed_uri.scheme) + 1); + for (i = 0; r->parsed_uri.scheme[i]; i++) { + scheme[i] = apr_tolower(r->parsed_uri.scheme[i]); + } + } + else { + scheme = "http"; + } + + /* If the content is locally generated, use the port-number of the + * current server. Otherwise. copy the URI's port-string (which may be a + * service name). If the URI contains no port-string, use apr-util's + * notion of the default port for that scheme - if available. + */ + if(r->proxyreq) { + if (r->parsed_uri.port_str) { + port_str = apr_pcalloc(p, strlen(r->parsed_uri.port_str) + 2); + port_str[0] = ':'; + for (i = 0; r->parsed_uri.port_str[i]; i++) { + port_str[i + 1] = apr_tolower(r->parsed_uri.port_str[i]); + } + } + else if (apr_uri_port_of_scheme(scheme)) { + port_str = apr_psprintf(p, ":%u", apr_uri_port_of_scheme(scheme)); + } + else { + /* No port string given in the AbsoluteUri, and we have no + * idea what the default port for the scheme is. Leave it + * blank and live with the inefficiency of some extra cached + * entities. + */ + port_str = ""; + } + } + else { + /* Use the server port */ + port_str = apr_psprintf(p, ":%u", ap_get_server_port(r)); + } + + /* Key format is a URI */ + *key = apr_pstrcat(p, scheme, "://", hostname, port_str, + r->parsed_uri.path, "?", r->args, NULL); + return APR_SUCCESS; } -