From 8716dd05d44f3b4cf0ff719240297cec57359815 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 25 Dec 2015 21:37:18 +0900 Subject: [PATCH] Return error from nghttp2_submit_{headers,request} when self dependency is made Return NGHTTP2_ERR_INVALID_ARGUMENT from nghttp2_submit_headers() if given stream ID and pri_spec->stream_id are the same (thus trying to depend on itself). Also return NGHTTP2_ERR_INVALID_ARGUMENT from nghttp2_submit_request() and nghttp2_submit_headers() with stream_id == 1, when new stream ID equals to pri_spec->stream_id. Previously, these cases are not checked, and just sent to peer. --- lib/includes/nghttp2/nghttp2.h | 6 +++++- lib/nghttp2_submit.c | 10 ++++++++++ tests/nghttp2_session_test.c | 26 ++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 5b7d8a28..cc4844de 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -3161,6 +3161,9 @@ nghttp2_priority_spec_check_default(const nghttp2_priority_spec *pri_spec); * :enum:`NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE` * No stream ID is available because maximum stream ID was * reached. + * :enum:`NGHTTP2_ERR_INVALID_ARGUMENT` + * Trying to depend on itself (new stream ID equals + * ``pri_spec->stream_id``). * * .. warning:: * @@ -3371,7 +3374,8 @@ NGHTTP2_EXTERN int nghttp2_submit_trailer(nghttp2_session *session, * No stream ID is available because maximum stream ID was * reached. * :enum:`NGHTTP2_ERR_INVALID_ARGUMENT` - * The |stream_id| is 0. + * The |stream_id| is 0; or trying to depend on itself (stream ID + * equals ``pri_spec->stream_id``). * :enum:`NGHTTP2_ERR_DATA_EXIST` * DATA or HEADERS has been already submitted and not fully * processed yet. This happens if stream denoted by |stream_id| diff --git a/lib/nghttp2_submit.c b/lib/nghttp2_submit.c index 2800b60e..67ecf4f6 100644 --- a/lib/nghttp2_submit.c +++ b/lib/nghttp2_submit.c @@ -55,6 +55,16 @@ static int32_t submit_headers_shared(nghttp2_session *session, uint8_t flags, goto fail; } + if (stream_id == -1) { + if ((int32_t)session->next_stream_id == pri_spec->stream_id) { + rv = NGHTTP2_ERR_INVALID_ARGUMENT; + goto fail; + } + } else if (stream_id == pri_spec->stream_id) { + rv = NGHTTP2_ERR_INVALID_ARGUMENT; + goto fail; + } + item = nghttp2_mem_malloc(mem, sizeof(nghttp2_outbound_item)); if (item == NULL) { rv = NGHTTP2_ERR_NOMEM; diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 6fb2768e..c8ed91ef 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -3965,6 +3965,7 @@ void test_nghttp2_submit_request_without_data(void) { nva_out out; nghttp2_bufs bufs; nghttp2_mem *mem; + nghttp2_priority_spec pri_spec; mem = nghttp2_mem_default(); frame_pack_bufs_init(&bufs); @@ -3998,6 +3999,15 @@ void test_nghttp2_submit_request_without_data(void) { nghttp2_bufs_free(&bufs); nghttp2_hd_inflate_free(&inflater); + + /* Try to depend on itself is error */ + nghttp2_priority_spec_init(&pri_spec, (int32_t)session->next_stream_id, 16, + 0); + + CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == + nghttp2_submit_request(session, &pri_spec, reqnv, ARRLEN(reqnv), + NULL, NULL)); + nghttp2_session_del(session); } @@ -4289,6 +4299,7 @@ void test_nghttp2_submit_headers(void) { nva_out out; nghttp2_bufs bufs; nghttp2_mem *mem; + nghttp2_priority_spec pri_spec; mem = nghttp2_mem_default(); frame_pack_bufs_init(&bufs); @@ -4343,6 +4354,21 @@ void test_nghttp2_submit_headers(void) { nghttp2_frame_headers_free(&frame.headers, mem); nghttp2_hd_inflate_free(&inflater); + + /* Try to depend on itself */ + nghttp2_priority_spec_init(&pri_spec, 3, 16, 0); + + CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == + nghttp2_submit_headers(session, NGHTTP2_FLAG_NONE, 3, &pri_spec, + reqnv, ARRLEN(reqnv), NULL)); + + session->next_stream_id = 5; + nghttp2_priority_spec_init(&pri_spec, 5, 16, 0); + + CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == + nghttp2_submit_headers(session, NGHTTP2_FLAG_NONE, -1, &pri_spec, + reqnv, ARRLEN(reqnv), NULL)); + nghttp2_session_del(session); }