diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 3a2a328c..bc3ae658 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -516,6 +516,8 @@ typedef enum { */ NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE = 0x04 } nghttp2_settings_id; +/* Note: If we add SETTINGS, update the capacity of + NGHTTP2_INBOUND_NUM_IV as well */ /** * @macro diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index a3dcf969..b2a00a49 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -282,6 +282,9 @@ static void session_inbound_frame_reset(nghttp2_session *session) iframe->payloadleft = 0; iframe->padlen = 0; iframe->headers_payload_length = 0; + iframe->iv[NGHTTP2_INBOUND_NUM_IV - 1].settings_id = + NGHTTP2_SETTINGS_HEADER_TABLE_SIZE; + iframe->iv[NGHTTP2_INBOUND_NUM_IV - 1].value = UINT32_MAX; } static void init_settings(nghttp2_settings_storage *settings) @@ -3299,11 +3302,7 @@ int nghttp2_session_on_settings_received(nghttp2_session *session, int noack) { int rv; - int i; - int header_table_size_seen = 0; - int enable_push_seen = 0; - int max_concurrent_streams_seen = 0; - int initial_window_size_seen = 0; + size_t i; if(frame->hd.stream_id != 0) { return session_handle_invalid_connection @@ -3338,20 +3337,11 @@ int nghttp2_session_on_settings_received(nghttp2_session *session, return session_call_on_frame_received(session, frame); } - for(i = (int)frame->settings.niv - 1; i >= 0; --i) { + for(i = 0; i < frame->settings.niv; ++i) { nghttp2_settings_entry *entry = &frame->settings.iv[i]; - /* The spec says the settings values are processed in the order - they appear in the payload. In other words, if the multiple - values for the same ID were found, use the last one and ignore - the rest. */ switch(entry->settings_id) { case NGHTTP2_SETTINGS_HEADER_TABLE_SIZE: - if(header_table_size_seen) { - break; - } - - header_table_size_seen = 1; if(entry->value > NGHTTP2_MAX_HEADER_TABLE_SIZE) { return session_handle_invalid_connection @@ -3374,11 +3364,6 @@ int nghttp2_session_on_settings_received(nghttp2_session *session, break; case NGHTTP2_SETTINGS_ENABLE_PUSH: - if(enable_push_seen) { - break; - } - - enable_push_seen = 1; if(entry->value != 0 && entry->value != 1) { return session_handle_invalid_connection @@ -3396,21 +3381,11 @@ int nghttp2_session_on_settings_received(nghttp2_session *session, break; case NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS: - if(max_concurrent_streams_seen) { - break; - } - - max_concurrent_streams_seen = 1; session->remote_settings.max_concurrent_streams = entry->value; break; case NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE: - if(initial_window_size_seen) { - break; - } - - initial_window_size_seen = 1; /* Update the initial window size of the all active streams */ /* Check that initial_window_size < (1u << 31) */ @@ -3458,6 +3433,27 @@ static int session_process_settings_frame(nghttp2_session *session) int rv; nghttp2_inbound_frame *iframe = &session->iframe; nghttp2_frame *frame = &iframe->frame; + size_t i; + nghttp2_settings_entry min_header_size_entry; + + min_header_size_entry = iframe->iv[NGHTTP2_INBOUND_NUM_IV - 1]; + + if(min_header_size_entry.value < UINT32_MAX) { + /* If we have less value, then we must have + SETTINGS_HEADER_TABLE_SIZE in i < iframe->niv */ + for(i = 0; i < iframe->niv; ++i) { + if(iframe->iv[i].settings_id == NGHTTP2_SETTINGS_HEADER_TABLE_SIZE) { + break; + } + } + + assert(i < iframe->niv); + + if(min_header_size_entry.value != iframe->iv[i].value) { + iframe->iv[iframe->niv++] = iframe->iv[i]; + iframe->iv[i] = min_header_size_entry; + } + } rv = nghttp2_frame_unpack_settings_payload(&frame->settings, iframe->iv, iframe->niv); @@ -4115,6 +4111,12 @@ static void inbound_frame_set_settings_entry(nghttp2_inbound_frame *iframe) if(i == iframe->niv) { iframe->iv[iframe->niv++] = iv; } + + if(iv.settings_id == NGHTTP2_SETTINGS_HEADER_TABLE_SIZE && + iv.value < iframe->iv[NGHTTP2_INBOUND_NUM_IV - 1].value) { + + iframe->iv[NGHTTP2_INBOUND_NUM_IV - 1] = iv; + } } /* diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h index 53885615..d9d6094a 100644 --- a/lib/nghttp2_session.h +++ b/lib/nghttp2_session.h @@ -81,6 +81,8 @@ typedef enum { NGHTTP2_IB_IGN_DATA } nghttp2_inbound_state; +#define NGHTTP2_INBOUND_NUM_IV 5 + typedef struct { nghttp2_frame frame; /* Storage for extension frame payload. frame->ext.payload points @@ -88,8 +90,9 @@ typedef struct { nghttp2_ext_frame_payload ext_frame_payload; /* The received SETTINGS entry. The protocol says that we only cares about the defined settings ID. If unknown ID is received, it is - subject to connection error */ - nghttp2_settings_entry iv[5]; + ignored. We use last entry to hold minimum header table size if + same settings are multiple times. */ + nghttp2_settings_entry iv[NGHTTP2_INBOUND_NUM_IV]; /* buffer pointers to small buffer, raw_sbuf */ nghttp2_buf sbuf; /* buffer pointers to large buffer, raw_lbuf */ diff --git a/tests/main.c b/tests/main.c index 0fda14a1..bb26c08f 100644 --- a/tests/main.c +++ b/tests/main.c @@ -93,6 +93,8 @@ int main(int argc, char* argv[]) test_nghttp2_session_recv_unknown_frame) || !CU_add_test(pSuite, "session_recv_unexpected_continuation", test_nghttp2_session_recv_unexpected_continuation) || + !CU_add_test(pSuite, "session_recv_settings_header_table_size", + test_nghttp2_session_recv_settings_header_table_size) || !CU_add_test(pSuite, "session_continue", test_nghttp2_session_continue) || !CU_add_test(pSuite, "session_add_frame", test_nghttp2_session_add_frame) || diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 308c3e1c..88df64bf 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -1231,6 +1231,182 @@ void test_nghttp2_session_recv_unexpected_continuation(void) nghttp2_session_del(session); } +void test_nghttp2_session_recv_settings_header_table_size(void) +{ + nghttp2_session *session; + nghttp2_session_callbacks callbacks; + nghttp2_frame frame; + nghttp2_bufs bufs; + nghttp2_buf *buf; + ssize_t rv; + my_user_data ud; + nghttp2_settings_entry iv[3]; + nghttp2_nv nv = MAKE_NV(":authority", "example.org"); + + frame_pack_bufs_init(&bufs); + + memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); + callbacks.on_frame_recv_callback = on_frame_recv_callback; + callbacks.send_callback = null_send_callback; + + nghttp2_session_client_new(&session, &callbacks, &ud); + + iv[0].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE; + iv[0].value = 3000; + + iv[1].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE; + iv[1].value = 16384; + + nghttp2_frame_settings_init(&frame.settings, NGHTTP2_FLAG_NONE, + dup_iv(iv, 2), 2); + + rv = nghttp2_frame_pack_settings(&bufs, &frame.settings); + + CU_ASSERT(0 == rv); + CU_ASSERT(nghttp2_bufs_len(&bufs) > 0); + + nghttp2_frame_settings_free(&frame.settings); + + buf = &bufs.head->buf; + assert(nghttp2_bufs_len(&bufs) == nghttp2_buf_len(buf)); + + ud.frame_recv_cb_called = 0; + + rv = nghttp2_session_mem_recv(session, buf->pos, nghttp2_buf_len(buf)); + + CU_ASSERT(rv == nghttp2_buf_len(buf)); + CU_ASSERT(1 == ud.frame_recv_cb_called); + + CU_ASSERT(3000 == session->remote_settings.header_table_size); + CU_ASSERT(16384 == session->remote_settings.initial_window_size); + + nghttp2_bufs_reset(&bufs); + + /* 2 SETTINGS_HEADER_TABLE_SIZE */ + iv[0].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE; + iv[0].value = 3001; + + iv[1].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE; + iv[1].value = 16383; + + iv[2].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE; + iv[2].value = 3001; + + nghttp2_frame_settings_init(&frame.settings, NGHTTP2_FLAG_NONE, + dup_iv(iv, 3), 3); + + rv = nghttp2_frame_pack_settings(&bufs, &frame.settings); + + CU_ASSERT(0 == rv); + CU_ASSERT(nghttp2_bufs_len(&bufs) > 0); + + nghttp2_frame_settings_free(&frame.settings); + + buf = &bufs.head->buf; + assert(nghttp2_bufs_len(&bufs) == nghttp2_buf_len(buf)); + + ud.frame_recv_cb_called = 0; + + rv = nghttp2_session_mem_recv(session, buf->pos, nghttp2_buf_len(buf)); + + CU_ASSERT(rv == nghttp2_buf_len(buf)); + CU_ASSERT(1 == ud.frame_recv_cb_called); + + CU_ASSERT(3001 == session->remote_settings.header_table_size); + CU_ASSERT(16383 == session->remote_settings.initial_window_size); + + nghttp2_bufs_reset(&bufs); + + /* 2 SETTINGS_HEADER_TABLE_SIZE; first entry clears dynamic header + table. */ + + nghttp2_submit_request(session, NULL, &nv, 1, NULL, NULL); + nghttp2_session_send(session); + + CU_ASSERT(0 < session->hd_deflater.ctx.hd_table.len); + + iv[0].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE; + iv[0].value = 0; + + iv[1].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE; + iv[1].value = 16382; + + iv[2].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE; + iv[2].value = 4096; + + nghttp2_frame_settings_init(&frame.settings, NGHTTP2_FLAG_NONE, + dup_iv(iv, 3), 3); + + rv = nghttp2_frame_pack_settings(&bufs, &frame.settings); + + CU_ASSERT(0 == rv); + CU_ASSERT(nghttp2_bufs_len(&bufs) > 0); + + nghttp2_frame_settings_free(&frame.settings); + + buf = &bufs.head->buf; + assert(nghttp2_bufs_len(&bufs) == nghttp2_buf_len(buf)); + + ud.frame_recv_cb_called = 0; + + rv = nghttp2_session_mem_recv(session, buf->pos, nghttp2_buf_len(buf)); + + CU_ASSERT(rv == nghttp2_buf_len(buf)); + CU_ASSERT(1 == ud.frame_recv_cb_called); + + CU_ASSERT(4096 == session->remote_settings.header_table_size); + CU_ASSERT(16382 == session->remote_settings.initial_window_size); + CU_ASSERT(0 == session->hd_deflater.ctx.hd_table.len); + + nghttp2_bufs_reset(&bufs); + + /* 2 SETTINGS_HEADER_TABLE_SIZE; second entry clears dynamic header + table. */ + + nghttp2_submit_request(session, NULL, &nv, 1, NULL, NULL); + nghttp2_session_send(session); + + CU_ASSERT(0 < session->hd_deflater.ctx.hd_table.len); + + iv[0].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE; + iv[0].value = 3000; + + iv[1].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE; + iv[1].value = 16381; + + iv[2].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE; + iv[2].value = 0; + + nghttp2_frame_settings_init(&frame.settings, NGHTTP2_FLAG_NONE, + dup_iv(iv, 3), 3); + + rv = nghttp2_frame_pack_settings(&bufs, &frame.settings); + + CU_ASSERT(0 == rv); + CU_ASSERT(nghttp2_bufs_len(&bufs) > 0); + + nghttp2_frame_settings_free(&frame.settings); + + buf = &bufs.head->buf; + assert(nghttp2_bufs_len(&bufs) == nghttp2_buf_len(buf)); + + ud.frame_recv_cb_called = 0; + + rv = nghttp2_session_mem_recv(session, buf->pos, nghttp2_buf_len(buf)); + + CU_ASSERT(rv == nghttp2_buf_len(buf)); + CU_ASSERT(1 == ud.frame_recv_cb_called); + + CU_ASSERT(0 == session->remote_settings.header_table_size); + CU_ASSERT(16381 == session->remote_settings.initial_window_size); + CU_ASSERT(0 == session->hd_deflater.ctx.hd_table.len); + + nghttp2_bufs_reset(&bufs); + + nghttp2_bufs_free(&bufs); + nghttp2_session_del(session); +} + void test_nghttp2_session_continue(void) { nghttp2_session *session; @@ -1868,6 +2044,7 @@ void test_nghttp2_session_on_settings_received(void) const size_t niv = 5; nghttp2_settings_entry iv[255]; nghttp2_outbound_item *item; + nghttp2_nv nv = MAKE_NV(":authority", "example.org"); iv[0].settings_id = NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS; iv[0].value = 50; @@ -1885,6 +2062,8 @@ void test_nghttp2_session_on_settings_received(void) iv[4].value = 0; memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); + callbacks.send_callback = null_send_callback; + nghttp2_session_client_new(&session, &callbacks, &user_data); session->remote_settings.initial_window_size = 16*1024; @@ -1952,6 +2131,34 @@ void test_nghttp2_session_on_settings_received(void) nghttp2_frame_settings_free(&frame.settings); nghttp2_session_del(session); + + /* Check that 2 SETTINGS_HEADER_TABLE_SIZE 0 and 4096 are included + and header table size is once cleared to 0. */ + nghttp2_session_client_new(&session, &callbacks, NULL); + + nghttp2_submit_request(session, NULL, &nv, 1, NULL, NULL); + + nghttp2_session_send(session); + + CU_ASSERT(session->hd_deflater.ctx.hd_table.len > 0); + + iv[0].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE; + iv[0].value = 0; + + iv[1].settings_id = NGHTTP2_SETTINGS_HEADER_TABLE_SIZE; + iv[1].value = 2048; + + nghttp2_frame_settings_init(&frame.settings, NGHTTP2_FLAG_NONE, + dup_iv(iv, 2), 2); + + CU_ASSERT(0 == nghttp2_session_on_settings_received(session, &frame, 0)); + + CU_ASSERT(0 == session->hd_deflater.ctx.hd_table.len); + CU_ASSERT(2048 == session->hd_deflater.ctx.hd_table_bufsize_max); + CU_ASSERT(2048 == session->remote_settings.header_table_size); + + nghttp2_frame_settings_free(&frame.settings); + nghttp2_session_del(session); } void test_nghttp2_session_on_push_promise_received(void) diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h index f9fbbd4f..57fb9695 100644 --- a/tests/nghttp2_session_test.h +++ b/tests/nghttp2_session_test.h @@ -36,6 +36,7 @@ void test_nghttp2_session_recv_premature_headers(void); void test_nghttp2_session_recv_altsvc(void); void test_nghttp2_session_recv_unknown_frame(void); void test_nghttp2_session_recv_unexpected_continuation(void); +void test_nghttp2_session_recv_settings_header_table_size(void); void test_nghttp2_session_continue(void); void test_nghttp2_session_add_frame(void); void test_nghttp2_session_on_request_headers_received(void);