From 458ccb3681676e7b27f9db379c88156b9939f6d3 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 7 Jun 2014 16:30:55 +0900 Subject: [PATCH] Ignore unknown frame types Unexpected CONTINUATION frame is handled separately as connection error. --- lib/nghttp2_session.c | 25 ++++++++---- tests/main.c | 4 ++ tests/nghttp2_session_test.c | 76 ++++++++++++++++++++++++++++++++++++ tests/nghttp2_session_test.h | 2 + 4 files changed, 100 insertions(+), 7 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index d4761aaa..29f5883c 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -4456,6 +4456,22 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, iframe->state = NGHTTP2_IB_READ_NBYTE; inbound_frame_set_mark(iframe, 8); + break; + case NGHTTP2_CONTINUATION: + DEBUGF(fprintf(stderr, "recv: unexpected CONTINUATION\n")); + + /* Receiving CONTINUATION in this state are subject to + connection error of type PROTOCOL_ERROR */ + rv = nghttp2_session_terminate_session(session, + NGHTTP2_PROTOCOL_ERROR); + if(nghttp2_is_fatal(rv)) { + return rv; + } + + busy = 1; + + iframe->state = NGHTTP2_IB_IGN_PAYLOAD; + break; case NGHTTP2_ALTSVC: DEBUGF(fprintf(stderr, "recv: ALTSVC\n")); @@ -4512,13 +4528,7 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, default: DEBUGF(fprintf(stderr, "recv: unknown frame\n")); - /* Receiving unknown frame type and CONTINUATION in this state - are subject to connection error of type PROTOCOL_ERROR */ - rv = nghttp2_session_terminate_session(session, - NGHTTP2_PROTOCOL_ERROR); - if(nghttp2_is_fatal(rv)) { - return rv; - } + /* Silently ignore unknown frame type. */ busy = 1; @@ -4849,6 +4859,7 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, switch(iframe->frame.hd.type) { case NGHTTP2_HEADERS: case NGHTTP2_PUSH_PROMISE: + case NGHTTP2_CONTINUATION: /* Mark inflater bad so that we won't perform further decoding */ session->hd_inflater.ctx.bad = 1; break; diff --git a/tests/main.c b/tests/main.c index 8fce69ba..302ecde2 100644 --- a/tests/main.c +++ b/tests/main.c @@ -89,6 +89,10 @@ int main(int argc, char* argv[]) test_nghttp2_session_recv_premature_headers) || !CU_add_test(pSuite, "session_recv_altsvc", test_nghttp2_session_recv_altsvc) || + !CU_add_test(pSuite, "session_recv_unknown_frame", + test_nghttp2_session_recv_unknown_frame) || + !CU_add_test(pSuite, "session_recv_unexpected_continuation", + test_nghttp2_session_recv_unexpected_continuation) || !CU_add_test(pSuite, "session_continue", test_nghttp2_session_continue) || !CU_add_test(pSuite, "session_add_frame", test_nghttp2_session_add_frame) || diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index ed77510d..c84a33d0 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -1146,6 +1146,82 @@ void test_nghttp2_session_recv_altsvc(void) nghttp2_session_del(session); } +void test_nghttp2_session_recv_unknown_frame(void) +{ + nghttp2_session *session; + nghttp2_session_callbacks callbacks; + my_user_data ud; + uint8_t data[16384]; + size_t datalen; + nghttp2_frame_hd hd; + ssize_t rv; + + hd.length = 16000; + hd.stream_id = 0; + hd.type = 99; + hd.flags = NGHTTP2_FLAG_NONE; + + nghttp2_frame_pack_frame_hd(data, &hd); + datalen = NGHTTP2_FRAME_HDLEN + hd.length; + + memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); + callbacks.on_frame_recv_callback = on_frame_recv_callback; + + nghttp2_session_server_new(&session, &callbacks, &ud); + + ud.frame_recv_cb_called = 0; + + /* Unknown frame must be ignored */ + rv = nghttp2_session_mem_recv(session, data, datalen); + + CU_ASSERT(rv == (ssize_t)datalen); + CU_ASSERT(0 == ud.frame_recv_cb_called); + CU_ASSERT(NULL == nghttp2_session_get_next_ob_item(session)); + + nghttp2_session_del(session); +} + +void test_nghttp2_session_recv_unexpected_continuation(void) +{ + nghttp2_session *session; + nghttp2_session_callbacks callbacks; + my_user_data ud; + uint8_t data[16384]; + size_t datalen; + nghttp2_frame_hd hd; + ssize_t rv; + nghttp2_outbound_item *item; + + hd.length = 16000; + hd.stream_id = 1; + hd.type = NGHTTP2_CONTINUATION; + hd.flags = NGHTTP2_FLAG_END_HEADERS; + + nghttp2_frame_pack_frame_hd(data, &hd); + datalen = NGHTTP2_FRAME_HDLEN + hd.length; + + memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); + callbacks.on_frame_recv_callback = on_frame_recv_callback; + + nghttp2_session_server_new(&session, &callbacks, &ud); + + open_stream(session, 1); + + ud.frame_recv_cb_called = 0; + + /* unexpected CONTINUATION must be treated as connection error */ + rv = nghttp2_session_mem_recv(session, data, datalen); + + CU_ASSERT(rv == (ssize_t)datalen); + CU_ASSERT(0 == ud.frame_recv_cb_called); + + item = nghttp2_session_get_next_ob_item(session); + + CU_ASSERT(NGHTTP2_GOAWAY == OB_CTRL_TYPE(item)); + + nghttp2_session_del(session); +} + void test_nghttp2_session_continue(void) { nghttp2_session *session; diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h index 8ef093cf..a49bf115 100644 --- a/tests/nghttp2_session_test.h +++ b/tests/nghttp2_session_test.h @@ -34,6 +34,8 @@ void test_nghttp2_session_recv_continuation(void); void test_nghttp2_session_recv_headers_with_priority(void); void test_nghttp2_session_recv_premature_headers(void); void test_nghttp2_session_recv_altsvc(void); +void test_nghttp2_session_recv_unknown_frame(void); +void test_nghttp2_session_recv_unexpected_continuation(void); void test_nghttp2_session_continue(void); void test_nghttp2_session_add_frame(void); void test_nghttp2_session_on_request_headers_received(void);