--On August 17, 2005 3:01:03 PM -0400 "Akins, Brian"
<[EMAIL PROTECTED]> wrote:
This patch allows one to override the values of some headers so that they
"vary" to the same value.
Config Example:
# all lines that have gzip set one variable
SetEnvIf Accept-Encoding gzip gzip=1
# browsers that have problems with gzip
BrowserMatch "MSIE [1-3]" gzip=0
BrowserMatch "MSIE [1-5].*Mac" gzip=0
BrowserMatch "^Mozilla/4\.0[678]" gzip=0
...
CacheOverrideHeader Accept-Encoding gzip
CacheOverrideHeader User-Agent gzip
This would allow all browsers that send "Accept-Encoding: gzip" and do not
match the BrowserMatches to be mapped to the same cache object. All the
other variants would point to another object. This would be very useful
in reverse proxy caches.
Only patched mod_disk_cache, but mod_mem_cache should be trivial.
The concept is fine. (And as Joshua pointed out 'CacheVaryOverride' would
be a better name.)
A few issues with the implementation (modulo style nits)...
diff -ru httpd-trunk.orig/modules/cache/cache_storage.c
httpd-trunk.new/modules/cache/cache_storage.c
--- httpd-trunk.orig/modules/cache/cache_util.c 2005-07-13
15:23:03.869381000 -0400
+++ httpd-trunk.new/modules/cache/cache_util.c 2005-08-17
14:53:29.890200893 -0400
@@ -534,3 +534,52 @@
}
return headers_out;
}
+
+CACHE_DECLARE(const char* )ap_cache_override_header(request_rec *r,
+ apr_table_t *t,
+ const char* key) {
+ const char *o;
+ cache_server_conf *conf =
ap_get_module_config(r->server->module_config,
+ &cache_module);
+ const char *header = NULL;
+
+ if((o = apr_table_get(conf->override_headers, key)) != NULL) {
+ if((header = apr_table_get(r->subprocess_env, o)) == NULL) {
+ header = "-";
+ }
+ }
I don't understand what the assignment to "-" is doing here.
+
+ if(!header) {
+ header = apr_table_get(t, key);
+ }
+
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
+ "ap_cache_override_header: %s => %s", key, header);
I don't think its necessary to log in the case where it is a no-op - so I'd
stick this in an else branch of the conditional above.
+
+ return header;
+}
+
...snip...
diff -ru httpd-trunk.orig/modules/cache/mod_disk_cache.c
httpd-trunk.new/modules/cache/mod_disk_cache.c
--- httpd-trunk.orig/modules/cache/mod_disk_cache.c 2005-08-09
11:51:09.473251000 -0400
+++ httpd-trunk.new/modules/cache/mod_disk_cache.c 2005-08-17
14:53:29.894200224 -0400
@@ -272,7 +272,7 @@
return APR_SUCCESS;
}
-static const char* regen_key(apr_pool_t *p, apr_table_t *headers,
+static const char* regen_key(request_rec *r,
apr_array_header_t *varray, const char
*oldkey)
I would prefer that we keep the headers argument explicit rather than
hardcoding that it is r->headers_in. -- justin
