Fixed framing error for resposnses received from cache and

with header modification

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

tfw_cache_h2_decode_write: after decoding header `TfwDecodeCacheIter`
was cleared and because of this caller of `tfw_cache_h2_decode_write`
couldn't receive a total length of header.

Author: const-t

fw/cache.c

@@ -683,12 +683,8 @@ 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));
-			break;
-		}
+		if (acc == len)
+			return r;
 
 		tr = *trec = tdb_next_rec_chunk(db, tr);
 		BUG_ON(!tr);
@@ -796,6 +792,8 @@ tfw_cache_build_resp_hdr(TDB *db, TfwHttpResp *resp, TfwHdrMods *hmods,
 		BUG_ON(s->flags);
 		*p += TFW_CSTR_HDRLEN;
 		r = write_actor(db, trec, resp, p, s->len, &dc_iter);
+		bzero_fast(dc_iter.__off, sizeof(dc_iter)
+			   - offsetof(TfwDecodeCacheIter, __off));
 		if (unlikely(r))
 			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->h2_data, 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->h2_data, 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->h2_data, 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->h2_data, prev, src - prev);
 
 		NEXT_STATE(HPACK_STATE_NAME_TEXT);
 
@@ -1898,7 +1899,8 @@ 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);
 
@@ -1914,41 +1916,32 @@ do {									\
 			for (; i < h_mods->sz; ++i) {
 				TfwHdrModsDesc *d = &h_mods->hdrs[i];
 
-				if (!__hdr_name_cmp(&dc_iter->hdr_data, d->hdr))
+				if (!desc->append
+				    && __hdr_name_cmp(&dc_iter->hdr_data,
+						      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)
+		if (h2_mode) {
+			/* Write HPACK related data. */
 			EXPAND_STR_DATA(&dc_iter->h2_data);
-
-		EXPAND_STR_DATA(&dc_iter->hdr_data);
-		TFW_STR_INIT(&dc_iter->hdr_data);
-
-		if (!h2_mode)
+			/* Write header name. */
+			if (!hp->index)
+				EXPAND_STR_DATA(&dc_iter->hdr_data);
+			TFW_STR_INIT(&dc_iter->h2_data);
+		} else {
+			EXPAND_STR_DATA(&dc_iter->hdr_data);
 			EXPAND_DATA(S_DLM, SLEN(S_DLM));
+		}
+		TFW_STR_INIT(&dc_iter->hdr_data);
 
 		T_DBG3("%s: name copied, n=%lu, tail=%lu, hp->length=%lu\n",
 		       __func__, n, last - src, hp->length);
@@ -1971,14 +1964,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->h2_data, 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->h2_data, prev, src - prev);
 
 		NEXT_STATE(HPACK_STATE_VALUE_TEXT);
 
@@ -2003,71 +1995,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->h2_data);
+
+		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 +2013,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->h2_data, prev, src - prev);
 		});
 		T_DBG3("%s: index finally decoded: %lu\n", __func__, hp->index);
 
@@ -2089,7 +2026,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->h2_data, prev, src - prev);
 		});
 		T_DBG3("%s: name length finally decoded: %lu\n", __func__,
 		       hp->length);
@@ -2103,8 +2040,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);