Skip to content

N
nghttp2
Commit a804117c authored by Tatsuhiro Tsujikawa's avatar Tatsuhiro Tsujikawa
Browse files
Options

Fix GOAWAY handling 

On reception of GOAWAY, new stream creation is disallowed regardless
of last-stream-id in GOAWAY is larger than next stream ID.
parent 2b14e4a6
Hide whitespace changes
Inline Side-by-side
Showing with 32 additions and 42 deletions
lib/nghttp2_session.c
View file @ a804117c
......@@ -1248,11 +1248,10 @@ static int stream_predicate_for_send(nghttp2_stream *stream) {
*/
static int session_predicate_request_headers_send(nghttp2_session *session,
nghttp2_headers *frame) {
/* If we are terminating session (session->goaway_flag is nonzero),
we don't send new request. Also if received last_stream_id is
strictly less than new stream ID, cancel its transmission. */
if (session->goaway_flags ||
session->remote_last_stream_id < frame->hd.stream_id) {
/* If we are terminating session (NGHTTP2_GOAWAY_TERM_ON_SEND) or
GOAWAY was received from peer, new request is not allowed. */
if (session->goaway_flags &
(NGHTTP2_GOAWAY_TERM_ON_SEND | NGHTTP2_GOAWAY_RECV)) {
return NGHTTP2_ERR_START_STREAM_NOT_ALLOWED;
}
return 0;
......@@ -1429,7 +1428,8 @@ static int session_predicate_push_promise_send(nghttp2_session *session,
if (stream->state == NGHTTP2_STREAM_CLOSING) {
return NGHTTP2_ERR_STREAM_CLOSING;
}
if (session->remote_last_stream_id < promised_stream_id) {
if (session->goaway_flags &
(NGHTTP2_GOAWAY_TERM_ON_SEND | NGHTTP2_GOAWAY_RECV)) {
return NGHTTP2_ERR_START_STREAM_NOT_ALLOWED;
}
return 0;
......@@ -2345,6 +2345,8 @@ static int session_after_frame_sent1(nghttp2_session *session) {
session->goaway_flags |= NGHTTP2_GOAWAY_TERM_SENT;
}
session->goaway_flags |= NGHTTP2_GOAWAY_SENT;
rv = session_close_stream_on_goaway(session, frame->goaway.last_stream_id,
1);
......@@ -3227,8 +3229,8 @@ int nghttp2_session_on_request_headers_received(nghttp2_session *session,
}
session->last_recv_stream_id = frame->hd.stream_id;
if (session->local_last_stream_id < frame->hd.stream_id) {
/* We just ignore stream rather than local_last_stream_id */
if (session->goaway_flags & NGHTTP2_GOAWAY_SENT) {
/* We just ignore stream after GOAWAY was queued */
return NGHTTP2_ERR_IGN_HEADER_BLOCK;
}
......@@ -4038,6 +4040,8 @@ int nghttp2_session_on_goaway_received(nghttp2_session *session,
"GOAWAY: invalid last_stream_id");
}
session->goaway_flags |= NGHTTP2_GOAWAY_RECV;
session->remote_last_stream_id = frame->goaway.last_stream_id;
rv = session_call_on_frame_received(session, frame);
......@@ -5615,10 +5619,9 @@ static size_t session_get_num_active_streams(nghttp2_session *session) {
int nghttp2_session_want_read(nghttp2_session *session) {
size_t num_active_streams;
/* If these flags are set, we don't want to read. The application
/* If this flag is set, we don't want to read. The application
should drop the connection. */
if ((session->goaway_flags & NGHTTP2_GOAWAY_TERM_ON_SEND) &&
(session->goaway_flags & NGHTTP2_GOAWAY_TERM_SENT)) {
if (session->goaway_flags & NGHTTP2_GOAWAY_TERM_SENT) {
return 0;
}
......@@ -5631,26 +5634,18 @@ int nghttp2_session_want_read(nghttp2_session *session) {
return 1;
}
/* If there is no active streams, we check last_stream_id peer sent
to us. Here is the asymmetry between server and client. For
server, if client cannot make new request, current conneciton is
no use. Server push might be going, but it is idempotent and can
be safely retried with the new connection. For client side, the
theory is the same. */
if (session->server) {
return session->local_last_stream_id > session->last_recv_stream_id;
} else {
return (uint32_t)session->remote_last_stream_id >= session->next_stream_id;
}
/* If there is no active streams and GOAWAY has been sent or
received, we are done with this session. */
return (session->goaway_flags &
(NGHTTP2_GOAWAY_SENT | NGHTTP2_GOAWAY_RECV)) == 0;
}
int nghttp2_session_want_write(nghttp2_session *session) {
size_t num_active_streams;
/* If these flags are set, we don't want to write any data. The
/* If these flag is set, we don't want to write any data. The
application should drop the connection. */
if ((session->goaway_flags & NGHTTP2_GOAWAY_TERM_ON_SEND) &&
(session->goaway_flags & NGHTTP2_GOAWAY_TERM_SENT)) {
if (session->goaway_flags & NGHTTP2_GOAWAY_TERM_SENT) {
return 0;
}
......@@ -5675,17 +5670,10 @@ int nghttp2_session_want_write(nghttp2_session *session) {
return 1;
}
/* If there is no active streams, we check last_stream_id peer sent
to us. Here is the asymmetry between server and client. For
server, if client cannot make new request, current conneciton is
no use. Server push might be going, but it is idempotent and can
be safely retried with the new connection. For client side, the
theory is the same. */
if (session->server) {
return session->local_last_stream_id > session->last_recv_stream_id;
} else {
return (uint32_t)session->remote_last_stream_id >= session->next_stream_id;
}
/* If there is no active streams and GOAWAY has been sent or
received, we are done with this session. */
return (session->goaway_flags &
(NGHTTP2_GOAWAY_SENT | NGHTTP2_GOAWAY_RECV)) == 0;
}
int nghttp2_session_add_ping(nghttp2_session *session, uint8_t flags,
......
lib/nghttp2_session.h
View file @ a804117c
......@@ -130,6 +130,10 @@ typedef enum {
NGHTTP2_GOAWAY_TERM_ON_SEND = 0x1,
/* Flag means GOAWAY to terminate session has been sent */
NGHTTP2_GOAWAY_TERM_SENT = 0x2,
/* Flag means GOAWAY was sent */
NGHTTP2_GOAWAY_SENT = 0x4,
/* Flag means GOAWAY was received */
NGHTTP2_GOAWAY_RECV = 0x8,
} nghttp2_goaway_flag;
struct nghttp2_session {
......
tests/nghttp2_session_test.c
View file @ a804117c
......@@ -1756,11 +1756,11 @@ void test_nghttp2_session_on_request_headers_received(void) {
nghttp2_frame_headers_free(&frame.headers, mem);
/* Stream ID which is greater than local_last_stream_id is
ignored */
/* If GOAWAY has been sent, new stream is ignored */
nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, 5,
NGHTTP2_HCAT_REQUEST, NULL, NULL, 0);
session->goaway_flags |= NGHTTP2_GOAWAY_SENT;
user_data.invalid_frame_recv_cb_called = 0;
CU_ASSERT(NGHTTP2_ERR_IGN_HEADER_BLOCK ==
nghttp2_session_on_request_headers_received(session, &frame));
......@@ -4847,10 +4847,8 @@ void test_nghttp2_session_on_ctrl_not_send(void) {
NULL, 0, NULL));
user_data.frame_not_send_cb_called = 0;
/* Terminating session */
CU_ASSERT(0 == nghttp2_session_add_goaway(session, 0, NGHTTP2_NO_ERROR, NULL,
0, 1));
/* GOAWAY received */
session->goaway_flags |= NGHTTP2_GOAWAY_RECV;
session->next_stream_id = 9;
CU_ASSERT(0 < nghttp2_submit_headers(session, NGHTTP2_FLAG_END_STREAM, -1,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment