From 67ce60544e7ea237147c2043917ca9f35e9f5eea Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 27 Oct 2013 21:17:09 +0900 Subject: [PATCH] Change maximum frame length to 16383 --- lib/nghttp2_frame.c | 2 +- lib/nghttp2_frame.h | 5 ++- lib/nghttp2_hd.c | 2 +- lib/nghttp2_session.c | 20 ++--------- tests/main.c | 2 -- tests/nghttp2_session_test.c | 66 ++++-------------------------------- tests/nghttp2_session_test.h | 1 - 7 files changed, 14 insertions(+), 84 deletions(-) diff --git a/lib/nghttp2_frame.c b/lib/nghttp2_frame.c index e42a3708..656da22a 100644 --- a/lib/nghttp2_frame.c +++ b/lib/nghttp2_frame.c @@ -47,7 +47,7 @@ void nghttp2_frame_pack_frame_hd(uint8_t* buf, const nghttp2_frame_hd *hd) void nghttp2_frame_unpack_frame_hd(nghttp2_frame_hd *hd, const uint8_t* buf) { - hd->length = nghttp2_get_uint16(&buf[0]); + hd->length = nghttp2_get_uint16(&buf[0]) & NGHTTP2_FRAME_LENGTH_MASK; hd->type = buf[2]; hd->flags = buf[3]; hd->stream_id = nghttp2_get_uint32(&buf[4]) & NGHTTP2_STREAM_ID_MASK; diff --git a/lib/nghttp2_frame.h b/lib/nghttp2_frame.h index 089098c9..1bc153e1 100644 --- a/lib/nghttp2_frame.h +++ b/lib/nghttp2_frame.h @@ -34,9 +34,7 @@ #include "nghttp2_buffer.h" /* The maximum payload length of a frame */ -#define NGHTTP2_MAX_FRAME_LENGTH ((1 << 16) - 1) -/* The maximum paylaod length of a frame used in HTTP */ -#define NGHTTP2_MAX_HTTP_FRAME_LENGTH ((1 << 14) - 1) +#define NGHTTP2_MAX_FRAME_LENGTH ((1 << 14) - 1) /* The maximum header block length. This is not specified by the spec. We just chose the arbitrary size */ @@ -45,6 +43,7 @@ by the spec. We just chose the arbitrary size */ #define NGHTTP2_MAX_HD_VALUE_LENGTH ((1 << 13) - 1) +#define NGHTTP2_FRAME_LENGTH_MASK 0x3fff #define NGHTTP2_STREAM_ID_MASK 0x7fffffff #define NGHTTP2_PRIORITY_MASK 0x7fffffff #define NGHTTP2_WINDOW_SIZE_INCREMENT_MASK 0x7fffffff diff --git a/lib/nghttp2_hd.c b/lib/nghttp2_hd.c index faa85b07..75f9a41c 100644 --- a/lib/nghttp2_hd.c +++ b/lib/nghttp2_hd.c @@ -470,7 +470,7 @@ static int ensure_write_buffer(uint8_t **buf_ptr, size_t *buflen_ptr, int rv; /* TODO Remove this limitation when header continuation is implemented. */ - if(need + offset > NGHTTP2_MAX_HTTP_FRAME_LENGTH) { + if(need + offset > NGHTTP2_MAX_FRAME_LENGTH) { return NGHTTP2_ERR_HEADER_COMP; } rv = nghttp2_reserve_buffer(buf_ptr, buflen_ptr, offset + need); diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 41307b38..57fc0b7a 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -3202,23 +3202,9 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, if(session->iframe.headbufoff == NGHTTP2_FRAME_HEAD_LENGTH) { session->iframe.state = NGHTTP2_RECV_PAYLOAD; session->iframe.payloadlen = - nghttp2_get_uint16(&session->iframe.headbuf[0]); - /* TODO Make payloadlen configurable up to - NGHTTP2_MAX_FRAME_LENGTH */ - if(session->iframe.payloadlen > NGHTTP2_MAX_HTTP_FRAME_LENGTH) { - session->iframe.error_code = NGHTTP2_ERR_FRAME_SIZE_ERROR; - session->iframe.state = NGHTTP2_RECV_PAYLOAD_IGN; - /* Make inflater fail forcibly to disallow reception of - further HEADERS or PUSH_PROMISE */ - session->hd_inflater.bad = 1; - /* Just tear down session for now */ - r = nghttp2_session_fail_session(session, NGHTTP2_FRAME_SIZE_ERROR); - if(r != 0) { - /* FATAL */ - assert(r < NGHTTP2_ERR_FATAL); - return r; - } - } else if(!nghttp2_frame_is_data_frame(session->iframe.headbuf)) { + nghttp2_get_uint16(&session->iframe.headbuf[0]) & + NGHTTP2_FRAME_LENGTH_MASK; + if(!nghttp2_frame_is_data_frame(session->iframe.headbuf)) { /* non-DATA frame */ ssize_t buflen = session->iframe.payloadlen; session->iframe.buflen = buflen; diff --git a/tests/main.c b/tests/main.c index 2141c02a..55678885 100644 --- a/tests/main.c +++ b/tests/main.c @@ -84,8 +84,6 @@ int main(int argc, char* argv[]) test_nghttp2_session_recv_eof) || !CU_add_test(pSuite, "session_recv_data", test_nghttp2_session_recv_data) || - !CU_add_test(pSuite, "session_recv_frame_too_large", - test_nghttp2_session_recv_frame_too_large) || !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 7ac8a7bb..cc3d4875 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -320,62 +320,6 @@ static const char *null_val_nv[] = { "Version", "HTTP/1.1", "Foo", NULL, NULL }; -void test_nghttp2_session_recv_frame_too_large(void) -{ - nghttp2_session *session; - nghttp2_session_callbacks callbacks; - scripted_data_feed df; - my_user_data user_data; - uint8_t data[NGHTTP2_FRAME_HEAD_LENGTH + NGHTTP2_MAX_HTTP_FRAME_LENGTH + 1]; - nghttp2_frame_hd hd; - nghttp2_outbound_item *item; - nghttp2_frame frame; - uint8_t *framebuf = NULL; - size_t framebuflen = 0; - size_t framelen; - - memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); - callbacks.send_callback = null_send_callback; - callbacks.recv_callback = scripted_recv_callback; - callbacks.on_frame_recv_callback = on_frame_recv_callback; - user_data.df = &df; - - nghttp2_session_client_new(&session, &callbacks, &user_data); - - nghttp2_session_open_stream(session, 1, NGHTTP2_FLAG_NONE, - NGHTTP2_PRI_DEFAULT, NGHTTP2_STREAM_OPENED, - NULL); - memset(data, 0, sizeof(data)); - hd.length = NGHTTP2_MAX_HTTP_FRAME_LENGTH + 1; - hd.type = NGHTTP2_DATA; - hd.flags = NGHTTP2_FLAG_END_STREAM; - hd.stream_id = 1; - nghttp2_frame_pack_frame_hd(data, &hd); - scripted_data_feed_init(&df, data, sizeof(data)); - - CU_ASSERT(0 == nghttp2_session_recv(session)); - - item = nghttp2_session_get_next_ob_item(session); - CU_ASSERT(item != NULL); - CU_ASSERT(NGHTTP2_GOAWAY == OB_CTRL_TYPE(item)); - CU_ASSERT(NGHTTP2_FRAME_SIZE_ERROR == OB_CTRL(item)->goaway.error_code); - - /* Check next frame can be received */ - nghttp2_frame_ping_init(&frame.ping, NGHTTP2_FLAG_NONE, NULL); - framelen = nghttp2_frame_pack_ping(&framebuf, &framebuflen, &frame.ping); - nghttp2_frame_ping_free(&frame.ping); - - scripted_data_feed_init(&df, framebuf, framelen); - - user_data.frame_recv_cb_called = 0; - CU_ASSERT(0 == nghttp2_session_recv(session)); - - CU_ASSERT(1 == user_data.frame_recv_cb_called); - - free(framebuf); - nghttp2_session_del(session); -} - void test_nghttp2_session_recv(void) { nghttp2_session *session; @@ -3224,6 +3168,7 @@ void test_nghttp2_session_flow_control_data_recv(void) uint8_t data[64*1024+16]; nghttp2_frame_hd hd; nghttp2_outbound_item *item; + nghttp2_stream *stream; memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); callbacks.send_callback = null_send_callback; @@ -3231,9 +3176,12 @@ void test_nghttp2_session_flow_control_data_recv(void) /* Initial window size to 64KiB - 1*/ nghttp2_session_client_new(&session, &callbacks, NULL); - nghttp2_session_open_stream(session, 1, NGHTTP2_FLAG_NONE, - NGHTTP2_PRI_DEFAULT, NGHTTP2_STREAM_OPENED, - NULL); + stream = nghttp2_session_open_stream(session, 1, NGHTTP2_FLAG_NONE, + NGHTTP2_PRI_DEFAULT, + NGHTTP2_STREAM_OPENED, NULL); + + session->local_window_size = NGHTTP2_MAX_FRAME_LENGTH; + stream->local_window_size = NGHTTP2_MAX_FRAME_LENGTH; /* Create DATA frame */ memset(data, 0, sizeof(data)); diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h index 6ddb7ffb..1cac4884 100644 --- a/tests/nghttp2_session_test.h +++ b/tests/nghttp2_session_test.h @@ -30,7 +30,6 @@ void test_nghttp2_session_recv_invalid_stream_id(void); void test_nghttp2_session_recv_invalid_frame(void); void test_nghttp2_session_recv_eof(void); void test_nghttp2_session_recv_data(void); -void test_nghttp2_session_recv_frame_too_large(void); void test_nghttp2_session_continue(void); void test_nghttp2_session_add_frame(void); void test_nghttp2_session_on_request_headers_received(void);