From: Christopher Faulet <cfaulet@haproxy.com>
Date: Tue, 11 Jun 2019 08:38:38 +0000 (+0200)
Subject: BUG/MEDIUM: compression/htx: Fix the adding of the last data block
X-Git-Tag: v2.0-dev7~13
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=86bc8df95579c69c0468722ac8b15acd2cb596fd;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: compression/htx: Fix the adding of the last data block

The function htx_add_data_before() is buggy and cannot work. It first add a data
block and then move it before another one, passed in argument. The problem
happens when a defragmentation is done to add the new block. In this case, the
reference is no longer valid, because the blocks are rearranged. So, instead of
moving the new block before the reference, it is moved at the head of the HTX
message.

So this function has been removed. It was only used by the compression filter to
add a last data block before a TLR, EOT or EOM block. Now, the new function
htx_add_last_data() is used. It adds a last data block, after all others and
before any TLR, EOT or EOM block. Then, the next bock is get. It is the first
non-data block after data in the HTX message. The compression loop continues
with it.

This patch must be backported to 1.9.
---

diff --git a/include/common/htx.h b/include/common/htx.h
index cb998b99e6..65f0ec3005 100644
--- a/include/common/htx.h
+++ b/include/common/htx.h
@@ -189,7 +189,7 @@ struct htx_blk *htx_add_all_trailers(struct htx *htx, const struct http_hdr *hdr
 struct htx_blk *htx_add_endof(struct htx *htx, enum htx_blk_type type);
 struct htx_blk *htx_add_data_atonce(struct htx *htx, const struct ist data);
 size_t htx_add_data(struct htx *htx, const struct ist data);
-struct htx_blk *htx_add_data_before(struct htx *htx, const struct htx_blk *ref, const struct ist data);
+struct htx_blk *htx_add_last_data(struct htx *htx, struct ist data);
 
 int htx_reqline_to_h1(const struct htx_sl *sl, struct buffer *chk);
 int htx_stline_to_h1(const struct htx_sl *sl, struct buffer *chk);
diff --git a/src/flt_http_comp.c b/src/flt_http_comp.c
index 13f87bead5..b04dcd145f 100644
--- a/src/flt_http_comp.c
+++ b/src/flt_http_comp.c
@@ -256,7 +256,10 @@ comp_http_payload(struct stream *s, struct filter *filter, struct http_msg *msg,
 					if (htx_compression_buffer_end(st, &trash, 1) < 0)
 						goto error;
 					if (b_data(&trash)) {
-						blk = htx_add_data_before(htx, blk, ist2(b_head(&trash), b_data(&trash)));
+						struct htx_blk *last = htx_add_last_data(htx, ist2(b_head(&trash), b_data(&trash)));
+						if (!last)
+							goto error;
+						blk = htx_get_next_blk(htx, last);
 						if (!blk)
 							goto error;
 						to_forward += b_data(&trash);
diff --git a/src/htx.c b/src/htx.c
index 8be38d6f70..8dce7c892c 100644
--- a/src/htx.c
+++ b/src/htx.c
@@ -868,22 +868,29 @@ size_t htx_add_data(struct htx *htx, const struct ist data)
 	return len;
 }
 
-struct htx_blk *htx_add_data_before(struct htx *htx, const struct htx_blk *ref,
-				    const struct ist data)
+
+/* Adds an HTX block of type DATA in <htx> just after all other DATA
+ * blocks. Because it relies on htx_add_data_atonce(), It may be happened to a
+ * DATA block if possible. But, if the function succeeds, it will be the last
+ * DATA block in all cases. If an error occurred, NULL is returned. Otherwise,
+ * on success, the updated block (or the new one) is returned.
+ */
+struct htx_blk *htx_add_last_data(struct htx *htx, struct ist data)
 {
-	struct htx_blk *blk;
-	int32_t prev;
+	struct htx_blk *blk, *pblk;
 
-	/* FIXME: check data.len (< 256MB) */
-	blk = htx_add_blk(htx, HTX_BLK_DATA, data.len);
+	blk = htx_add_data_atonce(htx, data);
 	if (!blk)
 		return NULL;
 
-	blk->info += data.len;
-	memcpy(htx_get_blk_ptr(htx, blk), data.ptr, data.len);
+	for (pblk = htx_get_prev_blk(htx, blk); pblk; pblk = htx_get_prev_blk(htx, pblk)) {
+		int32_t cur, prev;
+
+		if (htx_get_blk_type(pblk) <= HTX_BLK_DATA)
+			break;
 
-	for (prev = htx_get_prev(htx, htx->tail); prev != -1; prev = htx_get_prev(htx, prev)) {
-		struct htx_blk *pblk = htx_get_blk(htx, prev);
+		cur  = htx_get_blk_pos(htx, blk);
+		prev = htx_get_blk_pos(htx, pblk);
 
 		/* Swap .addr and .info fields */
 		blk->addr ^= pblk->addr; pblk->addr ^= blk->addr; blk->addr ^= pblk->addr;
@@ -891,11 +898,11 @@ struct htx_blk *htx_add_data_before(struct htx *htx, const struct htx_blk *ref,
 
 		if (blk->addr == pblk->addr)
 			blk->addr += htx_get_blksz(pblk);
-		htx->front = prev;
-
-		if (pblk == ref)
-			break;
 		blk = pblk;
+		if (htx->front == cur)
+			htx->front = prev;
+		else if (htx->front == prev)
+			htx->front = cur;
 	}
 
 	if (htx_get_blk_pos(htx, blk) != htx->front)