diff --git a/CHANGELOG b/CHANGELOG index 125c4c8..bb37739 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,7 @@ +2018-04-27 + + - HPACK: do not allow header block to end with table size update. + 2018-04-25 - [BUGFIX] Do not create gap in sent packnos when squeezing delayed diff --git a/src/liblsquic/lsquic_frame_reader.c b/src/liblsquic/lsquic_frame_reader.c index 12c2c6b..f2669dc 100644 --- a/src/liblsquic/lsquic_frame_reader.c +++ b/src/liblsquic/lsquic_frame_reader.c @@ -938,19 +938,15 @@ decode_and_pass_payload (struct lsquic_frame_reader *fr) s = lsquic_hdec_decode(fr->fr_hdec, &comp, end, hwc.buf, hwc.buf + 16 * 1024, &hwc.name_len, &hwc.val_len); - if (s > 0) + if (s == 0) { err = add_header_to_uh(fr, &hwc); - if (0 != err) - goto stream_error; - } - else if (s < 0) - { - err = FR_ERR_DECOMPRESS; - goto stream_error; + if (err == 0) + continue; } else - break; + err = FR_ERR_DECOMPRESS; + goto stream_error; } assert(comp == end); diff --git a/src/liblsquic/lsquic_hpack_dec.c b/src/liblsquic/lsquic_hpack_dec.c index 36b4f7f..5163c91 100644 --- a/src/liblsquic/lsquic_hpack_dec.c +++ b/src/liblsquic/lsquic_hpack_dec.c @@ -290,7 +290,7 @@ lsquic_hdec_decode (struct lsquic_hdec *dec, int indexed_type, len; if ((*src) == src_end) - return 0; + return -1; while ((*(*src) & 0xe0) == 0x20) //001 xxxxx { @@ -300,7 +300,7 @@ lsquic_hdec_decode (struct lsquic_hdec *dec, return -1; hdec_update_max_capacity(dec, new_capacity); if (*src == src_end) - return 0; + return -1; } /* lsquic_hdec_dec_int() sets `index' and advances `src'. If we do not call @@ -373,7 +373,7 @@ lsquic_hdec_decode (struct lsquic_hdec *dec, return -1; *val_len = lsquic_hpack_stx_tab[index - 1].val_len; memcpy(name + *name_len, lsquic_hpack_stx_tab[index - 1].val, *val_len); - return 1; + return 0; } } else @@ -392,7 +392,7 @@ lsquic_hdec_decode (struct lsquic_hdec *dec, return -1; *val_len = entry->dte_val_len; memcpy(name + *name_len, DTE_VALUE(entry), *val_len); - return 1; + return 0; } } } @@ -421,7 +421,7 @@ lsquic_hdec_decode (struct lsquic_hdec *dec, return -1; //error } - return 1; + return 0; } diff --git a/src/liblsquic/lsquic_hpack_dec.h b/src/liblsquic/lsquic_hpack_dec.h index 8f31f5b..b57ef18 100644 --- a/src/liblsquic/lsquic_hpack_dec.h +++ b/src/liblsquic/lsquic_hpack_dec.h @@ -22,16 +22,11 @@ lsquic_hdec_init (struct lsquic_hdec *); void lsquic_hdec_cleanup (struct lsquic_hdec *); -/** @lsquic_hdecode - * @brief HPACK decode one name/value item - * @param[in,out] dec - A pointer to a valid HPACK API struct - * @param[in,out] src - Address of pointer to source buffer - * @param[in] src_end - A pointer to end of source buffer - * @param[out] dst - A pointer to destination buffer - * @param[out] dst_end - A pointer to end of destination buffer - * @param[out] name_len - The item name's length - * @param[out] value_len - The item value's length - * @return 1: OK, 0: end, -1: FAIL +/* + * Returns 0 on success, a negative value on failure. + * + * If 0 is returned, `src' is advanced. Calling with a zero-length input + * buffer results in an error. */ int lsquic_hdec_decode (struct lsquic_hdec *dec, diff --git a/test/unittests/test_hpack.c b/test/unittests/test_hpack.c index f82aad3..1004e65 100644 --- a/test/unittests/test_hpack.c +++ b/test/unittests/test_hpack.c @@ -344,9 +344,11 @@ test_hpack_test_RFC_Example (void) char out[2048]; uint16_t name_len = 1024; uint16_t val_len = 1024; - while ((rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), &name_len, - &val_len)) > 0) + while (pSrc < bufEnd) { + rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), + &name_len, &val_len); + assert(rc == 0); char *name = out; char *val = name + name_len; printf("%.*s: %.*s\n", name_len, name, val_len, val); @@ -358,9 +360,11 @@ test_hpack_test_RFC_Example (void) "\x82\x86\x84\xbe\x58\x86\xa8\xeb\x10\x64\x9c\xbf"; pSrc = bufSamp; bufEnd = bufSamp + strlen((const char *)bufSamp); - while ((rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), &name_len, - &val_len)) > 0) + while (pSrc < bufEnd) { + rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), + &name_len, &val_len); + assert(rc == 0); char *name = out; char *val = name + name_len; printf("%.*s: %.*s\n", name_len, name, val_len, val); @@ -370,9 +374,11 @@ test_hpack_test_RFC_Example (void) "\x82\x87\x85\xbf\x40\x88\x25\xa8\x49\xe9\x5b\xa9\x7d\x7f\x89\x25\xa8\x49\xe9\x5b\xb8\xe8\xb4\xbf"; pSrc = bufSamp; bufEnd = bufSamp + strlen((const char *)bufSamp); - while ((rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), &name_len, - &val_len)) > 0) + while (pSrc < bufEnd) { + rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), + &name_len, &val_len); + assert(rc == 0); char *name = out; char *val = name + name_len; printf("%.*s: %.*s\n", name_len, name, val_len, val); @@ -420,7 +426,7 @@ test_decode_limits (void) src = comp; s = lsquic_hdec_decode(&hdec, &src, end, out, out + enough[n], &name_len, &val_len); - assert(1 == s); + assert(0 == s); assert(src == end); assert(16 == name_len); assert(17 == val_len); @@ -481,7 +487,7 @@ test_hpack_self_enc_dec_test (void) pSrc = respBuf; bufEnd = pBuf; while ((rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), &name_len, - &val_len)) > 0) + &val_len)) == 0) { char *name = out; char *val = name + name_len; @@ -515,7 +521,7 @@ test_hpack_self_enc_dec_test (void) pSrc = respBuf; bufEnd = pBuf; while ((rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), &name_len, - &val_len)) > 0) + &val_len)) == 0) { char *name = out; char *val = name + name_len; @@ -561,7 +567,7 @@ test_hpack_self_enc_dec_test (void) pSrc = respBuf; bufEnd = pBuf; while ((rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), &name_len, - &val_len)) > 0) + &val_len)) == 0) { char *name = out; char *val = name + name_len; @@ -602,7 +608,7 @@ test_hpack_self_enc_dec_test (void) pSrc = respBuf; bufEnd = pBuf; while ((rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), &name_len, - &val_len)) > 0) + &val_len)) == 0) { char *name = out; char *val = name + name_len; @@ -640,7 +646,7 @@ test_hpack_encode_and_decode (void) uint16_t name_len, val_len; int rc = lsquic_hdec_decode(&hdec, &dec, enc, out, out + sizeof(out), &name_len, &val_len); - assert(rc > 0); + assert(rc == 0); assert(name_len == 10); assert(val_len == 15); assert(dec == enc); @@ -715,7 +721,7 @@ test_hpack_self_enc_dec_test_firefox_error (void) while (pSrc < bufEnd) { rc = lsquic_hdec_decode(&hdec, &pSrc, bufEnd, out, out + sizeof(out), &name_len, &val_len); - assert(rc > 0); + assert(rc == 0); char *name = out; char *val = name + name_len; @@ -748,7 +754,7 @@ test_hdec_table_size_updates (void) * error. */ { - unsigned const char buf[] = { 0x20 | 0x1E }; + unsigned const char buf[] = { 0x20 | 0x1E, 0x88 }; src = buf; lsquic_hdec_init(&hdec); lsquic_hdec_set_max_capacity(&hdec, 0x11); @@ -762,7 +768,7 @@ test_hdec_table_size_updates (void) /* Test 2: inline update of capacity smaller than max succeeds. */ { - unsigned const char buf[] = { 0x20 | 0x1E }; + unsigned const char buf[] = { 0x20 | 0x1E, 0x88 }; src = buf; lsquic_hdec_init(&hdec); s = lsquic_hdec_decode(&hdec, &src, src + sizeof(buf), outbuf, @@ -784,6 +790,7 @@ test_hdec_table_size_updates (void) unsigned const char buf[] = { 0x20 | 0x00, 0x20 | 0x14, + 0x88, }; src = buf; lsquic_hdec_init(&hdec); @@ -795,6 +802,18 @@ test_hdec_table_size_updates (void) lsquic_hdec_cleanup(&hdec); } + /* Test 4: header block with table update at the end fails. + */ + { + unsigned const char buf[] = { 0x20 | 0x1E, }; + src = buf; + lsquic_hdec_init(&hdec); + s = lsquic_hdec_decode(&hdec, &src, src + sizeof(buf), outbuf, + outbuf + sizeof(outbuf), &name_len, &val_len); + assert(s < 0); + lsquic_hdec_cleanup(&hdec); + } + } @@ -886,7 +905,7 @@ test_henc_nonascii (void) src = comp; s = lsquic_hdec_decode(&hdec, &src, end, uncomp, uncomp + sizeof(uncomp), &name_len, &val_len); - assert(s == 1); + assert(s == 0); assert(sizeof(value) == val_len); assert(0 == memcmp(value, uncomp + name_len, val_len)); lsquic_hdec_cleanup(&hdec); @@ -927,7 +946,7 @@ test_henc_long_compressable (void) src = comp; s = lsquic_hdec_decode(&hdec, &src, end, uncomp, uncomp + sizeof(uncomp), &name_len, &val_len); - assert(s == 1); + assert(s == 0); assert(sizeof(value) == val_len); assert(0 == memcmp(value, uncomp + name_len, val_len)); lsquic_hdec_cleanup(&hdec); @@ -975,7 +994,7 @@ test_henc_long_uncompressable (void) src = comp; s = lsquic_hdec_decode(&hdec, &src, end, uncomp, uncomp + sizeof(uncomp), &name_len, &val_len); - assert(s == 1); + assert(s == 0); assert(sizeof(value) == val_len); assert(0 == memcmp(value, uncomp + name_len, val_len)); lsquic_hdec_cleanup(&hdec); @@ -1210,7 +1229,7 @@ main (int argc, char **argv) for (i = 0; comp < end; ++i) { s = lsquic_hdec_decode(&hdec, &comp, end, out, out + sizeof(out), &name_len, &val_len); - assert(s > 0); + assert(s == 0); assert(name_len == header_arr[i].name.iov_len); assert(0 == memcmp(header_arr[i].name.iov_base, out, name_len)); assert(val_len == header_arr[i].value.iov_len);