From d978f351dad7c930e2c601f902b8430ea9a7b584 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 11 Apr 2019 10:14:52 +0900 Subject: [PATCH] Fix bug that on_header callback is still called after stream is closed --- lib/nghttp2_session.c | 98 ++++++++++++++++++------------------ tests/main.c | 2 + tests/nghttp2_session_test.c | 85 +++++++++++++++++++++++++++++++ tests/nghttp2_session_test.h | 1 + 4 files changed, 138 insertions(+), 48 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index ef4932af..33d98766 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -3619,71 +3619,73 @@ static int inflate_header_block(nghttp2_session *session, nghttp2_frame *frame, if (call_header_cb && (inflate_flags & NGHTTP2_HD_INFLATE_EMIT)) { rv = 0; - if (subject_stream && session_enforce_http_messaging(session)) { - rv = nghttp2_http_on_header(session, subject_stream, frame, &nv, - trailer); + if (subject_stream) { + if (session_enforce_http_messaging(session)) { + rv = nghttp2_http_on_header(session, subject_stream, frame, &nv, + trailer); - if (rv == NGHTTP2_ERR_IGN_HTTP_HEADER) { - /* Don't overwrite rv here */ - int rv2; + if (rv == NGHTTP2_ERR_IGN_HTTP_HEADER) { + /* Don't overwrite rv here */ + int rv2; - rv2 = session_call_on_invalid_header(session, frame, &nv); - if (rv2 == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) { - rv = NGHTTP2_ERR_HTTP_HEADER; - } else { - if (rv2 != 0) { - return rv2; + rv2 = session_call_on_invalid_header(session, frame, &nv); + if (rv2 == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) { + rv = NGHTTP2_ERR_HTTP_HEADER; + } else { + if (rv2 != 0) { + return rv2; + } + + /* header is ignored */ + DEBUGF("recv: HTTP ignored: type=%u, id=%d, header %.*s: %.*s\n", + frame->hd.type, frame->hd.stream_id, (int)nv.name->len, + nv.name->base, (int)nv.value->len, nv.value->base); + + rv2 = session_call_error_callback( + session, NGHTTP2_ERR_HTTP_HEADER, + "Ignoring received invalid HTTP header field: frame type: " + "%u, stream: %d, name: [%.*s], value: [%.*s]", + frame->hd.type, frame->hd.stream_id, (int)nv.name->len, + nv.name->base, (int)nv.value->len, nv.value->base); + + if (nghttp2_is_fatal(rv2)) { + return rv2; + } } + } - /* header is ignored */ - DEBUGF("recv: HTTP ignored: type=%u, id=%d, header %.*s: %.*s\n", + if (rv == NGHTTP2_ERR_HTTP_HEADER) { + DEBUGF("recv: HTTP error: type=%u, id=%d, header %.*s: %.*s\n", frame->hd.type, frame->hd.stream_id, (int)nv.name->len, nv.name->base, (int)nv.value->len, nv.value->base); - rv2 = session_call_error_callback( + rv = session_call_error_callback( session, NGHTTP2_ERR_HTTP_HEADER, - "Ignoring received invalid HTTP header field: frame type: " + "Invalid HTTP header field was received: frame type: " "%u, stream: %d, name: [%.*s], value: [%.*s]", frame->hd.type, frame->hd.stream_id, (int)nv.name->len, nv.name->base, (int)nv.value->len, nv.value->base); - if (nghttp2_is_fatal(rv2)) { - return rv2; + if (nghttp2_is_fatal(rv)) { + return rv; } + + rv = session_handle_invalid_stream2(session, + subject_stream->stream_id, + frame, NGHTTP2_ERR_HTTP_HEADER); + if (nghttp2_is_fatal(rv)) { + return rv; + } + return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; } } - - if (rv == NGHTTP2_ERR_HTTP_HEADER) { - DEBUGF("recv: HTTP error: type=%u, id=%d, header %.*s: %.*s\n", - frame->hd.type, frame->hd.stream_id, (int)nv.name->len, - nv.name->base, (int)nv.value->len, nv.value->base); - - rv = session_call_error_callback( - session, NGHTTP2_ERR_HTTP_HEADER, - "Invalid HTTP header field was received: frame type: " - "%u, stream: %d, name: [%.*s], value: [%.*s]", - frame->hd.type, frame->hd.stream_id, (int)nv.name->len, - nv.name->base, (int)nv.value->len, nv.value->base); - - if (nghttp2_is_fatal(rv)) { + if (rv == 0) { + rv = session_call_on_header(session, frame, &nv); + /* This handles NGHTTP2_ERR_PAUSE and + NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE as well */ + if (rv != 0) { return rv; } - - rv = - session_handle_invalid_stream2(session, subject_stream->stream_id, - frame, NGHTTP2_ERR_HTTP_HEADER); - if (nghttp2_is_fatal(rv)) { - return rv; - } - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } - } - if (rv == 0) { - rv = session_call_on_header(session, frame, &nv); - /* This handles NGHTTP2_ERR_PAUSE and - NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE as well */ - if (rv != 0) { - return rv; } } } diff --git a/tests/main.c b/tests/main.c index 13865de7..bb6e8b6c 100644 --- a/tests/main.c +++ b/tests/main.c @@ -91,6 +91,8 @@ int main() { test_nghttp2_session_recv_headers_with_padding) || !CU_add_test(pSuite, "session_recv_headers_early_response", test_nghttp2_session_recv_headers_early_response) || + !CU_add_test(pSuite, "session_recv_headers_for_closed_stream", + test_nghttp2_session_recv_headers_for_closed_stream) || !CU_add_test(pSuite, "session_server_recv_push_response", test_nghttp2_session_server_recv_push_response) || !CU_add_test(pSuite, "session_recv_premature_headers", diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 296fb9d1..09f7576b 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -1743,6 +1743,91 @@ void test_nghttp2_session_recv_headers_early_response(void) { nghttp2_bufs_free(&bufs); } +void test_nghttp2_session_recv_headers_for_closed_stream(void) { + nghttp2_session *session; + nghttp2_session_callbacks callbacks; + nghttp2_nv *nva; + size_t nvlen; + nghttp2_frame frame; + nghttp2_bufs bufs; + nghttp2_buf *buf; + ssize_t rv; + my_user_data ud; + nghttp2_hd_deflater deflater; + nghttp2_stream *stream; + nghttp2_mem *mem; + const uint8_t *data; + + mem = nghttp2_mem_default(); + frame_pack_bufs_init(&bufs); + + memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); + callbacks.on_frame_recv_callback = on_frame_recv_callback; + callbacks.on_header_callback = on_header_callback; + + nghttp2_session_server_new(&session, &callbacks, &ud); + + nghttp2_hd_deflate_init(&deflater, mem); + + /* Make sure that on_header callback never be invoked for closed + stream */ + nvlen = ARRLEN(reqnv); + nghttp2_nv_array_copy(&nva, reqnv, nvlen, mem); + + nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, 1, + NGHTTP2_HCAT_HEADERS, NULL, nva, nvlen); + + rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater); + + CU_ASSERT(0 == rv); + CU_ASSERT(nghttp2_bufs_len(&bufs) > 0); + + nghttp2_frame_headers_free(&frame.headers, mem); + + buf = &bufs.head->buf; + assert(nghttp2_bufs_len(&bufs) == nghttp2_buf_len(buf)); + + ud.header_cb_called = 0; + ud.frame_recv_cb_called = 0; + + rv = nghttp2_session_mem_recv(session, buf->pos, NGHTTP2_FRAME_HDLEN); + + CU_ASSERT(NGHTTP2_FRAME_HDLEN == rv); + CU_ASSERT(0 == ud.header_cb_called); + CU_ASSERT(0 == ud.frame_recv_cb_called); + + stream = nghttp2_session_get_stream(session, 1); + + CU_ASSERT(NULL != stream); + + rv = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, 1, + NGHTTP2_NO_ERROR); + + CU_ASSERT(0 == rv); + + rv = nghttp2_session_mem_send(session, &data); + + CU_ASSERT(rv > 0); + + stream = nghttp2_session_get_stream(session, 1); + + CU_ASSERT(NULL == stream); + + ud.header_cb_called = 0; + ud.frame_recv_cb_called = 0; + + rv = nghttp2_session_mem_recv(session, buf->pos + NGHTTP2_FRAME_HDLEN, + nghttp2_buf_len(buf) - NGHTTP2_FRAME_HDLEN); + + CU_ASSERT((ssize_t)nghttp2_buf_len(buf) - NGHTTP2_FRAME_HDLEN == rv); + CU_ASSERT(0 == ud.header_cb_called); + CU_ASSERT(0 == ud.frame_recv_cb_called); + + nghttp2_bufs_free(&bufs); + nghttp2_hd_deflate_free(&deflater); + nghttp2_session_del(session); +} + void test_nghttp2_session_server_recv_push_response(void) { nghttp2_session *session; nghttp2_session_callbacks callbacks; diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h index 35a48b82..e872c5d0 100644 --- a/tests/nghttp2_session_test.h +++ b/tests/nghttp2_session_test.h @@ -39,6 +39,7 @@ void test_nghttp2_session_recv_continuation(void); void test_nghttp2_session_recv_headers_with_priority(void); void test_nghttp2_session_recv_headers_with_padding(void); void test_nghttp2_session_recv_headers_early_response(void); +void test_nghttp2_session_recv_headers_for_closed_stream(void); void test_nghttp2_session_server_recv_push_response(void); void test_nghttp2_session_recv_premature_headers(void); void test_nghttp2_session_recv_unknown_frame(void);