mirror of
https://github.com/nghttp2/nghttp2.git
synced 2025-12-07 18:48:54 +08:00
Cancel sending RST_STREAM if stream is not found
nghttp2_submit_rst_stream is intended to send RST_STREAM frame to the existing stream. Actually, nghttp2_session_add_rst_stream_continue does not add RST_STREAM if the stream is not found. There is a situation that the stream exists when nghttp2_submit_rst_stream is called, but it may be closed before actually sending RST_STREAM. Previously, we send the frame in this case hoping that this is noop on remote endpoint. This commit checks stream existence just before sending RST_STREAM, and if the stream is not found, cancel RST_STREAM. This is the consistent behavior of nghttp2_submit_rst_stream and fixes race condition.
This commit is contained in:
@@ -111,6 +111,10 @@ typedef struct {
|
||||
uint8_t flags;
|
||||
} nghttp2_goaway_aux_data;
|
||||
|
||||
typedef struct {
|
||||
int continue_without_stream;
|
||||
} nghttp2_rst_stream_aux_data;
|
||||
|
||||
/* struct used for extension frame */
|
||||
typedef struct {
|
||||
/* nonzero if this extension frame is serialized by library
|
||||
@@ -123,6 +127,7 @@ typedef union {
|
||||
nghttp2_data_aux_data data;
|
||||
nghttp2_headers_aux_data headers;
|
||||
nghttp2_goaway_aux_data goaway;
|
||||
nghttp2_rst_stream_aux_data rst_stream;
|
||||
nghttp2_ext_aux_data ext;
|
||||
} nghttp2_aux_data;
|
||||
|
||||
|
||||
@@ -1189,6 +1189,9 @@ int nghttp2_session_add_rst_stream_continue(nghttp2_session *session,
|
||||
frame = &item->frame;
|
||||
|
||||
nghttp2_frame_rst_stream_init(&frame->rst_stream, stream_id, error_code);
|
||||
|
||||
item->aux_data.rst_stream.continue_without_stream = continue_without_stream;
|
||||
|
||||
rv = nghttp2_session_add_item(session, item);
|
||||
if (rv != 0) {
|
||||
nghttp2_frame_rst_stream_free(&frame->rst_stream);
|
||||
@@ -2141,6 +2144,12 @@ static int session_prep_frame(nghttp2_session *session,
|
||||
if (session_is_closing(session)) {
|
||||
return NGHTTP2_ERR_SESSION_CLOSING;
|
||||
}
|
||||
|
||||
if (!item->aux_data.rst_stream.continue_without_stream &&
|
||||
!nghttp2_session_get_stream(session, frame->rst_stream.hd.stream_id)) {
|
||||
return NGHTTP2_ERR_STREAM_CLOSED;
|
||||
}
|
||||
|
||||
nghttp2_frame_pack_rst_stream(&session->aob.framebufs, &frame->rst_stream);
|
||||
return 0;
|
||||
case NGHTTP2_SETTINGS: {
|
||||
@@ -2856,8 +2865,12 @@ static nghttp2_ssize nghttp2_session_mem_send_internal(nghttp2_session *session,
|
||||
nghttp2_frame *frame = &item->frame;
|
||||
/* The library is responsible for the transmission of
|
||||
WINDOW_UPDATE frame, so we don't call error callback for
|
||||
it. */
|
||||
it. As for RST_STREAM, if it is not sent due to missing
|
||||
stream, we also do not call error callback because it may
|
||||
cause a lot of noises.*/
|
||||
if (frame->hd.type != NGHTTP2_WINDOW_UPDATE &&
|
||||
(frame->hd.type != NGHTTP2_RST_STREAM ||
|
||||
rv != NGHTTP2_ERR_STREAM_CLOSED) &&
|
||||
session->callbacks.on_frame_not_send_callback(
|
||||
session, frame, rv, session->user_data) != 0) {
|
||||
nghttp2_outbound_item_free(item, mem);
|
||||
|
||||
@@ -7043,8 +7043,13 @@ void test_nghttp2_submit_rst_stream(void) {
|
||||
int32_t stream_id;
|
||||
nghttp2_ssize datalen;
|
||||
const uint8_t *data;
|
||||
nghttp2_bufs bufs;
|
||||
nghttp2_buf *buf;
|
||||
nghttp2_frame frame;
|
||||
nghttp2_ssize nread;
|
||||
|
||||
memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
|
||||
frame_pack_bufs_init(&bufs);
|
||||
|
||||
/* Sending RST_STREAM to idle stream (local) is ignored */
|
||||
nghttp2_session_client_new(&session, &callbacks, NULL);
|
||||
@@ -7171,6 +7176,40 @@ void test_nghttp2_submit_rst_stream(void) {
|
||||
assert_null(item);
|
||||
|
||||
nghttp2_session_del(session);
|
||||
|
||||
/* Cancel sending RST_STREAM if stream is closed */
|
||||
nghttp2_session_client_new(&session, &callbacks, NULL);
|
||||
|
||||
open_sent_stream(session, 1);
|
||||
|
||||
rv =
|
||||
nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, 1, NGHTTP2_NO_ERROR);
|
||||
|
||||
assert_int(0, ==, rv);
|
||||
|
||||
item = nghttp2_outbound_queue_top(&session->ob_reg);
|
||||
|
||||
assert_not_null(item);
|
||||
assert_uint8(NGHTTP2_RST_STREAM, ==, item->frame.hd.type);
|
||||
assert_int32(1, ==, item->frame.hd.stream_id);
|
||||
|
||||
nghttp2_frame_rst_stream_init(&frame.rst_stream, 1, NGHTTP2_NO_ERROR);
|
||||
nghttp2_frame_pack_rst_stream(&bufs, &frame.rst_stream);
|
||||
nghttp2_frame_rst_stream_free(&frame.rst_stream);
|
||||
|
||||
buf = &bufs.head->buf;
|
||||
nread = nghttp2_session_mem_recv2(session, buf->pos, nghttp2_buf_len(buf));
|
||||
|
||||
assert_ptrdiff((nghttp2_ssize)nghttp2_buf_len(buf), ==, nread);
|
||||
assert_null(nghttp2_session_get_stream(session, 1));
|
||||
|
||||
datalen = nghttp2_session_mem_send2(session, &data);
|
||||
|
||||
assert_ptrdiff(0, ==, datalen);
|
||||
|
||||
nghttp2_session_del(session);
|
||||
|
||||
nghttp2_bufs_free(&bufs);
|
||||
}
|
||||
|
||||
void test_nghttp2_session_open_stream(void) {
|
||||
|
||||
Reference in New Issue
Block a user