From f3ec657e4f1cf02c3c1c593c5f6ecddfd4794493 Mon Sep 17 00:00:00 2001 From: Yann Ylavic Date: Wed, 30 Apr 2014 14:14:53 +0000 Subject: [PATCH] mod_cache: Don't add cached/revalidated entity headers to a 304 response. PR 55547. When the conditional request meets the conditions of the stale then revalidated entry, the forwarded 304 response includes the entity headers merged from the cached headers (before updating the entry). Strip them before returning a 304. Since the entity headers are stripped elsewhere, factorize the code using a new table (MOD_CACHE_ENTITY_HEADERS[]) containing these headers's names. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1591302 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 3 + modules/filters/mod_deflate.c | 145 ++++++++++++++++++++++------------ 2 files changed, 98 insertions(+), 50 deletions(-) diff --git a/CHANGES b/CHANGES index 9a1b0c0ce4..0eafe9dc38 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,9 @@ -*- coding: utf-8 -*- Changes with Apache 2.5.0 + *) mod_cache: Don't add cached/revalidated entity headers to a 304 response. + PR 55547. [Yann Ylavic] + *) mod_cache: Retry unconditional request with the full URL (including the query-string) when the origin server's 304 response does not match the conditions used to revalidate the stale entry. [Yann Ylavic]. diff --git a/modules/filters/mod_deflate.c b/modules/filters/mod_deflate.c index 92d39ff1af..e56d6d6a38 100644 --- a/modules/filters/mod_deflate.c +++ b/modules/filters/mod_deflate.c @@ -331,7 +331,7 @@ typedef struct deflate_ctx_t z_stream stream; unsigned char *buffer; unsigned long crc; - apr_bucket_brigade *bb, *proc_bb; + apr_bucket_brigade *bb, *proc_bb, *kept_bb; int (*libz_end_func)(z_streamp); unsigned char *validation_buffer; apr_size_t validation_buffer_length; @@ -1037,6 +1037,7 @@ static apr_status_t deflate_in_filter(ap_filter_t *f, int zRC; apr_status_t rv; deflate_filter_config *c; + apr_off_t plainbytes = 0; /* just get out of the way of things we don't want. */ if (mode != AP_MODE_READBYTES) { @@ -1161,28 +1162,33 @@ static apr_status_t deflate_in_filter(ap_filter_t *f, } if (APR_BRIGADE_EMPTY(ctx->proc_bb)) { - rv = ap_get_brigade(f->next, ctx->bb, mode, block, readbytes); - - /* Don't terminate on EAGAIN (or success with an empty brigade in - * non-blocking mode), just return focus. - */ - if (block == APR_NONBLOCK_READ - && (APR_STATUS_IS_EAGAIN(rv) - || (rv == APR_SUCCESS && APR_BRIGADE_EMPTY(ctx->bb)))) { - return rv; + if (ctx->kept_bb && !APR_BRIGADE_EMPTY(ctx->kept_bb)) { + APR_BRIGADE_CONCAT(ctx->bb, ctx->kept_bb); } - if (rv != APR_SUCCESS) { - inflateEnd(&ctx->stream); - return rv; + else { + rv = ap_get_brigade(f->next, ctx->bb, mode, block, readbytes); + + /* Don't terminate on EAGAIN (or success with an empty brigade in + * non-blocking mode), just return focus. + */ + if (block == APR_NONBLOCK_READ + && (APR_STATUS_IS_EAGAIN(rv) + || (rv == APR_SUCCESS + && APR_BRIGADE_EMPTY(ctx->bb)))) { + return rv; + } + if (rv != APR_SUCCESS) { + inflateEnd(&ctx->stream); + return rv; + } } - for (bkt = APR_BRIGADE_FIRST(ctx->bb); - bkt != APR_BRIGADE_SENTINEL(ctx->bb); - bkt = APR_BUCKET_NEXT(bkt)) - { + while (!APR_BRIGADE_EMPTY(ctx->bb)) { const char *data; apr_size_t len; + bkt = APR_BRIGADE_FIRST(ctx->bb); + if (APR_BUCKET_IS_EOS(bkt)) { if (!ctx->done) { inflateEnd(&ctx->stream); @@ -1198,7 +1204,7 @@ static apr_status_t deflate_in_filter(ap_filter_t *f, } if (APR_BUCKET_IS_FLUSH(bkt)) { - apr_bucket *tmp_b; + apr_bucket *tmp_heap; zRC = inflate(&(ctx->stream), Z_SYNC_FLUSH); if (zRC != Z_OK) { inflateEnd(&ctx->stream); @@ -1210,20 +1216,18 @@ static apr_status_t deflate_in_filter(ap_filter_t *f, ctx->stream.next_out = ctx->buffer; len = c->bufferSize - ctx->stream.avail_out; - + ctx->crc = crc32(ctx->crc, (const Bytef *)ctx->buffer, len); - tmp_b = apr_bucket_heap_create((char *)ctx->buffer, len, - NULL, f->c->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(ctx->proc_bb, tmp_b); + tmp_heap = apr_bucket_heap_create((char *)ctx->buffer, len, + NULL, f->c->bucket_alloc); + APR_BRIGADE_INSERT_TAIL(ctx->proc_bb, tmp_heap); ctx->stream.avail_out = c->bufferSize; /* Flush everything so far in the returning brigade, but continue * reading should EOS/more follow (don't lose them). */ - tmp_b = APR_BUCKET_PREV(bkt); APR_BUCKET_REMOVE(bkt); APR_BRIGADE_INSERT_TAIL(ctx->proc_bb, bkt); - bkt = tmp_b; continue; } @@ -1237,6 +1241,7 @@ static apr_status_t deflate_in_filter(ap_filter_t *f, /* read */ apr_bucket_read(bkt, &data, &len, APR_BLOCK_READ); if (!len) { + apr_bucket_delete(bkt); continue; } if (len > INT_MAX) { @@ -1250,6 +1255,7 @@ static apr_status_t deflate_in_filter(ap_filter_t *f, ctx->zlib_flags = 0; } if (!len) { + apr_bucket_delete(bkt); continue; } } @@ -1260,38 +1266,75 @@ static apr_status_t deflate_in_filter(ap_filter_t *f, zRC = Z_OK; - if (!ctx->validation_buffer) { - while (ctx->stream.avail_in != 0) { - if (ctx->stream.avail_out == 0) { - apr_bucket *tmp_heap; - ctx->stream.next_out = ctx->buffer; - len = c->bufferSize - ctx->stream.avail_out; - - ctx->crc = crc32(ctx->crc, (const Bytef *)ctx->buffer, len); - tmp_heap = apr_bucket_heap_create((char *)ctx->buffer, len, - NULL, f->c->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(ctx->proc_bb, tmp_heap); - ctx->stream.avail_out = c->bufferSize; - } + while (!ctx->validation_buffer) { + apr_size_t newbytes = c->bufferSize - ctx->stream.avail_out; + + if (ctx->stream.avail_out == 0 + || plainbytes + newbytes >= readbytes) { + apr_bucket *tmp_heap; + + ctx->crc = crc32(ctx->crc, (const Bytef *)ctx->buffer, + newbytes); + tmp_heap = apr_bucket_heap_create((char *)ctx->buffer, + newbytes, NULL, + f->c->bucket_alloc); + APR_BRIGADE_INSERT_TAIL(ctx->proc_bb, tmp_heap); - zRC = inflate(&ctx->stream, Z_NO_FLUSH); + ctx->stream.next_out = ctx->buffer; + ctx->stream.avail_out = c->bufferSize; - if (zRC == Z_STREAM_END) { - ctx->validation_buffer = apr_pcalloc(r->pool, - VALIDATION_SIZE); - ctx->validation_buffer_length = 0; + plainbytes += newbytes; + if (plainbytes >= readbytes) { + /* Split and free up to what's already handled */ + if (ctx->stream.avail_in > 0) { + apr_bucket_split(bkt, len - ctx->stream.avail_in); + } + ctx->kept_bb = apr_brigade_split_ex(ctx->bb, + APR_BUCKET_NEXT(bkt), + ctx->kept_bb); + apr_brigade_cleanup(ctx->bb); + + /* Save input_brigade. (At least) in the SSL case + * it contains transient buckets whose data would + * get overwritten should the downstream filters + * be called directly (this filter won't call + * ap_get_brigade() unless ctx->bb is empty, but + * be safe since we loose hand now). + */ + ap_save_brigade(f, &ctx->bb, &ctx->kept_bb, r->pool); + APR_BRIGADE_CONCAT(ctx->kept_bb, ctx->bb); + + /* We are done. */ break; } + } + if (ctx->stream.avail_in == 0) { + break; + } - if (zRC != Z_OK) { - inflateEnd(&ctx->stream); - ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01392) - "Zlib error %d inflating data (%s)", zRC, - ctx->stream.msg); - return APR_EGENERAL; - } + zRC = inflate(&ctx->stream, Z_NO_FLUSH); + + if (zRC == Z_STREAM_END) { + ctx->validation_buffer = apr_pcalloc(r->pool, + VALIDATION_SIZE); + ctx->validation_buffer_length = 0; + break; + } + + if (zRC != Z_OK) { + inflateEnd(&ctx->stream); + ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01392) + "Zlib error %d inflating data (%s)", zRC, + ctx->stream.msg); + return APR_EGENERAL; } } + if (plainbytes >= readbytes) { + /* We were done in the inner loop above, + * so we are for this one. + */ + break; + } if (ctx->validation_buffer) { apr_bucket *tmp_heap; @@ -1311,6 +1354,7 @@ static apr_status_t deflate_in_filter(ap_filter_t *f, memcpy(buf + ctx->validation_buffer_length, ctx->stream.next_in, avail); ctx->validation_buffer_length += avail; + apr_bucket_delete(bkt); continue; } memcpy(buf + ctx->validation_buffer_length, @@ -1363,8 +1407,9 @@ static apr_status_t deflate_in_filter(ap_filter_t *f, } } + apr_bucket_delete(bkt); } - apr_brigade_cleanup(ctx->bb); + } /* If we are about to return nothing for a 'blocking' read and we have -- 2.50.1