Skip to content

Commit 3af6091

Browse files
committed
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. Removed `desc` form `TfwDecodeCacheIter`. It's not used.
1 parent 7b509b2 commit 3af6091

File tree

3 files changed

+41
-110
lines changed

3 files changed

+41
-110
lines changed

fw/cache.c

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -683,12 +683,8 @@ tfw_cache_h2_decode_write(TDB *db, TdbVRec **trec, TfwHttpResp *resp,
683683

684684
acc += m_len;
685685
*data += m_len;
686-
if (acc == len) {
687-
bzero_fast(dc_iter->__off,
688-
sizeof(*dc_iter)
689-
- offsetof(TfwDecodeCacheIter, __off));
690-
break;
691-
}
686+
if (acc == len)
687+
return r;
692688

693689
tr = *trec = tdb_next_rec_chunk(db, tr);
694690
BUG_ON(!tr);
@@ -796,6 +792,8 @@ tfw_cache_build_resp_hdr(TDB *db, TfwHttpResp *resp, TfwHdrMods *hmods,
796792
BUG_ON(s->flags);
797793
*p += TFW_CSTR_HDRLEN;
798794
r = write_actor(db, trec, resp, p, s->len, &dc_iter);
795+
bzero_fast(dc_iter.__off, sizeof(dc_iter)
796+
- offsetof(TfwDecodeCacheIter, __off));
799797
if (unlikely(r))
800798
break;
801799
}

fw/hpack.c

Lines changed: 37 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -1738,7 +1738,8 @@ tfw_hpack_cache_decode_expand(TfwHPack *__restrict hp,
17381738
TfwHttpTransIter *mit = &resp->mit;
17391739
TfwMsgIter *it = &mit->iter;
17401740
bool h2_mode = TFW_MSG_H2(resp->req);
1741-
const unsigned char *prev, *last = src + n;
1741+
const unsigned char *last = src + n;
1742+
unsigned char *prev = src;
17421743
struct sk_buff **skb_head = &resp->msg.skb_head;
17431744

17441745
#define GET_NEXT_DATA(cond) \
@@ -1814,14 +1815,14 @@ do { \
18141815
if (hp->index == 0x0F) {
18151816
GET_FLEXIBLE_lambda(hp->index,
18161817
HPACK_STATE_INDEX, {
1817-
FIXUP_H2_DATA(&dc_iter->h2_data, src,
1818+
FIXUP_H2_DATA(&dc_iter->h2_data, prev,
18181819
src - prev);
18191820
});
18201821
}
18211822

18221823
T_DBG3("%s: name index: %lu\n", __func__, hp->index);
18231824

1824-
FIXUP_H2_DATA(&dc_iter->h2_data, src, src - prev);
1825+
FIXUP_H2_DATA(&dc_iter->h2_data, prev, src - prev);
18251826

18261827
NEXT_STATE(hp->index
18271828
? HPACK_STATE_INDEXED_NAME_TEXT
@@ -1846,7 +1847,7 @@ do { \
18461847
if (unlikely(hp->length == 0x7F)) {
18471848
GET_FLEXIBLE_lambda(hp->length,
18481849
HPACK_STATE_NAME_LENGTH, {
1849-
FIXUP_H2_DATA(&dc_iter->h2_data, src,
1850+
FIXUP_H2_DATA(&dc_iter->h2_data, prev,
18501851
src - prev);
18511852
});
18521853
}
@@ -1857,7 +1858,7 @@ do { \
18571858

18581859
T_DBG3("%s: name length: %lu\n", __func__, hp->length);
18591860

1860-
FIXUP_H2_DATA(&dc_iter->h2_data, src, src - prev);
1861+
FIXUP_H2_DATA(&dc_iter->h2_data, prev, src - prev);
18611862

18621863
NEXT_STATE(HPACK_STATE_NAME_TEXT);
18631864

@@ -1898,7 +1899,8 @@ do { \
18981899
get_name_text:
18991900
m_len = min((unsigned long)(last - src), hp->length);
19001901

1901-
T_DBG3("%s: decoding header name, m_len=%lu\n", __func__, m_len);
1902+
T_DBG3("%s: decoding header name, m_len=%lu\n", __func__,
1903+
m_len);
19021904

19031905
FIXUP_DATA(&dc_iter->hdr_data, src, m_len);
19041906

@@ -1909,46 +1911,36 @@ do { \
19091911
check_name_text:
19101912
i = 0;
19111913
h_mods = dc_iter->h_mods;
1912-
WARN_ON_ONCE(dc_iter->desc);
19131914
if (h_mods) {
19141915
for (; i < h_mods->sz; ++i) {
19151916
TfwHdrModsDesc *d = &h_mods->hdrs[i];
19161917

1917-
if (!__hdr_name_cmp(&dc_iter->hdr_data, d->hdr))
1918+
if (!d->append
1919+
&& !__hdr_name_cmp(&dc_iter->hdr_data,
1920+
d->hdr))
19181921
{
1919-
dc_iter->desc = d;
1920-
break;
1922+
/*
1923+
* All duplicate headers must be
1924+
* skipped by caller.
1925+
*/
1926+
dc_iter->skip = true;
1927+
goto out;
19211928
}
19221929
}
19231930
}
19241931

1925-
if (dc_iter->desc) {
1926-
/* All duplicate headers must be skipped by caller. */
1927-
WARN_ON_ONCE(test_bit(i, mit->found));
1928-
__set_bit(i, mit->found);
1929-
/*
1930-
* Header modifications format: 0 chunk - header name,
1931-
* optional 1st chunk - header value. If the value is
1932-
* empty, then the header is about to be removed,
1933-
* don't write it.
1934-
*/
1935-
1936-
/* FIXME: this is a temporary WA for GCC12, see #1695 for details */
1937-
if (TFW_STR_CHUNK(dc_iter->desc->hdr, 1) == NULL) {
1938-
dc_iter->skip = true;
1939-
goto out;
1940-
}
1941-
1942-
}
1943-
1944-
if (h2_mode)
1932+
if (h2_mode) {
1933+
/* Write HPACK related data. */
19451934
EXPAND_STR_DATA(&dc_iter->h2_data);
1946-
1947-
EXPAND_STR_DATA(&dc_iter->hdr_data);
1948-
TFW_STR_INIT(&dc_iter->hdr_data);
1949-
1950-
if (!h2_mode)
1935+
/* Write header name. */
1936+
if (!hp->index)
1937+
EXPAND_STR_DATA(&dc_iter->hdr_data);
1938+
TFW_STR_INIT(&dc_iter->h2_data);
1939+
} else {
1940+
EXPAND_STR_DATA(&dc_iter->hdr_data);
19511941
EXPAND_DATA(S_DLM, SLEN(S_DLM));
1942+
}
1943+
TFW_STR_INIT(&dc_iter->hdr_data);
19521944

19531945
T_DBG3("%s: name copied, n=%lu, tail=%lu, hp->length=%lu\n",
19541946
__func__, n, last - src, hp->length);
@@ -1971,14 +1963,13 @@ do { \
19711963
if (unlikely(hp->length == 0x7F))
19721964
GET_FLEXIBLE_lambda(hp->length,
19731965
HPACK_STATE_VALUE_LENGTH, {
1974-
if (!dc_iter->desc)
1975-
EXPAND_H2_DATA(src, src - prev);
1966+
FIXUP_H2_DATA(&dc_iter->h2_data, prev,
1967+
src - prev);
19761968
});
19771969

19781970
T_DBG3("%s: value length: %lu\n", __func__, hp->length);
19791971

1980-
if (!dc_iter->desc)
1981-
EXPAND_H2_DATA(src, src - prev);
1972+
FIXUP_H2_DATA(&dc_iter->h2_data, prev, src - prev);
19821973

19831974
NEXT_STATE(HPACK_STATE_VALUE_TEXT);
19841975

@@ -2003,71 +1994,16 @@ do { \
20031994
T_DBG3("%s: decode header value...\n", __func__);
20041995
m_len = min((unsigned long)(last - src), hp->length);
20051996

2006-
if (dc_iter->desc && dc_iter->desc->append && h2_mode) {
2007-
/*
2008-
* If the header value must be appended, we need to
2009-
* collect the value for HTTP/2-header, since it should
2010-
* be re-encoded in this case.
2011-
*/
2012-
FIXUP_DATA(&dc_iter->hdr_data, src, m_len);
2013-
}
2014-
else if (!dc_iter->desc || dc_iter->desc->append) {
2015-
EXPAND_DATA(src, m_len);
2016-
}
1997+
if (h2_mode)
1998+
EXPAND_STR_DATA(&dc_iter->h2_data);
1999+
2000+
EXPAND_DATA(src, m_len);
20172001

20182002
hp->length -= m_len;
20192003
src += m_len;
20202004

20212005
GET_NEXT_DATA(hp->length);
20222006

2023-
if (dc_iter->desc) {
2024-
TfwStr *val, *h = dc_iter->desc->hdr;
2025-
/*
2026-
* Header value is stored in chunk 1, see
2027-
* tfw_cfgop_mod_hdr_add().
2028-
*/
2029-
TfwStr n_val = {
2030-
.chunks = (TfwStr []){
2031-
{ .data = ", ", .len = 2 },
2032-
{ .data = __TFW_STR_CH(h, 1)->data,
2033-
.len = __TFW_STR_CH(h, 1)->len }
2034-
},
2035-
.len = __TFW_STR_CH(h, 1)->len + 2,
2036-
.nchunks = 2
2037-
};
2038-
2039-
dc_iter->skip = true;
2040-
2041-
if (h2_mode) {
2042-
TfwHPackInt vlen;
2043-
2044-
if (dc_iter->desc->append) {
2045-
val = &dc_iter->hdr_data;
2046-
if (tfw_strcat(resp->pool, val, &n_val))
2047-
{
2048-
r = T_DROP;
2049-
goto out;
2050-
}
2051-
}
2052-
else {
2053-
val = __TFW_STR_CH(&n_val, 1);
2054-
}
2055-
2056-
write_int(val->len, 0x7F, 0, &vlen);
2057-
2058-
EXPAND_DATA(vlen.buf, vlen.sz);
2059-
EXPAND_STR_DATA(val);
2060-
2061-
break;
2062-
}
2063-
2064-
val = dc_iter->desc->append
2065-
? &n_val
2066-
: __TFW_STR_CH(&n_val, 1);
2067-
2068-
EXPAND_STR_DATA(val);
2069-
}
2070-
20712007
if (!h2_mode)
20722008
EXPAND_DATA(S_CRLF, SLEN(S_CRLF));
20732009

@@ -2076,7 +2012,7 @@ do { \
20762012
case HPACK_STATE_INDEX:
20772013
prev = src;
20782014
GET_CONTINUE_lambda(hp->index, {
2079-
FIXUP_H2_DATA(&dc_iter->h2_data, src, src - prev);
2015+
FIXUP_H2_DATA(&dc_iter->h2_data, prev, src - prev);
20802016
});
20812017
T_DBG3("%s: index finally decoded: %lu\n", __func__, hp->index);
20822018

@@ -2089,7 +2025,7 @@ do { \
20892025
case HPACK_STATE_NAME_LENGTH:
20902026
prev = src;
20912027
GET_CONTINUE_lambda(hp->length, {
2092-
FIXUP_H2_DATA(&dc_iter->h2_data, src, src - prev);
2028+
FIXUP_H2_DATA(&dc_iter->h2_data, prev, src - prev);
20932029
});
20942030
T_DBG3("%s: name length finally decoded: %lu\n", __func__,
20952031
hp->length);
@@ -2103,8 +2039,7 @@ do { \
21032039
case HPACK_STATE_VALUE_LENGTH:
21042040
prev = src;
21052041
GET_CONTINUE_lambda(hp->length, {
2106-
if (!dc_iter->desc)
2107-
EXPAND_H2_DATA(src, src - prev);
2042+
EXPAND_H2_DATA(prev, src - prev);
21082043
});
21092044
T_DBG3("%s: value length finally decoded: %lu\n", __func__,
21102045
hp->length);

fw/hpack.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,6 @@ typedef struct {
236236
* between HTTP/2 and HTTP/1.1 resulting representation during
237237
* decoding from HTTP/2-cache;
238238
* @__off - offset to reinitialize the iterator non-persistent part;
239-
* @desc - pointer to the found header configured to be changed;
240239
* @acc_len - accumulated length of the resulting headers part of the
241240
* response;
242241
* @hdr_data - header's data currently received from cache;
@@ -246,7 +245,6 @@ typedef struct {
246245
TfwHdrMods *h_mods;
247246
bool skip;
248247
char __off[0];
249-
TfwHdrModsDesc *desc;
250248
unsigned long acc_len;
251249
TfwStr hdr_data;
252250
TfwStr h2_data;

0 commit comments

Comments
 (0)