From 505a300d93d8c6590c441e04b49f5eb33de02d44 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 7 Mar 2015 16:17:40 +0900 Subject: [PATCH] Refuse PUSH_PROMISE while unacked local ENABLE_PUSH is 0 After we sent SETTINGS including ENABLE_PUSH = 0, peer may already issue PUSH_PROMISE before receiving our SETTINGS and react it to SETTINGS ACK. Previously we accept this PUSH_PROMISE. In this commit, we check the pending ENABLE_PUSH value and if it means disabling push, we refuse PUSH_PROMISE with RST_STREAM of error REFUSED_STREAM. --- lib/nghttp2_session.c | 18 +++++++++++++++--- lib/nghttp2_session.h | 3 +++ tests/nghttp2_session_test.c | 20 ++++++++++++++++++++ 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 906b8580..0e5170bf 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -383,6 +383,7 @@ static int session_new(nghttp2_session **session_ptr, (*session_ptr)->pending_local_max_concurrent_stream = NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS; + (*session_ptr)->pending_enable_push = 1; if (server) { (*session_ptr)->server = 1; @@ -3901,6 +3902,7 @@ int nghttp2_session_update_local_settings(nghttp2_session *session, session->pending_local_max_concurrent_stream = NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS; + session->pending_enable_push = 1; return 0; } @@ -4129,7 +4131,8 @@ int nghttp2_session_on_push_promise_received(nghttp2_session *session, } session->last_recv_stream_id = frame->push_promise.promised_stream_id; stream = nghttp2_session_get_stream(session, frame->hd.stream_id); - if (!stream || stream->state == NGHTTP2_STREAM_CLOSING) { + if (!stream || stream->state == NGHTTP2_STREAM_CLOSING || + !session->pending_enable_push) { if (!stream) { if (session_detect_idle_stream(session, frame->hd.stream_id)) { return session_inflate_handle_invalid_connection( @@ -6105,8 +6108,10 @@ int nghttp2_session_add_settings(nghttp2_session *session, uint8_t flags, return rv; } - /* Extract NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS here and use - it to refuse the incoming streams with RST_STREAM. */ + /* Extract NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS and ENABLE_PUSH + here. We use it to refuse the incoming stream and PUSH_PROMISE + with RST_STREAM. */ + for (i = niv; i > 0; --i) { if (iv[i - 1].settings_id == NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS) { session->pending_local_max_concurrent_stream = iv[i - 1].value; @@ -6114,6 +6119,13 @@ int nghttp2_session_add_settings(nghttp2_session *session, uint8_t flags, } } + for (i = niv; i > 0; --i) { + if (iv[i - 1].settings_id == NGHTTP2_SETTINGS_ENABLE_PUSH) { + session->pending_enable_push = iv[i - 1].value; + break; + } + } + return 0; } diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h index 36939c16..505072d4 100644 --- a/lib/nghttp2_session.h +++ b/lib/nghttp2_session.h @@ -253,6 +253,9 @@ struct nghttp2_session { /* Unacked local SETTINGS_MAX_CONCURRENT_STREAMS value. We use this to refuse the incoming stream if it exceeds this value. */ uint32_t pending_local_max_concurrent_stream; + /* Unacked local ENABLE_PUSH value. We use this to refuse + PUSH_PROMISE before SETTINGS ACK is received. */ + uint8_t pending_enable_push; /* Nonzero if the session is server side. */ uint8_t server; /* Flags indicating GOAWAY is sent and/or recieved. The flags are diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 55bf3a74..ae8f7f5a 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -2201,6 +2201,7 @@ void test_nghttp2_session_on_push_promise_received(void) { nghttp2_nv *nva; size_t nvlen; nghttp2_mem *mem; + nghttp2_settings_entry iv = {NGHTTP2_SETTINGS_ENABLE_PUSH, 0}; mem = nghttp2_mem_default(); memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); @@ -2380,6 +2381,25 @@ void test_nghttp2_session_on_push_promise_received(void) { nghttp2_frame_push_promise_free(&frame.push_promise, mem); nghttp2_session_del(session); + + /* If local_settings.enable_push = 0 is pending, but not acked from + peer, incoming PUSH_PROMISE is rejected */ + nghttp2_session_client_new(&session, &callbacks, &user_data); + + stream = nghttp2_session_open_stream(session, 1, NGHTTP2_STREAM_FLAG_NONE, + &pri_spec_default, + NGHTTP2_STREAM_OPENING, NULL); + /* Submit settings with ENABLE_PUSH = 0 (thus disabling push) */ + nghttp2_submit_settings(session, NGHTTP2_FLAG_NONE, &iv, 1); + + nghttp2_frame_push_promise_init(&frame.push_promise, NGHTTP2_FLAG_END_HEADERS, + 1, 2, NULL, 0); + + CU_ASSERT(NGHTTP2_ERR_IGN_HEADER_BLOCK == + nghttp2_session_on_push_promise_received(session, &frame)); + + nghttp2_frame_push_promise_free(&frame.push_promise, mem); + nghttp2_session_del(session); } void test_nghttp2_session_on_ping_received(void) {