From 5a81e0349755a811ea84c8e6b84df8803ff64b8d Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 24 Oct 2013 21:52:02 +0900 Subject: [PATCH] nghttp2_hd: Add static table entry to dynamic table on emission --- lib/nghttp2_hd.c | 66 ++++++++++++++++++++++++++++------------- tests/nghttp2_hd_test.c | 32 +++++++++++++------- 2 files changed, 67 insertions(+), 31 deletions(-) diff --git a/lib/nghttp2_hd.c b/lib/nghttp2_hd.c index 03f72f28..3ab58201 100644 --- a/lib/nghttp2_hd.c +++ b/lib/nghttp2_hd.c @@ -796,7 +796,28 @@ static int deflate_nv(nghttp2_hd_context *deflater, if(rv != -1) { size_t index = rv; ent = nghttp2_hd_table_get(deflater, index); - if((ent->flags & NGHTTP2_HD_FLAG_REFSET) == 0) { + if(index >= deflater->hd_table.len) { + nghttp2_hd_entry *new_ent; + /* It is important to first add entry to the header table and + let eviction go. If NGHTTP2_HD_FLAG_IMPLICIT_EMIT entry is + evicted, it must be emitted before the |nv|. */ + new_ent = add_hd_table_incremental(deflater, buf_ptr, buflen_ptr, + offset_ptr, &ent->nv, + NGHTTP2_HD_FLAG_NONE); + if(!new_ent) { + return NGHTTP2_ERR_HEADER_COMP; + } + if(new_ent->ref == 0) { + nghttp2_hd_entry_free(new_ent); + free(new_ent); + } else { + new_ent->flags |= NGHTTP2_HD_FLAG_EMIT; + } + rv = emit_indexed_block(buf_ptr, buflen_ptr, offset_ptr, index); + if(rv != 0) { + return rv; + } + } else if((ent->flags & NGHTTP2_HD_FLAG_REFSET) == 0) { ent->flags |= NGHTTP2_HD_FLAG_REFSET | NGHTTP2_HD_FLAG_EMIT; rv = emit_indexed_block(buf_ptr, buflen_ptr, offset_ptr, index); if(rv != 0) { @@ -859,7 +880,12 @@ static int deflate_nv(nghttp2_hd_context *deflater, if(!new_ent) { return NGHTTP2_ERR_HEADER_COMP; } - new_ent->flags |= NGHTTP2_HD_FLAG_EMIT; + if(new_ent->ref == 0) { + nghttp2_hd_entry_free(new_ent); + free(new_ent); + } else { + new_ent->flags |= NGHTTP2_HD_FLAG_EMIT; + } incidx = 1; } if(index == -1) { @@ -923,14 +949,6 @@ ssize_t nghttp2_hd_deflate_hd(nghttp2_hd_context *deflater, goto fail; } } - for(i = 0; deflater->static_hd_table[i]; ++i) { - nghttp2_hd_entry *ent = deflater->static_hd_table[i]; - rv = deflate_post_process_hd_entry(ent, i + deflater->hd_table.len, - buf_ptr, buflen_ptr, &offset); - if(rv != 0) { - goto fail; - } - } return offset - nv_offset; fail: deflater->bad = 1; @@ -1003,12 +1021,25 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_context *inflater, goto fail; } ent = nghttp2_hd_table_get(inflater, index); - ent->flags ^= NGHTTP2_HD_FLAG_REFSET; - if(ent->flags & NGHTTP2_HD_FLAG_REFSET) { - rv = emit_indexed_header(inflater, &nva_out, ent); - if(rv != 0) { + if(index >= (ssize_t)inflater->hd_table.len) { + nghttp2_hd_entry *new_ent; + new_ent = add_hd_table_incremental(inflater, NULL, NULL, NULL, + &ent->nv, NGHTTP2_HD_FLAG_NONE); + if(!new_ent) { + rv = NGHTTP2_ERR_HEADER_COMP; goto fail; } + /* new_ent->ref == 0 may be hold but emit_indexed_header + tracks new_ent, so there is no leak. */ + rv = emit_indexed_header(inflater, &nva_out, new_ent); + } else { + ent->flags ^= NGHTTP2_HD_FLAG_REFSET; + if(ent->flags & NGHTTP2_HD_FLAG_REFSET) { + rv = emit_indexed_header(inflater, &nva_out, ent); + } + } + if(rv != 0) { + goto fail; } } else if(c == 0x40u || c == 0) { /* Literal Header Repr - New Name */ @@ -1191,13 +1222,6 @@ ssize_t nghttp2_hd_inflate_hd(nghttp2_hd_context *inflater, goto fail; } } - for(i = 0; inflater->static_hd_table[i]; ++i) { - nghttp2_hd_entry *ent = inflater->static_hd_table[i]; - rv = inflater_post_process_hd_entry(inflater, ent, &nva_out); - if(rv != 0) { - goto fail; - } - } nghttp2_nv_array_sort(nva_out.nva, nva_out.nvlen); *nva_ptr = nva_out.nva; return nva_out.nvlen; diff --git a/tests/nghttp2_hd_test.c b/tests/nghttp2_hd_test.c index d3967202..5a578449 100644 --- a/tests/nghttp2_hd_test.c +++ b/tests/nghttp2_hd_test.c @@ -198,13 +198,13 @@ void test_nghttp2_hd_deflate_common_header_eviction(void) size_t buflen = 0; ssize_t blocklen; nghttp2_nv *resnva; - /* Default header table capacity is 1262. Adding 2835 bytes, - including overhead, to the table evicts first entry. - use name ":host" which index 2 and value length 2798. */ - uint8_t value[2798]; + /* Default header table capacity is 4096. Adding 2 byte header name + and 4060 byte value, which is 4094 bytes including overhead, to + the table evicts first entry. */ + uint8_t value[4060]; memset(value, '0', sizeof(value)); - nva[1].name = (uint8_t*)":host"; + nva[1].name = (uint8_t*)"hd"; nva[1].namelen = strlen((const char*)nva[1].name); nva[1].value = value; nva[1].valuelen = sizeof(value); @@ -212,11 +212,20 @@ void test_nghttp2_hd_deflate_common_header_eviction(void) nghttp2_hd_deflate_init(&deflater, NGHTTP2_HD_SIDE_REQUEST); nghttp2_hd_inflate_init(&inflater, NGHTTP2_HD_SIDE_REQUEST); - /* Put :scheme: http (index = 0) in reference set */ - GET_TABLE_ENT(&deflater, 0)->flags |= NGHTTP2_HD_FLAG_REFSET; - GET_TABLE_ENT(&inflater, 0)->flags |= NGHTTP2_HD_FLAG_REFSET; - blocklen = nghttp2_hd_deflate_hd(&deflater, &buf, &buflen, 0, nva, - sizeof(nva)/sizeof(nghttp2_nv)); + /* First emit ":scheme: http" to put it in the reference set (index + = 0). */ + blocklen = nghttp2_hd_deflate_hd(&deflater, &buf, &buflen, 0, nva, 1); + CU_ASSERT(blocklen > 0); + nghttp2_hd_end_headers(&deflater); + + CU_ASSERT(1 == nghttp2_hd_inflate_hd(&inflater, &resnva, buf, blocklen)); + nghttp2_nv_array_sort(nva, 1); + assert_nv_equal(nva, resnva, 1); + nghttp2_nv_array_del(resnva); + nghttp2_hd_end_headers(&inflater); + + /* Encode with large header */ + blocklen = nghttp2_hd_deflate_hd(&deflater, &buf, &buflen, 0, nva, 2); CU_ASSERT(blocklen > 0); nghttp2_hd_end_headers(&deflater); @@ -230,6 +239,9 @@ void test_nghttp2_hd_deflate_common_header_eviction(void) nghttp2_nv_array_del(resnva); nghttp2_hd_end_headers(&inflater); + CU_ASSERT(1 == deflater.hd_table.len); + CU_ASSERT(1 == inflater.hd_table.len); + free(buf); nghttp2_hd_inflate_free(&inflater); nghttp2_hd_deflate_free(&deflater);