hi again...
ok, I think I've worked out all the issues - attached is a new patch with
logic (and spelling :) errors hopefully worked out. also included is a
patch against the perl-framework for testing if you so choose.
one of the issues that needed working out was dealing with multiple ETag
headers. my original idea was to have ap_weaken_etag guarantee that ETag
headers would be weak. with ETag headers entering err_headers_out via a
third party, there exists the possibility that the server would send
multiple ETag headers for a single request. while I'm not sure if this is
actually legal, I can't find anything that says it isn't. so, the header
table is iterated and all ETag headers adjusted. oh, and if there is an
easier way to alter table entries with multi-valued keys other than creating
a temporary table, I'm all ears.
anyway, thanks for your patience in reviewing. all told it's more logic
than I wanted or expected but it's probably valuable nonetheless, even if it
requires more work.
--Geoff
Index: include/http_protocol.h
===================================================================
RCS file: /home/cvspublic/httpd-2.0/include/http_protocol.h,v
retrieving revision 1.85
diff -u -r1.85 http_protocol.h
--- include/http_protocol.h 12 Dec 2003 17:03:58 -0000 1.85
+++ include/http_protocol.h 15 Dec 2003 20:47:56 -0000
@@ -203,11 +203,28 @@
AP_DECLARE(char *) ap_make_etag(request_rec *r, int force_weak);
/**
- * Set the E-tag outgoing header
+ * Set the ETag outgoing header
* @param The current request
* @deffunc void ap_set_etag(request_rec *r)
*/
AP_DECLARE(void) ap_set_etag(request_rec *r);
+
+/**
+ * Supress the ETag response header
+ * @param The current request
+ * @deffunc void ap_suppress_etag(request_rec *r)
+ */
+AP_DECLARE(void) ap_suppress_etag(request_rec *r);
+
+/**
+ * Force generation of weak ETag headers.
+ * weak tags are allowed when the resource is semantically
+ * equivalent to a prior version, even though the content
+ * bits are different. see RFC-2616 13.3.3
+ * @param The current request
+ * @deffunc void ap_weaken_etag(request_rec *r)
+ */
+AP_DECLARE(void) ap_weaken_etag(request_rec *r);
/**
* Set the last modified time for the file being sent
Index: modules/filters/mod_include.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/filters/mod_include.c,v
retrieving revision 1.292
diff -u -r1.292 mod_include.c
--- modules/filters/mod_include.c 10 Dec 2003 02:30:20 -0000 1.292
+++ modules/filters/mod_include.c 15 Dec 2003 20:48:09 -0000
@@ -3584,7 +3584,7 @@
* We don't know if we are going to be including a file or executing
* a program - in either case a strong ETag header will likely be invalid.
*/
- apr_table_setn(f->r->notes, "no-etag", "");
+ ap_suppress_etag(f->r);
return OK;
}
Index: modules/http/http_protocol.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/http/http_protocol.c,v
retrieving revision 1.474
diff -u -r1.474 http_protocol.c
--- modules/http/http_protocol.c 12 Dec 2003 17:03:58 -0000 1.474
+++ modules/http/http_protocol.c 15 Dec 2003 20:48:13 -0000
@@ -1560,6 +1560,55 @@
}
}
+typedef struct {
+ request_rec *r;
+ apr_table_t *etag_table;
+} weaken_struct;
+
+#define ETAG_WEAK "W/"
+
+/* apr_table_do iterator for weakening ETag header fields */
+static int make_etags_weak(weaken_struct *w, const char *key, const char *val)
+{
+ if (strncmp(val, ETAG_WEAK, strlen(ETAG_WEAK))) {
+ apr_table_addn(w->etag_table, key,
+ apr_pstrcat(w->r->pool, ETAG_WEAK, val, NULL));
+ }
+ else {
+ apr_table_addn(w->etag_table, key, val);
+ }
+
+ return 1;
+}
+
+/*
+ * weaken all the ETag headers in r->headers_out.
+ *
+ * there might be multiple ETag headers to weaken, specifically
+ * if both r->headers_out and r->err_headers out contained
+ * entries that were merged via ap_http_header_filter. perhaps
+ * there are legitimate reasons for multiple ETag headers as
+ * well?
+ */
+static void weaken_etags(request_rec *r)
+{
+ if (apr_table_get(r->headers_out, "ETag")) {
+
+ apr_table_t *etag_table = apr_table_make(r->pool, 2);
+ weaken_struct w;
+ w.r = r;
+ w.etag_table = etag_table;
+
+ apr_table_do((int (*) (void *, const char *, const char *))
+ make_etags_weak, (void *) &w, r->headers_out,
+ "ETag", NULL);
+
+ apr_table_unset(r->headers_out, "ETag");
+ r->headers_out = apr_table_overlay(r->pool, r->headers_out,
+ etag_table);
+ }
+}
+
typedef struct header_filter_ctx {
int headers_sent;
} header_filter_ctx;
@@ -1612,7 +1661,7 @@
* later attempts to set or unset a given fieldname might be bypassed.
*/
if (!apr_is_empty_table(r->err_headers_out)) {
- r->headers_out = apr_table_overlay(r->pool, r->err_headers_out,
+ r->headers_out = apr_table_overlay(r->pool, r->err_headers_out,
r->headers_out);
}
@@ -1634,12 +1683,23 @@
}
/*
- * Now remove any ETag response header field if earlier processing
- * says so (such as a 'FileETag None' directive).
+ * Now take some steps to make sure that the Etag response
+ * header looks like it should.
+ *
+ * ap_suppress_etag() mandates that no ETag be allowed in the
+ * response. this could have happened via an API call or
+ * a directive like 'FileETag None'
+ *
+ * ap_weaken_etag() mandates that if an ETag is allowed in
+ * the response headers it should be in its weak form.
*/
+
if (apr_table_get(r->notes, "no-etag") != NULL) {
apr_table_unset(r->headers_out, "ETag");
}
+ else if (apr_table_get(r->notes, "weak-etag") != NULL) {
+ weaken_etags(r);
+ }
/* determine the protocol and whether we should use keepalives. */
basic_http_header_check(r, &protocol);
@@ -2671,7 +2731,6 @@
return next;
}
-#define ETAG_WEAK "W/"
#define CHARS_PER_UNSIGNED_LONG (sizeof(unsigned long) * 2)
/*
* Construct an entity tag (ETag) from resource information. If it's a real
@@ -2700,7 +2759,7 @@
* note it for the header-sender to ignore.
*/
if (etag_bits & ETAG_NONE) {
- apr_table_setn(r->notes, "no-etag", "omit");
+ ap_suppress_etag(r);
return "";
}
@@ -2718,7 +2777,13 @@
* we send a weak tag instead of a strong one, since it could
* be modified again later in the second, and the validation
* would be incorrect.
+ *
+ * additional criteria for a weak tag is if force_weak is true
+ * or ap_weaken_etag() was previously called
*/
+
+ force_weak = (force_weak || apr_table_get(r->notes, "weak-etag"));
+
if ((r->request_time - r->mtime > (1 * APR_USEC_PER_SEC)) &&
!force_weak) {
weak = NULL;
@@ -2838,6 +2903,29 @@
}
apr_table_setn(r->headers_out, "ETag", etag);
+}
+
+/*
+ * make certain that any ETag response header is marked as weak.
+ *
+ * strong tags need to be unique for every version of a resource
+ * that has ever existed. weak tags are acceptable whenever the
+ * the resource is has altered but it's meaning has not - when
+ * the bits have changed but the content is semantically equivalent.
+ * in some (perhaps rare) cases output filters may want to weaken
+ * the ETag instead of removing it entirely.
+ */
+AP_DECLARE(void) ap_weaken_etag(request_rec *r)
+{
+ apr_table_setn(r->notes, "weak-etag", "yes");
+}
+
+/*
+ * make certain that no ETag header ends up in the response
+ */
+AP_DECLARE(void) ap_suppress_etag(request_rec *r)
+{
+ apr_table_setn(r->notes, "no-etag", "omit");
}
static int parse_byterange(char *range, apr_off_t clength,
Index: t/conf/extra.conf.in
===================================================================
RCS file: /home/cvspublic/httpd-test/perl-framework/t/conf/extra.conf.in,v
retrieving revision 1.43
diff -u -r1.43 extra.conf.in
--- t/conf/extra.conf.in 13 Aug 2003 01:28:47 -0000 1.43
+++ t/conf/extra.conf.in 15 Dec 2003 20:42:06 -0000
@@ -318,6 +318,11 @@
Options +Indexes
AllowOverride All
</Directory>
+ <Directory @SERVERROOT@/htdocs/etags>
+ Options +Indexes
+ IndexOptions +TrackModified
+ AllowOverride All
+ </Directory>
</IfModule>
##
--- /dev/null 2003-01-30 05:24:37.000000000 -0500
+++ t/apache/etagapi.t 2003-12-15 15:21:18.000000000 -0500
@@ -0,0 +1,113 @@
+use strict;
+use warnings FATAL => 'all';
+
+use Apache::Test;
+use Apache::TestRequest;
+use Apache::TestUtil;
+
+my $strong = qr!^"!;
+my $weak = qr!^W/"!;
+
+my %tests = (
+ '/index.html' => $strong,
+ '/index.html?weaken' => $weak,
+ '/index.html?weaken-init' => $weak,
+ '/index.html?suppress' => undef,
+ '/index.html?suppress-init' => undef,
+ '/etags/' => $strong,
+ '/etags/?weaken' => $weak,
+ '/etags/?weaken-init' => $weak,
+ '/etags/?suppress' => undef,
+ '/etags/?suppress-init' => undef,
+ '/modules/cgi/perl.pl' => undef,
+ '/modules/cgi/perl.pl?weaken' => undef,
+ '/modules/cgi/perl.pl?weaken-init' => undef,
+);
+
+# each hash entry gets two basic tests
+my $tests = (scalar keys %tests) * 2;
+
+# each 'index.html' entry gets an additonal
+# 2 304 tests
+# 4 (Err)HeadersOut(Init) tests
+$tests += (grep { /index/ } keys %tests) * 6;
+
+# X-ErrHeaderOut(Init) variants add an additional 4 tests
+# to unsupressed index.html entries
+$tests += (grep { /index/ && !/suppress/ } keys %tests) * 4;
+
+plan tests => $tests , (have_min_apache_version(2.1) &&
+ have_module('etag_api_filter') &&
+ have_module('autoindex') &&
+ have_cgi);
+
+foreach my $uri (sort keys %tests) {
+
+ my ($last_modified, $etag);
+
+ {
+ my $response = GET $uri, 'X-AddOutputFilter' => 'etag_api_filter';
+
+ ok t_cmp(200,
+ $response->code,
+ "GET $uri status code");
+
+ $etag = $response->header('ETag');
+ $last_modified = $response->header('Last-Modified');
+
+ ok t_cmp($tests{$uri},
+ $etag,
+ "GET $uri ETag header");
+
+ }
+
+ next unless $uri =~ m/index/;
+
+ {
+ my $response = GET $uri, 'X-AddOutputFilter' => 'etag_api_filter',
+ 'If-Modified-Since' => $last_modified;
+
+ my $not_modified_etag = $response->header('ETag');
+
+ ok t_cmp(304,
+ $response->code,
+ "GET $uri, If-Modified-Since: $last_modified");
+
+ # calling ap_weaken_etag or ap_suppress_etag after
+ # filter_init has no effect on 304s by design - the 304 is
+ # returned before the filter ever runs
+ $etag = $strong unless $uri =~ m/init/;
+
+ ok t_cmp($etag,
+ $not_modified_etag,
+ "GET $uri, If-Modified-Since: $last_modified ETag compare");
+ }
+
+ foreach my $header (qw(X-ErrHeadersOut X-ErrHeadersOutInit
+ X-HeadersOut X-HeadersOutInit)) {
+
+ my $response = GET $uri, 'X-AddOutputFilter' => 'etag_api_filter',
+ $header => 1;
+
+ my @etags = $response->header('ETag');
+
+ # suppress tests are expecting undef headers
+ $etags[0] ||= undef;
+
+ # unsuppressed X-ErrHeadersOut(Init) requests should
+ # result in two ETag headers
+ if ($uri !~ m/suppress/ && $header =~ m/ErrHeadersOut/) {
+ ok t_cmp(2,
+ scalar @etags,
+ "GET $uri, $header expects 2 ETags");
+ }
+
+ my $i = 1;
+ foreach my $etag (@etags) {
+ ok t_cmp($tests{$uri},
+ $etag,
+ "GET $uri, $header " .
+ $i++ . " of " . scalar @etags);
+ }
+ }
+}
--- /dev/null 2003-01-30 05:24:37.000000000 -0500
+++ c-modules/etag_api_filter/mod_etag_api_filter.c 2003-12-15 13:56:33.000000000
-0500
@@ -0,0 +1,73 @@
+#define HTTPD_TEST_REQUIRE_APACHE 2.1
+
+#include "httpd.h"
+#include "http_config.h"
+#include "apr_buckets.h"
+#include "apr_general.h"
+#include "apr_lib.h"
+#include "util_filter.h"
+#include "http_request.h"
+#include "http_protocol.h"
+
+module AP_MODULE_DECLARE_DATA etag_api_filter_module;
+
+static apr_status_t etag_api_filter(ap_filter_t *f,
+ apr_bucket_brigade *b) {
+
+ if (f->r->args && strcmp(f->r->args, "weaken") == 0) {
+ ap_weaken_etag(f->r);
+ }
+ if (f->r->args && strcmp(f->r->args, "suppress") == 0) {
+ ap_suppress_etag(f->r);
+ }
+
+ if (apr_table_get(f->r->headers_in, "X-ErrHeadersOut")) {
+ apr_table_setn(f->r->err_headers_out, "ETag", "\"ErrHeadersOutEtag\"");
+ }
+ if (apr_table_get(f->r->headers_in, "X-HeadersOut")) {
+ apr_table_setn(f->r->headers_out, "ETag", "\"HeadersOutEtag\"");
+ }
+
+ ap_remove_output_filter(f);
+ return ap_pass_brigade(f->next, b);
+}
+
+static apr_status_t etag_api_filter_init(ap_filter_t *f) {
+
+ if (f->r->args && strcmp(f->r->args, "weaken-init") == 0) {
+ ap_weaken_etag(f->r);
+ }
+ if (f->r->args && strcmp(f->r->args, "suppress-init") == 0) {
+ ap_suppress_etag(f->r);
+ }
+
+ if (apr_table_get(f->r->headers_in, "X-ErrHeadersOutInit")) {
+ apr_table_setn(f->r->err_headers_out, "ETag", "\"ErrHeadersOutInitEtag\"");
+ }
+ if (apr_table_get(f->r->headers_in, "X-HeadersOutInit")) {
+ apr_table_setn(f->r->headers_out, "ETag", "\"HeadersOutInitEtag\"");
+ }
+
+ return OK;
+}
+
+static void etag_api_filter_insert(request_rec *r) {
+ ap_add_output_filter("etag_api_filter",NULL,r,r->connection);
+}
+
+static void etag_api_filter_register(apr_pool_t *p) {
+ ap_hook_insert_filter(etag_api_filter_insert,NULL,NULL,APR_HOOK_MIDDLE);
+ ap_register_output_filter("etag_api_filter",etag_api_filter,
+ etag_api_filter_init,AP_FTYPE_RESOURCE);
+}
+
+module AP_MODULE_DECLARE_DATA etag_api_filter_module =
+{
+ STANDARD20_MODULE_STUFF,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ NULL,
+ etag_api_filter_register
+};
