diff --git a/lib/includes/spdylay/spdylay.h b/lib/includes/spdylay/spdylay.h index f3d13392..7af755f0 100644 --- a/lib/includes/spdylay/spdylay.h +++ b/lib/includes/spdylay/spdylay.h @@ -43,6 +43,7 @@ typedef struct spdylay_session spdylay_session; typedef enum { SPDYLAY_ERR_INVALID_ARGUMENT = -501, + SPDYLAY_ERR_ZLIB = -502, SPDYLAY_ERR_WOULDBLOCK = -504, SPDYLAY_ERR_PROTO = -505, SPDYLAY_ERR_INVALID_FRAME = -506, @@ -54,7 +55,6 @@ typedef enum { SPDYLAY_ERR_FATAL = -900, SPDYLAY_ERR_NOMEM = -901, SPDYLAY_ERR_CALLBACK_FAILURE = -902, - SPDYLAY_ERR_ZLIB = -903, } spdylay_error; typedef enum { @@ -373,12 +373,20 @@ int spdylay_session_recv(spdylay_session *session); /* * Returns non-zero value if |session| want to receive data from the * remote peer, or 0. + * + * if both spdylay_session_want_read() and + * spdylay_session_want_write() return 0, the application should drop + * the connection. */ int spdylay_session_want_read(spdylay_session *session); /* * Returns non-zero value if |session| want to send data to the remote * peer, or 0. + * + * if both spdylay_session_want_read() and + * spdylay_session_want_write() return 0, the application should drop + * the connection. */ int spdylay_session_want_write(spdylay_session *session); diff --git a/lib/spdylay_session.c b/lib/spdylay_session.c index 65956f94..ac944e8e 100644 --- a/lib/spdylay_session.c +++ b/lib/spdylay_session.c @@ -1299,6 +1299,25 @@ 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. For example, when it receives bad + * zlib data. + */ +static int spdylay_session_fail_session(spdylay_session *session) +{ + session->goaway_flags |= SPDYLAY_GOAWAY_FAIL_ON_SEND; + return spdylay_submit_goaway(session); +} + +/* + * Returns non-zero if |error| is non-fatal error. + */ +static int spdylay_is_non_fatal(int error) +{ + return error < 0 && error > SPDYLAY_ERR_FATAL; +} + static int spdylay_session_process_ctrl_frame(spdylay_session *session) { int r = 0; @@ -1321,10 +1340,11 @@ static int spdylay_session_process_ctrl_frame(spdylay_session *session) if(r == 0) { r = spdylay_session_on_syn_stream_received(session, &frame); spdylay_frame_syn_stream_free(&frame.syn_stream); - } else { /* TODO if r indicates mulformed NV pairs (multiple nulls) or invalid frame, send RST_STREAM with PROTOCOL_ERROR. Same for other control frames. */ + } else if(spdylay_is_non_fatal(r)) { + r = spdylay_session_fail_session(session); } break; case SPDYLAY_SYN_REPLY: @@ -1341,6 +1361,8 @@ static int spdylay_session_process_ctrl_frame(spdylay_session *session) if(r == 0) { r = spdylay_session_on_syn_reply_received(session, &frame); spdylay_frame_syn_reply_free(&frame.syn_reply); + } else if(spdylay_is_non_fatal(r)) { + r = spdylay_session_fail_session(session); } break; case SPDYLAY_RST_STREAM: @@ -1352,6 +1374,8 @@ static int spdylay_session_process_ctrl_frame(spdylay_session *session) if(r == 0) { r = spdylay_session_on_rst_stream_received(session, &frame); spdylay_frame_rst_stream_free(&frame.rst_stream); + } else if(spdylay_is_non_fatal(r)) { + r = spdylay_session_fail_session(session); } break; case SPDYLAY_SETTINGS: @@ -1363,6 +1387,8 @@ static int spdylay_session_process_ctrl_frame(spdylay_session *session) if(r == 0) { r = spdylay_session_on_settings_received(session, &frame); spdylay_frame_settings_free(&frame.settings); + } else if(spdylay_is_non_fatal(r)) { + r = spdylay_session_fail_session(session); } break; case SPDYLAY_NOOP: @@ -1376,6 +1402,8 @@ static int spdylay_session_process_ctrl_frame(spdylay_session *session) if(r == 0) { r = spdylay_session_on_ping_received(session, &frame); spdylay_frame_ping_free(&frame.ping); + } else if(spdylay_is_non_fatal(r)) { + r = spdylay_session_fail_session(session); } break; case SPDYLAY_GOAWAY: @@ -1387,6 +1415,8 @@ static int spdylay_session_process_ctrl_frame(spdylay_session *session) if(r == 0) { r = spdylay_session_on_goaway_received(session, &frame); spdylay_frame_goaway_free(&frame.goaway); + } else if(spdylay_is_non_fatal(r)) { + r = spdylay_session_fail_session(session); } break; case SPDYLAY_HEADERS: @@ -1403,6 +1433,8 @@ static int spdylay_session_process_ctrl_frame(spdylay_session *session) if(r == 0) { r = spdylay_session_on_headers_received(session, &frame); spdylay_frame_headers_free(&frame.headers); + } else if(spdylay_is_non_fatal(r)) { + r = spdylay_session_fail_session(session); } break; } @@ -1577,6 +1609,12 @@ int spdylay_session_recv(spdylay_session *session) int spdylay_session_want_read(spdylay_session *session) { + /* If these flags are set, we don't want to read. The application + should drop the connection. */ + if((session->goaway_flags & SPDYLAY_GOAWAY_FAIL_ON_SEND) && + (session->goaway_flags & SPDYLAY_GOAWAY_SEND)) { + return 0; + } /* Unless GOAWAY is sent or received, we always want to read incoming frames. After GOAWAY is sent or received, we are only interested in active streams. */ @@ -1585,6 +1623,12 @@ int spdylay_session_want_read(spdylay_session *session) int spdylay_session_want_write(spdylay_session *session) { + /* If these flags are set, we don't want to write any data. The + application should drop the connection. */ + if((session->goaway_flags & SPDYLAY_GOAWAY_FAIL_ON_SEND) && + (session->goaway_flags & SPDYLAY_GOAWAY_SEND)) { + return 0; + } /* * Unless GOAWAY is sent or received, we want to write frames if * there is pending ones. If pending frame is SYN_STREAM and diff --git a/lib/spdylay_session.h b/lib/spdylay_session.h index 8602244b..15071f0c 100644 --- a/lib/spdylay_session.h +++ b/lib/spdylay_session.h @@ -104,7 +104,9 @@ typedef enum { /* Flag means GOAWAY frame is sent to the remote peer. */ SPDYLAY_GOAWAY_SEND = 0x1, /* Flag means GOAWAY frame is received from the remote peer. */ - SPDYLAY_GOAWAY_RECV = 0x2 + SPDYLAY_GOAWAY_RECV = 0x2, + /* Flag means connection should be dropped after sending GOAWAY. */ + SPDYLAY_GOAWAY_FAIL_ON_SEND = 0x4 } spdylay_goaway_flag; struct spdylay_session { diff --git a/tests/main.c b/tests/main.c index 78fc7a76..901722f8 100644 --- a/tests/main.c +++ b/tests/main.c @@ -119,6 +119,8 @@ int main(int argc, char* argv[]) 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, "session_recv_invalid_frame", + test_spdylay_session_recv_invalid_frame) || !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 3c949d82..0e9bce05 100644 --- a/tests/spdylay_session_test.c +++ b/tests/spdylay_session_test.c @@ -1214,3 +1214,52 @@ void test_spdylay_session_stream_close_on_syn_stream() spdylay_frame_syn_stream_free(&frame.syn_stream); spdylay_session_del(session); } + +void test_spdylay_session_recv_invalid_frame() +{ + spdylay_session *session; + spdylay_session_callbacks callbacks; + scripted_data_feed df; + my_user_data user_data; + const char *nv[] = { + "url", "/", NULL + }; + uint8_t *framedata = NULL, *nvbuf = NULL; + size_t framedatalen = 0, nvbuflen = 0; + ssize_t framelen; + spdylay_frame frame; + + memset(&callbacks, 0, sizeof(spdylay_session_callbacks)); + callbacks.recv_callback = scripted_recv_callback; + callbacks.send_callback = null_send_callback; + callbacks.on_ctrl_send_callback = on_ctrl_send_callback; + + user_data.df = &df; + user_data.ctrl_send_cb_called = 0; + spdylay_session_server_new(&session, &callbacks, &user_data); + spdylay_frame_syn_stream_init(&frame.syn_stream, SPDYLAY_FLAG_NONE, 1, 0, 3, + dup_nv(nv)); + framelen = spdylay_frame_pack_syn_stream(&framedata, &framedatalen, + &nvbuf, &nvbuflen, + &frame.syn_stream, + &session->hd_deflater); + scripted_data_feed_init(&df, framedata, framelen); + + CU_ASSERT(0 == spdylay_session_recv(session)); + CU_ASSERT(0 == spdylay_session_send(session)); + CU_ASSERT(0 == user_data.ctrl_send_cb_called); + + /* Receive exactly same bytes of SYN_STREAM causes error */ + scripted_data_feed_init(&df, framedata, framelen); + + CU_ASSERT(0 == spdylay_session_recv(session)); + CU_ASSERT(0 == spdylay_session_send(session)); + CU_ASSERT(1 == user_data.ctrl_send_cb_called); + CU_ASSERT(SPDYLAY_GOAWAY == user_data.sent_frame_type); + + free(framedata); + free(nvbuf); + 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 37ba779a..4ee2a9f2 100644 --- a/tests/spdylay_session_test.h +++ b/tests/spdylay_session_test.h @@ -51,5 +51,6 @@ 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(); +void test_spdylay_session_recv_invalid_frame(); #endif // SPDYLAY_SESSION_TEST_H