From f26270b5b462ffc535fad30da60d1d628375b4cf Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 6 Feb 2014 22:06:42 +0900 Subject: [PATCH] Change SETTINGS payload format according to the spec --- lib/nghttp2_frame.c | 20 ++++++++++---------- lib/nghttp2_frame.h | 3 +++ lib/nghttp2_session.c | 8 ++++---- lib/nghttp2_submit.c | 3 ++- tests/nghttp2_frame_test.c | 6 ++++-- tests/nghttp2_session_test.c | 4 ++-- 6 files changed, 25 insertions(+), 19 deletions(-) diff --git a/lib/nghttp2_frame.c b/lib/nghttp2_frame.c index 451aea55..1b9a4c6b 100644 --- a/lib/nghttp2_frame.c +++ b/lib/nghttp2_frame.c @@ -110,7 +110,8 @@ void nghttp2_frame_settings_init(nghttp2_settings *frame, uint8_t flags, nghttp2_settings_entry *iv, size_t niv) { memset(frame, 0, sizeof(nghttp2_settings)); - nghttp2_frame_set_hd(&frame->hd, niv*8, NGHTTP2_SETTINGS, flags, 0); + nghttp2_frame_set_hd(&frame->hd, niv * NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH, + NGHTTP2_SETTINGS, flags, 0); frame->niv = niv; frame->iv = iv; } @@ -341,11 +342,11 @@ size_t nghttp2_frame_pack_settings_payload(uint8_t *buf, size_t niv) { size_t i; - for(i = 0; i < niv; ++i, buf += 8) { - nghttp2_put_uint32be(buf, iv[i].settings_id); - nghttp2_put_uint32be(buf + 4, iv[i].value); + for(i = 0; i < niv; ++i, buf += NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH) { + buf[0] = iv[i].settings_id; + nghttp2_put_uint32be(buf + 1, iv[i].value); } - return 8 * niv; + return NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH * niv; } int nghttp2_frame_unpack_settings_payload(nghttp2_settings *frame, @@ -366,9 +367,8 @@ int nghttp2_frame_unpack_settings_payload(nghttp2_settings *frame, void nghttp2_frame_unpack_settings_entry(nghttp2_settings_entry *iv, const uint8_t *payload) { - iv->settings_id = nghttp2_get_uint32(&payload[0]) & - NGHTTP2_SETTINGS_ID_MASK; - iv->value = nghttp2_get_uint32(&payload[4]); + iv->settings_id = payload[0]; + iv->value = nghttp2_get_uint32(&payload[1]); } int nghttp2_frame_unpack_settings_payload2(nghttp2_settings_entry **iv_ptr, @@ -377,13 +377,13 @@ int nghttp2_frame_unpack_settings_payload2(nghttp2_settings_entry **iv_ptr, size_t payloadlen) { size_t i; - *niv_ptr = payloadlen / 8; + *niv_ptr = payloadlen / NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH; *iv_ptr = malloc((*niv_ptr)*sizeof(nghttp2_settings_entry)); if(*iv_ptr == NULL) { return NGHTTP2_ERR_NOMEM; } for(i = 0; i < *niv_ptr; ++i) { - size_t off = i*8; + size_t off = i * NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH; nghttp2_frame_unpack_settings_entry(&(*iv_ptr)[i], &payload[off]); } return 0; diff --git a/lib/nghttp2_frame.h b/lib/nghttp2_frame.h index fe6b995f..841bce35 100644 --- a/lib/nghttp2_frame.h +++ b/lib/nghttp2_frame.h @@ -48,6 +48,9 @@ /* The number of bytes of frame header. */ #define NGHTTP2_FRAME_HEAD_LENGTH 8 +/* The number of bytes for each SETTINGS entry */ +#define NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH 5 + /* Category of frames. */ typedef enum { /* non-DATA frame */ diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 4432d0fd..7020947a 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -3390,7 +3390,7 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, break; case NGHTTP2_SETTINGS: DEBUGF(fprintf(stderr, "SETTINGS\n")); - if((iframe->frame.hd.length & 0x7) || + if((iframe->frame.hd.length % NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH) || ((iframe->frame.hd.flags & NGHTTP2_FLAG_ACK) && iframe->payloadleft > 0)) { busy = 1; @@ -3399,7 +3399,7 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, } iframe->state = NGHTTP2_IB_READ_SETTINGS; if(iframe->payloadleft) { - iframe->left = 8; + iframe->left = NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH; break; } busy = 1; @@ -3629,7 +3629,7 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, } } if(iframe->payloadleft) { - iframe->left = 8; + iframe->left = NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH; iframe->buflen = 0; break; } @@ -4070,7 +4070,7 @@ int nghttp2_session_upgrade(nghttp2_session *session, (session->server && session->last_recv_stream_id >= 1)) { return NGHTTP2_ERR_PROTO; } - if(settings_payloadlen % 8) { + if(settings_payloadlen % NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH) { return NGHTTP2_ERR_INVALID_ARGUMENT; } rv = nghttp2_frame_unpack_settings_payload2(&iv, &niv, settings_payload, diff --git a/lib/nghttp2_submit.c b/lib/nghttp2_submit.c index 4096dece..3ef60ad4 100644 --- a/lib/nghttp2_submit.c +++ b/lib/nghttp2_submit.c @@ -323,8 +323,9 @@ ssize_t nghttp2_pack_settings_payload(uint8_t *buf, return NGHTTP2_ERR_INVALID_ARGUMENT; } - if(buflen < (niv * 8)) + if(buflen < (niv * NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH)) { return NGHTTP2_ERR_INSUFF_BUFSIZE; + } return nghttp2_frame_pack_settings_payload(buf, iv, niv); } diff --git a/tests/nghttp2_frame_test.c b/tests/nghttp2_frame_test.c index 08e7f698..74d01563 100644 --- a/tests/nghttp2_frame_test.c +++ b/tests/nghttp2_frame_test.c @@ -226,9 +226,11 @@ void test_nghttp2_frame_pack_settings() nghttp2_frame_settings_init(&frame, NGHTTP2_FLAG_NONE, nghttp2_frame_iv_copy(iv, 3), 3); framelen = nghttp2_frame_pack_settings(&buf, &buflen, &frame); - CU_ASSERT(NGHTTP2_FRAME_HEAD_LENGTH+3*8 == framelen); + CU_ASSERT(NGHTTP2_FRAME_HEAD_LENGTH + + 3 * NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH == framelen); CU_ASSERT(0 == unpack_frame((nghttp2_frame*)&oframe, buf, framelen)); - check_frame_header(3*8, NGHTTP2_SETTINGS, NGHTTP2_FLAG_NONE, 0, &oframe.hd); + check_frame_header(3 * NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH, + NGHTTP2_SETTINGS, NGHTTP2_FLAG_NONE, 0, &oframe.hd); CU_ASSERT(3 == oframe.niv); for(i = 0; i < 3; ++i) { CU_ASSERT(iv[i].settings_id == oframe.iv[i].settings_id); diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 5ec1b04a..e42529f7 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -3828,7 +3828,7 @@ void test_nghttp2_pack_settings_payload(void) iv[1].value = 4095; len = nghttp2_pack_settings_payload(buf, sizeof(buf), iv, 2); - CU_ASSERT(16 == len); + CU_ASSERT(2 * NGHTTP2_FRAME_SETTINGS_ENTRY_LENGTH == len); CU_ASSERT(0 == nghttp2_frame_unpack_settings_payload2(&resiv, &resniv, buf, len)); CU_ASSERT(2 == resniv); @@ -3839,6 +3839,6 @@ void test_nghttp2_pack_settings_payload(void) free(resiv); - len = nghttp2_pack_settings_payload(buf, 15 /* too small */, iv, 2); + len = nghttp2_pack_settings_payload(buf, 9 /* too small */, iv, 2); CU_ASSERT(NGHTTP2_ERR_INSUFF_BUFSIZE == len); }