From b4bb6a61010d6d25096742964a6574774dba30a6 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 28 Aug 2014 23:30:42 +0900 Subject: [PATCH] Add reserved bits to header and frames Currently reserved bit is always set to 0. The addition of reserved bit is for future extension. --- lib/includes/nghttp2/nghttp2.h | 20 +++++++++++ lib/nghttp2_frame.c | 43 ++++++++++++---------- lib/nghttp2_frame.h | 8 +++++ lib/nghttp2_session.c | 6 ++-- tests/nghttp2_frame_test.c | 1 + tests/nghttp2_session_test.c | 65 +++++++++++++--------------------- 6 files changed, 80 insertions(+), 63 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 637b1648..d3f81a30 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -605,6 +605,11 @@ typedef struct { * The flags. */ uint8_t flags; + /** + * Reserved bit in frame header. Currently, this is always set to 0 + * and application should not expect something useful in here. + */ + uint8_t reserved; } nghttp2_frame_hd; @@ -887,6 +892,11 @@ typedef struct { * The promised stream ID */ int32_t promised_stream_id; + /** + * Reserved bit. Currently this is always set to 0 and application + * should not expect something useful in here. + */ + uint8_t reserved; } nghttp2_push_promise; /** @@ -931,6 +941,11 @@ typedef struct { * The length of |opaque_data| member. */ size_t opaque_data_len; + /** + * Reserved bit. Currently this is always set to 0 and application + * should not expect something useful in here. + */ + uint8_t reserved; } nghttp2_goaway; /** @@ -947,6 +962,11 @@ typedef struct { * The window size increment. */ int32_t window_size_increment; + /** + * Reserved bit. Currently this is always set to 0 and application + * should not expect something useful in here. + */ + uint8_t reserved; } nghttp2_window_update; /** diff --git a/lib/nghttp2_frame.c b/lib/nghttp2_frame.c index b351e4f3..41e4cf67 100644 --- a/lib/nghttp2_frame.c +++ b/lib/nghttp2_frame.c @@ -44,6 +44,7 @@ void nghttp2_frame_pack_frame_hd(uint8_t* buf, const nghttp2_frame_hd *hd) buf[3]= hd->type; buf[4] = hd->flags; nghttp2_put_uint32be(&buf[5], hd->stream_id); + /* ignore hd->reserved for now */ } void nghttp2_frame_unpack_frame_hd(nghttp2_frame_hd *hd, const uint8_t* buf) @@ -52,16 +53,18 @@ void nghttp2_frame_unpack_frame_hd(nghttp2_frame_hd *hd, const uint8_t* buf) hd->type = buf[3]; hd->flags = buf[4]; hd->stream_id = nghttp2_get_uint32(&buf[5]) & NGHTTP2_STREAM_ID_MASK; + hd->reserved = 0; } -static void frame_set_hd(nghttp2_frame_hd *hd, size_t length, - uint8_t type, uint8_t flags, - int32_t stream_id) +void nghttp2_frame_hd_init(nghttp2_frame_hd *hd, size_t length, + uint8_t type, uint8_t flags, + int32_t stream_id) { hd->length = length; hd->type = type; hd->flags = flags; hd->stream_id = stream_id; + hd->reserved = 0; } void nghttp2_frame_headers_init(nghttp2_headers *frame, @@ -70,7 +73,7 @@ void nghttp2_frame_headers_init(nghttp2_headers *frame, const nghttp2_priority_spec *pri_spec, nghttp2_nv *nva, size_t nvlen) { - frame_set_hd(&frame->hd, 0, NGHTTP2_HEADERS, flags, stream_id); + nghttp2_frame_hd_init(&frame->hd, 0, NGHTTP2_HEADERS, flags, stream_id); frame->padlen = 0; frame->nva = nva; frame->nvlen = nvlen; @@ -91,8 +94,8 @@ void nghttp2_frame_headers_free(nghttp2_headers *frame) void nghttp2_frame_priority_init(nghttp2_priority *frame, int32_t stream_id, const nghttp2_priority_spec *pri_spec) { - frame_set_hd(&frame->hd, NGHTTP2_PRIORITY_SPECLEN, NGHTTP2_PRIORITY, - NGHTTP2_FLAG_NONE, stream_id); + nghttp2_frame_hd_init(&frame->hd, NGHTTP2_PRIORITY_SPECLEN, NGHTTP2_PRIORITY, + NGHTTP2_FLAG_NONE, stream_id); frame->pri_spec = *pri_spec; } @@ -103,8 +106,8 @@ void nghttp2_frame_rst_stream_init(nghttp2_rst_stream *frame, int32_t stream_id, uint32_t error_code) { - frame_set_hd(&frame->hd, 4, NGHTTP2_RST_STREAM, NGHTTP2_FLAG_NONE, - stream_id); + nghttp2_frame_hd_init(&frame->hd, 4, NGHTTP2_RST_STREAM, NGHTTP2_FLAG_NONE, + stream_id); frame->error_code = error_code; } @@ -115,8 +118,8 @@ void nghttp2_frame_rst_stream_free(nghttp2_rst_stream *frame) void nghttp2_frame_settings_init(nghttp2_settings *frame, uint8_t flags, nghttp2_settings_entry *iv, size_t niv) { - frame_set_hd(&frame->hd, niv * NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH, - NGHTTP2_SETTINGS, flags, 0); + nghttp2_frame_hd_init(&frame->hd, niv * NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH, + NGHTTP2_SETTINGS, flags, 0); frame->niv = niv; frame->iv = iv; } @@ -131,11 +134,12 @@ void nghttp2_frame_push_promise_init(nghttp2_push_promise *frame, int32_t promised_stream_id, nghttp2_nv *nva, size_t nvlen) { - frame_set_hd(&frame->hd, 0, NGHTTP2_PUSH_PROMISE, flags, stream_id); + nghttp2_frame_hd_init(&frame->hd, 0, NGHTTP2_PUSH_PROMISE, flags, stream_id); frame->padlen = 0; frame->nva = nva; frame->nvlen = nvlen; frame->promised_stream_id = promised_stream_id; + frame->reserved = 0; } void nghttp2_frame_push_promise_free(nghttp2_push_promise *frame) @@ -146,7 +150,7 @@ void nghttp2_frame_push_promise_free(nghttp2_push_promise *frame) void nghttp2_frame_ping_init(nghttp2_ping *frame, uint8_t flags, const uint8_t *opaque_data) { - frame_set_hd(&frame->hd, 8, NGHTTP2_PING, flags, 0); + nghttp2_frame_hd_init(&frame->hd, 8, NGHTTP2_PING, flags, 0); if(opaque_data) { memcpy(frame->opaque_data, opaque_data, sizeof(frame->opaque_data)); } else { @@ -161,12 +165,13 @@ void nghttp2_frame_goaway_init(nghttp2_goaway *frame, int32_t last_stream_id, uint32_t error_code, uint8_t *opaque_data, size_t opaque_data_len) { - frame_set_hd(&frame->hd, 8+opaque_data_len, NGHTTP2_GOAWAY, - NGHTTP2_FLAG_NONE, 0); + nghttp2_frame_hd_init(&frame->hd, 8+opaque_data_len, NGHTTP2_GOAWAY, + NGHTTP2_FLAG_NONE, 0); frame->last_stream_id = last_stream_id; frame->error_code = error_code; frame->opaque_data = opaque_data; frame->opaque_data_len = opaque_data_len; + frame->reserved = 0; } void nghttp2_frame_goaway_free(nghttp2_goaway *frame) @@ -179,8 +184,10 @@ void nghttp2_frame_window_update_init(nghttp2_window_update *frame, int32_t stream_id, int32_t window_size_increment) { - frame_set_hd(&frame->hd, 4, NGHTTP2_WINDOW_UPDATE, flags, stream_id); + nghttp2_frame_hd_init(&frame->hd, 4, NGHTTP2_WINDOW_UPDATE, flags, + stream_id); frame->window_size_increment = window_size_increment; + frame->reserved = 0; } void nghttp2_frame_window_update_free(nghttp2_window_update *frame) @@ -201,8 +208,8 @@ void nghttp2_frame_altsvc_init(nghttp2_extension *frame, int32_t stream_id, payloadlen = NGHTTP2_ALTSVC_MINLEN + protocol_id_len + host_len + origin_len; - frame_set_hd(&frame->hd, payloadlen, NGHTTP2_EXT_ALTSVC, NGHTTP2_FLAG_NONE, - stream_id); + nghttp2_frame_hd_init(&frame->hd, payloadlen, NGHTTP2_EXT_ALTSVC, + NGHTTP2_FLAG_NONE, stream_id); altsvc->max_age = max_age; altsvc->port = port; @@ -249,7 +256,7 @@ void nghttp2_frame_private_data_init(nghttp2_private_data *frame, const nghttp2_data_provider *data_prd) { /* At this moment, the length of DATA frame is unknown */ - frame_set_hd(&frame->hd, 0, NGHTTP2_DATA, flags, stream_id); + nghttp2_frame_hd_init(&frame->hd, 0, NGHTTP2_DATA, flags, stream_id); frame->data_prd = *data_prd; frame->padlen = 0; frame->eof = 0; diff --git a/lib/nghttp2_frame.h b/lib/nghttp2_frame.h index b0a04438..fe2c350e 100644 --- a/lib/nghttp2_frame.h +++ b/lib/nghttp2_frame.h @@ -127,6 +127,14 @@ 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); +/** + * Initializes frame header |hd| with given parameters. Reserved bit + * is set to 0. + */ +void nghttp2_frame_hd_init(nghttp2_frame_hd *hd, size_t length, + uint8_t type, uint8_t flags, + int32_t stream_id); + /** * Returns the number of priority field depending on the |flags|. If * |flags| has neither NGHTTP2_FLAG_PRIORITY_GROUP nor diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index c02bd702..e0711281 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -5736,10 +5736,8 @@ int nghttp2_session_pack_data(nghttp2_session *session, } /* The primary reason of data_frame is pass to the user callback */ - data_frame.hd.length = payloadlen; - data_frame.hd.stream_id = frame->hd.stream_id; - data_frame.hd.type = NGHTTP2_DATA; - data_frame.hd.flags = flags; + nghttp2_frame_hd_init(&data_frame.hd, payloadlen, NGHTTP2_DATA, flags, + frame->hd.stream_id); data_frame.data.padlen = 0; max_payloadlen = nghttp2_min(datamax, diff --git a/tests/nghttp2_frame_test.c b/tests/nghttp2_frame_test.c index 21f6c710..55bdfa57 100644 --- a/tests/nghttp2_frame_test.c +++ b/tests/nghttp2_frame_test.c @@ -68,6 +68,7 @@ static void check_frame_header(size_t length, uint8_t type, uint8_t flags, CU_ASSERT(type == hd->type); CU_ASSERT(flags == hd->flags); CU_ASSERT(stream_id == hd->stream_id); + CU_ASSERT(0 == hd->reserved); } void test_nghttp2_frame_pack_headers() diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 73dce6b0..cfbecf53 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -830,10 +830,8 @@ void test_nghttp2_session_recv_continuation(void) nghttp2_put_uint32be(data, (1 << 8) + data[3]); /* First CONTINUATION, 2 bytes */ - cont_hd.length = 2; - cont_hd.type = NGHTTP2_CONTINUATION; - cont_hd.flags = NGHTTP2_FLAG_NONE; - cont_hd.stream_id = 1; + nghttp2_frame_hd_init(&cont_hd, 2, NGHTTP2_CONTINUATION, + NGHTTP2_FLAG_NONE, 1); nghttp2_frame_pack_frame_hd(data + datalen, &cont_hd); datalen += NGHTTP2_FRAME_HDLEN; @@ -843,9 +841,8 @@ void test_nghttp2_session_recv_continuation(void) buf->pos += cont_hd.length; /* Second CONTINUATION, rest of the bytes */ - cont_hd.length = nghttp2_buf_len(buf); - cont_hd.flags = NGHTTP2_FLAG_END_HEADERS; - cont_hd.stream_id = 1; + nghttp2_frame_hd_init(&cont_hd, nghttp2_buf_len(buf), NGHTTP2_CONTINUATION, + NGHTTP2_FLAG_END_HEADERS, 1); nghttp2_frame_pack_frame_hd(data + datalen, &cont_hd); datalen += NGHTTP2_FRAME_HDLEN; @@ -1239,10 +1236,7 @@ void test_nghttp2_session_recv_unknown_frame(void) nghttp2_frame_hd hd; ssize_t rv; - hd.length = 16000; - hd.stream_id = 0; - hd.type = 99; - hd.flags = NGHTTP2_FLAG_NONE; + nghttp2_frame_hd_init(&hd, 16000, 99, NGHTTP2_FLAG_NONE, 0); nghttp2_frame_pack_frame_hd(data, &hd); datalen = NGHTTP2_FRAME_HDLEN + hd.length; @@ -1275,10 +1269,8 @@ void test_nghttp2_session_recv_unexpected_continuation(void) 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_hd_init(&hd, 16000, NGHTTP2_CONTINUATION, + NGHTTP2_FLAG_END_HEADERS, 1); nghttp2_frame_pack_frame_hd(data, &hd); datalen = NGHTTP2_FRAME_HDLEN + hd.length; @@ -1487,13 +1479,12 @@ void test_nghttp2_session_recv_too_large_frame_length(void) nghttp2_session_callbacks callbacks; uint8_t buf[NGHTTP2_FRAME_HDLEN]; nghttp2_outbound_item *item; - nghttp2_frame_hd hd = { - /* Initial max frame size is NGHTTP2_MAX_FRAME_SIZE_MIN */ - NGHTTP2_MAX_FRAME_SIZE_MIN + 1, - 1, - NGHTTP2_HEADERS, - NGHTTP2_FLAG_NONE - }; + nghttp2_frame_hd hd; + + /* Initial max frame size is NGHTTP2_MAX_FRAME_SIZE_MIN */ + nghttp2_frame_hd_init(&hd, NGHTTP2_MAX_FRAME_SIZE_MIN + 1, + NGHTTP2_HEADERS, NGHTTP2_FLAG_NONE, 1); + memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); nghttp2_session_server_new(&session, &callbacks, NULL); @@ -1665,10 +1656,7 @@ void test_nghttp2_session_continue(void) CU_ASSERT(1 == user_data.frame_recv_cb_called); /* Receive DATA */ - data_hd.length = 16; - data_hd.type = NGHTTP2_DATA; - data_hd.flags = NGHTTP2_FLAG_NONE; - data_hd.stream_id = 1; + nghttp2_frame_hd_init(&data_hd, 16, NGHTTP2_DATA, NGHTTP2_FLAG_NONE, 1); nghttp2_buf_reset(&databuf); nghttp2_frame_pack_frame_hd(databuf.pos, &data_hd); @@ -2630,9 +2618,7 @@ void test_nghttp2_session_on_data_received(void) &pri_spec_default, NGHTTP2_STREAM_OPENING, NULL); - frame.hd.length = 4096; - frame.hd.flags = NGHTTP2_FLAG_NONE; - frame.hd.stream_id = 2; + nghttp2_frame_hd_init(&frame.hd, 4096, NGHTTP2_DATA, NGHTTP2_FLAG_NONE, 2); CU_ASSERT(0 == nghttp2_session_on_data_received(session, &frame)); CU_ASSERT(0 == stream->shut_flags); @@ -4725,10 +4711,9 @@ void test_nghttp2_session_flow_control_data_recv(void) /* Create DATA frame */ memset(data, 0, sizeof(data)); - hd.length = NGHTTP2_MAX_PAYLOADLEN; - hd.type = NGHTTP2_DATA; - hd.flags = NGHTTP2_FLAG_END_STREAM; - hd.stream_id = 1; + nghttp2_frame_hd_init(&hd, NGHTTP2_MAX_PAYLOADLEN, NGHTTP2_DATA, + NGHTTP2_FLAG_END_STREAM, 1); + nghttp2_frame_pack_frame_hd(data, &hd); CU_ASSERT(NGHTTP2_MAX_PAYLOADLEN+NGHTTP2_FRAME_HDLEN == nghttp2_session_mem_recv(session, data, @@ -4792,10 +4777,9 @@ void test_nghttp2_session_flow_control_data_with_padding_recv(void) /* Create DATA frame */ memset(data, 0, sizeof(data)); - hd.length = 357; - hd.type = NGHTTP2_DATA; - hd.flags = NGHTTP2_FLAG_END_STREAM | NGHTTP2_FLAG_PADDED; - hd.stream_id = 1; + nghttp2_frame_hd_init(&hd, 357, NGHTTP2_DATA, + NGHTTP2_FLAG_END_STREAM | NGHTTP2_FLAG_PADDED, 1); + nghttp2_frame_pack_frame_hd(data, &hd); /* Set Pad Length field, which itself is padding */ data[NGHTTP2_FRAME_HDLEN] = 255; @@ -6396,10 +6380,9 @@ void test_nghttp2_session_on_header_temporal_failure(void) nghttp2_hd_deflate_hd_bufs(&deflater, &bufs, &nv[1], 1); - hd.length = nghttp2_bufs_len(&bufs) - hdpos - NGHTTP2_FRAME_HDLEN; - hd.type = NGHTTP2_CONTINUATION; - hd.flags = NGHTTP2_FLAG_END_HEADERS; - hd.stream_id = 1; + nghttp2_frame_hd_init(&hd, + nghttp2_bufs_len(&bufs) - hdpos - NGHTTP2_FRAME_HDLEN, + NGHTTP2_CONTINUATION, NGHTTP2_FLAG_END_HEADERS, 1); nghttp2_frame_pack_frame_hd(&buf->pos[hdpos], &hd);