Sander Striker wrote:
>> [EMAIL PROTECTED] wrote:
>
> [...]
[...]
>
>> I am aware that this forces a full request to the backend for requests
>> without conditionals
>> to expired resources. So I am not very happy with this solution. Maybe
>> it is better to let
>> the default handler pass 304 responses down the filter chain.
>
>
> Might as well not do revalidation in that case; actually that would be
> better, because the
> 304's that are returned may not even be correct. The conditions are
> replaced with the
> ones from the cache, remember?
>
Yes, I remember, but I must admit that I am slightly confused now. When should
we avoid revalidation with the conditionals from the cache?
If the original request does not contain any conditionals? This is what my
patch does
[..cut..]
>
> I can see the application. Are you up for submitting a patch to the
> default handler? :)
I have attached a patch for this. Two comments:
1. I am not very familar with buckets and brigades, so please check closely if
I did it correct (my tests make me think so). If I did something wrong
feedback
is appreciated such that I can do it better next time :-)
2. ap_meets_conditions returns 3 different values: OK, HTTP_PRECONDITION_FAILED
and
HTTP_NOT_MODIFIED. In my patch I assume that in all cases the response should
go down the filter chain.
Regards
Rüdiger
--- core.c.orig 2005-05-21 10:30:23.000000000 +0200
+++ core.c 2005-05-22 12:08:20.000000000 +0200
@@ -3492,46 +3492,48 @@ static int default_handler(request_rec *
ap_set_etag(r);
apr_table_setn(r->headers_out, "Accept-Ranges", "bytes");
ap_set_content_length(r, r->finfo.size);
- if ((errstatus = ap_meets_conditions(r)) != OK) {
- apr_file_close(fd);
- return errstatus;
- }
-
- if (bld_content_md5) {
- apr_table_setn(r->headers_out, "Content-MD5",
- ap_md5digest(r->pool, fd));
- }
bb = apr_brigade_create(r->pool, c->bucket_alloc);
- /* For platforms where the size of the file may be larger than
- * that which can be stored in a single bucket (where the
- * length field is an apr_size_t), split it into several
- * buckets: */
- if (sizeof(apr_off_t) > sizeof(apr_size_t)
- && r->finfo.size > AP_MAX_SENDFILE) {
- apr_off_t fsize = r->finfo.size;
- e = apr_bucket_file_create(fd, 0, AP_MAX_SENDFILE, r->pool,
- c->bucket_alloc);
- while (fsize > AP_MAX_SENDFILE) {
- apr_bucket *ce;
- apr_bucket_copy(e, &ce);
- APR_BRIGADE_INSERT_TAIL(bb, ce);
- e->start += AP_MAX_SENDFILE;
- fsize -= AP_MAX_SENDFILE;
+ if ((errstatus = ap_meets_conditions(r)) != OK) {
+ apr_file_close(fd);
+ r->status = errstatus;
+ }
+ else {
+ if (bld_content_md5) {
+ apr_table_setn(r->headers_out, "Content-MD5",
+ ap_md5digest(r->pool, fd));
+ }
+
+ /* For platforms where the size of the file may be larger than
+ * that which can be stored in a single bucket (where the
+ * length field is an apr_size_t), split it into several
+ * buckets: */
+ if (sizeof(apr_off_t) > sizeof(apr_size_t)
+ && r->finfo.size > AP_MAX_SENDFILE) {
+ apr_off_t fsize = r->finfo.size;
+ e = apr_bucket_file_create(fd, 0, AP_MAX_SENDFILE, r->pool,
+ c->bucket_alloc);
+ while (fsize > AP_MAX_SENDFILE) {
+ apr_bucket *ce;
+ apr_bucket_copy(e, &ce);
+ APR_BRIGADE_INSERT_TAIL(bb, ce);
+ e->start += AP_MAX_SENDFILE;
+ fsize -= AP_MAX_SENDFILE;
+ }
+ e->length = (apr_size_t)fsize; /* Resize just the last bucket
*/
}
- e->length = (apr_size_t)fsize; /* Resize just the last bucket */
- }
- else
- e = apr_bucket_file_create(fd, 0, (apr_size_t)r->finfo.size,
- r->pool, c->bucket_alloc);
+ else
+ e = apr_bucket_file_create(fd, 0, (apr_size_t)r->finfo.size,
+ r->pool, c->bucket_alloc);
#if APR_HAS_MMAP
- if (d->enable_mmap == ENABLE_MMAP_OFF) {
- (void)apr_bucket_file_enable_mmap(e, 0);
- }
+ if (d->enable_mmap == ENABLE_MMAP_OFF) {
+ (void)apr_bucket_file_enable_mmap(e, 0);
+ }
#endif
- APR_BRIGADE_INSERT_TAIL(bb, e);
+ APR_BRIGADE_INSERT_TAIL(bb, e);
+ }
e = apr_bucket_eos_create(c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(bb, e);
