From ce6dc1303ebd35226264b62c780cc246ed94b564 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 11 Mar 2012 19:27:33 +0900 Subject: [PATCH] Issue session error with PROTOCOL_ERROR if SYN_STREAM with a stream ID which is less than any previously received SYN_STREAM. --- lib/spdylay_session.c | 53 ++++++++++++++++++++++++------------ tests/spdylay_session_test.c | 27 ++++++++++++++---- 2 files changed, 56 insertions(+), 24 deletions(-) diff --git a/lib/spdylay_session.c b/lib/spdylay_session.c index 04e31507..ee829294 100644 --- a/lib/spdylay_session.c +++ b/lib/spdylay_session.c @@ -63,6 +63,18 @@ static int spdylay_is_fatal(int error) return error < SPDYLAY_ERR_FATAL; } +/* + * This function should be called when the session wants to drop + * connection after sending GOAWAY. These cases are called as the + * session error. For example, when it receives bad zlib data. + */ +static int spdylay_session_fail_session(spdylay_session *session, + uint32_t status_code) +{ + session->goaway_flags |= SPDYLAY_GOAWAY_FAIL_ON_SEND; + return spdylay_submit_goaway(session, status_code); +} + int spdylay_session_is_my_stream_id(spdylay_session *session, int32_t stream_id) { @@ -1376,9 +1388,6 @@ static int spdylay_session_check_nv(char **nv) static int spdylay_session_validate_syn_stream(spdylay_session *session, spdylay_syn_stream *frame) { - if(!spdylay_session_is_new_peer_stream_id(session, frame->stream_id)) { - return SPDYLAY_PROTOCOL_ERROR; - } if(!spdylay_session_check_version(session, frame->hd.version)) { return SPDYLAY_UNSUPPORTED_VERSION; } @@ -1443,8 +1452,29 @@ int spdylay_session_on_syn_stream_received(spdylay_session *session, /* We don't accept SYN_STREAM after GOAWAY is sent or received. */ return 0; } - status_code = spdylay_session_validate_syn_stream(session, - &frame->syn_stream); + if(session->last_recv_stream_id == frame->syn_stream.stream_id) { + /* SPDY/3 spec says if an endpoint receives same stream ID twice, + it MUST issue a stream error with status code + PROTOCOL_ERROR. */ + status_code = SPDYLAY_PROTOCOL_ERROR; + } else if(!spdylay_session_is_new_peer_stream_id + (session, frame->syn_stream.stream_id)) { + /* SPDY/3 spec says if an endpoint receives a SYN_STREAM with a + stream ID which is less than any previously received + SYN_STREAM, it MUST issue a session error with status + PROTOCOL_ERROR */ + if(session->callbacks.on_invalid_ctrl_recv_callback) { + session->callbacks.on_invalid_ctrl_recv_callback(session, + SPDYLAY_SYN_STREAM, + frame, + session->user_data); + } + return spdylay_session_fail_session(session, SPDYLAY_GOAWAY_PROTOCOL_ERROR); + } else { + session->last_recv_stream_id = frame->syn_stream.stream_id; + status_code = spdylay_session_validate_syn_stream(session, + &frame->syn_stream); + } if(status_code == 0) { uint8_t flags = frame->syn_stream.hd.flags; if((flags & SPDYLAY_CTRL_FLAG_FIN) && @@ -1471,7 +1501,6 @@ int spdylay_session_on_syn_stream_received(spdylay_session *session, SPDYLAY_CTRL_FLAG_UNIDIRECTIONAL is not set here. */ } } - session->last_recv_stream_id = frame->syn_stream.stream_id; spdylay_session_call_on_ctrl_frame_received(session, SPDYLAY_SYN_STREAM, frame); if(flags & SPDYLAY_CTRL_FLAG_FIN) { @@ -1749,18 +1778,6 @@ int spdylay_session_on_headers_received(spdylay_session *session, return r; } -/* - * This function should be called when the session wants to drop - * connection after sending GOAWAY. These cases are called as the - * session error. For example, when it receives bad zlib data. - */ -static int spdylay_session_fail_session(spdylay_session *session, - uint32_t status_code) -{ - session->goaway_flags |= SPDYLAY_GOAWAY_FAIL_ON_SEND; - return spdylay_submit_goaway(session, status_code); -} - /* For errors, this function only returns FATAL error. */ static int spdylay_session_process_ctrl_frame(spdylay_session *session) { diff --git a/tests/spdylay_session_test.c b/tests/spdylay_session_test.c index 98a5b881..4d99dd2f 100644 --- a/tests/spdylay_session_test.c +++ b/tests/spdylay_session_test.c @@ -377,25 +377,40 @@ void test_spdylay_session_on_syn_stream_received() CU_ASSERT(SPDYLAY_STREAM_OPENING == stream->state); CU_ASSERT(pri == stream->pri); - /* Same stream ID twice leads stream closing */ + /* Same stream ID twice leads stream error */ + user_data.invalid_ctrl_recv_cb_called = 0; CU_ASSERT(0 == spdylay_session_on_syn_stream_received(session, &frame)); CU_ASSERT(1 == user_data.invalid_ctrl_recv_cb_called); - CU_ASSERT(SPDYLAY_STREAM_CLOSING == - spdylay_session_get_stream(session, stream_id)->state); + CU_ASSERT(SPDYLAY_STREAM_CLOSING == stream->state); /* assoc_stream_id != 0 from client is invalid. */ + frame.syn_stream.stream_id = 3; frame.syn_stream.assoc_stream_id = 1; + user_data.invalid_ctrl_recv_cb_called = 0; CU_ASSERT(0 == spdylay_session_on_syn_stream_received(session, &frame)); - CU_ASSERT(2 == user_data.invalid_ctrl_recv_cb_called); + CU_ASSERT(1 == user_data.invalid_ctrl_recv_cb_called); spdylay_frame_syn_stream_free(&frame.syn_stream); /* Upper cased name/value pairs */ spdylay_frame_syn_stream_init(&frame.syn_stream, SPDYLAY_PROTO_SPDY2, SPDYLAY_CTRL_FLAG_NONE, - 3, 0, 3, dup_nv(upcase_nv)); + 5, 0, 3, dup_nv(upcase_nv)); + user_data.invalid_ctrl_recv_cb_called = 0; CU_ASSERT(0 == spdylay_session_on_syn_stream_received(session, &frame)); - CU_ASSERT(3 == user_data.invalid_ctrl_recv_cb_called); + CU_ASSERT(1 == user_data.invalid_ctrl_recv_cb_called); + + spdylay_frame_syn_stream_free(&frame.syn_stream); + + /* Stream ID less than previouly received SYN_STREAM leads session + error */ + spdylay_frame_syn_stream_init(&frame.syn_stream, SPDYLAY_PROTO_SPDY2, + SPDYLAY_CTRL_FLAG_NONE, + 3, 0, 3, dup_nv(nv)); + user_data.invalid_ctrl_recv_cb_called = 0; + CU_ASSERT(0 == spdylay_session_on_syn_stream_received(session, &frame)); + CU_ASSERT(1 == user_data.invalid_ctrl_recv_cb_called); + CU_ASSERT(session->goaway_flags & SPDYLAY_GOAWAY_FAIL_ON_SEND); spdylay_frame_syn_stream_free(&frame.syn_stream); spdylay_session_del(session);