From 63398f30ddbd9a3f0c850ca65b3f124dc0baed69 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 9 Jul 2014 22:40:31 +0900 Subject: [PATCH] Extend frame length field to 24 bits --- lib/nghttp2_frame.c | 36 +++++++++++++------------ lib/nghttp2_frame.h | 6 ++--- tests/nghttp2_frame_test.c | 21 ++++++++------- tests/nghttp2_session_test.c | 51 +++++++++++++++++++----------------- 4 files changed, 60 insertions(+), 54 deletions(-) diff --git a/lib/nghttp2_frame.c b/lib/nghttp2_frame.c index c48ba3e8..93009674 100644 --- a/lib/nghttp2_frame.c +++ b/lib/nghttp2_frame.c @@ -40,21 +40,21 @@ int nghttp2_frame_is_data_frame(uint8_t *head) void nghttp2_frame_pack_frame_hd(uint8_t* buf, const nghttp2_frame_hd *hd) { - nghttp2_put_uint16be(&buf[0], hd->length); - buf[2]= hd->type; - buf[3] = hd->flags; - nghttp2_put_uint32be(&buf[4], hd->stream_id); + nghttp2_put_uint32be(&buf[0], (uint32_t)(hd->length << 8)); + buf[3]= hd->type; + buf[4] = hd->flags; + nghttp2_put_uint32be(&buf[5], hd->stream_id); } void nghttp2_frame_unpack_frame_hd(nghttp2_frame_hd *hd, const uint8_t* buf) { - 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; + hd->length = nghttp2_get_uint32(&buf[0]) >> 8; + hd->type = buf[3]; + hd->flags = buf[4]; + hd->stream_id = nghttp2_get_uint32(&buf[5]) & NGHTTP2_STREAM_ID_MASK; } -static void frame_set_hd(nghttp2_frame_hd *hd, uint16_t length, +static void frame_set_hd(nghttp2_frame_hd *hd, size_t length, uint8_t type, uint8_t flags, int32_t stream_id) { @@ -1047,6 +1047,7 @@ int nghttp2_iv_check(const nghttp2_settings_entry *iv, size_t niv) static void frame_set_pad(nghttp2_buf *buf, size_t padlen) { size_t trail_padlen; + size_t newlen; DEBUGF(fprintf(stderr, "send: padlen=%zu, shift left 1 bytes\n", padlen)); @@ -1054,9 +1055,10 @@ static void frame_set_pad(nghttp2_buf *buf, size_t padlen) --buf->pos; - buf->pos[3] |= NGHTTP2_FLAG_PADDED; + buf->pos[4] |= NGHTTP2_FLAG_PADDED; - nghttp2_put_uint16be(buf->pos, nghttp2_get_uint16(buf->pos) + padlen); + newlen = (nghttp2_get_uint32(buf->pos) >> 8) + padlen; + nghttp2_put_uint32be(buf->pos, (uint32_t)((newlen << 8) + buf->pos[3])); trail_padlen = padlen - 1; buf->pos[NGHTTP2_FRAME_HDLEN] = trail_padlen; @@ -1087,12 +1089,12 @@ int nghttp2_frame_add_pad(nghttp2_bufs *bufs, nghttp2_frame_hd *hd, * 0 1 2 3 * 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - * | |Frame header | Frame payload... : - * +-+---------------+---------------------------------------------+ - * | |Frame header | Frame payload... : - * +-+---------------+---------------------------------------------+ - * | |Frame header | Frame payload... : - * +-+---------------+---------------------------------------------+ + * | |Frame header | Frame payload... : + * +-+-----------------+-------------------------------------------+ + * | |Frame header | Frame payload... : + * +-+-----------------+-------------------------------------------+ + * | |Frame header | Frame payload... : + * +-+-----------------+-------------------------------------------+ * * We arranged padding so that it is included in the first frame * completely. For padded frame, we are going to adjust buf->pos of diff --git a/lib/nghttp2_frame.h b/lib/nghttp2_frame.h index 0bd2e77c..7c262eea 100644 --- a/lib/nghttp2_frame.h +++ b/lib/nghttp2_frame.h @@ -33,7 +33,7 @@ #include "nghttp2_hd.h" #include "nghttp2_buf.h" -#define NGHTTP2_FRAME_LENGTH_MASK ((1 << 14) - 1) +#define NGHTTP2_FRAME_LENGTH_MASK ((1 << 24) - 1) #define NGHTTP2_STREAM_ID_MASK ((1u << 31) - 1) #define NGHTTP2_PRI_GROUP_ID_MASK ((1u << 31) - 1) #define NGHTTP2_PRIORITY_MASK ((1u << 31) - 1) @@ -41,9 +41,9 @@ #define NGHTTP2_SETTINGS_ID_MASK ((1 << 24) - 1) /* The number of bytes of frame header. */ -#define NGHTTP2_FRAME_HDLEN 8 +#define NGHTTP2_FRAME_HDLEN 9 -#define NGHTTP2_MAX_PAYLOADLEN 16383 +#define NGHTTP2_MAX_PAYLOADLEN 16384 /* The one frame buffer length for tranmission. We may use several of them to support CONTINUATION. To account for Pad Length field, we allocate extra 1 byte, which saves extra large memcopying. */ diff --git a/tests/nghttp2_frame_test.c b/tests/nghttp2_frame_test.c index 0e96dbed..7b3baaf3 100644 --- a/tests/nghttp2_frame_test.c +++ b/tests/nghttp2_frame_test.c @@ -61,7 +61,7 @@ static nghttp2_nv* headers(void) return nva; } -static void check_frame_header(uint16_t length, uint8_t type, uint8_t flags, +static void check_frame_header(size_t length, uint8_t type, uint8_t flags, int32_t stream_id, nghttp2_frame_hd *hd) { CU_ASSERT(length == hd->length); @@ -223,7 +223,7 @@ void test_nghttp2_frame_pack_priority(void) rv = nghttp2_frame_pack_priority(&bufs, &frame); CU_ASSERT(0 == rv); - CU_ASSERT(13 == nghttp2_bufs_len(&bufs)); + CU_ASSERT(NGHTTP2_FRAME_HDLEN + 5 == nghttp2_bufs_len(&bufs)); CU_ASSERT(0 == unpack_framebuf((nghttp2_frame*)&oframe, &bufs)); check_frame_header(5, NGHTTP2_PRIORITY, NGHTTP2_FLAG_NONE, 1000000007, &oframe.hd); @@ -251,7 +251,7 @@ void test_nghttp2_frame_pack_rst_stream(void) rv = nghttp2_frame_pack_rst_stream(&bufs, &frame); CU_ASSERT(0 == rv); - CU_ASSERT(12 == nghttp2_bufs_len(&bufs)); + CU_ASSERT(NGHTTP2_FRAME_HDLEN + 4 == nghttp2_bufs_len(&bufs)); CU_ASSERT(0 == unpack_framebuf((nghttp2_frame*)&oframe, &bufs)); check_frame_header(4, NGHTTP2_RST_STREAM, NGHTTP2_FLAG_NONE, 1000000007, &oframe.hd); @@ -384,7 +384,7 @@ void test_nghttp2_frame_pack_ping(void) rv = nghttp2_frame_pack_ping(&bufs, &frame); CU_ASSERT(0 == rv); - CU_ASSERT(16 == nghttp2_bufs_len(&bufs)); + CU_ASSERT(NGHTTP2_FRAME_HDLEN + 8 == nghttp2_bufs_len(&bufs)); CU_ASSERT(0 == unpack_framebuf((nghttp2_frame*)&oframe, &bufs)); check_frame_header(8, NGHTTP2_PING, NGHTTP2_FLAG_ACK, 0, &oframe.hd); CU_ASSERT(memcmp(opaque_data, oframe.opaque_data, sizeof(opaque_data) - 1) @@ -411,7 +411,8 @@ void test_nghttp2_frame_pack_goaway() rv = nghttp2_frame_pack_goaway(&bufs, &frame); CU_ASSERT(0 == rv); - CU_ASSERT((ssize_t)(16 + opaque_data_len) == nghttp2_bufs_len(&bufs)); + CU_ASSERT((ssize_t)(NGHTTP2_FRAME_HDLEN + 8 + opaque_data_len) == + nghttp2_bufs_len(&bufs)); CU_ASSERT(0 == unpack_framebuf((nghttp2_frame*)&oframe, &bufs)); check_frame_header(24, NGHTTP2_GOAWAY, NGHTTP2_FLAG_NONE, 0, &oframe.hd); CU_ASSERT(1000000007 == oframe.last_stream_id); @@ -453,7 +454,7 @@ void test_nghttp2_frame_pack_window_update(void) rv = nghttp2_frame_pack_window_update(&bufs, &frame); CU_ASSERT(0 == rv); - CU_ASSERT(12 == nghttp2_bufs_len(&bufs)); + CU_ASSERT(NGHTTP2_FRAME_HDLEN + 4 == nghttp2_bufs_len(&bufs)); CU_ASSERT(0 == unpack_framebuf((nghttp2_frame*)&oframe, &bufs)); check_frame_header(4, NGHTTP2_WINDOW_UPDATE, NGHTTP2_FLAG_NONE, 1000000007, &oframe.hd); @@ -541,7 +542,7 @@ void test_nghttp2_frame_pack_altsvc(void) /* Check no origin case */ payloadlen = NGHTTP2_ALTSVC_MINLEN + protocol_id_len + host_len; - nghttp2_put_uint16be(buf->pos, payloadlen); + nghttp2_put_uint32be(buf->pos, (uint32_t)((payloadlen << 8) + buf->pos[3])); oframe.payload = &oaltsvc; @@ -562,7 +563,7 @@ void test_nghttp2_frame_pack_altsvc(void) /* Check insufficient payload length for host */ payloadlen = NGHTTP2_ALTSVC_MINLEN + protocol_id_len + host_len - 1; - nghttp2_put_uint16be(buf->pos, payloadlen); + nghttp2_put_uint32be(buf->pos, (uint32_t)((payloadlen << 8) + buf->pos[3])); oframe.payload = &oaltsvc; @@ -579,7 +580,7 @@ void test_nghttp2_frame_pack_altsvc(void) /* Check no host case */ payloadlen = NGHTTP2_ALTSVC_MINLEN + protocol_id_len; - nghttp2_put_uint16be(buf->pos, payloadlen); + nghttp2_put_uint32be(buf->pos, (uint32_t)((payloadlen << 8) + buf->pos[3])); buf->pos[NGHTTP2_FRAME_HDLEN + NGHTTP2_ALTSVC_FIXED_PARTLEN + protocol_id_len] = 0; @@ -602,7 +603,7 @@ void test_nghttp2_frame_pack_altsvc(void) /* Check missing Host-Len */ payloadlen = NGHTTP2_ALTSVC_FIXED_PARTLEN + protocol_id_len; - nghttp2_put_uint16be(buf->pos, payloadlen); + nghttp2_put_uint32be(buf->pos, (uint32_t)((payloadlen << 8) + buf->pos[3])); oframe.payload = &oaltsvc; diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 88df64bf..4d4b41bb 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -450,7 +450,9 @@ void test_nghttp2_session_recv(void) assert(nghttp2_buf_len(&bufs.cur->buf) >= 16); bufs.cur->buf.last += 16; - nghttp2_put_uint16be(bufs.cur->buf.pos, frame.hd.length + 16); + nghttp2_put_uint32be(bufs.cur->buf.pos, + (uint32_t)(((frame.hd.length + 16) << 8) + + bufs.cur->buf.pos[3])); nghttp2_frame_ping_free(&frame.ping); @@ -610,8 +612,8 @@ void test_nghttp2_session_recv_data(void) error. This is not mandated by the spec */ ud.data_chunk_recv_cb_called = 0; ud.frame_recv_cb_called = 0; - rv = nghttp2_session_mem_recv(session, data, 8+4096); - CU_ASSERT(8+4096 == rv); + rv = nghttp2_session_mem_recv(session, data, NGHTTP2_FRAME_HDLEN + 4096); + CU_ASSERT(NGHTTP2_FRAME_HDLEN + 4096 == rv); CU_ASSERT(0 == ud.data_chunk_recv_cb_called); CU_ASSERT(0 == ud.frame_recv_cb_called); @@ -633,8 +635,8 @@ void test_nghttp2_session_recv_data(void) ud.data_chunk_recv_cb_called = 0; ud.frame_recv_cb_called = 0; - rv = nghttp2_session_mem_recv(session, data, 8+4096); - CU_ASSERT(8+4096 == rv); + rv = nghttp2_session_mem_recv(session, data, NGHTTP2_FRAME_HDLEN + 4096); + CU_ASSERT(NGHTTP2_FRAME_HDLEN + 4096 == rv); CU_ASSERT(0 == ud.data_chunk_recv_cb_called); CU_ASSERT(0 == ud.frame_recv_cb_called); @@ -646,8 +648,8 @@ void test_nghttp2_session_recv_data(void) ud.data_chunk_recv_cb_called = 0; ud.frame_recv_cb_called = 0; - rv = nghttp2_session_mem_recv(session, data, 8+4096); - CU_ASSERT(8+4096 == rv); + rv = nghttp2_session_mem_recv(session, data, NGHTTP2_FRAME_HDLEN + 4096); + CU_ASSERT(NGHTTP2_FRAME_HDLEN + 4096 == rv); CU_ASSERT(1 == ud.data_chunk_recv_cb_called); CU_ASSERT(1 == ud.frame_recv_cb_called); @@ -656,8 +658,8 @@ void test_nghttp2_session_recv_data(void) ud.data_chunk_recv_cb_called = 0; ud.frame_recv_cb_called = 0; - rv = nghttp2_session_mem_recv(session, data, 8+4096); - CU_ASSERT(8+4096 == rv); + rv = nghttp2_session_mem_recv(session, data, NGHTTP2_FRAME_HDLEN + 4096); + CU_ASSERT(NGHTTP2_FRAME_HDLEN + 4096 == rv); /* Now we got data more than initial-window-size / 2, WINDOW_UPDATE must be queued */ @@ -676,8 +678,8 @@ void test_nghttp2_session_recv_data(void) DATA. Additional 4 DATA frames, connection flow control will kick in. */ for(i = 0; i < 5; ++i) { - rv = nghttp2_session_mem_recv(session, data, 8+4096); - CU_ASSERT(8+4096 == rv); + rv = nghttp2_session_mem_recv(session, data, NGHTTP2_FRAME_HDLEN + 4096); + CU_ASSERT(NGHTTP2_FRAME_HDLEN + 4096 == rv); } item = nghttp2_session_get_next_ob_item(session); CU_ASSERT(NGHTTP2_WINDOW_UPDATE == OB_CTRL_TYPE(item)); @@ -693,8 +695,8 @@ void test_nghttp2_session_recv_data(void) ud.data_chunk_recv_cb_called = 0; ud.frame_recv_cb_called = 0; - rv = nghttp2_session_mem_recv(session, data, 8+4096); - CU_ASSERT(8+4096 == rv); + rv = nghttp2_session_mem_recv(session, data, NGHTTP2_FRAME_HDLEN + 4096); + CU_ASSERT(NGHTTP2_FRAME_HDLEN + 4096 == rv); CU_ASSERT(0 == ud.data_chunk_recv_cb_called); CU_ASSERT(0 == ud.frame_recv_cb_called); @@ -753,11 +755,11 @@ void test_nghttp2_session_recv_continuation(void) nghttp2_frame_headers_free(&frame.headers); /* HEADERS's payload is 1 byte */ - memcpy(data, buf->pos, 9); - datalen = 9; - buf->pos += 9; + memcpy(data, buf->pos, NGHTTP2_FRAME_HDLEN + 1); + datalen = NGHTTP2_FRAME_HDLEN + 1; + buf->pos += NGHTTP2_FRAME_HDLEN + 1; - nghttp2_put_uint16be(data, 1); + nghttp2_put_uint32be(data, (1 << 8) + data[3]); /* First CONTINUATION, 2 bytes */ cont_hd.length = 2; @@ -925,14 +927,14 @@ void test_nghttp2_session_recv_headers_with_priority(void) rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater); CU_ASSERT(0 == rv); - CU_ASSERT(13 == nghttp2_bufs_len(&bufs)); + CU_ASSERT(NGHTTP2_FRAME_HDLEN + 5 == nghttp2_bufs_len(&bufs)); nghttp2_frame_headers_free(&frame.headers); buf = &bufs.head->buf; /* Make payload shorter than required length to store priroty group */ - nghttp2_put_uint16be(buf->pos, 4); + nghttp2_put_uint32be(buf->pos, (4 << 8) + buf->pos[3]); ud.frame_recv_cb_called = 0; @@ -1043,7 +1045,8 @@ void test_nghttp2_session_recv_premature_headers(void) assert(nghttp2_bufs_len(&bufs) == nghttp2_buf_len(buf)); /* Intentionally feed payload cutting last 1 byte off */ - nghttp2_put_uint16be(buf->pos, frame.hd.length - 1); + nghttp2_put_uint32be(buf->pos, + (uint32_t)(((frame.hd.length - 1) << 8) + buf->pos[3])); rv = nghttp2_session_mem_recv(session, buf->pos, nghttp2_buf_len(buf) - 1); CU_ASSERT((ssize_t)(nghttp2_buf_len(buf) - 1) == rv); @@ -1138,7 +1141,7 @@ void test_nghttp2_session_recv_altsvc(void) CU_ASSERT(NGHTTP2_EXT_ALTSVC == ud.recv_frame_type); /* premature payload */ - nghttp2_put_uint16be(buf->pos, 8); + nghttp2_put_uint32be(buf->pos, (8 << 8) + buf->pos[3]); ud.frame_recv_cb_called = 0; @@ -1642,11 +1645,11 @@ void test_nghttp2_session_add_frame(void) aux_data)); CU_ASSERT(0 == nghttp2_pq_empty(&session->ob_ss_pq)); CU_ASSERT(0 == nghttp2_session_send(session)); - CU_ASSERT(NGHTTP2_HEADERS == acc.buf[2]); + CU_ASSERT(NGHTTP2_HEADERS == acc.buf[3]); CU_ASSERT((NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_PRIORITY) == - acc.buf[3]); + acc.buf[4]); /* check stream id */ - CU_ASSERT(1 == nghttp2_get_uint32(&acc.buf[4])); + CU_ASSERT(1 == nghttp2_get_uint32(&acc.buf[5])); nghttp2_session_del(session); }