diff --git a/lib/nghttp2_hd_huffman.c b/lib/nghttp2_hd_huffman.c index 1f5388c7..a129796f 100644 --- a/lib/nghttp2_hd_huffman.c +++ b/lib/nghttp2_hd_huffman.c @@ -43,57 +43,73 @@ extern const nghttp2_huff_decode huff_decode_table[][16]; static ssize_t huff_encode_sym(nghttp2_bufs *bufs, size_t *avail_ptr, size_t rembits, const nghttp2_huff_sym *sym) { int rv; - size_t nbits = sym->nbits; + uint32_t nbits = sym->nbits; + uint32_t code = sym->code; - for (;;) { - if (rembits > nbits) { - if (*avail_ptr) { - nghttp2_bufs_fast_orb_hold(bufs, sym->code << (rembits - nbits)); - } else { - rv = nghttp2_bufs_orb_hold(bufs, sym->code << (rembits - nbits)); - if (rv != 0) { - return rv; - } - - *avail_ptr = nghttp2_bufs_cur_avail(bufs); - } - - rembits -= nbits; - - break; - } - - if (*avail_ptr) { - nghttp2_bufs_fast_orb(bufs, sym->code >> (nbits - rembits)); - --*avail_ptr; - } else { - rv = nghttp2_bufs_orb(bufs, sym->code >> (nbits - rembits)); - if (rv != 0) { - return rv; - } - - *avail_ptr = nghttp2_bufs_cur_avail(bufs); - } - - nbits -= rembits; - rembits = 8; - - if (nbits == 0) { - break; - } - - if (*avail_ptr) { - nghttp2_bufs_fast_addb_hold(bufs, 0); - } else { - rv = nghttp2_bufs_addb_hold(bufs, 0); - if (rv != 0) { - return rv; - } - - *avail_ptr = nghttp2_bufs_cur_avail(bufs); - } + /* We assume that sym->nbits <= 32 */ + if (rembits > nbits) { + nghttp2_bufs_fast_orb_hold(bufs, code << (rembits - nbits)); + return (ssize_t)(rembits - nbits); } - return (ssize_t)rembits; + + if (rembits == nbits) { + nghttp2_bufs_fast_orb(bufs, code); + --*avail_ptr; + return 8; + } + + nghttp2_bufs_fast_orb(bufs, code >> (nbits - rembits)); + --*avail_ptr; + + nbits -= rembits; + if (nbits & 0x7) { + /* align code to MSB byte boundary */ + code <<= 8 - (nbits & 0x7); + } + + /* we lose at most 3 bytes, but it is not critical in practice */ + if (*avail_ptr < (nbits + 7) / 8) { + rv = nghttp2_bufs_advance(bufs); + if (rv != 0) { + return rv; + } + *avail_ptr = nghttp2_bufs_cur_avail(bufs); + /* we assume that we at least 3 buffer space available */ + assert(*avail_ptr >= 3); + } + + /* fast path, since most code is less than 8 */ + if (nbits < 8) { + nghttp2_bufs_fast_addb_hold(bufs, code); + *avail_ptr = nghttp2_bufs_cur_avail(bufs); + return (ssize_t)(8 - nbits); + } + + /* handle longer code path */ + if (nbits > 24) { + nghttp2_bufs_fast_addb(bufs, code >> 24); + nbits -= 8; + } + + if (nbits > 16) { + nghttp2_bufs_fast_addb(bufs, code >> 16); + nbits -= 8; + } + + if (nbits > 8) { + nghttp2_bufs_fast_addb(bufs, code >> 8); + nbits -= 8; + } + + if (nbits == 8) { + nghttp2_bufs_fast_addb(bufs, code); + *avail_ptr = nghttp2_bufs_cur_avail(bufs); + return 8; + } + + nghttp2_bufs_fast_addb_hold(bufs, code); + *avail_ptr = nghttp2_bufs_cur_avail(bufs); + return (ssize_t)(8 - nbits); } size_t nghttp2_hd_huff_encode_count(const uint8_t *src, size_t len) { @@ -136,17 +152,12 @@ int nghttp2_hd_huff_encode(nghttp2_bufs *bufs, const uint8_t *src, } /* 256 is special terminal symbol, pad with its prefix */ if (rembits < 8) { + /* if rembits < 8, we should have at least 1 buffer space + available */ const nghttp2_huff_sym *sym = &huff_sym_table[256]; - + assert(avail); /* Caution we no longer adjust avail here */ - if (avail) { - nghttp2_bufs_fast_orb(bufs, sym->code >> (sym->nbits - rembits)); - } else { - rv = nghttp2_bufs_orb(bufs, sym->code >> (sym->nbits - rembits)); - if (rv != 0) { - return rv; - } - } + nghttp2_bufs_fast_orb(bufs, sym->code >> (sym->nbits - rembits)); } return 0; diff --git a/tests/main.c b/tests/main.c index a5e8ebde..4093abc5 100644 --- a/tests/main.c +++ b/tests/main.c @@ -303,6 +303,7 @@ int main(int argc _U_, char *argv[] _U_) { !CU_add_test(pSuite, "hd_deflate_bound", test_nghttp2_hd_deflate_bound) || !CU_add_test(pSuite, "hd_public_api", test_nghttp2_hd_public_api) || !CU_add_test(pSuite, "hd_decode_length", test_nghttp2_hd_decode_length) || + !CU_add_test(pSuite, "hd_huff_encode", test_nghttp2_hd_huff_encode) || !CU_add_test(pSuite, "adjust_local_window_size", test_nghttp2_adjust_local_window_size) || !CU_add_test(pSuite, "check_header_name", diff --git a/tests/nghttp2_hd_test.c b/tests/nghttp2_hd_test.c index 1374054e..56300029 100644 --- a/tests/nghttp2_hd_test.c +++ b/tests/nghttp2_hd_test.c @@ -1211,3 +1211,32 @@ void test_nghttp2_hd_decode_length(void) { CU_ASSERT(-1 == rv); } + +void test_nghttp2_hd_huff_encode(void) { + int rv; + ssize_t len; + nghttp2_bufs bufs, outbufs; + nghttp2_hd_huff_decode_context ctx; + const uint8_t t1[] = {22, 21, 20, 19, 18, 17, 16, 15, 14, 13, 12, 11, + 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0}; + + frame_pack_bufs_init(&bufs); + frame_pack_bufs_init(&outbufs); + + rv = nghttp2_hd_huff_encode(&bufs, t1, sizeof(t1)); + + CU_ASSERT(rv == 0); + + nghttp2_hd_huff_decode_context_init(&ctx); + + len = nghttp2_hd_huff_decode(&ctx, &outbufs, bufs.cur->buf.pos, + nghttp2_bufs_len(&bufs), 1); + + CU_ASSERT(nghttp2_bufs_len(&bufs) == len); + CU_ASSERT((ssize_t)sizeof(t1) == nghttp2_bufs_len(&outbufs)); + + CU_ASSERT(0 == memcmp(t1, outbufs.cur->buf.pos, sizeof(t1))); + + nghttp2_bufs_free(&bufs); + nghttp2_bufs_free(&outbufs); +} diff --git a/tests/nghttp2_hd_test.h b/tests/nghttp2_hd_test.h index b726c95d..e41fad73 100644 --- a/tests/nghttp2_hd_test.h +++ b/tests/nghttp2_hd_test.h @@ -42,5 +42,6 @@ void test_nghttp2_hd_no_index(void); void test_nghttp2_hd_deflate_bound(void); void test_nghttp2_hd_public_api(void); void test_nghttp2_hd_decode_length(void); +void test_nghttp2_hd_huff_encode(void); #endif /* NGHTTP2_HD_TEST_H */