diff --git a/lib/nghttp2_hd.c b/lib/nghttp2_hd.c index 6ffe59ac..1c0cb919 100644 --- a/lib/nghttp2_hd.c +++ b/lib/nghttp2_hd.c @@ -800,6 +800,7 @@ int nghttp2_hd_inflate_init(nghttp2_hd_inflater *inflater, nghttp2_mem *mem) { } inflater->settings_hd_table_bufsize_max = NGHTTP2_HD_DEFAULT_MAX_BUFFER_SIZE; + inflater->min_hd_table_bufsize_max = UINT32_MAX; inflater->ent_keep = NULL; inflater->nv_keep = NULL; @@ -1383,9 +1384,24 @@ int nghttp2_hd_inflate_change_table_size(nghttp2_hd_inflater *inflater, return NGHTTP2_ERR_INVALID_STATE; } - inflater->state = NGHTTP2_HD_STATE_EXPECT_TABLE_SIZE; + /* It seems that encoder is not required to send dynamic table size + update if the table size is not changed after applying + SETTINGS_HEADER_TABLE_SIZE. RFC 7541 is ambiguous here, but this + is the intention of the editor. If new maximum table size is + strictly smaller than the current negotiated maximum size, + encoder must send dynamic table size update. In other cases, we + cannot expect it to do so. */ + if (inflater->ctx.hd_table_bufsize_max > settings_hd_table_bufsize_max) { + inflater->state = NGHTTP2_HD_STATE_EXPECT_TABLE_SIZE; + /* Remember minimum value, and validate that encoder sends the + value less than or equal to this. */ + inflater->min_hd_table_bufsize_max = settings_hd_table_bufsize_max; + } + inflater->settings_hd_table_bufsize_max = settings_hd_table_bufsize_max; + inflater->ctx.hd_table_bufsize_max = settings_hd_table_bufsize_max; + hd_context_shrink_table_size(&inflater->ctx, NULL); return 0; } @@ -2136,8 +2152,10 @@ ssize_t nghttp2_hd_inflate_hd2(nghttp2_hd_inflater *inflater, break; case NGHTTP2_HD_STATE_READ_TABLE_SIZE: rfin = 0; - rv = hd_inflate_read_len(inflater, &rfin, in, last, 5, - inflater->settings_hd_table_bufsize_max); + rv = hd_inflate_read_len( + inflater, &rfin, in, last, 5, + nghttp2_min(inflater->min_hd_table_bufsize_max, + inflater->settings_hd_table_bufsize_max)); if (rv < 0) { goto fail; } @@ -2146,6 +2164,7 @@ ssize_t nghttp2_hd_inflate_hd2(nghttp2_hd_inflater *inflater, goto almost_ok; } DEBUGF(fprintf(stderr, "inflatehd: table_size=%zu\n", inflater->left)); + inflater->min_hd_table_bufsize_max = UINT32_MAX; inflater->ctx.hd_table_bufsize_max = inflater->left; hd_context_shrink_table_size(&inflater->ctx, NULL); inflater->state = NGHTTP2_HD_STATE_INFLATE_START; @@ -2403,7 +2422,8 @@ ssize_t nghttp2_hd_inflate_hd2(nghttp2_hd_inflater *inflater, if (in_final) { DEBUGF(fprintf(stderr, "inflatehd: in_final set\n")); - if (inflater->state != NGHTTP2_HD_STATE_OPCODE) { + if (inflater->state != NGHTTP2_HD_STATE_OPCODE && + inflater->state != NGHTTP2_HD_STATE_INFLATE_START) { DEBUGF(fprintf(stderr, "inflatehd: unacceptable state=%d\n", inflater->state)); rv = NGHTTP2_ERR_HEADER_COMP; @@ -2415,7 +2435,7 @@ ssize_t nghttp2_hd_inflate_hd2(nghttp2_hd_inflater *inflater, return (ssize_t)(in - first); almost_ok: - if (in_final && inflater->state != NGHTTP2_HD_STATE_OPCODE) { + if (in_final) { DEBUGF(fprintf(stderr, "inflatehd: input ended prematurely\n")); rv = NGHTTP2_ERR_HEADER_COMP; diff --git a/lib/nghttp2_hd.h b/lib/nghttp2_hd.h index 3d7fc593..c667888e 100644 --- a/lib/nghttp2_hd.h +++ b/lib/nghttp2_hd.h @@ -240,6 +240,8 @@ struct nghttp2_hd_inflater { /* The maximum header table size the inflater supports. This is the same value transmitted in SETTINGS_HEADER_TABLE_SIZE */ size_t settings_hd_table_bufsize_max; + /* Minimum header table size set by nghttp2_hd_inflate_change_table_size */ + size_t min_hd_table_bufsize_max; /* The number of next shift to decode integer */ size_t shift; nghttp2_hd_opcode opcode; diff --git a/tests/nghttp2_hd_test.c b/tests/nghttp2_hd_test.c index 50ed04a1..c71117ce 100644 --- a/tests/nghttp2_hd_test.c +++ b/tests/nghttp2_hd_test.c @@ -563,13 +563,79 @@ void test_nghttp2_hd_inflate_expect_table_size_update(void) { nghttp2_hd_inflate_init(&inflater, mem); /* This will make inflater require table size update in the next inflation. */ + nghttp2_hd_inflate_change_table_size(&inflater, 4095); nghttp2_hd_inflate_change_table_size(&inflater, 4096); CU_ASSERT(NGHTTP2_ERR_HEADER_COMP == inflate_hd(&inflater, &out, &bufs, 0, mem)); nva_out_reset(&out, mem); - nghttp2_bufs_free(&bufs); nghttp2_hd_inflate_free(&inflater); + + /* This does not require for encoder to emit table size update since + * size is not changed. */ + nghttp2_hd_inflate_init(&inflater, mem); + nghttp2_hd_inflate_change_table_size(&inflater, 4096); + CU_ASSERT((ssize_t)nghttp2_bufs_len(&bufs) == + inflate_hd(&inflater, &out, &bufs, 0, mem)); + + nva_out_reset(&out, mem); + nghttp2_hd_inflate_free(&inflater); + + /* This does not require for encodre to emit table size update since + new size is larger than current size. */ + nghttp2_hd_inflate_init(&inflater, mem); + nghttp2_hd_inflate_change_table_size(&inflater, 4097); + CU_ASSERT((ssize_t)nghttp2_bufs_len(&bufs) == + inflate_hd(&inflater, &out, &bufs, 0, mem)); + + nva_out_reset(&out, mem); + nghttp2_hd_inflate_free(&inflater); + + /* Received table size is strictly larger than minimum table size */ + nghttp2_hd_inflate_init(&inflater, mem); + nghttp2_hd_inflate_change_table_size(&inflater, 111); + nghttp2_hd_inflate_change_table_size(&inflater, 4096); + + nghttp2_bufs_reset(&bufs); + nghttp2_hd_emit_table_size(&bufs, 112); + + CU_ASSERT(NGHTTP2_ERR_HEADER_COMP == + inflate_hd(&inflater, &out, &bufs, 0, mem)); + + nva_out_reset(&out, mem); + nghttp2_hd_inflate_free(&inflater); + + /* Receiving 2 table size updates, min and last value */ + nghttp2_hd_inflate_init(&inflater, mem); + nghttp2_hd_inflate_change_table_size(&inflater, 111); + nghttp2_hd_inflate_change_table_size(&inflater, 4096); + + nghttp2_bufs_reset(&bufs); + nghttp2_hd_emit_table_size(&bufs, 111); + nghttp2_hd_emit_table_size(&bufs, 4096); + + CU_ASSERT((ssize_t)nghttp2_bufs_len(&bufs) == + inflate_hd(&inflater, &out, &bufs, 0, mem)); + + nva_out_reset(&out, mem); + nghttp2_hd_inflate_free(&inflater); + + /* 2nd update is larger than last value */ + nghttp2_hd_inflate_init(&inflater, mem); + nghttp2_hd_inflate_change_table_size(&inflater, 111); + nghttp2_hd_inflate_change_table_size(&inflater, 4095); + + nghttp2_bufs_reset(&bufs); + nghttp2_hd_emit_table_size(&bufs, 111); + nghttp2_hd_emit_table_size(&bufs, 4096); + + CU_ASSERT(NGHTTP2_ERR_HEADER_COMP == + inflate_hd(&inflater, &out, &bufs, 0, mem)); + + nva_out_reset(&out, mem); + nghttp2_hd_inflate_free(&inflater); + + nghttp2_bufs_free(&bufs); } void test_nghttp2_hd_inflate_unexpected_table_size_update(void) { diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index c2376b30..b3441f5c 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -4214,13 +4214,16 @@ void test_nghttp2_submit_settings(void) { iv[2].value = 50; iv[3].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE; - iv[3].value = 0; + iv[3].value = 111; iv[4].settings_id = UNKNOWN_ID; iv[4].value = 999; - iv[5].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE; - iv[5].value = (uint32_t)NGHTTP2_MAX_WINDOW_SIZE + 1; + iv[5].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE; + iv[5].value = 1023; + + iv[6].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE; + iv[6].value = (uint32_t)NGHTTP2_MAX_WINDOW_SIZE + 1; memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); callbacks.send_callback = null_send_callback; @@ -4228,7 +4231,7 @@ void test_nghttp2_submit_settings(void) { nghttp2_session_server_new(&session, &callbacks, &ud); CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == - nghttp2_submit_settings(session, NGHTTP2_FLAG_NONE, iv, 6)); + nghttp2_submit_settings(session, NGHTTP2_FLAG_NONE, iv, 7)); /* Make sure that local settings are not changed */ CU_ASSERT(NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS == @@ -4237,14 +4240,14 @@ void test_nghttp2_submit_settings(void) { session->local_settings.initial_window_size); /* Now sends without 6th one */ - CU_ASSERT(0 == nghttp2_submit_settings(session, NGHTTP2_FLAG_NONE, iv, 5)); + CU_ASSERT(0 == nghttp2_submit_settings(session, NGHTTP2_FLAG_NONE, iv, 6)); item = nghttp2_session_get_next_ob_item(session); CU_ASSERT(NGHTTP2_SETTINGS == item->frame.hd.type); frame = &item->frame; - CU_ASSERT(5 == frame->settings.niv); + CU_ASSERT(6 == frame->settings.niv); CU_ASSERT(5 == frame->settings.iv[0].value); CU_ASSERT(NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS == frame->settings.iv[0].settings_id); @@ -4267,7 +4270,8 @@ void test_nghttp2_submit_settings(void) { nghttp2_frame_settings_free(&ack_frame.settings, mem); CU_ASSERT(16 * 1024 == session->local_settings.initial_window_size); - CU_ASSERT(0 == session->hd_inflater.ctx.hd_table_bufsize_max); + CU_ASSERT(1023 == session->hd_inflater.ctx.hd_table_bufsize_max); + CU_ASSERT(111 == session->hd_inflater.min_hd_table_bufsize_max); CU_ASSERT(50 == session->local_settings.max_concurrent_streams); /* We just keep the last seen value */ CU_ASSERT(50 == session->pending_local_max_concurrent_stream);