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);