From 06dae79b28a5d60e5136af1fc555a91b33325cc8 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 29 Jan 2012 16:27:00 +0900 Subject: [PATCH] Discard inbound HEADERS and DATA in CLOSING state. Handle stream shutdown when DATA is received with FIN bit set. --- lib/spdylay_session.c | 60 ++++++++++++++++++++++----------- lib/spdylay_session.h | 7 ++++ tests/main.c | 2 ++ tests/spdylay_session_test.c | 64 ++++++++++++++++++++++++++++++++++++ tests/spdylay_session_test.h | 1 + 5 files changed, 114 insertions(+), 20 deletions(-) diff --git a/lib/spdylay_session.c b/lib/spdylay_session.c index 0f46d36..4b74be7 100644 --- a/lib/spdylay_session.c +++ b/lib/spdylay_session.c @@ -875,7 +875,7 @@ int spdylay_session_on_headers_received(spdylay_session *session, /* First we check readability from this stream. */ if(stream && (stream->shut_flags & SPDYLAY_SHUT_RD) == 0) { if(spdylay_session_is_my_stream_id(session, frame->headers.stream_id)) { - if(stream && stream->state == SPDYLAY_STREAM_OPENED) { + if(stream->state == SPDYLAY_STREAM_OPENED) { valid = 1; spdylay_session_call_on_ctrl_frame_received(session, SPDYLAY_HEADERS, frame); @@ -891,13 +891,17 @@ int spdylay_session_on_headers_received(spdylay_session *session, } } else { /* If this is remote peer initiated stream, it is OK unless it - have sent FIN frame already. */ + have sent FIN frame already. But if stream is in + SPDYLAY_STREAM_CLOSING, we discard the frame. This is a race + condition. */ valid = 1; - spdylay_session_call_on_ctrl_frame_received(session, SPDYLAY_HEADERS, - frame); - if(frame->headers.hd.flags & SPDYLAY_FLAG_FIN) { - spdylay_stream_shutdown(stream, SPDYLAY_SHUT_RD); - spdylay_session_close_stream_if_shut_rdwr(session, stream); + if(stream->state != SPDYLAY_STREAM_CLOSING) { + spdylay_session_call_on_ctrl_frame_received(session, SPDYLAY_HEADERS, + frame); + if(frame->headers.hd.flags & SPDYLAY_FLAG_FIN) { + spdylay_stream_shutdown(stream, SPDYLAY_SHUT_RD); + spdylay_session_close_stream_if_shut_rdwr(session, stream); + } } } } @@ -1000,20 +1004,14 @@ static int spdylay_session_process_ctrl_frame(spdylay_session *session) } } -static int spdylay_session_process_data_frame(spdylay_session *session) +int spdylay_session_on_data_received(spdylay_session *session, + uint8_t flags, int32_t length, + int32_t stream_id) { - int32_t stream_id; - uint8_t flags; - int32_t length; - spdylay_stream *stream; - int status_code = 0; int valid = 0; int r = 0; - stream_id = spdylay_get_uint32(session->iframe.headbuf) & - SPDYLAY_STREAM_ID_MASK; - flags = session->iframe.headbuf[4]; - length = spdylay_get_uint32(&session->iframe.headbuf[4]) & - SPDYLAY_LENGTH_MASK; + spdylay_status_code status_code = 0; + spdylay_stream *stream; stream = spdylay_session_get_stream(session, stream_id); if(stream) { if((stream->shut_flags & SPDYLAY_SHUT_RD) == 0) { @@ -1023,11 +1021,18 @@ static int spdylay_session_process_data_frame(spdylay_session *session) } else if(stream->state != SPDYLAY_STREAM_CLOSING) { status_code = SPDYLAY_PROTOCOL_ERROR; } - } else { + } else if(stream->state != SPDYLAY_STREAM_CLOSING) { /* It is OK if this is remote peer initiated stream and we did - not receive FIN. */ + not receive FIN unless stream is in SPDYLAY_STREAM_CLOSING + state. This is a race condition. */ valid = 1; } + if(valid) { + if(flags & SPDYLAY_FLAG_FIN) { + spdylay_stream_shutdown(stream, SPDYLAY_SHUT_RD); + spdylay_session_close_stream_if_shut_rdwr(session, stream); + } + } } else { status_code = SPDYLAY_PROTOCOL_ERROR; } @@ -1042,6 +1047,21 @@ static int spdylay_session_process_data_frame(spdylay_session *session) } else if(status_code != 0) { r = spdylay_session_add_rst_stream(session, stream_id, status_code); } + return r; +} + +static int spdylay_session_process_data_frame(spdylay_session *session) +{ + uint8_t flags; + int32_t length; + int32_t stream_id; + int r; + stream_id = spdylay_get_uint32(session->iframe.headbuf) & + SPDYLAY_STREAM_ID_MASK; + flags = session->iframe.headbuf[4]; + length = spdylay_get_uint32(&session->iframe.headbuf[4]) & + SPDYLAY_LENGTH_MASK; + r = spdylay_session_on_data_received(session, flags, length, stream_id); if(r < SPDYLAY_ERR_FATAL) { return r; } else { diff --git a/lib/spdylay_session.h b/lib/spdylay_session.h index 87d9f71..d2a16ad 100644 --- a/lib/spdylay_session.h +++ b/lib/spdylay_session.h @@ -208,6 +208,13 @@ int spdylay_session_on_goaway_received(spdylay_session *session, int spdylay_session_on_headers_received(spdylay_session *session, spdylay_frame *frame); +/* + * Called when DATA is received. + */ +int spdylay_session_on_data_received(spdylay_session *session, + uint8_t flags, int32_t length, + int32_t stream_id); + /* * Returns spdylay_stream* object whose stream ID is |stream_id|. It * could be NULL if such stream does not exist. diff --git a/tests/main.c b/tests/main.c index d480f07..8b6bb5d 100644 --- a/tests/main.c +++ b/tests/main.c @@ -88,6 +88,8 @@ int main() test_spdylay_session_on_ping_received) || !CU_add_test(pSuite, "session_on_goaway_received", test_spdylay_session_on_goaway_received) || + !CU_add_test(pSuite, "session_on_data_received", + test_spdylay_session_on_data_received) || !CU_add_test(pSuite, "frame_unpack_nv", test_spdylay_frame_unpack_nv) || !CU_add_test(pSuite, "frame_count_nv_space", test_spdylay_frame_count_nv_space) || diff --git a/tests/spdylay_session_test.c b/tests/spdylay_session_test.c index 5967e3e..77f93bf 100644 --- a/tests/spdylay_session_test.c +++ b/tests/spdylay_session_test.c @@ -330,6 +330,7 @@ void test_spdylay_session_on_syn_reply_received() my_user_data user_data; const char *nv[] = { NULL }; spdylay_frame frame; + spdylay_stream *stream; user_data.valid = 0; user_data.invalid = 0; @@ -349,6 +350,16 @@ void test_spdylay_session_on_syn_reply_received() CU_ASSERT(SPDYLAY_STREAM_CLOSING == ((spdylay_stream*)spdylay_map_find(&session->streams, 1))->state); + /* Check the situation when SYN_REPLY is received after peer sends + FIN */ + stream = spdylay_session_open_stream(session, 3, SPDYLAY_FLAG_NONE, 0, + SPDYLAY_STREAM_OPENED); + spdylay_stream_shutdown(stream, SPDYLAY_SHUT_RD); + frame.syn_reply.stream_id = 3; + + CU_ASSERT(0 == spdylay_session_on_syn_reply_received(session, &frame)); + CU_ASSERT(2 == user_data.invalid); + spdylay_frame_syn_reply_free(&frame.syn_reply); spdylay_session_del(session); } @@ -522,6 +533,16 @@ void test_spdylay_session_on_headers_received() CU_ASSERT(0 == spdylay_session_on_headers_received(session, &frame)); CU_ASSERT(1 == user_data.invalid); + /* Check to see when SPDYLAY_STREAM_CLOSING, incoming HEADERS is + discarded. */ + spdylay_session_open_stream(session, 3, SPDYLAY_FLAG_NONE, 0, + SPDYLAY_STREAM_CLOSING); + frame.headers.stream_id = 3; + frame.headers.hd.flags = SPDYLAY_FLAG_NONE; + CU_ASSERT(0 == spdylay_session_on_headers_received(session, &frame)); + CU_ASSERT(2 == user_data.valid); + CU_ASSERT(1 == user_data.invalid); + /* Server initiated stream */ spdylay_session_open_stream(session, 2, SPDYLAY_FLAG_NONE, 0, SPDYLAY_STREAM_OPENING); @@ -609,3 +630,46 @@ void test_spdylay_session_on_goaway_received() spdylay_frame_goaway_free(&frame.goaway); spdylay_session_del(session); } + +void test_spdylay_session_on_data_received() +{ + spdylay_session *session; + spdylay_session_callbacks callbacks; + memset(&callbacks, 0, sizeof(spdylay_session_callbacks)); + my_user_data user_data; + spdylay_outbound_item *top; + int32_t stream_id = 2; + spdylay_stream *stream; + + spdylay_session_client_new(&session, &callbacks, &user_data); + stream = spdylay_session_open_stream(session, stream_id, SPDYLAY_FLAG_NONE, + 3, SPDYLAY_STREAM_OPENING); + CU_ASSERT(0 == spdylay_session_on_data_received(session, SPDYLAY_FLAG_NONE, + 4096, stream_id)); + CU_ASSERT(0 == stream->shut_flags); + + CU_ASSERT(0 == spdylay_session_on_data_received(session, SPDYLAY_FLAG_FIN, + 4096, stream_id)); + CU_ASSERT(SPDYLAY_SHUT_RD == stream->shut_flags); + + /* If SPDYLAY_STREAM_CLOSING state, DATA frame is discarded. */ + stream_id = 4; + + spdylay_session_open_stream(session, stream_id, SPDYLAY_FLAG_NONE, + 3, SPDYLAY_STREAM_CLOSING); + CU_ASSERT(0 == spdylay_session_on_data_received(session, SPDYLAY_FLAG_NONE, + 4096, stream_id)); + CU_ASSERT(NULL == spdylay_session_get_ob_pq_top(session)); + + /* Check INVALID_STREAM case: DATA frame with stream ID which does + not exist. */ + stream_id = 6; + + CU_ASSERT(0 == spdylay_session_on_data_received(session, SPDYLAY_FLAG_NONE, + 4096, stream_id)); + top = spdylay_session_get_ob_pq_top(session); + CU_ASSERT(SPDYLAY_RST_STREAM == top->frame_type); + CU_ASSERT(SPDYLAY_INVALID_STREAM == top->frame->rst_stream.status_code); + + spdylay_session_del(session); +} diff --git a/tests/spdylay_session_test.h b/tests/spdylay_session_test.h index 31f3ffe..40742f0 100644 --- a/tests/spdylay_session_test.h +++ b/tests/spdylay_session_test.h @@ -37,5 +37,6 @@ void test_spdylay_session_reply_fail(); void test_spdylay_session_on_headers_received(); void test_spdylay_session_on_ping_received(); void test_spdylay_session_on_goaway_received(); +void test_spdylay_session_on_data_received(); #endif // SPDYLAY_SESSION_TEST_H