From c46d3dafc6c92f64b9a2fa06947f8c3b10d6261a Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 7 Jun 2014 18:15:36 +0900 Subject: [PATCH] Remove PAD_HIGH and Pad High field CONTINUATION now doesn't have padding. --- lib/includes/nghttp2/nghttp2.h | 8 +- lib/nghttp2_frame.c | 135 ++++----------------- lib/nghttp2_frame.h | 32 +++-- lib/nghttp2_session.c | 167 ++++++-------------------- lib/nghttp2_session.h | 2 - src/app_helper.cc | 30 +---- tests/main.c | 6 - tests/nghttp2_frame_test.c | 2 +- tests/nghttp2_session_test.c | 213 ++------------------------------- tests/nghttp2_session_test.h | 3 - tests/nghttp2_test_helper.c | 11 +- 11 files changed, 98 insertions(+), 511 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 752594b7..5666d7ae 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -477,13 +477,9 @@ typedef enum { */ NGHTTP2_FLAG_END_SEGMENT = 0x02, /** - * The PAD_LOW flag. + * The PADDED flag. */ - NGHTTP2_FLAG_PAD_LOW = 0x08, - /** - * The PAD_HIGH flag. - */ - NGHTTP2_FLAG_PAD_HIGH = 0x10, + NGHTTP2_FLAG_PADDED = 0x08, /** * The PRIORITY flag. */ diff --git a/lib/nghttp2_frame.c b/lib/nghttp2_frame.c index e6e6d655..72f03dcb 100644 --- a/lib/nghttp2_frame.c +++ b/lib/nghttp2_frame.c @@ -239,9 +239,7 @@ void nghttp2_frame_data_init(nghttp2_data *frame, nghttp2_private_data *pdata) size_t nghttp2_frame_trail_padlen(nghttp2_frame *frame, size_t padlen) { - return padlen - - ((frame->hd.flags & NGHTTP2_FLAG_PAD_HIGH) > 0) - - ((frame->hd.flags & NGHTTP2_FLAG_PAD_LOW) > 0); + return padlen - ((frame->hd.flags & NGHTTP2_FLAG_PADDED) > 0); } void nghttp2_frame_private_data_init(nghttp2_private_data *frame, @@ -1042,40 +1040,18 @@ static void frame_set_pad(nghttp2_buf *buf, size_t padlen) { size_t trail_padlen; - if(padlen > 256) { - DEBUGF(fprintf(stderr, "send: padlen=%zu, shift left 2 bytes\n", padlen)); + DEBUGF(fprintf(stderr, "send: padlen=%zu, shift left 1 bytes\n", padlen)); - memmove(buf->pos - 2, buf->pos, NGHTTP2_FRAME_HDLEN); + memmove(buf->pos - 1, buf->pos, NGHTTP2_FRAME_HDLEN); - buf->pos -= 2; + --buf->pos; - buf->pos[3] |= NGHTTP2_FLAG_PAD_HIGH | NGHTTP2_FLAG_PAD_LOW; + buf->pos[3] |= NGHTTP2_FLAG_PADDED; - nghttp2_put_uint16be(buf->pos, nghttp2_get_uint16(buf->pos) + padlen); + nghttp2_put_uint16be(buf->pos, nghttp2_get_uint16(buf->pos) + padlen); - trail_padlen = padlen - 2; - buf->pos[NGHTTP2_FRAME_HDLEN] = trail_padlen >> 8; - buf->pos[NGHTTP2_FRAME_HDLEN + 1] = trail_padlen & 0xff; - - } else if(padlen > 0) { - DEBUGF(fprintf(stderr, "send: padlen=%zu, shift left 1 bytes\n", padlen)); - - memmove(buf->pos - 1, buf->pos, NGHTTP2_FRAME_HDLEN); - - --buf->pos; - - buf->pos[3] |= NGHTTP2_FLAG_PAD_LOW; - - nghttp2_put_uint16be(buf->pos, nghttp2_get_uint16(buf->pos) + padlen); - - trail_padlen = padlen - 1; - buf->pos[NGHTTP2_FRAME_HDLEN] = trail_padlen; - - } else { - DEBUGF(fprintf(stderr, "send: padlen=0, no shift left was made\n")); - - return; - } + trail_padlen = padlen - 1; + buf->pos[NGHTTP2_FRAME_HDLEN] = trail_padlen; /* zero out padding */ memset(buf->last, 0, trail_padlen); @@ -1087,13 +1063,9 @@ static void frame_set_pad(nghttp2_buf *buf, size_t padlen) } int nghttp2_frame_add_pad(nghttp2_bufs *bufs, nghttp2_frame_hd *hd, - size_t padlen, nghttp2_frame_type type) + size_t padlen) { - int rv; - size_t trail_padlen; - size_t last_avail; nghttp2_buf *buf; - nghttp2_frame_hd last_hd; if(padlen == 0) { DEBUGF(fprintf(stderr, "send: padlen = 0, nothing to do\n")); @@ -1107,87 +1079,28 @@ 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... : + * +-+---------------+---------------------------------------------+ * - * Since we limit the length of the padding bytes, they are - * completely included in one frame payload or across 2 frames. We - * are going to adjust buf->pos of frame which includes part of - * padding. And serialize (memmove) frame header in the correct - * position. Also extends buf->last to include padding. + * We arranged padding so that it is included in the first frame + * completely. For padded frame, we are going to adjust buf->pos of + * frame which includes padding and serialize (memmove) frame header + * in the correct position. Also extends buf->last to include + * padding. */ - nghttp2_bufs_seek_last_present(bufs); + buf = &bufs->head->buf; - buf = &bufs->cur->buf; + assert(nghttp2_buf_avail(buf) >= (ssize_t)padlen - 1); - last_avail = nghttp2_buf_avail(buf); - - if(last_avail >= padlen) { - /* Last frame can include all paddings bytes */ - DEBUGF(fprintf(stderr, "send: last frame includes all paddings\n")); - - frame_set_pad(buf, padlen); - - } else { - /* padding across 2 frames */ - - /* type = DATA must not be here */ - assert(type == NGHTTP2_CONTINUATION); - - /* This will seek to the last chain */ - rv = nghttp2_bufs_advance(bufs); - if(rv == NGHTTP2_ERR_BUFFER_ERROR) { - return NGHTTP2_ERR_FRAME_SIZE_ERROR; - } - - if(rv != 0) { - return rv; - } - - if(type == NGHTTP2_CONTINUATION) { - /* former last frame has END_HEADERS flag set. Clear it. */ - buf->pos[3] &= ~NGHTTP2_FLAG_END_HEADERS; - } - - trail_padlen = nghttp2_buf_avail(buf); - - /* former last frame may have zero buffer available */ - if(trail_padlen == 0) { - DEBUGF(fprintf(stderr, - "send: last frame has no space to include padding\n")); - } else { - DEBUGF(fprintf(stderr, "send: padding across 2 frames\n")); - - frame_set_pad(buf, trail_padlen); - } - - /* This buffer does not have frame header serialized */ - buf = &bufs->cur->buf; - - trail_padlen = padlen - trail_padlen; - - last_hd.length = 0; - last_hd.type = type; - last_hd.stream_id = hd->stream_id; - - if(type == NGHTTP2_CONTINUATION) { - last_hd.flags = NGHTTP2_FLAG_END_HEADERS; - } else { - last_hd.flags = NGHTTP2_FLAG_NONE; - } - - buf->pos -= NGHTTP2_FRAME_HDLEN; - nghttp2_frame_pack_frame_hd(buf->pos, &last_hd); - - frame_set_pad(buf, trail_padlen); - } + frame_set_pad(buf, padlen); hd->length += padlen; + hd->flags |= NGHTTP2_FLAG_PADDED; DEBUGF(fprintf(stderr, "send: final payloadlen=%zu, padlen=%zu\n", hd->length, padlen)); diff --git a/lib/nghttp2_frame.h b/lib/nghttp2_frame.h index cd9e9d27..0d2cfb49 100644 --- a/lib/nghttp2_frame.h +++ b/lib/nghttp2_frame.h @@ -45,11 +45,10 @@ #define NGHTTP2_MAX_PAYLOADLEN 16383 /* The one frame buffer length for tranmission. We may use several of - them to support CONTINUATION. To account for padding specifiers - (PAD_HIGH and PAD_LOW), we allocate extra 2 bytes, which saves - extra large memcopying. */ + them to support CONTINUATION. To account for Pad Length field, we + allocate extra 1 byte, which saves extra large memcopying. */ #define NGHTTP2_FRAMEBUF_CHUNKLEN \ - (NGHTTP2_FRAME_HDLEN + 2 + NGHTTP2_MAX_PAYLOADLEN) + (NGHTTP2_FRAME_HDLEN + 1 + NGHTTP2_MAX_PAYLOADLEN) /* The maximum length of DATA frame payload. */ #define NGHTTP2_DATA_PAYLOADLEN 4096 @@ -93,8 +92,8 @@ typedef struct { */ nghttp2_data_provider data_prd; /** - * The number of bytes added as padding. This includes PAD_HIGH and - * PAD_LOW. + * The number of bytes added as padding. This includes Pad Length + * field (1 byte). */ size_t padlen; /** @@ -169,7 +168,7 @@ int nghttp2_frame_pack_headers(nghttp2_bufs *bufs, /* * Unpacks HEADERS frame byte sequence into |frame|. This function * only unapcks bytes that come before name/value header block and - * after PAD_HIGH and PAD_LOW. + * after possible Pad Length field. * * This function always succeeds and returns 0. */ @@ -301,7 +300,7 @@ int nghttp2_frame_pack_push_promise(nghttp2_bufs *bufs, /* * Unpacks PUSH_PROMISE frame byte sequence into |frame|. This * function only unapcks bytes that come before name/value header - * block and after PAD_HIGH and PAD_LOW. + * block and after possible Pad Length field. * * This function returns 0 if it succeeds or one of the following * negative error codes: @@ -542,9 +541,9 @@ void nghttp2_frame_blocked_free(nghttp2_blocked *frame); void nghttp2_frame_data_init(nghttp2_data *frame, nghttp2_private_data *pdata); /* - * Returns the number of padding bytes after payload. The total - * padding length is given in the |padlen|. The returned value does - * not include the PAD_HIGH and PAD_LOW. + * Returns the number of padding bytes after payload. The total + * padding length is given in the |padlen|. The returned value does + * not include the Pad Length field. */ size_t nghttp2_frame_trail_padlen(nghttp2_frame *frame, size_t padlen); @@ -606,11 +605,10 @@ void nghttp2_nv_array_del(nghttp2_nv *nva); int nghttp2_iv_check(const nghttp2_settings_entry *iv, size_t niv); /* - * Sets PAD_HIGH and PAD_LOW fields, flags and adjust frame header - * position of each buffers in |bufs|. The padding is given in the - * |padlen|. The |hd| is the frame header for the serialized data. - * The |type| is used as a frame type when padding requires additional - * buffers. + * Sets Pad Length field and flags and adjusts frame header position + * of each buffers in |bufs|. The number of padding is given in the + * |padlen| including Pad Length field. The |hd| is the frame header + * for the serialized data. * * This function returns 0 if it succeeds, or one of the following * negative error codes: @@ -621,6 +619,6 @@ int nghttp2_iv_check(const nghttp2_settings_entry *iv, size_t niv); * The length of the resulting frame is too large. */ int nghttp2_frame_add_pad(nghttp2_bufs *bufs, nghttp2_frame_hd *hd, - size_t padlen, nghttp2_frame_type type); + size_t padlen); #endif /* NGHTTP2_FRAME_H */ diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index e8752f02..13ed17a5 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -308,10 +308,10 @@ static int session_new(nghttp2_session **session_ptr, (*session_ptr)->server = 1; } - /* 2 for PAD_HIGH and PAD_LOW. */ + /* 1 for Pad Field. */ rv = nghttp2_bufs_init3(&(*session_ptr)->aob.framebufs, NGHTTP2_FRAMEBUF_CHUNKLEN, 8, 1, - NGHTTP2_FRAME_HDLEN + 2); + NGHTTP2_FRAME_HDLEN + 1); if(rv != 0) { goto fail_aob_framebuf; } @@ -1391,11 +1391,21 @@ static ssize_t session_call_select_padding(nghttp2_session *session, size_t max_payloadlen) { ssize_t rv; + + if(frame->hd.length >= max_payloadlen) { + return frame->hd.length; + } + if(session->callbacks.select_padding_callback) { + size_t max_paddedlen; + + /* 256 is maximum padding size */ + max_paddedlen = nghttp2_min(frame->hd.length + 256, max_payloadlen); + rv = session->callbacks.select_padding_callback(session, frame, - max_payloadlen, + max_paddedlen, session->user_data); - if(rv < (ssize_t)frame->hd.length || rv > (ssize_t)max_payloadlen) { + if(rv < (ssize_t)frame->hd.length || rv > (ssize_t)max_paddedlen) { return NGHTTP2_ERR_CALLBACK_FAILURE; } return rv; @@ -1419,7 +1429,7 @@ static int session_headers_add_pad(nghttp2_session *session, framebufs = &aob->framebufs; padded_payloadlen = session_call_select_padding(session, frame, - frame->hd.length + 1024); + NGHTTP2_MAX_PAYLOADLEN); if(nghttp2_is_fatal(padded_payloadlen)) { return padded_payloadlen; } @@ -1430,8 +1440,8 @@ static int session_headers_add_pad(nghttp2_session *session, "send: padding selected: payloadlen=%zu, padlen=%zu\n", padded_payloadlen, padlen)); - rv = nghttp2_frame_add_pad(framebufs, &frame->hd, padlen, - NGHTTP2_CONTINUATION); + rv = nghttp2_frame_add_pad(framebufs, &frame->hd, padlen); + if(rv != 0) { return rv; } @@ -4097,24 +4107,14 @@ static int inbound_frame_set_settings_entry(nghttp2_inbound_frame *iframe) } /* - * Checks PAD_HIGH and PAD_LOW flags and set iframe->sbuf to read them - * accordingly. If padding is set, this function returns 1. If no - * padding is set, this function returns 0. On error, returns -1. + * Checks PADDED flags and set iframe->sbuf to read them accordingly. + * If padding is set, this function returns 1. If no padding is set, + * this function returns 0. On error, returns -1. */ static int inbound_frame_handle_pad(nghttp2_inbound_frame *iframe, nghttp2_frame_hd *hd) { - if(hd->flags & NGHTTP2_FLAG_PAD_HIGH) { - if((hd->flags & NGHTTP2_FLAG_PAD_LOW) == 0) { - return -1; - } - if(hd->length < 2) { - return -1; - } - inbound_frame_set_mark(iframe, 2); - return 1; - } - if(hd->flags & NGHTTP2_FLAG_PAD_LOW) { + if(hd->flags & NGHTTP2_FLAG_PADDED) { if(hd->length < 1) { return -1; } @@ -4133,20 +4133,13 @@ static ssize_t inbound_frame_compute_pad(nghttp2_inbound_frame *iframe) { size_t padlen; - padlen = iframe->sbuf.pos[0]; - - if(iframe->frame.hd.flags & NGHTTP2_FLAG_PAD_HIGH) { - padlen <<= 8; - padlen |= iframe->sbuf.pos[1]; - ++padlen; - } - - ++padlen; + /* 1 for Pad Length field */ + padlen = iframe->sbuf.pos[0] + 1; DEBUGF(fprintf(stderr, "recv: padlen=%zu\n", padlen)); /* We cannot use iframe->frame.hd.length because of CONTINUATION */ - if(padlen - (padlen > 255) - 1 > iframe->payloadleft) { + if(padlen - 1 > iframe->payloadleft) { return -1; } @@ -4225,8 +4218,7 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, iframe->frame.hd.flags &= (NGHTTP2_FLAG_END_STREAM | NGHTTP2_FLAG_END_SEGMENT | - NGHTTP2_FLAG_PAD_LOW | - NGHTTP2_FLAG_PAD_HIGH); + NGHTTP2_FLAG_PADDED); /* Check stream is open. If it is not open or closing, ignore payload. */ busy = 1; @@ -4270,8 +4262,7 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, iframe->frame.hd.flags &= (NGHTTP2_FLAG_END_STREAM | NGHTTP2_FLAG_END_SEGMENT | NGHTTP2_FLAG_END_HEADERS | - NGHTTP2_FLAG_PAD_LOW | - NGHTTP2_FLAG_PAD_HIGH | + NGHTTP2_FLAG_PADDED | NGHTTP2_FLAG_PRIORITY); rv = inbound_frame_handle_pad(iframe, &iframe->frame.hd); @@ -4397,8 +4388,7 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, DEBUGF(fprintf(stderr, "recv: PUSH_PROMISE\n")); iframe->frame.hd.flags &= (NGHTTP2_FLAG_END_HEADERS | - NGHTTP2_FLAG_PAD_LOW | - NGHTTP2_FLAG_PAD_HIGH); + NGHTTP2_FLAG_PADDED); rv = inbound_frame_handle_pad(iframe, &iframe->frame.hd); if(rv < 0) { @@ -4556,7 +4546,7 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, switch(iframe->frame.hd.type) { case NGHTTP2_HEADERS: if(iframe->padlen == 0 && - iframe->frame.hd.flags & NGHTTP2_FLAG_PAD_LOW) { + iframe->frame.hd.flags & NGHTTP2_FLAG_PADDED) { rv = inbound_frame_compute_pad(iframe); if(rv < 0) { busy = 1; @@ -4622,7 +4612,7 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, break; case NGHTTP2_PUSH_PROMISE: if(iframe->padlen == 0 && - iframe->frame.hd.flags & NGHTTP2_FLAG_PAD_LOW) { + iframe->frame.hd.flags & NGHTTP2_FLAG_PADDED) { rv = inbound_frame_compute_pad(iframe); if(rv < 0) { busy = 1; @@ -4815,11 +4805,6 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, if(iframe->payloadleft) { break; } - /* Clear PAD_HIGH and PAD_LOW, because we rely on those flags in - the next CONTINUATION frame. Also we don't show these flags - to user callback */ - iframe->frame.hd.flags &= - ~(NGHTTP2_FLAG_PAD_HIGH | NGHTTP2_FLAG_PAD_LOW); if((iframe->frame.hd.flags & NGHTTP2_FLAG_END_HEADERS) == 0) { @@ -5000,34 +4985,11 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, break; } - iframe->frame.hd.flags |= cont_hd.flags & - (NGHTTP2_FLAG_END_HEADERS | - NGHTTP2_FLAG_PAD_HIGH | NGHTTP2_FLAG_PAD_LOW); + /* CONTINUATION won't bear NGHTTP2_PADDED flag */ + + iframe->frame.hd.flags |= cont_hd.flags & NGHTTP2_FLAG_END_HEADERS; iframe->frame.hd.length += cont_hd.length; - rv = inbound_frame_handle_pad(iframe, &cont_hd); - if(rv < 0) { - busy = 1; - - iframe->state = NGHTTP2_IB_IGN_PAYLOAD; - - rv = nghttp2_session_terminate_session(session, - NGHTTP2_PROTOCOL_ERROR); - if(nghttp2_is_fatal(rv)) { - return rv; - } - break; - } - - if(rv == 1) { - if(iframe->state == NGHTTP2_IB_EXPECT_CONTINUATION) { - iframe->state = NGHTTP2_IB_READ_PAD_CONTINUATION; - } else { - iframe->state = NGHTTP2_IB_IGN_PAD_CONTINUATION; - } - break; - } - busy = 1; if(iframe->state == NGHTTP2_IB_EXPECT_CONTINUATION) { @@ -5036,58 +4998,6 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, iframe->state = NGHTTP2_IB_IGN_HEADER_BLOCK; } - break; - case NGHTTP2_IB_READ_PAD_CONTINUATION: - case NGHTTP2_IB_IGN_PAD_CONTINUATION: -#ifdef DEBUGBUILD - if(iframe->state == NGHTTP2_IB_READ_PAD_CONTINUATION) { - fprintf(stderr, "recv: [IB_READ_PAD_CONTINUATION]\n"); - } else { - fprintf(stderr, "recv: [IB_IGN_PAD_CONTINUATION]\n"); - } -#endif /* DEBUGBUILD */ - - readlen = inbound_frame_buf_read(iframe, in, last); - in += readlen; - iframe->payloadleft -= readlen; - - DEBUGF(fprintf(stderr, "recv: readlen=%zu, payloadleft=%zu, left=%zu\n", - readlen, iframe->payloadleft, - nghttp2_buf_mark_avail(&iframe->sbuf))); - - if(nghttp2_buf_mark_avail(&iframe->sbuf)) { - return in - first; - } - - busy = 1; - - rv = inbound_frame_compute_pad(iframe); - if(rv < 0) { - rv = nghttp2_session_terminate_session(session, - NGHTTP2_PROTOCOL_ERROR); - if(nghttp2_is_fatal(rv)) { - return rv; - } - - iframe->state = NGHTTP2_IB_IGN_PAYLOAD; - - break; - } - - iframe->padlen = rv; - - if(iframe->frame.hd.type == NGHTTP2_HEADERS) { - iframe->frame.headers.padlen += rv; - } else { - iframe->frame.push_promise.padlen += rv; - } - - if(iframe->state == NGHTTP2_IB_READ_PAD_CONTINUATION) { - iframe->state = NGHTTP2_IB_READ_HEADER_BLOCK; - } else { - iframe->state = NGHTTP2_IB_IGN_HEADER_BLOCK; - } - break; case NGHTTP2_IB_READ_PAD_DATA: DEBUGF(fprintf(stderr, "recv: [IB_READ_PAD_DATA]\n")); @@ -5100,7 +5010,7 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, readlen, iframe->payloadleft, nghttp2_buf_mark_avail(&iframe->sbuf))); - /* PAD_HIGH and PAD_LOW are subject to flow control */ + /* Pad Length field is subject to flow control */ rv = session_update_recv_connection_window_size(session, readlen); if(nghttp2_is_fatal(rv)) { return rv; @@ -5118,9 +5028,7 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, } } - if(nghttp2_buf_mark_avail(&iframe->sbuf)) { - return in - first; - } + assert(nghttp2_buf_mark_avail(&iframe->sbuf) == 0); busy = 1; @@ -5198,11 +5106,6 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, break; } - /* Clear PAD_HIGH and PAD_LOW, because we don't show these flags - to user callback */ - session->iframe.frame.hd.flags &= - ~(NGHTTP2_FLAG_PAD_HIGH | NGHTTP2_FLAG_PAD_LOW); - rv = session_process_data_frame(session); if(nghttp2_is_fatal(rv)) { return rv; @@ -5561,7 +5464,7 @@ int nghttp2_session_pack_data(nghttp2_session *session, nghttp2_frame_pack_frame_hd(buf->pos, &hd); - rv = nghttp2_frame_add_pad(bufs, &hd, padlen, NGHTTP2_DATA); + rv = nghttp2_frame_add_pad(bufs, &hd, padlen); if(rv != 0) { return rv; } diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h index 74117e74..514adf8c 100644 --- a/lib/nghttp2_session.h +++ b/lib/nghttp2_session.h @@ -76,8 +76,6 @@ typedef enum { NGHTTP2_IB_READ_ALTSVC, NGHTTP2_IB_EXPECT_CONTINUATION, NGHTTP2_IB_IGN_CONTINUATION, - NGHTTP2_IB_READ_PAD_CONTINUATION, - NGHTTP2_IB_IGN_PAD_CONTINUATION, NGHTTP2_IB_READ_PAD_DATA, NGHTTP2_IB_READ_DATA, NGHTTP2_IB_IGN_DATA diff --git a/src/app_helper.cc b/src/app_helper.cc index c07caa26..f7e1905b 100644 --- a/src/app_helper.cc +++ b/src/app_helper.cc @@ -229,17 +229,11 @@ void print_flags(const nghttp2_frame_hd& hd) } s += "END_SEGMENT"; } - if(hd.flags & NGHTTP2_FLAG_PAD_LOW) { + if(hd.flags & NGHTTP2_FLAG_PADDED) { if(!s.empty()) { s += " | "; } - s += "PAD_LOW"; - } - if(hd.flags & NGHTTP2_FLAG_PAD_HIGH) { - if(!s.empty()) { - s += " | "; - } - s += "PAD_HIGH"; + s += "PADDED"; } break; case NGHTTP2_HEADERS: @@ -258,17 +252,11 @@ void print_flags(const nghttp2_frame_hd& hd) } s += "END_HEADERS"; } - if(hd.flags & NGHTTP2_FLAG_PAD_LOW) { + if(hd.flags & NGHTTP2_FLAG_PADDED) { if(!s.empty()) { s += " | "; } - s += "PAD_LOW"; - } - if(hd.flags & NGHTTP2_FLAG_PAD_HIGH) { - if(!s.empty()) { - s += " | "; - } - s += "PAD_HIGH"; + s += "PADDED"; } if(hd.flags & NGHTTP2_FLAG_PRIORITY) { if(!s.empty()) { @@ -289,17 +277,11 @@ void print_flags(const nghttp2_frame_hd& hd) if(hd.flags & NGHTTP2_FLAG_END_HEADERS) { s += "END_HEADERS"; } - if(hd.flags & NGHTTP2_FLAG_PAD_LOW) { + if(hd.flags & NGHTTP2_FLAG_PADDED) { if(!s.empty()) { s += " | "; } - s += "PAD_LOW"; - } - if(hd.flags & NGHTTP2_FLAG_PAD_HIGH) { - if(!s.empty()) { - s += " | "; - } - s += "PAD_HIGH"; + s += "PADDED"; } break; case NGHTTP2_PING: diff --git a/tests/main.c b/tests/main.c index 302ecde2..5c168ad2 100644 --- a/tests/main.c +++ b/tests/main.c @@ -209,12 +209,6 @@ int main(int argc, char* argv[]) test_nghttp2_session_pack_data_with_padding) || !CU_add_test(pSuite, "session_pack_headers_with_padding", test_nghttp2_session_pack_headers_with_padding) || - !CU_add_test(pSuite, "session_pack_headers_with_padding2", - test_nghttp2_session_pack_headers_with_padding2) || - !CU_add_test(pSuite, "session_pack_headers_with_padding3", - test_nghttp2_session_pack_headers_with_padding3) || - !CU_add_test(pSuite, "session_pack_headers_with_padding4", - test_nghttp2_session_pack_headers_with_padding4) || !CU_add_test(pSuite, "pack_settings_payload", test_nghttp2_pack_settings_payload) || !CU_add_test(pSuite, "session_stream_dep_add", diff --git a/tests/nghttp2_frame_test.c b/tests/nghttp2_frame_test.c index 28080c87..7ccb8077 100644 --- a/tests/nghttp2_frame_test.c +++ b/tests/nghttp2_frame_test.c @@ -499,7 +499,7 @@ void test_nghttp2_frame_pack_altsvc(void) buf = &bufs.head->buf; - CU_ASSERT(buf->pos - buf->begin == 2); + CU_ASSERT(buf->pos - buf->begin == 1); /* Check no origin case */ diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index c84a33d0..9a04bdd0 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -74,7 +74,7 @@ typedef struct { int begin_headers_cb_called; nghttp2_nv nv; size_t data_chunk_len; - size_t padding_boundary; + size_t padlen; } my_user_data; static void scripted_data_feed_init2(scripted_data_feed *df, @@ -225,9 +225,7 @@ static ssize_t select_padding_callback(nghttp2_session *session, void *user_data) { my_user_data *ud = (my_user_data*)user_data; - return nghttp2_min(max_payloadlen, - (frame->hd.length + ud->padding_boundary - 1) - / ud->padding_boundary * ud->padding_boundary); + return nghttp2_min(max_payloadlen, frame->hd.length + ud->padlen); } static ssize_t fixed_length_data_source_read_callback @@ -4240,13 +4238,11 @@ void test_nghttp2_session_flow_control_data_with_padding_recv(void) memset(data, 0, sizeof(data)); hd.length = 357; hd.type = NGHTTP2_DATA; - hd.flags = NGHTTP2_FLAG_END_STREAM | - NGHTTP2_FLAG_PAD_HIGH | NGHTTP2_FLAG_PAD_LOW;; + hd.flags = NGHTTP2_FLAG_END_STREAM | NGHTTP2_FLAG_PADDED; hd.stream_id = 1; nghttp2_frame_pack_frame_hd(data, &hd); - /* Add 2 byte padding (PAD_LOW itself is padding) */ - data[NGHTTP2_FRAME_HDLEN] = 1; - data[NGHTTP2_FRAME_HDLEN + 1] = 1; + /* Set Pad Length field, which itself is padding */ + data[NGHTTP2_FRAME_HDLEN] = 255; CU_ASSERT((ssize_t)(NGHTTP2_FRAME_HDLEN + hd.length) == nghttp2_session_mem_recv(session, data, @@ -4665,7 +4661,7 @@ void test_nghttp2_session_pack_data_with_padding(void) nghttp2_session_client_new(&session, &callbacks, &ud); - ud.padding_boundary = 512; + ud.padlen = 63; nghttp2_submit_request(session, NULL, NULL, 0, &data_prd, NULL); ud.block_count = 1; @@ -4676,33 +4672,8 @@ void test_nghttp2_session_pack_data_with_padding(void) frame = OB_DATA(session->aob.item); - CU_ASSERT(ud.padding_boundary - datalen == frame->padlen); - /* We no longer set PAD_HIGH and PAD_LOW flags in frame->hd */ - /* CU_ASSERT(frame->hd.flags & NGHTTP2_FLAG_PAD_LOW); */ - /* CU_ASSERT(frame->hd.flags & NGHTTP2_FLAG_PAD_HIGH); */ - - /* Check reception of this DATA frame */ - check_session_recv_data_with_padding(&session->aob.framebufs, datalen); - - nghttp2_session_del(session); - - /* Check without PAD_HIGH */ - nghttp2_session_client_new(&session, &callbacks, &ud); - - ud.padding_boundary = 64; - - nghttp2_submit_request(session, NULL, NULL, 0, &data_prd, NULL); - ud.block_count = 1; - ud.data_source_length = datalen; - /* Sends HEADERS */ - CU_ASSERT(0 == nghttp2_session_send(session)); - CU_ASSERT(NGHTTP2_HEADERS == ud.sent_frame_type); - - frame = OB_DATA(session->aob.item); - CU_ASSERT((frame->padlen + datalen) % ud.padding_boundary == 0); - /* We no longer set PAD_HIGH and PAD_LOW flags in frame->hd */ - /* CU_ASSERT(frame->hd.flags & NGHTTP2_FLAG_PAD_LOW); */ - CU_ASSERT(0 == (frame->hd.flags & NGHTTP2_FLAG_PAD_HIGH)); + CU_ASSERT(ud.padlen == frame->padlen); + CU_ASSERT(frame->hd.flags & NGHTTP2_FLAG_PADDED); /* Check reception of this DATA frame */ check_session_recv_data_with_padding(&session->aob.framebufs, datalen); @@ -4711,167 +4682,6 @@ void test_nghttp2_session_pack_data_with_padding(void) } void test_nghttp2_session_pack_headers_with_padding(void) -{ - nghttp2_session *session, *sv_session; - accumulator acc; - my_user_data ud; - nghttp2_session_callbacks callbacks; - nghttp2_nv nva[8172]; - size_t i; - - for(i = 0; i < ARRLEN(nva); ++i) { - nva[i].name = (uint8_t*)":path"; - nva[i].namelen = 5; - nva[i].value = (uint8_t*)"/"; - nva[i].valuelen = 1; - nva[i].flags = NGHTTP2_NV_FLAG_NONE; - } - - memset(&callbacks, 0, sizeof(callbacks)); - callbacks.send_callback = accumulator_send_callback; - callbacks.on_frame_send_callback = on_frame_send_callback; - callbacks.select_padding_callback = select_padding_callback; - callbacks.on_frame_recv_callback = on_frame_recv_callback; - - acc.length = 0; - ud.acc = &acc; - - /* In this test, padding is laid out across 2 frames: HEADERS and - CONTINUATION frames */ - nghttp2_session_client_new(&session, &callbacks, &ud); - nghttp2_session_server_new(&sv_session, &callbacks, &ud); - - ud.padding_boundary = 16385; - - CU_ASSERT(1 == - nghttp2_submit_request(session, NULL, - nva, ARRLEN(nva), NULL, NULL)); - CU_ASSERT(0 == nghttp2_session_send(session)); - - CU_ASSERT(acc.length > NGHTTP2_MAX_PAYLOADLEN); - ud.frame_recv_cb_called = 0; - CU_ASSERT((ssize_t)acc.length == - nghttp2_session_mem_recv(sv_session, acc.buf, acc.length)); - CU_ASSERT(1 == ud.frame_recv_cb_called); - CU_ASSERT(NULL == nghttp2_session_get_next_ob_item(sv_session)); - - /* Check PUSH_PROMISE */ - CU_ASSERT(2 == - nghttp2_submit_push_promise(sv_session, NGHTTP2_FLAG_NONE, 1, - nva, ARRLEN(nva), NULL)); - acc.length = 0; - CU_ASSERT(0 == nghttp2_session_send(sv_session)); - - CU_ASSERT(acc.length > NGHTTP2_MAX_PAYLOADLEN); - ud.frame_recv_cb_called = 0; - CU_ASSERT((ssize_t)acc.length == - nghttp2_session_mem_recv(session, acc.buf, acc.length)); - CU_ASSERT(1 == ud.frame_recv_cb_called); - CU_ASSERT(NULL == nghttp2_session_get_next_ob_item(session)); - - nghttp2_session_del(sv_session); - nghttp2_session_del(session); -} - -void test_nghttp2_session_pack_headers_with_padding2(void) -{ - nghttp2_session *session, *sv_session; - accumulator acc; - my_user_data ud; - nghttp2_session_callbacks callbacks; - nghttp2_nv nva[16364]; - size_t i; - - for(i = 0; i < ARRLEN(nva); ++i) { - nva[i].name = (uint8_t*)":path"; - nva[i].namelen = 5; - nva[i].value = (uint8_t*)"/"; - nva[i].valuelen = 1; - nva[i].flags = NGHTTP2_NV_FLAG_NONE; - } - - memset(&callbacks, 0, sizeof(callbacks)); - callbacks.send_callback = accumulator_send_callback; - callbacks.on_frame_send_callback = on_frame_send_callback; - callbacks.select_padding_callback = select_padding_callback; - callbacks.on_frame_recv_callback = on_frame_recv_callback; - - acc.length = 0; - ud.acc = &acc; - - /* In this test, padding is laid out across 2 CONTINUATION frames */ - nghttp2_session_client_new(&session, &callbacks, &ud); - nghttp2_session_server_new(&sv_session, &callbacks, &ud); - - ud.padding_boundary = 16385; - - CU_ASSERT(1 == - nghttp2_submit_request(session, NULL, - nva, ARRLEN(nva), NULL, NULL)); - CU_ASSERT(0 == nghttp2_session_send(session)); - - CU_ASSERT(acc.length > NGHTTP2_MAX_PAYLOADLEN); - - ud.frame_recv_cb_called = 0; - CU_ASSERT((ssize_t)acc.length == - nghttp2_session_mem_recv(sv_session, acc.buf, acc.length)); - CU_ASSERT(1 == ud.frame_recv_cb_called); - CU_ASSERT(NULL == nghttp2_session_get_next_ob_item(sv_session)); - - nghttp2_session_del(sv_session); - nghttp2_session_del(session); -} - -void test_nghttp2_session_pack_headers_with_padding3(void) -{ - nghttp2_session *session, *sv_session; - accumulator acc; - my_user_data ud; - nghttp2_session_callbacks callbacks; - nghttp2_nv nva[9120]; - size_t i; - - for(i = 0; i < ARRLEN(nva); ++i) { - nva[i].name = (uint8_t*)":path"; - nva[i].namelen = 5; - nva[i].value = (uint8_t*)"/"; - nva[i].valuelen = 1; - nva[i].flags = NGHTTP2_NV_FLAG_NONE; - } - - memset(&callbacks, 0, sizeof(callbacks)); - callbacks.send_callback = accumulator_send_callback; - callbacks.on_frame_send_callback = on_frame_send_callback; - callbacks.select_padding_callback = select_padding_callback; - callbacks.on_frame_recv_callback = on_frame_recv_callback; - - acc.length = 0; - ud.acc = &acc; - - /* In this test, padding is included in the last CONTINUATION - frame */ - nghttp2_session_client_new(&session, &callbacks, &ud); - nghttp2_session_server_new(&sv_session, &callbacks, &ud); - - ud.padding_boundary = 16385; - - CU_ASSERT(1 == - nghttp2_submit_request(session, NULL, - nva, ARRLEN(nva), NULL, NULL)); - CU_ASSERT(0 == nghttp2_session_send(session)); - - CU_ASSERT(acc.length > NGHTTP2_MAX_PAYLOADLEN); - ud.frame_recv_cb_called = 0; - CU_ASSERT((ssize_t)acc.length == - nghttp2_session_mem_recv(sv_session, acc.buf, acc.length)); - CU_ASSERT(1 == ud.frame_recv_cb_called); - CU_ASSERT(NULL == nghttp2_session_get_next_ob_item(sv_session)); - - nghttp2_session_del(sv_session); - nghttp2_session_del(session); -} - -void test_nghttp2_session_pack_headers_with_padding4(void) { nghttp2_session *session, *sv_session; accumulator acc; @@ -4888,15 +4698,12 @@ void test_nghttp2_session_pack_headers_with_padding4(void) acc.length = 0; ud.acc = &acc; - /* In this test, padding is included in the first HEADERS frame */ nghttp2_session_client_new(&session, &callbacks, &ud); nghttp2_session_server_new(&sv_session, &callbacks, &ud); - ud.padding_boundary = 16385; + ud.padlen = 163; - CU_ASSERT(1 == - nghttp2_submit_request(session, NULL, - &nv, 1, NULL, NULL)); + CU_ASSERT(1 == nghttp2_submit_request(session, NULL, &nv, 1, NULL, NULL)); CU_ASSERT(0 == nghttp2_session_send(session)); CU_ASSERT(acc.length < NGHTTP2_MAX_PAYLOADLEN); diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h index a49bf115..03834cd0 100644 --- a/tests/nghttp2_session_test.h +++ b/tests/nghttp2_session_test.h @@ -97,9 +97,6 @@ void test_nghttp2_session_set_option(void); void test_nghttp2_session_data_backoff_by_high_pri_frame(void); void test_nghttp2_session_pack_data_with_padding(void); void test_nghttp2_session_pack_headers_with_padding(void); -void test_nghttp2_session_pack_headers_with_padding2(void); -void test_nghttp2_session_pack_headers_with_padding3(void); -void test_nghttp2_session_pack_headers_with_padding4(void); void test_nghttp2_pack_settings_payload(void); void test_nghttp2_session_stream_dep_add(void); void test_nghttp2_session_stream_dep_remove(void); diff --git a/tests/nghttp2_test_helper.c b/tests/nghttp2_test_helper.c index 828bf4f5..42869e31 100644 --- a/tests/nghttp2_test_helper.c +++ b/tests/nghttp2_test_helper.c @@ -53,8 +53,7 @@ int unpack_frame(nghttp2_frame *frame, const uint8_t *in, size_t len) nghttp2_frame_unpack_frame_hd(&frame->hd, in); switch(frame->hd.type) { case NGHTTP2_HEADERS: - payloadoff = ((frame->hd.flags & NGHTTP2_FLAG_PAD_HIGH) > 0) + - ((frame->hd.flags & NGHTTP2_FLAG_PAD_LOW) > 0); + payloadoff = ((frame->hd.flags & NGHTTP2_FLAG_PADDED) > 0); rv = nghttp2_frame_unpack_headers_payload (&frame->headers, payload + payloadoff, payloadlen - payloadoff); break; @@ -224,14 +223,14 @@ ssize_t inflate_hd(nghttp2_hd_inflater *inflater, nva_out *out, int frame_pack_bufs_init(nghttp2_bufs *bufs) { - /* 2 for PAD_HIGH and PAD_LOW */ - return nghttp2_bufs_init2(bufs, 4096, 16, NGHTTP2_FRAME_HDLEN + 2); + /* 1 for Pad Length */ + return nghttp2_bufs_init2(bufs, 4096, 16, NGHTTP2_FRAME_HDLEN + 1); } void bufs_large_init(nghttp2_bufs *bufs, size_t chunk_size) { - /* 2 for PAD_HIGH and PAD_LOW */ - nghttp2_bufs_init2(bufs, chunk_size, 16, NGHTTP2_FRAME_HDLEN + 2); + /* 1 for Pad Length */ + nghttp2_bufs_init2(bufs, chunk_size, 16, NGHTTP2_FRAME_HDLEN + 1); } static nghttp2_stream* open_stream_with_all(nghttp2_session *session,