Hi, Here are 2 patches fixing bugs in data filtering.
-- Christopher
>From bdfa4535d6fe1f0f5bca52a48d2e4205e41bbd81 Mon Sep 17 00:00:00 2001 From: Christopher Faulet <[email protected]> Date: Tue, 21 Jun 2016 10:44:32 +0200 Subject: [PATCH 1/2] BUG/MEDIUM: filters: Fix data filtering when data are modified Filters can alter data during the parsing, i.e when http_data or tcp_data callbacks are called. For now, the update must be done by hand. So we must handle changes in the channel buffers, especially on the number of input bytes pending (buf->i). In addition, a filter can choose to switch channel buffers to do its updates. So, during data filtering, we must always use the right buffer and not use variable to reference them. Without this patch, filters cannot safely alter data during the data parsing. --- src/filters.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/filters.c b/src/filters.c index 139440d..7f8fae4 100644 --- a/src/filters.c +++ b/src/filters.c @@ -418,12 +418,11 @@ int flt_http_data(struct stream *s, struct http_msg *msg) { struct filter *filter; - struct buffer *buf = msg->chn->buf; unsigned int buf_i; - int ret = 0; + int delta = 0, ret = 0; /* Save buffer state */ - buf_i = buf->i; + buf_i = msg->chn->buf->i; list_for_each_entry(filter, &strm_flt(s)->filters, list) { unsigned int *nxt; @@ -440,9 +439,12 @@ flt_http_data(struct stream *s, struct http_msg *msg) *nxt = msg->next; if (FLT_OPS(filter)->http_data) { + unsigned int i = msg->chn->buf->i; + ret = FLT_OPS(filter)->http_data(s, filter, msg); if (ret < 0) break; + delta += (int)(msg->chn->buf->i - i); /* Update the next offset of the current filter */ *nxt += ret; @@ -450,18 +452,18 @@ flt_http_data(struct stream *s, struct http_msg *msg) /* And set this value as the bound for the next * filter. It will not able to parse more data than this * one. */ - buf->i = *nxt; + msg->chn->buf->i = *nxt; } else { /* Consume all available data and update the next offset * of the current filter. buf->i is untouched here. */ - ret = MIN(msg->chunk_len + msg->next, buf->i) - *nxt; + ret = MIN(msg->chunk_len + msg->next, msg->chn->buf->i) - *nxt; *nxt += ret; } } /* Restore the original buffer state */ - buf->i = buf_i; + msg->chn->buf->i = buf_i + delta; return ret; } @@ -806,12 +808,11 @@ static int flt_data(struct stream *s, struct channel *chn) { struct filter *filter; - struct buffer *buf = chn->buf; unsigned int buf_i; - int ret = 0; + int delta = 0, ret = 0; /* Save buffer state */ - buf_i = buf->i; + buf_i = chn->buf->i; list_for_each_entry(filter, &strm_flt(s)->filters, list) { unsigned int *nxt; @@ -822,9 +823,12 @@ flt_data(struct stream *s, struct channel *chn) nxt = &FLT_NXT(filter, chn); if (FLT_OPS(filter)->tcp_data) { + unsigned int i = chn->buf->i; + ret = FLT_OPS(filter)->tcp_data(s, filter, chn); if (ret < 0) break; + delta += (int)(chn->buf->i - i); /* Increase next offset of the current filter */ *nxt += ret; @@ -832,11 +836,11 @@ flt_data(struct stream *s, struct channel *chn) /* And set this value as the bound for the next * filter. It will not able to parse more data than the * current one. */ - buf->i = *nxt; + chn->buf->i = *nxt; } else { /* Consume all available data */ - *nxt = buf->i; + *nxt = chn->buf->i; } /* Update <ret> value to be sure to have the last one when we @@ -846,7 +850,7 @@ flt_data(struct stream *s, struct channel *chn) } /* Restore the original buffer state */ - chn->buf->i = buf_i; + chn->buf->i = buf_i + delta; return ret; } -- 2.5.5
>From f7b54e739b1d2e56b0969fbd8ee9ba2f0e229639 Mon Sep 17 00:00:00 2001 From: Christopher Faulet <[email protected]> Date: Tue, 21 Jun 2016 11:04:34 +0200 Subject: [PATCH 2/2] BUG/MINOR: filters: Fix HTTP parsing when a filter loops on data forwarding A filter can choose to loop on data forwarding. When this loop occurs in HTTP_MSG_ENDING state, http_foward_data callbacks are called twice because of a goto on the wrong label. A filter can also choose to loop at the end of a HTTP message, in http_end callback. Here the goto is good but the label is not at the right place. We must be sure to upate msg->sov value. --- src/proto_http.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/proto_http.c b/src/proto_http.c index 78c1ac3..4a6e6d3 100644 --- a/src/proto_http.c +++ b/src/proto_http.c @@ -6851,7 +6851,7 @@ http_msg_forward_body(struct stream *s, struct http_msg *msg) b_adv(chn->buf, ret); msg->next -= ret; if (msg->next) - goto missing_data_or_waiting; + goto waiting; FLT_STRM_DATA_CB(s, chn, flt_http_end(s, msg), /* default_ret */ 1, @@ -6867,11 +6867,12 @@ http_msg_forward_body(struct stream *s, struct http_msg *msg) /* on_error */ goto error); b_adv(chn->buf, ret); msg->next -= ret; + + waiting: if (!(chn->flags & CF_WROTE_DATA) || msg->sov > 0) msg->sov -= ret; if (!HAS_DATA_FILTERS(s, chn)) msg->chunk_len -= channel_forward(chn, msg->chunk_len); - waiting: return 0; error: return -1; @@ -6965,7 +6966,7 @@ http_msg_forward_chunked_body(struct stream *s, struct http_msg *msg) b_adv(chn->buf, ret); msg->next -= ret; if (msg->next) - goto missing_data_or_waiting; + goto waiting; FLT_STRM_DATA_CB(s, chn, flt_http_end(s, msg), /* default_ret */ 1, @@ -6981,11 +6982,12 @@ http_msg_forward_chunked_body(struct stream *s, struct http_msg *msg) /* on_error */ goto error); b_adv(chn->buf, ret); msg->next -= ret; + + waiting: if (!(chn->flags & CF_WROTE_DATA) || msg->sov > 0) msg->sov -= ret; if (!HAS_DATA_FILTERS(s, chn)) msg->chunk_len -= channel_forward(chn, msg->chunk_len); - waiting: return 0; chunk_parsing_error: -- 2.5.5
