From babfa414244217e7a124c529ba658e8fb4a02163 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 28 Nov 2014 01:01:33 +0900 Subject: [PATCH] Just ignore HEADERS with non-idle stream ID and not found in stream map --- lib/nghttp2_session.c | 16 ++++++++++++---- tests/nghttp2_session_test.c | 15 ++++++++++++++- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index d77ffa30..b1445952 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -2996,10 +2996,18 @@ int nghttp2_session_on_request_headers_received(nghttp2_session *session, if (!session_is_new_peer_stream_id(session, frame->hd.stream_id)) { /* The spec says if an endpoint receives a HEADERS with invalid stream ID, it MUST issue connection error with error code - PROTOCOL_ERROR */ - return session_inflate_handle_invalid_connection( - session, frame, NGHTTP2_PROTOCOL_ERROR, - "request HEADERS: invalid stream_id"); + PROTOCOL_ERROR. But we could get trailer HEADERS after we have + sent RST_STREAM to this stream and peer have not received it. + Then connection error is too harsh. It means that we only use + connection error if stream ID refers idle stream. OTherwise we + just ignore HEADERS for now. */ + if (session_detect_idle_stream(session, frame->hd.stream_id)) { + return session_inflate_handle_invalid_connection( + session, frame, NGHTTP2_PROTOCOL_ERROR, + "request HEADERS: invalid stream_id"); + } + + return NGHTTP2_ERR_IGN_HEADER_BLOCK; } session->last_recv_stream_id = frame->hd.stream_id; diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 7a1a109a..444f2e70 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -1723,11 +1723,24 @@ void test_nghttp2_session_on_request_headers_received(void) { NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS; /* Stream ID less than or equal to the previouly received request - HEADERS leads to connection error */ + HEADERS is just ignored due to race condition */ nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_PRIORITY, 3, NGHTTP2_HCAT_HEADERS, NULL, NULL, 0); user_data.invalid_frame_recv_cb_called = 0; + CU_ASSERT(NGHTTP2_ERR_IGN_HEADER_BLOCK == + nghttp2_session_on_request_headers_received(session, &frame)); + CU_ASSERT(0 == user_data.invalid_frame_recv_cb_called); + CU_ASSERT(0 == (session->goaway_flags & NGHTTP2_GOAWAY_FAIL_ON_SEND)); + + nghttp2_frame_headers_free(&frame.headers); + + /* Stream ID is our side and it is idle stream ID, then treat it as + connection error */ + nghttp2_frame_headers_init(&frame.headers, + NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_PRIORITY, + 2, NGHTTP2_HCAT_HEADERS, NULL, NULL, 0); + user_data.invalid_frame_recv_cb_called = 0; CU_ASSERT(NGHTTP2_ERR_IGN_HEADER_BLOCK == nghttp2_session_on_request_headers_received(session, &frame)); CU_ASSERT(1 == user_data.invalid_frame_recv_cb_called);