diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index cfa71f8a..c401aabb 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -3840,52 +3840,13 @@ int nghttp2_session_on_request_headers_received(nghttp2_session *session, * implementation since client is not wrong if it did not get * RST_STREAM when it issued trailer HEADERS. * - * For server session, we remember closed streams as long as the - * sum of closed streams and opened streams are under current max - * concurrent streams. We can use these closed streams to detect - * the error in some cases. - * - * If the stream cannot be found in either closed or opened - * streams, it is considered to be closed, or it has not exist - * (e.g., peer skipped sending the stream). Actually, it is - * impossible to detect which is which, since that information was - * lost forever. For these cases, we send back GOAWAY with - * PROTOCOL_ERROR. - * - * If the stream is found, and we know that it is in half closed - * (remote), or closed by peer's explicit action (e.g., received - * RST_STREAM from peer, or peer sends HEADERS/DATA frame with - * END_STREAM), getting new frame on that stream is clearly error. - * In this case, we send GOAWAY with error code STREAM_CLOSED. - * - * There is one corner case here. Server can change the max - * concurrent streams. The initial value of max concurrent - * streams is unlimited (NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS, - * which is UINT32_MAX). When sending out SETTINGS with - * MAX_CONCURRENT_STREAMS, we save its value as pending max - * concurrent streams, and use it as a cap to remember closed - * stream to save memory. This means that we might not sure that - * stream surely closed or has not exist when it is not found in - * closed or opened stream. To workaround this issue, we ignore - * incoming frame if the current max concurrent streams is - * NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS, and pending max - * concurrent streams is less than that. + * At the moment, we are very conservative here. We only use + * connection error if stream ID refers idle stream, or we are + * sure that stream is half-closed(remote) or closed. Otherwise + * we just ignore HEADERS for now. */ stream = nghttp2_session_get_stream_raw(session, frame->hd.stream_id); - - if (!stream) { - if (session->local_settings.max_concurrent_streams == - NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS && - session->pending_local_max_concurrent_stream < - NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS) { - return NGHTTP2_ERR_IGN_HEADER_BLOCK; - } - - return session_inflate_handle_invalid_connection( - session, frame, NGHTTP2_ERR_PROTO, "HEADERS: stream does not exist"); - } - - if (stream->shut_flags & NGHTTP2_SHUT_RD) { + if (stream && (stream->shut_flags & NGHTTP2_SHUT_RD)) { return session_inflate_handle_invalid_connection( session, frame, NGHTTP2_ERR_STREAM_CLOSED, "HEADERS: stream closed"); } @@ -5159,25 +5120,7 @@ static int session_on_data_received_fail_fast(nghttp2_session *session) { stream = nghttp2_session_get_stream(session, stream_id); if (!stream) { stream = nghttp2_session_get_stream_raw(session, stream_id); - - if (!stream) { - if (session->server) { - if (session->local_settings.max_concurrent_streams == - NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS && - session->pending_local_max_concurrent_stream < - NGHTTP2_DEFAULT_MAX_CONCURRENT_STREAMS) { - return NGHTTP2_ERR_IGN_PAYLOAD; - } - - failure_reason = "DATA: stream does not exist"; - error_code = NGHTTP2_PROTOCOL_ERROR; - goto fail; - } - - return NGHTTP2_ERR_IGN_PAYLOAD; - } - - if (stream->shut_flags & NGHTTP2_SHUT_RD) { + if (stream && (stream->shut_flags & NGHTTP2_SHUT_RD)) { failure_reason = "DATA: stream closed"; error_code = NGHTTP2_STREAM_CLOSED; goto fail; diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 31d47d1a..de6c557a 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -9971,8 +9971,9 @@ void test_nghttp2_session_removed_closed_stream(void) { prepare_session_removed_closed_stream(session, &deflater); - /* Now current max concurrent streams is 2, so receiving frame on - stream 3 is treated as connection error */ + /* Now current max concurrent streams is 2. Receiving frame on + stream 3 is ignored because we have no stream object for stream + 3. */ nghttp2_bufs_reset(&bufs); rv = pack_headers(&bufs, &deflater, 3, NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_END_STREAM, @@ -9987,9 +9988,7 @@ void test_nghttp2_session_removed_closed_stream(void) { item = nghttp2_session_get_next_ob_item(session); - CU_ASSERT(NULL != item); - CU_ASSERT(NGHTTP2_GOAWAY == item->frame.hd.type); - CU_ASSERT(NGHTTP2_PROTOCOL_ERROR == item->frame.goaway.error_code); + CU_ASSERT(NULL == item); nghttp2_hd_deflate_free(&deflater); nghttp2_session_del(session); @@ -10012,9 +10011,7 @@ void test_nghttp2_session_removed_closed_stream(void) { item = nghttp2_session_get_next_ob_item(session); - CU_ASSERT(NULL != item); - CU_ASSERT(NGHTTP2_GOAWAY == item->frame.hd.type); - CU_ASSERT(NGHTTP2_PROTOCOL_ERROR == item->frame.goaway.error_code); + CU_ASSERT(NULL == item); nghttp2_hd_deflate_free(&deflater); nghttp2_session_del(session);