Fixed framing error for resposnses received from cache and

with header modification

tfw_hpack_cache_decode_expand: fixed http2 framing. However,
function looks overcomplicated and combines http1 and http2
related logic.

tfw_cache_h2_decode_write: after decoding header part of
`TfwDecodeCacheIter` was cleared including `acc_len` and
because of this caller of `tfw_cache_h2_decode_write`
couldn't receive a total length of header. Fixed by
clearing only `hdr_data` and `h2_data`.

Removed `__off` from `TfwDecodeCacheIter`, because we need
to clear only two last members in this structure and it can
be done via inner struct.

Removed `desc` from `TfwDecodeCacheIter`. It's not used.

Author: const-t

fw/cache.c

@@ -684,9 +684,7 @@ tfw_cache_h2_decode_write(TDB *db, TdbVRec **trec, TfwHttpResp *resp,
 		acc += m_len;
 		*data += m_len;
 		if (acc == len) {
-			bzero_fast(dc_iter->__off,
-				   sizeof(*dc_iter)
-				   - offsetof(TfwDecodeCacheIter, __off));
+			bzero_fast(&dc_iter->data, sizeof(dc_iter->data));
 			break;
 		}
 

fw/hpack.c

@@ -1738,7 +1738,8 @@ tfw_hpack_cache_decode_expand(TfwHPack *__restrict hp,
 	TfwHttpTransIter *mit = &resp->mit;
 	TfwMsgIter *it = &mit->iter;
 	bool h2_mode = TFW_MSG_H2(resp->req);
-	const unsigned char *prev, *last = src + n;
+	const unsigned char *last = src + n;
+	unsigned char *prev = src;
 	struct sk_buff **skb_head = &resp->msg.skb_head;
 
 #define GET_NEXT_DATA(cond)						\
@@ -1814,14 +1815,14 @@ do {									\
 		if (hp->index == 0x0F) {
 			GET_FLEXIBLE_lambda(hp->index,
 				HPACK_STATE_INDEX, {
-				FIXUP_H2_DATA(&dc_iter->h2_data, src,
+				FIXUP_H2_DATA(&dc_iter->data.h2, prev,
 					      src - prev);
 			});
 		}
 
 		T_DBG3("%s: name index: %lu\n", __func__, hp->index);
 
-		FIXUP_H2_DATA(&dc_iter->h2_data, src, src - prev);
+		FIXUP_H2_DATA(&dc_iter->data.h2, prev, src - prev);
 
 		NEXT_STATE(hp->index
 			   ? HPACK_STATE_INDEXED_NAME_TEXT
@@ -1846,7 +1847,7 @@ do {									\
 		if (unlikely(hp->length == 0x7F)) {
 			GET_FLEXIBLE_lambda(hp->length,
 				HPACK_STATE_NAME_LENGTH, {
-				FIXUP_H2_DATA(&dc_iter->h2_data, src,
+				FIXUP_H2_DATA(&dc_iter->data.h2, prev,
 					      src - prev);
 			});
 		}
@@ -1857,7 +1858,7 @@ do {									\
 
 		T_DBG3("%s: name length: %lu\n", __func__, hp->length);
 
-		FIXUP_H2_DATA(&dc_iter->h2_data, src, src - prev);
+		FIXUP_H2_DATA(&dc_iter->data.h2, prev, src - prev);
 
 		NEXT_STATE(HPACK_STATE_NAME_TEXT);
 
@@ -1884,8 +1885,8 @@ do {									\
 			goto out;
 		}
 
-		dc_iter->hdr_data.len = entry->name_len;
-		dc_iter->hdr_data.data = __TFW_STR_CH(entry->hdr, 0)->data;
+		dc_iter->data.hdr.len = entry->name_len;
+		dc_iter->data.hdr.data = __TFW_STR_CH(entry->hdr, 0)->data;
 
 		goto check_name_text;
 
@@ -1898,9 +1899,10 @@ do {									\
 get_name_text:
 		m_len = min((unsigned long)(last - src), hp->length);
 
-		T_DBG3("%s: decoding header name, m_len=%lu\n", __func__, m_len);
+		T_DBG3("%s: decoding header name, m_len=%lu\n", __func__,
+		       m_len);
 
-		FIXUP_DATA(&dc_iter->hdr_data, src, m_len);
+		FIXUP_DATA(&dc_iter->data.hdr, src, m_len);
 
 		hp->length -= m_len;
 		src += m_len;
@@ -1909,46 +1911,36 @@ do {									\
 check_name_text:
 		i = 0;
 		h_mods = dc_iter->h_mods;
-		WARN_ON_ONCE(dc_iter->desc);
 		if (h_mods) {
 			for (; i < h_mods->sz; ++i) {
 				TfwHdrModsDesc *d = &h_mods->hdrs[i];
 
-				if (!__hdr_name_cmp(&dc_iter->hdr_data, d->hdr))
+				if (!d->append
+				    && !__hdr_name_cmp(&dc_iter->data.hdr,
+						       d->hdr))
 				{
-					dc_iter->desc = d;
-					break;
+					/*
+					 * All duplicate headers must be
+					 * skipped by caller.
+					 */
+					dc_iter->skip = true;
+					goto out;
 				}
 			}
 		}
 
-		if (dc_iter->desc) {
-			/* All duplicate headers must be skipped by caller. */
-			WARN_ON_ONCE(test_bit(i, mit->found));
-			__set_bit(i, mit->found);
-			/*
-			 * Header modifications format: 0 chunk - header name,
-			 * optional 1st chunk - header value. If the value is
-			 * empty, then the header is about to be removed,
-			 * don't write it.
-			 */
-
-			/* FIXME: this is a temporary WA for GCC12, see #1695 for details */
-			if (TFW_STR_CHUNK(dc_iter->desc->hdr, 1) == NULL) {
-				dc_iter->skip = true;
-				goto out;
-			}
-
-		}
-
-		if (h2_mode)
-			EXPAND_STR_DATA(&dc_iter->h2_data);
-
-		EXPAND_STR_DATA(&dc_iter->hdr_data);
-		TFW_STR_INIT(&dc_iter->hdr_data);
-
-		if (!h2_mode)
+		if (h2_mode) {
+			/* Write HPACK related data. */
+			EXPAND_STR_DATA(&dc_iter->data.h2);
+			/* Write header name. */
+			if (!hp->index)
+				EXPAND_STR_DATA(&dc_iter->data.hdr);
+			TFW_STR_INIT(&dc_iter->data.h2);
+		} else {
+			EXPAND_STR_DATA(&dc_iter->data.hdr);
 			EXPAND_DATA(S_DLM, SLEN(S_DLM));
+		}
+		TFW_STR_INIT(&dc_iter->data.hdr);
 
 		T_DBG3("%s: name copied, n=%lu, tail=%lu, hp->length=%lu\n",
 		       __func__, n, last - src, hp->length);
@@ -1971,14 +1963,13 @@ do {									\
 		if (unlikely(hp->length == 0x7F))
 			GET_FLEXIBLE_lambda(hp->length,
 				HPACK_STATE_VALUE_LENGTH, {
-				if (!dc_iter->desc)
-					EXPAND_H2_DATA(src, src - prev);
+				FIXUP_H2_DATA(&dc_iter->data.h2, prev,
+					      src - prev);
 			});
 
 		T_DBG3("%s: value length: %lu\n", __func__, hp->length);
 
-		if (!dc_iter->desc)
-			EXPAND_H2_DATA(src, src - prev);
+		FIXUP_H2_DATA(&dc_iter->data.h2, prev, src - prev);
 
 		NEXT_STATE(HPACK_STATE_VALUE_TEXT);
 
@@ -2003,71 +1994,16 @@ do {									\
 		T_DBG3("%s: decode header value...\n", __func__);
 		m_len = min((unsigned long)(last - src), hp->length);
 
-		if (dc_iter->desc && dc_iter->desc->append && h2_mode) {
-			/*
-			 * If the header value must be appended, we need to
-			 * collect the value for HTTP/2-header, since it should
-			 * be re-encoded in this case.
-			 */
-			FIXUP_DATA(&dc_iter->hdr_data, src, m_len);
-		}
-		else if (!dc_iter->desc || dc_iter->desc->append) {
-			EXPAND_DATA(src, m_len);
-		}
+		if (h2_mode)
+			EXPAND_STR_DATA(&dc_iter->data.h2);
+
+		EXPAND_DATA(src, m_len);
 
 		hp->length -= m_len;
 		src += m_len;
 
 		GET_NEXT_DATA(hp->length);
 
-		if (dc_iter->desc) {
-			TfwStr *val, *h = dc_iter->desc->hdr;
-			/*
-			 * Header value is stored in chunk 1, see
-			 * tfw_cfgop_mod_hdr_add().
-			 */
-			TfwStr n_val = {
-				.chunks = (TfwStr []){
-					{ .data = ", ", .len = 2 },
-					{ .data = __TFW_STR_CH(h, 1)->data,
-					  .len = __TFW_STR_CH(h, 1)->len }
-				},
-				.len = __TFW_STR_CH(h, 1)->len + 2,
-				.nchunks = 2
-			};
-
-			dc_iter->skip = true;
-
-			if (h2_mode) {
-				TfwHPackInt vlen;
-
-				if (dc_iter->desc->append) {
-					val = &dc_iter->hdr_data;
-					if (tfw_strcat(resp->pool, val, &n_val))
-					{
-						r = T_DROP;
-						goto out;
-					}
-				}
-				else {
-					val = __TFW_STR_CH(&n_val, 1);
-				}
-
-				write_int(val->len, 0x7F, 0, &vlen);
-
-				EXPAND_DATA(vlen.buf, vlen.sz);
-				EXPAND_STR_DATA(val);
-
-				break;
-			}
-
-			val = dc_iter->desc->append
-				? &n_val
-				: __TFW_STR_CH(&n_val, 1);
-
-			EXPAND_STR_DATA(val);
-		}
-
 		if (!h2_mode)
 			EXPAND_DATA(S_CRLF, SLEN(S_CRLF));
 
@@ -2076,7 +2012,7 @@ do {									\
 	case HPACK_STATE_INDEX:
 		prev = src;
 		GET_CONTINUE_lambda(hp->index, {
-			FIXUP_H2_DATA(&dc_iter->h2_data, src, src - prev);
+			FIXUP_H2_DATA(&dc_iter->data.h2, prev, src - prev);
 		});
 		T_DBG3("%s: index finally decoded: %lu\n", __func__, hp->index);
 
@@ -2089,7 +2025,7 @@ do {									\
 	case HPACK_STATE_NAME_LENGTH:
 		prev = src;
 		GET_CONTINUE_lambda(hp->length, {
-			FIXUP_H2_DATA(&dc_iter->h2_data, src, src - prev);
+			FIXUP_H2_DATA(&dc_iter->data.h2, prev, src - prev);
 		});
 		T_DBG3("%s: name length finally decoded: %lu\n", __func__,
 		       hp->length);
@@ -2103,8 +2039,7 @@ do {									\
 	case HPACK_STATE_VALUE_LENGTH:
 		prev = src;
 		GET_CONTINUE_lambda(hp->length, {
-			if (!dc_iter->desc)
-				EXPAND_H2_DATA(src, src - prev);
+			EXPAND_H2_DATA(prev, src - prev);
 		});
 		T_DBG3("%s: value length finally decoded: %lu\n", __func__,
 		       hp->length);

fw/hpack.h

@@ -235,8 +235,6 @@ typedef struct {
  * @skip	- flag to skip particular cached data in order to switch
  *		  between HTTP/2 and HTTP/1.1 resulting representation during
  *		  decoding from HTTP/2-cache;
- * @__off	- offset to reinitialize the iterator non-persistent part;
- * @desc	- pointer to the found header configured to be changed;
  * @acc_len	- accumulated length of the resulting headers part of the
  *		  response;
  * @hdr_data	- header's data currently received from cache;
@@ -245,11 +243,11 @@ typedef struct {
 typedef struct {
 	TfwHdrMods		*h_mods;
 	bool			skip;
-	char			__off[0];
-	TfwHdrModsDesc		*desc;
 	unsigned long		acc_len;
-	TfwStr			hdr_data;
-	TfwStr			h2_data;
+	struct {
+		TfwStr			hdr;
+		TfwStr			h2;
+	} data;
 } TfwDecodeCacheIter;
 
 #define	BUFFER_GET(len, it)					\