From e67096fef32207ea96dcad7d3c583c3499744527 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 8 Aug 2013 21:12:49 +0900 Subject: [PATCH] Handle overflow in initial window update in stream Rename window_size in nghttp2_stream as remote_window_size. --- lib/includes/nghttp2/nghttp2.h | 11 ++++++++++ lib/nghttp2_session.c | 39 ++++++++++++++++++++++------------ lib/nghttp2_stream.c | 24 +++++++++++++++------ lib/nghttp2_stream.h | 23 ++++++++++++++------ tests/nghttp2_session_test.c | 23 ++++++++++---------- 5 files changed, 81 insertions(+), 39 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index bfb2dcfc..0c246ba2 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -71,6 +71,13 @@ typedef struct nghttp2_session nghttp2_session; */ #define NGHTTP2_PRI_LOWEST ((1U << 31) - 1) +/** + * @macro + * + * The maximum window size + */ +#define NGHTTP2_MAX_WINDOW_SIZE ((int32_t)((1U << 31) - 1)) + /** * @macro * @@ -207,6 +214,10 @@ typedef enum { * Header block inflate/deflate error. */ NGHTTP2_ERR_HEADER_COMP = -523, + /** + * Flow control error + */ + NGHTTP2_ERR_FLOW_CONTROL = -524, /** * The errors < :enum:`NGHTTP2_ERR_FATAL` mean that the library is * under unexpected condition and cannot process any further data diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 6a41d7ad..14459118 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -457,6 +457,8 @@ nghttp2_stream* nghttp2_session_open_stream(nghttp2_session *session, [NGHTTP2_SETTINGS_FLOW_CONTROL_OPTIONS], session->remote_settings [NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE], + session->local_settings + [NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE], stream_user_data); r = nghttp2_map_insert(&session->streams, &stream->map_entry); if(r != 0) { @@ -818,7 +820,7 @@ static size_t nghttp2_session_next_data_read(nghttp2_session *session, int32_t session_window_size = session->remote_flow_control ? session->window_size : INT32_MAX; int32_t stream_window_size = - stream->remote_flow_control ? stream->window_size : INT32_MAX; + stream->remote_flow_control ? stream->remote_window_size : INT32_MAX; int32_t window_size = nghttp2_min(session_window_size, stream_window_size); if(window_size > 0) { @@ -1509,7 +1511,7 @@ int nghttp2_session_send(nghttp2_session *session) frame = nghttp2_outbound_item_get_data_frame(session->aob.item); stream = nghttp2_session_get_stream(session, frame->hd.stream_id); if(stream && stream->remote_flow_control) { - stream->window_size -= len; + stream->remote_window_size -= len; } if(session->remote_flow_control) { session->window_size -= len; @@ -1857,21 +1859,24 @@ int nghttp2_session_on_rst_stream_received(nghttp2_session *session, static int nghttp2_update_initial_window_size_func(nghttp2_map_entry *entry, void *ptr) { + int rv; nghttp2_update_window_size_arg *arg; nghttp2_stream *stream; arg = (nghttp2_update_window_size_arg*)ptr; stream = (nghttp2_stream*)entry; - nghttp2_stream_update_initial_window_size(stream, - arg->new_window_size, - arg->old_window_size); + rv = nghttp2_stream_update_remote_initial_window_size(stream, + arg->new_window_size, + arg->old_window_size); + if(rv != 0) { + return NGHTTP2_ERR_FLOW_CONTROL; + } /* If window size gets positive, push deferred DATA frame to outbound queue. */ if(stream->deferred_data && (stream->deferred_flags & NGHTTP2_DEFERRED_FLOW_CONTROL) && - stream->window_size > 0 && + stream->remote_window_size > 0 && (arg->session->remote_flow_control == 0 || arg->session->window_size > 0)) { - int rv; rv = nghttp2_pq_push(&arg->session->ob_pq, stream->deferred_data); if(rv == 0) { nghttp2_stream_detach_deferred_data(stream); @@ -2011,10 +2016,15 @@ int nghttp2_session_on_settings_received(nghttp2_session *session, case NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE: /* Update the initial window size of the all active streams */ /* Check that initial_window_size < (1u << 31) */ - if(entry->value < (1u << 31)) { + if(entry->value <= NGHTTP2_MAX_WINDOW_SIZE) { rv = nghttp2_session_update_initial_window_size(session, entry->value); if(rv != 0) { - return rv; + if(nghttp2_is_fatal(rv)) { + return rv; + } else { + return nghttp2_session_handle_invalid_connection + (session, frame, NGHTTP2_FLOW_CONTROL_ERROR); + } } } else { return nghttp2_session_handle_invalid_connection @@ -2143,7 +2153,7 @@ static int nghttp2_push_back_deferred_data_func(nghttp2_map_entry *entry, outbound queue. */ if(stream->deferred_data && (stream->deferred_flags & NGHTTP2_DEFERRED_FLOW_CONTROL) && - (stream->remote_flow_control == 0 || stream->window_size > 0)) { + (stream->remote_flow_control == 0 || stream->remote_window_size > 0)) { int rv; rv = nghttp2_pq_push(&session->ob_pq, stream->deferred_data); if(rv == 0) { @@ -2232,15 +2242,16 @@ int nghttp2_session_on_window_update_received(nghttp2_session *session, nghttp2_session_call_on_frame_received(session, frame); return 0; } - if(INT32_MAX - frame->window_update.window_size_increment < - stream->window_size) { + if(NGHTTP2_MAX_WINDOW_SIZE - frame->window_update.window_size_increment < + stream->remote_window_size) { int r; r = nghttp2_session_handle_invalid_stream (session, frame, NGHTTP2_FLOW_CONTROL_ERROR); return r; } else { - stream->window_size += frame->window_update.window_size_increment; - if(stream->window_size > 0 && + stream->remote_window_size += + frame->window_update.window_size_increment; + if(stream->remote_window_size > 0 && (session->remote_flow_control == 0 || session->window_size > 0) && stream->deferred_data != NULL && diff --git a/lib/nghttp2_stream.c b/lib/nghttp2_stream.c index 3b7bc474..757ba0d9 100644 --- a/lib/nghttp2_stream.c +++ b/lib/nghttp2_stream.c @@ -31,7 +31,8 @@ void nghttp2_stream_init(nghttp2_stream *stream, int32_t stream_id, nghttp2_stream_state initial_state, uint8_t remote_flow_control, uint8_t local_flow_control, - int32_t initial_window_size, + int32_t remote_initial_window_size, + int32_t local_initial_window_size, void *stream_user_data) { nghttp2_map_entry_init(&stream->map_entry, stream_id); @@ -45,7 +46,8 @@ void nghttp2_stream_init(nghttp2_stream *stream, int32_t stream_id, stream->deferred_flags = NGHTTP2_DEFERRED_NONE; stream->remote_flow_control = remote_flow_control; stream->local_flow_control = local_flow_control; - stream->window_size = initial_window_size; + stream->remote_window_size = remote_initial_window_size; + stream->local_window_size = local_initial_window_size; stream->recv_window_size = 0; } @@ -75,12 +77,20 @@ void nghttp2_stream_detach_deferred_data(nghttp2_stream *stream) stream->deferred_flags = NGHTTP2_DEFERRED_NONE; } -void nghttp2_stream_update_initial_window_size(nghttp2_stream *stream, - int32_t new_initial_window_size, - int32_t old_initial_window_size) +int nghttp2_stream_update_remote_initial_window_size +(nghttp2_stream *stream, + int32_t new_initial_window_size, + int32_t old_initial_window_size) { - stream->window_size = - new_initial_window_size-(old_initial_window_size-stream->window_size); + int64_t new_window_size = (int64_t)stream->remote_window_size + + new_initial_window_size - old_initial_window_size; + if(INT32_MIN > new_window_size || + new_window_size > NGHTTP2_MAX_WINDOW_SIZE) { + return -1; + } + stream->remote_window_size += + new_initial_window_size - old_initial_window_size; + return 0; } void nghttp2_stream_promise_fulfilled(nghttp2_stream *stream) diff --git a/lib/nghttp2_stream.h b/lib/nghttp2_stream.h index e38557be..f0613891 100644 --- a/lib/nghttp2_stream.h +++ b/lib/nghttp2_stream.h @@ -109,12 +109,16 @@ typedef struct { flow control options off or sending WINDOW_UPDATE with END_FLOW_CONTROL bit set. */ uint8_t local_flow_control; - /* Current sender window size. This value is computed against the + /* Current remote window size. This value is computed against the current initial window size of remote endpoint. */ - int32_t window_size; + int32_t remote_window_size; /* Keep track of the number of bytes received without WINDOW_UPDATE. */ int32_t recv_window_size; + /* window size for local window control. It is initially set to + NGHTTP2_INITIAL_WINDOW_SIZE and could be increased/decreased by + submitting WINDOW_UPDATE. See nghttp2_submit_window_update(). */ + int32_t local_window_size; } nghttp2_stream; void nghttp2_stream_init(nghttp2_stream *stream, int32_t stream_id, @@ -122,7 +126,8 @@ void nghttp2_stream_init(nghttp2_stream *stream, int32_t stream_id, nghttp2_stream_state initial_state, uint8_t remote_flow_control, uint8_t local_flow_control, - int32_t initial_window_size, + int32_t remote_initial_window_size, + int32_t local_initial_window_size, void *stream_user_data); void nghttp2_stream_free(nghttp2_stream *stream); @@ -149,13 +154,17 @@ void nghttp2_stream_defer_data(nghttp2_stream *stream, void nghttp2_stream_detach_deferred_data(nghttp2_stream *stream); /* - * Updates the initial window size with the new value + * Updates the remote window size with the new value * |new_initial_window_size|. The |old_initial_window_size| is used to * calculate the current window size. + * + * This function returns 0 if it succeeds or -1. The failure is due to + * overflow. */ -void nghttp2_stream_update_initial_window_size(nghttp2_stream *stream, - int32_t new_initial_window_size, - int32_t old_initial_window_size); +int nghttp2_stream_update_remote_initial_window_size +(nghttp2_stream *stream, + int32_t new_initial_window_size, + int32_t old_initial_window_size); /* * Call this function if promised stream |stream| is replied with diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 93dd4833..4aab5292 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -906,8 +906,8 @@ void test_nghttp2_session_on_settings_received(void) NGHTTP2_STREAM_OPENING, NULL); /* Set window size for each streams and will see how settings updates these values */ - stream1->window_size = 16*1024; - stream2->window_size = -48*1024; + stream1->remote_window_size = 16*1024; + stream2->remote_window_size = -48*1024; nghttp2_frame_settings_init(&frame.settings, dup_iv(iv, niv), niv); @@ -919,15 +919,15 @@ void test_nghttp2_session_on_settings_received(void) CU_ASSERT(1 == session->remote_settings[NGHTTP2_SETTINGS_FLOW_CONTROL_OPTIONS]); - CU_ASSERT(64*1024 == stream1->window_size); - CU_ASSERT(0 == stream2->window_size); + CU_ASSERT(64*1024 == stream1->remote_window_size); + CU_ASSERT(0 == stream2->remote_window_size); frame.settings.iv[2].value = 16*1024; CU_ASSERT(0 == nghttp2_session_on_settings_received(session, &frame)); - CU_ASSERT(16*1024 == stream1->window_size); - CU_ASSERT(-48*1024 == stream2->window_size); + CU_ASSERT(16*1024 == stream1->remote_window_size); + CU_ASSERT(-48*1024 == stream2->remote_window_size); CU_ASSERT(0 == stream1->remote_flow_control); CU_ASSERT(0 == stream2->remote_flow_control); @@ -1141,7 +1141,7 @@ void test_nghttp2_session_on_window_update_received(void) CU_ASSERT(0 == nghttp2_session_on_window_update_received(session, &frame)); CU_ASSERT(1 == user_data.frame_recv_cb_called); - CU_ASSERT(NGHTTP2_INITIAL_WINDOW_SIZE+16*1024 == stream->window_size); + CU_ASSERT(NGHTTP2_INITIAL_WINDOW_SIZE+16*1024 == stream->remote_window_size); data_item = malloc(sizeof(nghttp2_outbound_item)); memset(data_item, 0, sizeof(nghttp2_outbound_item)); @@ -1150,7 +1150,8 @@ void test_nghttp2_session_on_window_update_received(void) CU_ASSERT(0 == nghttp2_session_on_window_update_received(session, &frame)); CU_ASSERT(2 == user_data.frame_recv_cb_called); - CU_ASSERT(NGHTTP2_INITIAL_WINDOW_SIZE+16*1024*2 == stream->window_size); + CU_ASSERT(NGHTTP2_INITIAL_WINDOW_SIZE+16*1024*2 == + stream->remote_window_size); CU_ASSERT(NULL == stream->deferred_data); nghttp2_frame_window_update_free(&frame.window_update); @@ -2475,12 +2476,12 @@ void test_nghttp2_session_flow_control(void) /* Change initial window size to 16KiB. The window_size becomes negative. */ new_initial_window_size = 16*1024; - stream->window_size = new_initial_window_size- + stream->remote_window_size = new_initial_window_size- (session->remote_settings[NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE] - -stream->window_size); + - stream->remote_window_size); session->remote_settings[NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE] = new_initial_window_size; - CU_ASSERT(-48*1024 == stream->window_size); + CU_ASSERT(-48*1024 == stream->remote_window_size); /* Back 48KiB to stream window */ frame.hd.stream_id = 1;