From: Christopher Faulet <cfaulet@haproxy.com>
Date: Thu, 13 Jun 2019 09:16:45 +0000 (+0200)
Subject: BUG/MEDIUM: htx: Don't change position of the first block during HTX analysis
X-Git-Tag: v2.0.0~41
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=421e769783ec4a3640a1f74c782e80b1cb1fb0d3;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: htx: Don't change position of the first block during HTX analysis

In the HTX structure, the field <first> is used to know where to (re)start the
analysis. It may differ from the message's head. It is especially important to
update it to handle 1xx messages, to be sure to restart the analysis on the next
message (another 1xx message or the final one). It is also updated when some
data are forwarded (the headers or part of the body). But this update is an
error and must never be done at the analysis level. It is a bug, because some
sample fetches may be used after the data forwarding (but before the first send
of course). At this stage, if the first block position does not point on the
start-line, most of HTTP sample fetches fail.

So now, when something is forwarding by HTX analyzers, the first block position
is not update anymore.

This issue was reported on Github. See #119. No backport needed.
---

diff --git a/include/proto/channel.h b/include/proto/channel.h
index 73288584be..c072c661f7 100644
--- a/include/proto/channel.h
+++ b/include/proto/channel.h
@@ -915,9 +915,10 @@ static inline void channel_slow_realign(struct channel *chn, char *swap)
 
 
 /* Forward all headers of an HTX message, starting from the SL to the EOH. This
- * function also updates the first block position.
+ * function returns the position of the block after the EOH, if
+ * found. Otherwise, it returns -1.
  */
-static inline void channel_htx_fwd_headers(struct channel *chn, struct htx *htx)
+static inline int32_t channel_htx_fwd_headers(struct channel *chn, struct htx *htx)
 {
 	int32_t pos;
 	size_t  data = 0;
@@ -926,11 +927,12 @@ static inline void channel_htx_fwd_headers(struct channel *chn, struct htx *htx)
 		struct htx_blk *blk = htx_get_blk(htx, pos);
 		data += htx_get_blksz(blk);
 		if (htx_get_blk_type(blk) == HTX_BLK_EOH) {
-			htx->first = htx_get_next(htx, pos);
+			pos = htx_get_next(htx, pos);
 			break;
 		}
 	}
 	c_adv(chn, data);
+	return pos;
 }
 
 /* Forward <data> bytes of payload of an HTX message. This function also updates
diff --git a/src/filters.c b/src/filters.c
index f0cf9b6c24..ff7807565b 100644
--- a/src/filters.c
+++ b/src/filters.c
@@ -782,7 +782,8 @@ flt_http_payload(struct stream *s, struct http_msg *msg, unsigned int len)
 {
 	struct filter *filter;
 	unsigned long long *strm_off = &FLT_STRM_OFF(s, msg->chn);
-	int ret = len - co_data(msg->chn);
+	unsigned int out = co_data(msg->chn);
+	int ret = len - out;
 
 	list_for_each_entry(filter, &strm_flt(s)->filters, list) {
 		/* Call "data" filters only */
@@ -792,7 +793,7 @@ flt_http_payload(struct stream *s, struct http_msg *msg, unsigned int len)
 			unsigned long long *flt_off = &FLT_OFF(filter, msg->chn);
 			unsigned int offset = *flt_off - *strm_off;
 
-			ret = FLT_OPS(filter)->http_payload(s, filter, msg, offset, ret - offset);
+			ret = FLT_OPS(filter)->http_payload(s, filter, msg, out + offset, ret - offset);
 			if (ret < 0)
 				goto end;
 			*flt_off += ret;
diff --git a/src/proto_htx.c b/src/proto_htx.c
index 7a1f9c5168..64595a3c42 100644
--- a/src/proto_htx.c
+++ b/src/proto_htx.c
@@ -1220,12 +1220,12 @@ int htx_request_forward_body(struct stream *s, struct channel *req, int an_bit)
 		ret  = flt_http_payload(s, msg, htx->data);
 		if (ret < 0)
 			goto return_bad_req;
-		channel_htx_fwd_payload(req, htx, ret);
+		c_adv(req, ret);
 		if (htx->data != co_data(req) || htx->extra)
 			goto missing_data_or_waiting;
 	}
 	else {
-		channel_htx_fwd_all(req, htx);
+		c_adv(req, htx->data - co_data(req));
 		if (msg->flags & HTTP_MSGF_XFER_LEN)
 			channel_htx_forward_forever(req, htx);
 	}
@@ -1698,7 +1698,7 @@ int htx_wait_for_response(struct stream *s, struct channel *rep, int an_bit)
 	if (txn->status < 200 &&
 	    (txn->status == 100 || txn->status >= 102)) {
 		FLT_STRM_CB(s, flt_http_reset(s, msg));
-		channel_htx_fwd_headers(rep, htx);
+		htx->first = channel_htx_fwd_headers(rep, htx);
 		msg->msg_state = HTTP_MSG_RPBEFORE;
 		txn->status = 0;
 		s->logs.t_data = -1; /* was not a response yet */
@@ -2219,12 +2219,12 @@ int htx_response_forward_body(struct stream *s, struct channel *res, int an_bit)
 		ret  = flt_http_payload(s, msg, htx->data);
 		if (ret < 0)
 			goto return_bad_res;
-		channel_htx_fwd_payload(res, htx, ret);
+		c_adv(res, ret);
 		if (htx->data != co_data(res) || htx->extra)
 			goto missing_data_or_waiting;
 	}
 	else {
-		channel_htx_fwd_all(res, htx);
+		c_adv(res, htx->data - co_data(res));
 		if (msg->flags & HTTP_MSGF_XFER_LEN)
 			channel_htx_forward_forever(res, htx);
 	}