On Tue, Nov 28, 2017 at 10:56:56AM +0100, Cyril Bonté wrote:
> Hi all,
>
> Late this night, I had an issue I didn't met during my previous tests,
> related to the cache feature : sometimes, I had some corrupted images in the
> browser.
> After adding some debug messages, it seems there's an issue in
> cache_store_http_forward_data() when there is not enough contiguous space
> available.
> For example, with an image of 6532 bytes :
> - first, the headers are captured = 287 bytes
> - then, it tries to fill the cache but can only read 2609 bytes, skipping the
> first 287 bytes to ignore headers
> - cache_store_http_forward_data() has to be called a second time, that's
> where I think the issue occurs : it will bypass 287 bytes of headers again,
> while it shouldn't => only the last 3636 bytes will be read instead of the
> remaining 3923 ones :
> [code]
> /* Skip remaining headers to fill the cache */
> b_adv(msg->chn->buf, st->hdrs_len);
> ret = shctx_row_data_append(shctx,
> st->first_block,
> (unsigned char
> *)bi_ptr(msg->chn->buf),
>
> MIN(bi_contig_data(msg->chn->buf), len - st->hdrs_len));
> /* Rewind the buffer to forward all data */
> b_rew(msg->chn->buf, st->hdrs_len);
> [/code]
>
> I won't be able to make more tests before this evening, but I hope there is
> enough details ;-)
>
> Also, I've not checked yet but I was wondering : is it specific to the cache
> filter or can we find the same issue in other ones (compression for example) ?
>
> Cyril
You're absolutely right, we spot this one before but it seems the patch didn't
make it to the master.
This should be specific to the cache, we don't rewind and store the headers in
the other filters.
Patch attached.
--
William Lallemand
>From 82f39e7d1b408345aba5804608ee9ea4c4933d5a Mon Sep 17 00:00:00 2001
From: William Lallemand
Date: Tue, 28 Nov 2017 11:33:02 +0100
Subject: [PATCH] BUG/MEDIUM: cache: bad computation of the remaining size
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The cache was not setting the hdrs_len to zero when we are called
in the http_forward_data with headers + body.
The consequence is to always try to store a size - the size of headers,
during the calls to http_forward_data even when it has already forwarded
the headers.
Thanks to Cyril Bonté for reporting this bug.
Must be backported to 1.8.
---
src/cache.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/cache.c b/src/cache.c
index 06d7ce881..cdfd43d57 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -228,6 +228,7 @@ cache_store_http_forward_data(struct stream *s, struct filter *filter,
MIN(bi_contig_data(msg->chn->buf), len - st->hdrs_len));
/* Rewind the buffer to forward all data */
b_rew(msg->chn->buf, st->hdrs_len);
+st->hdrs_len = 0;
if (ret)
goto disable_cache;
}
--
2.13.6
