MINOR: qpack: improve decoding function

Adjust decoding loop by using temporary ist for header name and value.
The header is inserted at the end of an iteration, which guarantee that
we do not insert name only in the list in case of an error on value
decoding. This also helps the function readability by centralizing the
LIST insert operation.

The return value of the decoding function is also changed. Now on
success the number of headers inserted in the input list is returned.
This change as no impact as success value is not used by the caller.
This is mainly done to have a behavior similar to hpack decoding
function.
This commit is contained in:
Amaury Denoyelle 2022-06-14 17:17:07 +02:00
parent 60ef19f137
commit b666c6b26e

View File

@ -181,12 +181,13 @@ static int qpack_decode_fs_pfx(uint64_t *enc_ric, uint64_t *db, int *sign_bit,
* a storage for some elements pointing into it. An end marker is inserted at * a storage for some elements pointing into it. An end marker is inserted at
* the end of the list with empty strings as name/value. * the end of the list with empty strings as name/value.
* *
* Returns 0 on success. In case of error, a negative code QPACK_ERR_* is * Returns the number of headers inserted into list excluding the end marker.
* returned. * In case of error, a negative code QPACK_ERR_* is returned.
*/ */
int qpack_decode_fs(const unsigned char *raw, size_t len, struct buffer *tmp, int qpack_decode_fs(const unsigned char *raw, size_t len, struct buffer *tmp,
struct http_hdr *list, int list_size) struct http_hdr *list, int list_size)
{ {
struct ist name, value;
uint64_t enc_ric, db; uint64_t enc_ric, db;
int s; int s;
unsigned int efl_type; unsigned int efl_type;
@ -299,8 +300,10 @@ int qpack_decode_fs(const unsigned char *raw, size_t len, struct buffer *tmp,
goto out; goto out;
} }
if (t) if (t) {
list[hdr_idx++] = qpack_sht[index]; name = qpack_sht[index].n;
value = qpack_sht[index].v;
}
qpack_debug_printf(stderr, " t=%d index=%llu", !!t, (unsigned long long)index); qpack_debug_printf(stderr, " t=%d index=%llu", !!t, (unsigned long long)index);
} }
@ -320,7 +323,7 @@ int qpack_decode_fs(const unsigned char *raw, size_t len, struct buffer *tmp,
} }
if (t) if (t)
list[hdr_idx] = qpack_sht[index]; name = qpack_sht[index].n;
qpack_debug_printf(stderr, " n=%d t=%d index=%llu", !!n, !!t, (unsigned long long)index); qpack_debug_printf(stderr, " n=%d t=%d index=%llu", !!n, !!t, (unsigned long long)index);
h = *raw & 0x80; h = *raw & 0x80;
@ -352,10 +355,10 @@ int qpack_decode_fs(const unsigned char *raw, size_t len, struct buffer *tmp,
qpack_debug_printf(stderr, " [name huff %d->%d '%s']", (int)length, (int)nlen, trash); qpack_debug_printf(stderr, " [name huff %d->%d '%s']", (int)length, (int)nlen, trash);
/* makes an ist from tmp storage */ /* makes an ist from tmp storage */
b_add(tmp, nlen); b_add(tmp, nlen);
list[hdr_idx].v = ist2(trash, nlen); value = ist2(trash, nlen);
} }
else { else {
list[hdr_idx].v = ist2(raw, length); value = ist2(raw, length);
} }
if (len < length) { if (len < length) {
@ -366,7 +369,6 @@ int qpack_decode_fs(const unsigned char *raw, size_t len, struct buffer *tmp,
raw += length; raw += length;
len -= length; len -= length;
++hdr_idx;
} }
else if (efl_type & QPACK_LFL_WLN_BIT) { else if (efl_type & QPACK_LFL_WLN_BIT) {
/* Literal field line with literal name */ /* Literal field line with literal name */
@ -412,14 +414,15 @@ int qpack_decode_fs(const unsigned char *raw, size_t len, struct buffer *tmp,
qpack_debug_printf(stderr, " [name huff %d->%d '%s']", (int)name_len, (int)nlen, trash); qpack_debug_printf(stderr, " [name huff %d->%d '%s']", (int)name_len, (int)nlen, trash);
/* makes an ist from tmp storage */ /* makes an ist from tmp storage */
b_add(tmp, nlen); b_add(tmp, nlen);
list[hdr_idx].n = ist2(trash, nlen); name = ist2(trash, nlen);
} }
else { else {
list[hdr_idx].n = ist2(raw, name_len); name = ist2(raw, name_len);
} }
raw += name_len; raw += name_len;
len -= name_len; len -= name_len;
hvalue = *raw & 0x80; hvalue = *raw & 0x80;
value_len = qpack_get_varint(&raw, &len, 7); value_len = qpack_get_varint(&raw, &len, 7);
if (len == (uint64_t)-1) { if (len == (uint64_t)-1) {
@ -456,17 +459,20 @@ int qpack_decode_fs(const unsigned char *raw, size_t len, struct buffer *tmp,
qpack_debug_printf(stderr, " [name huff %d->%d '%s']", (int)value_len, (int)nlen, trash); qpack_debug_printf(stderr, " [name huff %d->%d '%s']", (int)value_len, (int)nlen, trash);
/* makes an ist from tmp storage */ /* makes an ist from tmp storage */
b_add(tmp, nlen); b_add(tmp, nlen);
list[hdr_idx].v = ist2(trash, nlen); value = ist2(trash, nlen);
} }
else { else {
list[hdr_idx].v = ist2(raw, value_len); value = ist2(raw, value_len);
} }
raw += value_len; raw += value_len;
len -= value_len; len -= value_len;
++hdr_idx;
} }
list[hdr_idx].n = name;
list[hdr_idx].v = value;
++hdr_idx;
qpack_debug_printf(stderr, "\n"); qpack_debug_printf(stderr, "\n");
} }
@ -478,6 +484,7 @@ int qpack_decode_fs(const unsigned char *raw, size_t len, struct buffer *tmp,
/* put an end marker */ /* put an end marker */
list[hdr_idx].n = list[hdr_idx].v = IST_NULL; list[hdr_idx].n = list[hdr_idx].v = IST_NULL;
ret = hdr_idx;
out: out:
qpack_debug_printf(stderr, "-- done: ret=%d\n", ret); qpack_debug_printf(stderr, "-- done: ret=%d\n", ret);