diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index b975644f..7fab91f8 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -1141,10 +1141,7 @@ static ssize_t nghttp2_session_prep_frame(nghttp2_session *session, return framebuflen; } padded_payloadlen = session_call_select_padding - (session, frame, - (frame->hd.length == 0 ? NGHTTP2_MAX_FRAME_LENGTH : - (frame->hd.length + NGHTTP2_MAX_FRAME_LENGTH - 1) - / NGHTTP2_MAX_FRAME_LENGTH * NGHTTP2_MAX_FRAME_LENGTH)); + (session, frame, frame->hd.length + 1024); if(nghttp2_is_fatal(padded_payloadlen)) { return padded_payloadlen; } @@ -1152,13 +1149,46 @@ static ssize_t nghttp2_session_prep_frame(nghttp2_session *session, frame->headers.padlen = padded_payloadlen - frame->hd.length; frame->hd.length = padded_payloadlen; + DEBUGF(fprintf(stderr, "payloadlen=%zu, padlen=%zu\n", + frame->hd.length, frame->headers.padlen)); + if(frame->hd.length > NGHTTP2_MAX_FRAME_LENGTH) { - /* PAD_HIGH and PAD_LOW will be added in - nghttp2_session_after_frame_sent(). */ - /* This may make framebuflen > session->aob.framebufmax. But + if(NGHTTP2_MAX_FRAME_LENGTH > + frame->hd.length - frame->headers.padlen) { + size_t padlen = NGHTTP2_MAX_FRAME_LENGTH - + (frame->hd.length - frame->headers.padlen); + nghttp2_frame_hd hd = frame->hd; + + DEBUGF(fprintf(stderr, "padding across 2 frames\n")); + + hd.flags &= ~NGHTTP2_FLAG_END_HEADERS; + hd.length = NGHTTP2_MAX_FRAME_LENGTH; + + DEBUGF(fprintf(stderr, "first HEADERS payloadlen=%zu, padlen=%zu\n", + hd.length, padlen)); + + r = nghttp2_frame_add_pad(&session->aob.framebuf, + &session->aob.framebufmax, + &session->aob.framebufoff, + &hd.flags, + hd.length - padlen, + padlen); + if(nghttp2_is_fatal(r)) { + return r; + } + framebuflen = session->aob.framebufoff + frame->hd.length + + NGHTTP2_FRAME_HEAD_LENGTH; + + nghttp2_frame_pack_frame_hd + (session->aob.framebuf + session->aob.framebufoff, &hd); + } else { + /* PAD_HIGH and PAD_LOW will be added in + nghttp2_session_after_frame_sent(). */ + framebuflen += frame->headers.padlen; + } + /* At this point, framebuflen > session->aob.framebufmax. But before we access the missing part, we will allocate it in nghttp2_session_after_frame_sent(). */ - framebuflen += frame->headers.padlen; } else if(frame->hd.length <= NGHTTP2_MAX_FRAME_LENGTH && frame->headers.padlen > 0) { r = nghttp2_frame_add_pad(&session->aob.framebuf, @@ -1504,17 +1534,24 @@ static int nghttp2_session_after_frame_sent(nghttp2_session *session) session->aob.framebufoff -= NGHTTP2_FRAME_HEAD_LENGTH; if(cont_hd.length + session->aob.framebufmark == session->aob.framebuflen) { + size_t padlen; + if(cont_hd.length < frame->headers.padlen) { + padlen = cont_hd.length; + } else { + padlen = frame->headers.padlen; + } DEBUGF(fprintf(stderr, "last CONTINUATION payloadlen=%zu, padlen=%zu\n", - cont_hd.length, frame->headers.padlen)); + cont_hd.length, padlen)); cont_hd.flags = NGHTTP2_FLAG_END_HEADERS; + rv = nghttp2_frame_add_pad(&session->aob.framebuf, &session->aob.framebufmax, &session->aob.framebufoff, &cont_hd.flags, - cont_hd.length - frame->headers.padlen, - frame->headers.padlen); + cont_hd.length - padlen, + padlen); if(nghttp2_is_fatal(rv)) { return rv; } @@ -1524,9 +1561,6 @@ static int nghttp2_session_after_frame_sent(nghttp2_session *session) session->aob.framebuflen = session->aob.framebufmark = session->aob.framebufoff + NGHTTP2_FRAME_HEAD_LENGTH + cont_hd.length; - /* Or-ing flags so that we can show these flags in - callback */ - frame->hd.flags |= cont_hd.flags; } else { cont_hd.flags = NGHTTP2_FLAG_NONE; session->aob.framebufmark += cont_hd.length; @@ -3506,6 +3540,8 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, nghttp2_frame_unpack_frame_hd(&iframe->frame.hd, iframe->buf); iframe->payloadleft = iframe->frame.hd.length; + DEBUGF(fprintf(stderr, "payloadlen=%zu\n", iframe->payloadleft)); + switch(iframe->frame.hd.type) { case NGHTTP2_DATA: { DEBUGF(fprintf(stderr, "DATA\n")); @@ -3841,6 +3877,10 @@ 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. */ + iframe->frame.hd.flags &= + ~(NGHTTP2_FLAG_PAD_HIGH | NGHTTP2_FLAG_PAD_LOW); if((iframe->frame.hd.flags & NGHTTP2_FLAG_END_HEADERS) == 0) { inbound_frame_reset_left(iframe, NGHTTP2_FRAME_HEAD_LENGTH); iframe->padlen = 0; @@ -3950,6 +3990,9 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, } nghttp2_frame_unpack_frame_hd(&cont_hd, iframe->buf); iframe->payloadleft = cont_hd.length; + + DEBUGF(fprintf(stderr, "payloadlen=%zu\n", iframe->payloadleft)); + if(cont_hd.type != NGHTTP2_CONTINUATION || cont_hd.stream_id != iframe->frame.hd.stream_id) { DEBUGF(fprintf(stderr, "expected stream_id=%d, type=%d, but " diff --git a/tests/main.c b/tests/main.c index 415acc03..2c038721 100644 --- a/tests/main.c +++ b/tests/main.c @@ -199,6 +199,8 @@ int main(int argc, char* argv[]) test_nghttp2_session_data_backoff_by_high_pri_frame) || !CU_add_test(pSuite, "session_pack_data_with_padding", 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, "pack_settings_payload", test_nghttp2_pack_settings_payload) || !CU_add_test(pSuite, "frame_pack_headers", diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 0669ee96..dca5ba86 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -40,7 +40,7 @@ #define OB_DATA(ITEM) nghttp2_outbound_item_get_data_frame(ITEM) typedef struct { - uint8_t buf[4096]; + uint8_t buf[65535]; size_t length; } accumulator; @@ -4039,6 +4039,54 @@ void test_nghttp2_session_pack_data_with_padding(void) nghttp2_session_del(session); } +void test_nghttp2_session_pack_headers_with_padding(void) +{ + nghttp2_session *session, *sv_session; + accumulator acc; + my_user_data ud; + nghttp2_session_callbacks callbacks, sv_callbacks; + nghttp2_nv nva[8190]; + 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; + } + + 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; + + acc.length = 0; + ud.acc = &acc; + + memset(&sv_callbacks, 0, sizeof(sv_callbacks)); + sv_callbacks.on_frame_recv_callback = on_frame_recv_callback; + + nghttp2_session_client_new(&session, &callbacks, &ud); + nghttp2_session_server_new(&sv_session, &sv_callbacks, &ud); + + ud.padding_boundary = 16385; + + CU_ASSERT(0 == + nghttp2_submit_request(session, NGHTTP2_PRI_DEFAULT, + nva, ARRLEN(nva), NULL, NULL)); + CU_ASSERT(0 == nghttp2_session_send(session)); + + CU_ASSERT(acc.length > NGHTTP2_MAX_FRAME_LENGTH); + 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_pack_settings_payload(void) { nghttp2_settings_entry iv[2]; diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h index 626963a4..26680a58 100644 --- a/tests/nghttp2_session_test.h +++ b/tests/nghttp2_session_test.h @@ -90,6 +90,7 @@ void test_nghttp2_session_get_effective_local_window_size(void); 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_pack_settings_payload(void); #endif /* NGHTTP2_SESSION_TEST_H */