BUG/MEDIUM: htx: Always do a defrag if a block value is replace by a bigger one
Otherwise, after such replaces, the HTX message appears to wrap but the head
block address is not necessarily the first one. So adding new blocks will
override data of old ones.
diff --git a/src/htx.c b/src/htx.c
index cd16bf3..78bfd57 100644
--- a/src/htx.c
+++ b/src/htx.c
@@ -368,26 +368,27 @@
struct htx_blk *htx_replace_blk_value(struct htx *htx, struct htx_blk *blk,
const struct ist old, const struct ist new)
{
- struct htx_blk *frtblk;
- struct buffer *tmp;
+ struct buffer *tmp;
struct ist n, v;
- uint32_t info, room;
+ int32_t delta;
n = htx_get_blk_name(htx, blk);
v = htx_get_blk_value(htx, blk);
+ delta = new.len - old.len;
+
/* easy case, new data are smaller, so replace it in-place */
- if (new.len <= old.len) {
+ if (delta <= 0) {
memcpy(old.ptr, new.ptr, new.len);
if (old.len != v.len)
memmove(old.ptr + new.len, old.ptr + old.len, (v.ptr + v.len) - (old.ptr + old.len));
- htx_set_blk_value_len(blk, v.len - old.len + new.len);
- htx->data -= (old.len - new.len);
+ htx_set_blk_value_len(blk, v.len + delta);
+ htx->data += delta;
return blk;
}
/* we need to allocate more space to store the new header value */
- if ((new.len - old.len) > htx_free_space(htx))
+ if (delta > htx_free_space(htx))
return NULL; /* not enough space */
/*
@@ -409,36 +410,16 @@
if (old.len != v.len)
chunk_memcat(tmp, old.ptr + old.len, (v.ptr + v.len) - (old.ptr + old.len));
- /*
- * temporarely remove space reserved for the header
- */
- info = blk->info;
- blk->info &= 0xf0000000;
- htx->data -= (n.len + v.len);
- /*
- * Try to find right addr to copy all the data
+ /* Expand the block size. But data are not copied yet. Then defragment
+ * the HTX message.
*/
- if (htx->tail + 1 == htx->wrap) {
- frtblk = htx_get_blk(htx, htx->front);
- room = sizeof(htx->blocks[0]) * htx_pos_to_idx(htx, htx->tail) - (frtblk->addr + htx_get_blksz(frtblk));
- if (room >= htx->data) {
- blk->addr = frtblk->addr + htx_get_blksz(frtblk);
- goto replace_value;
- }
- }
+ htx_set_blk_value_len(blk, v.len + delta);
+ htx->data += delta;
+ blk = htx_defrag(htx, blk);
- /* HTX message need to be defragmented first */
- blk = htx_defrag(htx, blk);
- frtblk = htx_get_blk(htx, htx->front);
- blk->addr = frtblk->addr + htx_get_blksz(frtblk);
-
- replace_value:
- blk->info = info;
- htx_set_blk_value_len(blk, v.len - old.len + new.len);
+ /* Finaly, copy data. */
memcpy(htx_get_blk_ptr(htx, blk), tmp->area, tmp->data);
- htx->data += tmp->data;
- htx->front = htx_get_blk_pos(htx, blk);
return blk;
}
@@ -514,57 +495,6 @@
return (struct htx_ret){.ret = ret, .blk = dstblk};
}
-static struct htx_blk *htx_new_blk_value(struct htx *htx, struct htx_blk *blk,
- uint32_t newsz)
-{
- struct htx_blk *frtblk;
- uint32_t sz, room;
- int32_t delta;
-
- sz = htx_get_blksz(blk);
- delta = newsz - sz;
-
- /* easy case, new value is smaller, so replace it in-place */
- if (delta <= 0) {
- /* Reset value size. It is the caller responsibility to set the new one */
- blk->info &= 0xf0000000;
- htx->data += delta;
- return blk;
- }
-
- /* we need to allocate more space to store the new value */
- if (delta > htx_free_space(htx))
- return NULL; /* not enough space */
-
- /*
- * temporarely remove space reserved for the old value
- */
- /* Reset value size. It is the caller responsibility to set the new one */
- blk->info &= 0xf0000000;
- htx->data -= sz;
-
- /*
- * Try to find right addr to copy all the data
- */
- if (htx->tail + 1 == htx->wrap) {
- frtblk = htx_get_blk(htx, htx->front);
- room = sizeof(htx->blocks[0]) * htx_pos_to_idx(htx, htx->tail) - (frtblk->addr + htx_get_blksz(frtblk));
- if (room >= newsz)
- goto replace_value;
- }
-
- /* HTX message need to be defragmented first */
- blk = htx_defrag(htx, blk);
- frtblk = htx_get_blk(htx, htx->front);
-
- replace_value:
- blk->addr = frtblk->addr + htx_get_blksz(frtblk);
- htx->data += newsz;
- htx->front = htx_get_blk_pos(htx, blk);
-
- return blk;
-}
-
/* Replaces an header by a new one. The new header can be smaller or larger than
* the old one. It returns the new block on success, otherwise it returns NULL.
* The header name is always lower cased.
@@ -573,19 +503,36 @@
const struct ist name, const struct ist value)
{
enum htx_blk_type type;
+ int32_t delta;
type = htx_get_blk_type(blk);
if (type != HTX_BLK_HDR)
return NULL;
- blk = htx_new_blk_value(htx, blk, (name.len + value.len));
- if (!blk)
- return NULL;
+ delta = name.len + value.len - htx_get_blksz(blk);
+
+ /* easy case, new value is smaller, so replace it in-place */
+ if (delta <= 0) {
+ blk->info = (type << 28) + (value.len << 8) + name.len;
+ htx->data += delta;
+ goto copy;
+ }
+
+ /* we need to allocate more space to store the new value */
+ if (delta > htx_free_space(htx))
+ return NULL; /* not enough space */
+ /* Expand the block size. But data are not copied yet. Then defragment
+ * the HTX message.
+ */
blk->info = (type << 28) + (value.len << 8) + name.len;
+ htx->data += delta;
+ blk = htx_defrag(htx, blk);
+
+ copy:
+ /* Finaly, copy data. */
ist2bin_lc(htx_get_blk_ptr(htx, blk), name);
memcpy(htx_get_blk_ptr(htx, blk) + name.len, value.ptr, value.len);
-
return blk;
}
@@ -600,6 +547,7 @@
struct htx_sl tmp; /* used to save sl->info and sl->flags */
enum htx_blk_type type;
uint32_t size;
+ int32_t delta;
type = htx_get_blk_type(blk);
if (type != HTX_BLK_REQ_SL && type != HTX_BLK_RES_SL)
@@ -614,12 +562,28 @@
size = sizeof(*sl) + p1.len + p2.len + p3.len;
- blk = htx_new_blk_value(htx, blk, size);
- if (!blk)
- return NULL;
- blk->info = (type << 28) + size;
+ delta = size - htx_get_blksz(blk);
+
+ /* easy case, new data are smaller, so replace it in-place */
+ if (delta <= 0) {
+ htx_set_blk_value_len(blk, size);
+ htx->data += delta;
+ goto copy;
+ }
+
+ /* we need to allocate more space to store the new value */
+ if (delta > htx_free_space(htx))
+ return NULL; /* not enough space */
+
+ /* Expand the block size. But data are not copied yet. Then defragment
+ * the HTX message.
+ */
+ htx_set_blk_value_len(blk, size);
+ htx->data += delta;
+ blk = htx_defrag(htx, blk);
- /* Restore start-line info and flags*/
+ copy:
+ /* Restore start-line info and flags and copy parts of the start-line */
sl = htx_get_blk_ptr(htx, blk);
sl->info = tmp.info;
sl->flags = tmp.flags;