HTTP/2 Parser implementation (#309):

Corrections as a result of HPACK decoder/encoder/parser unit-tests debugging.

tempesta_fw/hpack.c

@@ -1183,8 +1183,10 @@ do {								\
 do {								\
 	WARN_ON_ONCE(TFW_STR_EMPTY(it->parsed_hdr));		\
 	it->nm_len = it->parsed_hdr->len;			\
-	it->nm_num = it->parsed_hdr->nchunks;			\
-	if (state & HPACK_FLAGS_HUFFMAN_NAME) {			\
+	it->nm_num = it->parsed_hdr->nchunks			\
+		? it->parsed_hdr->nchunks			\
+		: 1;						\
+	if (state & HPACK_FLAGS_HUFFMAN_VALUE) {		\
 		BUFFER_GET(length, it);				\
 		if (!it->pos) {					\
 			r = T_DROP;				\
@@ -1752,15 +1754,13 @@ tfw_hpack_find_index(TfwHPackDTbl *__restrict tbl, unsigned long index)
 {
 	const TfwHPackEntry *entry = NULL;
 
+	WARN_ON_ONCE(tbl->n > tbl->length);
+
 	if (index <= HPACK_STATIC_ENTRIES) {
 		entry = static_table + index - 1;
-		BUG_ON(!entry->hdr);
-		return entry;
+		WARN_ON_ONCE(entry->name_num != 1);
 	}
-
-	WARN_ON_ONCE(tbl->n > tbl->length);
-	index -= HPACK_STATIC_ENTRIES;
-	if (index <= tbl->n) {
+	else if ((index -= HPACK_STATIC_ENTRIES) <= tbl->n) {
 		unsigned int curr = tbl->curr;
 
 		if (index <= curr) {
@@ -1772,12 +1772,12 @@ tfw_hpack_find_index(TfwHPackDTbl *__restrict tbl, unsigned long index)
 		      __func__, tbl->length, tbl->curr, curr, index);
 
 		entry = tbl->entries + curr;
-		WARN_ON_ONCE(!entry->hdr->nchunks || entry->name_num != 1);
-
-		return entry;
+		WARN_ON_ONCE(!entry->name_num);
 	}
 
-	return NULL;
+	WARN_ON_ONCE(entry && (!entry->hdr || !entry->hdr->nchunks));
+
+	return entry;
 }
 
 static int
@@ -1905,11 +1905,12 @@ tfw_hpack_reinit(TfwHPack *__restrict hp, TfwMsgParseIter *__restrict it)
 static inline int
 tfw_hpack_process_hdr_name(TfwHttpReq *req)
 {
-	int ret = T_BAD;
+	const TfwStr *c, *end;
 	TfwMsgParseIter *it = &req->pit;
-	const TfwStr *c, *end, *next = it->next;
+	const TfwStr *hdr = &it->hdr, *next = it->next;
+	int ret = T_BAD;
 
-	WARN_ON_ONCE(next != &it->hdr);
+	WARN_ON_ONCE(next != hdr);
 	TFW_STR_FOR_EACH_CHUNK(c, next, end) {
 		bool last = c + 1 == end;
 
@@ -1924,13 +1925,28 @@ tfw_hpack_process_hdr_name(TfwHttpReq *req)
 static inline int
 tfw_hpack_process_hdr_value(TfwHttpReq *req)
 {
-	int ret = T_BAD;
+	const TfwStr *chunk, *end;
 	TfwMsgParseIter *it = &req->pit;
-	const TfwStr *hdr = &it->hdr;
-	const TfwStr *chunk = it->next;
-	const TfwStr *end = hdr->chunks + hdr->nchunks;
+	const TfwStr *hdr = &it->hdr, *next = it->next;
+	int ret = T_BAD;
+
+	BUG_ON(TFW_STR_DUP(hdr));
+	if (TFW_STR_PLAIN(hdr)) {
+		WARN_ON_ONCE(hdr != next);
+		chunk = hdr;
+		end = hdr + 1;
+	} else {
+		/*
+		 * In case of compound @hdr the @next can point either to the
+		 * @hdr itself (if only header's value has been Huffman-decoded,
+		 * i.e. in case of indexed or raw header's name), or to some
+		 * chunk inside the @hdr (if both, the name and the value, has
+		 * been Huffman-decoded).
+		 */
+		chunk = (hdr != next) ? next : next->chunks;
+		end = hdr->chunks + hdr->nchunks;
+	}
 
-	WARN_ON_ONCE(chunk == hdr);
 	while (chunk < end) {
 		bool last = chunk + 1 == end;
 
@@ -2056,40 +2072,58 @@ tfw_hpack_hdr_set(TfwHPack *__restrict hp, TfwHttpReq *__restrict req,
 	switch (entry->tag) {
 	case TFW_TAG_HDR_H2_METHOD:
 		parser->_hdr_tag = TFW_HTTP_HDR_H2_METHOD;
+		break;
 	case TFW_TAG_HDR_H2_SCHEME:
 		parser->_hdr_tag = TFW_HTTP_HDR_H2_SCHEME;
+		break;
 	case TFW_TAG_HDR_H2_AUTHORITY:
 		parser->_hdr_tag = TFW_HTTP_HDR_H2_AUTHORITY;
+		break;
 	case TFW_TAG_HDR_H2_PATH:
 		parser->_hdr_tag = TFW_HTTP_HDR_H2_PATH;
+		break;
 	case TFW_TAG_HDR_ACCEPT:
 		parser->_hdr_tag = TFW_HTTP_HDR_RAW;
+		break;
 	case TFW_TAG_HDR_AUTHORIZATION:
 		parser->_hdr_tag = TFW_HTTP_HDR_RAW;
+		break;
 	case TFW_TAG_HDR_CACHE_CONTROL:
 		parser->_hdr_tag = TFW_HTTP_HDR_RAW;
+		break;
 	case TFW_TAG_HDR_CONTENT_LENGTH:
 		parser->_hdr_tag = TFW_HTTP_HDR_CONTENT_LENGTH;
+		break;
 	case TFW_TAG_HDR_CONTENT_TYPE:
 		parser->_hdr_tag = TFW_HTTP_HDR_CONTENT_TYPE;
+		break;
 	case TFW_TAG_HDR_COOKIE:
 		parser->_hdr_tag = TFW_HTTP_HDR_COOKIE;
+		break;
 	case TFW_TAG_HDR_HOST:
 		parser->_hdr_tag = TFW_HTTP_HDR_HOST;
+		break;
 	case TFW_TAG_HDR_IF_MODIFIED_SINCE:
 		parser->_hdr_tag = TFW_HTTP_HDR_RAW;
+		break;
 	case TFW_TAG_HDR_IF_NONE_MATCH:
 		parser->_hdr_tag = TFW_HTTP_HDR_IF_NONE_MATCH;
+		break;
 	case TFW_TAG_HDR_PRAGMA:
 		parser->_hdr_tag = TFW_HTTP_HDR_RAW;
+		break;
 	case TFW_TAG_HDR_REFERER:
 		parser->_hdr_tag = TFW_HTTP_HDR_REFERER;
+		break;
 	case TFW_TAG_HDR_X_FORWARDED_FOR:
 		parser->_hdr_tag = TFW_HTTP_HDR_X_FORWARDED_FOR;
+		break;
 	case TFW_TAG_HDR_USER_AGENT:
 		parser->_hdr_tag = TFW_HTTP_HDR_USER_AGENT;
+		break;
 	case TFW_TAG_HDR_RAW:
 		parser->_hdr_tag = TFW_HTTP_HDR_RAW;
+		break;
 	default:
 		WARN_ON_ONCE(1);
 		return T_DROP;
@@ -3856,8 +3890,8 @@ tfw_hpack_hdr_expand(TfwHttpResp *__restrict resp, TfwStr *__restrict hdr,
 		mit->acc_len += s_name->len;
 
 		T_DBG3("%s: name str, acc_len=%lu, s_name.len=%lu, s_name.data"
-		       "='%.*s'\n", __func__, mit->acc_len, s_name.len,
-		       (int)s_name.len, s_name.data);
+		       "='%.*s'\n", __func__, mit->acc_len, s_name->len,
+		       (int)s_name->len, s_name->data);
 	}
 	/*
 	 * The @hdr must have the HTTP/2-format chunk structure (without the

tempesta_fw/http.c

@@ -3106,8 +3106,8 @@ do {									\
 	WRITE_LIT(dst, S_CRLF);
 
 	T_DBG3("%s: req adjusted, it->hdrs_len=%lu, it->hdrs_cnt=%u, dst=[%p],"
-	       " p=[%p]\n", __func__, it->hdrs_len, it->hdrs_cnt, dst,
-	       (char *)page_address(p));
+	       " pg=[%p]\n", __func__, it->hdrs_len, it->hdrs_cnt, dst,
+	       (char *)page_address(pg));
 	WARN_ON_ONCE(dst - (char *)page_address(pg) != it->hdrs_len);
 
 	r = ss_skb_replace_page(&req->msg.skb_head, pg, it->hdrs_len,
@@ -3434,7 +3434,7 @@ tfw_h2_add_hdr_via(TfwHttpResp *resp)
 	if (unlikely(r))
 		T_ERR("HTTP/2: unable to add 'via' header (resp=[%p])\n", resp);
 	else
-		T_DBG3("%s: added 'via' header, resp=[%p]\n", resp);
+		T_DBG3("%s: added 'via' header, resp=[%p]\n", __func__, resp);
 	return r;
 #undef NM_VIA
 #undef V_PROTO
@@ -3457,7 +3457,7 @@ tfw_h2_set_hdr_date(TfwHttpResp *resp)
 		T_ERR("HTTP/2: unable to add 'date' header to response"
 			" [%p]\n", resp);
 	else
-		T_DBG3("%s: added 'date' header, resp=[%p]\n", resp);
+		T_DBG3("%s: added 'date' header, resp=[%p]\n", __func__, resp);
 
 	return r;
 }

tempesta_fw/http_msg.c

@@ -25,6 +25,7 @@
 #if DBG_HTTP_PARSER == 0
 #undef DEBUG
 #endif
+
 #include "lib/str.h"
 #include "gfsm.h"
 #include "http_msg.h"
@@ -104,8 +105,9 @@ static inline unsigned int
 __tfw_http_msg_spec_hid(const TfwStr *hdr, const TfwHdrDef array[])
 {
 	const TfwHdrDef *def;
+	size_t size = TFW_HTTP_HDR_RAW - TFW_HTTP_HDR_REGULAR;
 	/* TODO: return error if @hdr can't be applied to response or client. */
-	def = (TfwHdrDef *)__tfw_http_msg_find_hdr(hdr, array, TFW_HTTP_HDR_RAW,
+	def = (TfwHdrDef *)__tfw_http_msg_find_hdr(hdr, array, size,
 						   sizeof(TfwHdrDef));
 
 	return def ? def->id : TFW_HTTP_HDR_RAW;
@@ -511,7 +513,6 @@ tfw_http_msg_hdr_close(TfwHttpMsg *hm)
 		if (TFW_STR_EMPTY(h))
 			/* Just store the special header in empty slot. */
 			goto done;
-
 		/*
 		 * Process duplicate header.
 		 *
@@ -1484,8 +1485,9 @@ tfw_http_msg_expand_data(TfwMsgIter *it, struct sk_buff **skb_head,
 				++it->frag;
 				skb_fill_page_desc(it->skb, it->frag, page,
 						   0, 0);
-				T_DBG3("message expanded by new frag %d,"
-				       " page=[%p], skb=[%p]\n", i,
+				T_DBG3("message expanded by new frag %u,"
+				       " page=[%p], skb=[%p]\n",
+				       skb_shinfo(it->skb)->nr_frags,
 				       page_address(page), it->skb);
 			}