Don't always expect dynamic table size update

The encoder is not required to send dynamic table size update if the
table size is not changed from the previous value after accepting new
maximum value.
This commit is contained in:
Tatsuhiro Tsujikawa 2015-10-24 17:27:24 +09:00
parent 5bc3dfa1cd
commit 25bf567cd7
4 changed files with 105 additions and 13 deletions

View File

@ -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->settings_hd_table_bufsize_max = NGHTTP2_HD_DEFAULT_MAX_BUFFER_SIZE;
inflater->min_hd_table_bufsize_max = UINT32_MAX;
inflater->ent_keep = NULL; inflater->ent_keep = NULL;
inflater->nv_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; 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->settings_hd_table_bufsize_max = settings_hd_table_bufsize_max;
inflater->ctx.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); hd_context_shrink_table_size(&inflater->ctx, NULL);
return 0; return 0;
} }
@ -2136,8 +2152,10 @@ ssize_t nghttp2_hd_inflate_hd2(nghttp2_hd_inflater *inflater,
break; break;
case NGHTTP2_HD_STATE_READ_TABLE_SIZE: case NGHTTP2_HD_STATE_READ_TABLE_SIZE:
rfin = 0; rfin = 0;
rv = hd_inflate_read_len(inflater, &rfin, in, last, 5, rv = hd_inflate_read_len(
inflater->settings_hd_table_bufsize_max); inflater, &rfin, in, last, 5,
nghttp2_min(inflater->min_hd_table_bufsize_max,
inflater->settings_hd_table_bufsize_max));
if (rv < 0) { if (rv < 0) {
goto fail; goto fail;
} }
@ -2146,6 +2164,7 @@ ssize_t nghttp2_hd_inflate_hd2(nghttp2_hd_inflater *inflater,
goto almost_ok; goto almost_ok;
} }
DEBUGF(fprintf(stderr, "inflatehd: table_size=%zu\n", inflater->left)); 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; inflater->ctx.hd_table_bufsize_max = inflater->left;
hd_context_shrink_table_size(&inflater->ctx, NULL); hd_context_shrink_table_size(&inflater->ctx, NULL);
inflater->state = NGHTTP2_HD_STATE_INFLATE_START; inflater->state = NGHTTP2_HD_STATE_INFLATE_START;
@ -2403,7 +2422,8 @@ ssize_t nghttp2_hd_inflate_hd2(nghttp2_hd_inflater *inflater,
if (in_final) { if (in_final) {
DEBUGF(fprintf(stderr, "inflatehd: in_final set\n")); 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", DEBUGF(fprintf(stderr, "inflatehd: unacceptable state=%d\n",
inflater->state)); inflater->state));
rv = NGHTTP2_ERR_HEADER_COMP; rv = NGHTTP2_ERR_HEADER_COMP;
@ -2415,7 +2435,7 @@ ssize_t nghttp2_hd_inflate_hd2(nghttp2_hd_inflater *inflater,
return (ssize_t)(in - first); return (ssize_t)(in - first);
almost_ok: almost_ok:
if (in_final && inflater->state != NGHTTP2_HD_STATE_OPCODE) { if (in_final) {
DEBUGF(fprintf(stderr, "inflatehd: input ended prematurely\n")); DEBUGF(fprintf(stderr, "inflatehd: input ended prematurely\n"));
rv = NGHTTP2_ERR_HEADER_COMP; rv = NGHTTP2_ERR_HEADER_COMP;

View File

@ -240,6 +240,8 @@ struct nghttp2_hd_inflater {
/* The maximum header table size the inflater supports. This is the /* The maximum header table size the inflater supports. This is the
same value transmitted in SETTINGS_HEADER_TABLE_SIZE */ same value transmitted in SETTINGS_HEADER_TABLE_SIZE */
size_t settings_hd_table_bufsize_max; 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 */ /* The number of next shift to decode integer */
size_t shift; size_t shift;
nghttp2_hd_opcode opcode; nghttp2_hd_opcode opcode;

View File

@ -563,13 +563,79 @@ void test_nghttp2_hd_inflate_expect_table_size_update(void) {
nghttp2_hd_inflate_init(&inflater, mem); nghttp2_hd_inflate_init(&inflater, mem);
/* This will make inflater require table size update in the next /* This will make inflater require table size update in the next
inflation. */ inflation. */
nghttp2_hd_inflate_change_table_size(&inflater, 4095);
nghttp2_hd_inflate_change_table_size(&inflater, 4096); nghttp2_hd_inflate_change_table_size(&inflater, 4096);
CU_ASSERT(NGHTTP2_ERR_HEADER_COMP == CU_ASSERT(NGHTTP2_ERR_HEADER_COMP ==
inflate_hd(&inflater, &out, &bufs, 0, mem)); inflate_hd(&inflater, &out, &bufs, 0, mem));
nva_out_reset(&out, mem); nva_out_reset(&out, mem);
nghttp2_bufs_free(&bufs);
nghttp2_hd_inflate_free(&inflater); 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) { void test_nghttp2_hd_inflate_unexpected_table_size_update(void) {

View File

@ -4214,13 +4214,16 @@ void test_nghttp2_submit_settings(void) {
iv[2].value = 50; iv[2].value = 50;
iv[3].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE; 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].settings_id = UNKNOWN_ID;
iv[4].value = 999; iv[4].value = 999;
iv[5].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE; iv[5].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE;
iv[5].value = (uint32_t)NGHTTP2_MAX_WINDOW_SIZE + 1; 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)); memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
callbacks.send_callback = null_send_callback; callbacks.send_callback = null_send_callback;
@ -4228,7 +4231,7 @@ void test_nghttp2_submit_settings(void) {
nghttp2_session_server_new(&session, &callbacks, &ud); nghttp2_session_server_new(&session, &callbacks, &ud);
CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == 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 */ /* Make sure that local settings are not changed */
CU_ASSERT(NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS == CU_ASSERT(NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS ==
@ -4237,14 +4240,14 @@ void test_nghttp2_submit_settings(void) {
session->local_settings.initial_window_size); session->local_settings.initial_window_size);
/* Now sends without 6th one */ /* 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); item = nghttp2_session_get_next_ob_item(session);
CU_ASSERT(NGHTTP2_SETTINGS == item->frame.hd.type); CU_ASSERT(NGHTTP2_SETTINGS == item->frame.hd.type);
frame = &item->frame; 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(5 == frame->settings.iv[0].value);
CU_ASSERT(NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS == CU_ASSERT(NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS ==
frame->settings.iv[0].settings_id); frame->settings.iv[0].settings_id);
@ -4267,7 +4270,8 @@ void test_nghttp2_submit_settings(void) {
nghttp2_frame_settings_free(&ack_frame.settings, mem); nghttp2_frame_settings_free(&ack_frame.settings, mem);
CU_ASSERT(16 * 1024 == session->local_settings.initial_window_size); 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); CU_ASSERT(50 == session->local_settings.max_concurrent_streams);
/* We just keep the last seen value */ /* We just keep the last seen value */
CU_ASSERT(50 == session->pending_local_max_concurrent_stream); CU_ASSERT(50 == session->pending_local_max_concurrent_stream);