From fd06d216385e1f0bd08b9264189be183e4c00630 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 15 Feb 2012 00:45:09 +0900 Subject: [PATCH] Call on_stream_close_callback when server pushed SYN_STREAM has FIN flag set. Don't mix status code and return value in spdylay_session_on_syn_stream_received. --- lib/spdylay_session.c | 33 ++++++++++++++++---------- tests/main.c | 2 ++ tests/spdylay_session_test.c | 45 ++++++++++++++++++++++++++++++++++-- tests/spdylay_session_test.h | 1 + 4 files changed, 67 insertions(+), 14 deletions(-) diff --git a/lib/spdylay_session.c b/lib/spdylay_session.c index be35d4ad..be8f9908 100644 --- a/lib/spdylay_session.c +++ b/lib/spdylay_session.c @@ -1003,19 +1003,20 @@ static int spdylay_session_handle_invalid_stream int spdylay_session_on_syn_stream_received(spdylay_session *session, spdylay_frame *frame) { - int r; + int r = 0; + int status_code; if(session->goaway_flags) { /* We don't accept SYN_STREAM after GOAWAY is sent or received. */ return 0; } - r = spdylay_session_validate_syn_stream(session, &frame->syn_stream); - if(r == 0) { + 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_FLAG_FIN) && (flags & SPDYLAY_FLAG_UNIDIRECTIONAL)) { /* If the stream is UNIDIRECTIONAL and FIN bit set, we can close stream upon receiving SYN_STREAM. So, the stream needs not to be opened. */ - r = 0; } else { spdylay_stream *stream; stream = spdylay_session_open_stream(session, frame->syn_stream.stream_id, @@ -1035,18 +1036,26 @@ int spdylay_session_on_syn_stream_received(spdylay_session *session, SPDYLAY_FLAG_UNIDIRECTIONAL is not set here. */ } } - if(r == 0) { - 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_FLAG_FIN) { - spdylay_session_call_on_request_recv(session, - frame->syn_stream.stream_id); + 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_FLAG_FIN) { + spdylay_session_call_on_request_recv(session, + frame->syn_stream.stream_id); + if(flags & SPDYLAY_FLAG_UNIDIRECTIONAL) { + /* Note that we call on_stream_close_callback without opening + stream. */ + if(session->callbacks.on_stream_close_callback) { + session->callbacks.on_stream_close_callback + (session, frame->syn_stream.stream_id, SPDYLAY_OK, + session->user_data); + } } } } else { r = spdylay_session_handle_invalid_stream - (session, frame->syn_stream.stream_id, SPDYLAY_SYN_STREAM, frame, r); + (session, frame->syn_stream.stream_id, SPDYLAY_SYN_STREAM, frame, + status_code); } return r; } diff --git a/tests/main.c b/tests/main.c index 6d46bf86..1a1892d7 100644 --- a/tests/main.c +++ b/tests/main.c @@ -117,6 +117,8 @@ int main(int argc, char* argv[]) test_spdylay_session_data_backoff_by_high_pri_frame) || !CU_add_test(pSuite, "session_stop_data_with_rst_stream", test_spdylay_session_stop_data_with_rst_stream) || + !CU_add_test(pSuite, "session_stream_close_on_syn_stream", + test_spdylay_session_stream_close_on_syn_stream) || !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 f1d3927b..b725ae73 100644 --- a/tests/spdylay_session_test.c +++ b/tests/spdylay_session_test.c @@ -163,6 +163,16 @@ static void on_request_recv_callback(spdylay_session *session, ud->stream_id = stream_id; } +static void no_stream_user_data_stream_close_callback +(spdylay_session *session, + int32_t stream_id, + spdylay_status_code status_code, + void *user_data) +{ + my_user_data* my_data = (my_user_data*)user_data; + ++my_data->stream_close_cb_called; +} + static char** dup_nv(const char **src) { return spdylay_frame_nv_copy(src); @@ -960,8 +970,9 @@ void test_spdylay_session_on_request_recv_callback() spdylay_session_del(session); } -void stream_close_callback(spdylay_session *session, int32_t stream_id, - spdylay_status_code status_code, void *user_data) +static void stream_close_callback(spdylay_session *session, int32_t stream_id, + spdylay_status_code status_code, + void *user_data) { my_user_data* my_data = (my_user_data*)user_data; void *stream_data = spdylay_session_get_stream_user_data(session, stream_id); @@ -1124,3 +1135,33 @@ void test_spdylay_session_stop_data_with_rst_stream() spdylay_session_del(session); } + +/* + * Check that on_stream_close_callback is called when server pushed + * SYN_STREAM have SPDYLAY_FLAG_FIN. + */ +void test_spdylay_session_stream_close_on_syn_stream() +{ + spdylay_session *session; + spdylay_session_callbacks callbacks; + const char *nv[] = { NULL }; + my_user_data ud; + spdylay_frame frame; + + memset(&callbacks, 0, sizeof(spdylay_session_callbacks)); + callbacks.on_stream_close_callback = + no_stream_user_data_stream_close_callback; + ud.stream_close_cb_called = 0; + + spdylay_session_client_new(&session, &callbacks, &ud); + spdylay_session_open_stream(session, 1, SPDYLAY_FLAG_NONE, 3, + SPDYLAY_STREAM_OPENING, NULL); + spdylay_frame_syn_stream_init(&frame.syn_stream, + SPDYLAY_FLAG_FIN | SPDYLAY_FLAG_UNIDIRECTIONAL, + 2, 1, 3, dup_nv(nv)); + + CU_ASSERT(0 == spdylay_session_on_syn_stream_received(session, &frame)); + + spdylay_frame_syn_stream_free(&frame.syn_stream); + spdylay_session_del(session); +} diff --git a/tests/spdylay_session_test.h b/tests/spdylay_session_test.h index 63a7f97f..37ba779a 100644 --- a/tests/spdylay_session_test.h +++ b/tests/spdylay_session_test.h @@ -50,5 +50,6 @@ void test_spdylay_session_on_stream_close(); void test_spdylay_session_max_concurrent_streams(); void test_spdylay_session_data_backoff_by_high_pri_frame(); void test_spdylay_session_stop_data_with_rst_stream(); +void test_spdylay_session_stream_close_on_syn_stream(); #endif // SPDYLAY_SESSION_TEST_H