From 22a4e3eab83ef81c798ba3fe2fe632d3113f152d Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 17 Jan 2014 22:52:30 +0900 Subject: [PATCH] Remove nghttp2_session_continue nghttp2_session_continue is removed. Now just call nghttp2_session_mem_recv to continue after NGHTTP2_ERR_PAUSE. --- lib/includes/nghttp2/nghttp2.h | 54 +++--------- lib/nghttp2_session.c | 96 +++++++++++++++++---- tests/nghttp2_session_test.c | 148 ++++++++++++++++++++++++++------- 3 files changed, 209 insertions(+), 89 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 54e0727d..5b1a987e 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -913,13 +913,12 @@ typedef int (*nghttp2_on_invalid_frame_recv_callback) * * If the application uses `nghttp2_session_mem_recv()`, it can return * :enum:`NGHTTP2_ERR_PAUSE` to make `nghttp2_session_mem_recv()` - * return without processing further input bytes. The |frame| - * parameter is retained until `nghttp2_session_continue()` is + * return without processing further input bytes. The memory by + * pointed by the |data| is retained until + * `nghttp2_session_mem_recv()` or `nghttp2_session_recv()` is * called. The application must retain the input bytes which was used - * to produce the |frame| parameter, because it may refer to the - * memory region included in the input bytes. The application which - * returns :enum:`NGHTTP2_ERR_PAUSE` must call - * `nghttp2_session_continue()` before `nghttp2_session_mem_recv()`. + * to produce the |data| parameter, because it may refer to the memory + * region included in the input bytes. * * The implementation of this function must return 0 if it * succeeds. If nonzero is returned, it is treated as fatal error and @@ -1124,14 +1123,12 @@ typedef int (*nghttp2_on_unknown_frame_recv_callback) * * If the application uses `nghttp2_session_mem_recv()`, it can return * :enum:`NGHTTP2_ERR_PAUSE` to make `nghttp2_session_mem_recv()` - * return without processing further input bytes. The |frame|, - * |name|, |namelen|, |value| and |valuelen| parameters are retained - * until `nghttp2_session_continue()` is called. The application must - * retain the input bytes which was used to produce the |frame| - * parameter, because it may refer to the memory region included in - * the input bytes. The application which returns - * :enum:`NGHTTP2_ERR_PAUSE` must call `nghttp2_session_continue()` - * before `nghttp2_session_mem_recv()`. + * return without processing further input bytes. The memory pointed + * by |frame|, |name| and |value| parameters are retained until + * `nghttp2_session_mem_recv()` or `nghttp2_session_recv()` is + * called. The application must retain the input bytes which was used + * to produce these parameters, because it may refer to the memory + * region included in the input bytes. * * The implementation of this function must return 0 if it * succeeds. It may return :enum:`NGHTTP2_ERR_PAUSE`. If the other @@ -1548,7 +1545,7 @@ int nghttp2_session_recv(nghttp2_session *session); * In the current implementation, this function always tries to * processes all input data unless either an error occurs or * :enum:`NGHTTP2_ERR_PAUSE` is returned from - * :member:`nghttp2_session_callbacks.on_frame_recv_callback` or + * :member:`nghttp2_session_callbacks.on_header_callback` or * :member:`nghttp2_session_callbacks.on_data_chunk_recv_callback`. * If :enum:`NGHTTP2_ERR_PAUSE` is used, the return value includes the * number of bytes which was used to produce the data or frame for the @@ -1565,33 +1562,6 @@ int nghttp2_session_recv(nghttp2_session *session); ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in, size_t inlen); -/** - * @function - * - * Perform post-processing after `nghttp2_session_mem_recv()` was - * paused by :enum:`NGHTTP2_ERR_PAUSE` from - * :member:`nghttp2_session_callbacks.on_frame_recv_callback` or - * :member:`nghttp2_session_callbacks.on_data_chunk_recv_callback`. - * - * This function frees resources associated with paused frames. It - * may also call additional callbacks, such as - * :member:`nghttp2_session_callbacks.on_stream_close_callback`. - * - * If this function succeeds, the application can call - * `nghttp2_session_mem_recv()` again. - * - * This function returns 0 if it succeeds, or one of the following - * negative error codes: - * - * - * :enum:`NGHTTP2_ERR_NOMEM` - * Out of memory. - * :enum:`NGHTTP2_ERR_CALLBACK_FAILURE` - * The callback function failed. - * - */ -int nghttp2_session_continue(nghttp2_session *session); - /** * @function * diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 07f8b3a1..9cdbd414 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -2046,6 +2046,21 @@ static int nghttp2_session_inflate_handle_invalid_connection return nghttp2_session_handle_invalid_connection(session, frame, error_code); } +/* + * Decompress header blocks of incoming request HEADERS and also call + * additional callbacks. This function can be called again if this + * function returns NGHTTP2_ERR_PAUSE. + * + * This function returns 0 if it succeeds, or one of negative error + * codes: + * + * NGHTTP2_ERR_CALLBACK_FAILURE + * The callback function failed. + * NGHTTP2_ERR_NOMEM + * Out of memory. + * NGHTTP2_ERR_PAUSE + * The callback function returned NGHTTP2_ERR_PAUSE + */ static int session_end_request_headers_received(nghttp2_session *session, nghttp2_frame *frame) { @@ -2067,6 +2082,21 @@ static int session_end_request_headers_received(nghttp2_session *session, return 0; } +/* + * Decompress header blocks of incoming (push-)response HEADERS and + * also call additional callbacks. This function can be called again + * if this function returns NGHTTP2_ERR_PAUSE. + * + * This function returns 0 if it succeeds, or one of negative error + * codes: + * + * NGHTTP2_ERR_CALLBACK_FAILURE + * The callback function failed. + * NGHTTP2_ERR_NOMEM + * Out of memory. + * NGHTTP2_ERR_PAUSE + * The callback function returned NGHTTP2_ERR_PAUSE + */ static int session_end_response_headers_received(nghttp2_session *session, nghttp2_frame *frame) { @@ -2089,6 +2119,21 @@ static int session_end_response_headers_received(nghttp2_session *session, return 0; } +/* + * Decompress header blocks of incoming HEADERS and also call + * additional callbacks. This function can be called again if this + * function returns NGHTTP2_ERR_PAUSE. + * + * This function returns 0 if it succeeds, or one of negative error + * codes: + * + * NGHTTP2_ERR_CALLBACK_FAILURE + * The callback function failed. + * NGHTTP2_ERR_NOMEM + * Out of memory. + * NGHTTP2_ERR_PAUSE + * The callback function returned NGHTTP2_ERR_PAUSE + */ static int session_end_headers_received(nghttp2_session *session, nghttp2_frame *frame) { @@ -3390,40 +3435,46 @@ static int nghttp2_session_check_data_recv_allowed(nghttp2_session *session, return 0; } -int nghttp2_session_continue(nghttp2_session *session) +/* + * Handles paused header emission callbacks. This function may return + * NGHTTP2_ERR_PAUSE again if the application just do so in a + * callback. + * + * This function returns 0 if it succeeds, or one of the following + * negative error codes: + * + * :enum:`NGHTTP2_ERR_NOMEM` + * Out of memory. + * :enum:`NGHTTP2_ERR_CALLBACK_FAILURE` + * The callback function failed. + * :enum:`NGHTTP2_ERR_PAUSE` + * The callback function returned NGHTTP2_ERR_PAUSE + */ +static int session_continue(nghttp2_session *session) { nghttp2_frame *frame = &session->iframe.frame; int rv = 0; - if(session->iframe.error_code != NGHTTP2_ERR_PAUSE) { - return 0; - } switch(frame->hd.type) { - case NGHTTP2_DATA: - /* To call on_data_recv_callback */ - return nghttp2_session_mem_recv(session, NULL, 0); case NGHTTP2_HEADERS: - switch(session->iframe.frame.headers.cat) { + switch(frame->headers.cat) { case NGHTTP2_HCAT_REQUEST: - rv = session_end_request_headers_received(session, - &session->iframe.frame); + rv = session_end_request_headers_received(session, frame); break; case NGHTTP2_HCAT_RESPONSE: case NGHTTP2_HCAT_PUSH_RESPONSE: - rv = session_end_response_headers_received(session, - &session->iframe.frame); + rv = session_end_response_headers_received(session, frame); break; case NGHTTP2_HCAT_HEADERS: - rv = session_end_headers_received(session, &session->iframe.frame); + rv = session_end_headers_received(session, frame); break; } break; case NGHTTP2_PUSH_PROMISE: - rv = inflate_header_block(session, &session->iframe.frame, 1); + rv = inflate_header_block(session, frame, 1); break; default: break; } - nghttp2_inbound_frame_reset(session); return rv; } @@ -3431,6 +3482,21 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in, size_t inlen) { const uint8_t *inmark, *inlimit; + if(session->iframe.error_code == NGHTTP2_ERR_PAUSE) { + int rv; + rv = session_continue(session); + if(rv == NGHTTP2_ERR_PAUSE) { + return 0; + } + if(rv != 0) { + return rv; + } + /* For DATA, we have to go without resetting in order to call + on_data_recv_callback. */ + if(session->iframe.frame.hd.type != NGHTTP2_DATA) { + nghttp2_inbound_frame_reset(session); + } + } inmark = in; inlimit = in+inlen; while(1) { diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index af97741b..3ccd593e 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -69,6 +69,9 @@ typedef struct { int data_recv_cb_called; const nghttp2_frame *frame; size_t fixed_sendlen; + int header_cb_called; + int end_headers_cb_called; + nghttp2_nv nv; } my_user_data; static void scripted_data_feed_init(scripted_data_feed *df, @@ -146,16 +149,6 @@ static int on_frame_recv_callback(nghttp2_session *session, return 0; } -static int pause_on_frame_recv_callback(nghttp2_session *session, - const nghttp2_frame *frame, - void *user_data) -{ - my_user_data *ud = (my_user_data*)user_data; - ++ud->frame_recv_cb_called; - ud->frame = frame; - return NGHTTP2_ERR_PAUSE; -} - static int on_invalid_frame_recv_callback(nghttp2_session *session, const nghttp2_frame *frame, nghttp2_error_code error_code, @@ -287,6 +280,44 @@ static ssize_t block_count_send_callback(nghttp2_session* session, return r; } +static int on_header_callback(nghttp2_session *session, + const nghttp2_frame *frame, + const uint8_t *name, size_t namelen, + const uint8_t *value, size_t valuelen, + void *user_data) +{ + my_user_data *ud = (my_user_data*)user_data; + ++ud->header_cb_called; + ud->nv.name = (uint8_t*)name; + ud->nv.namelen = namelen; + ud->nv.value = (uint8_t*)value; + ud->nv.valuelen = valuelen; + + ud->frame = frame; + return 0; +} + +static int pause_on_header_callback(nghttp2_session *session, + const nghttp2_frame *frame, + const uint8_t *name, size_t namelen, + const uint8_t *value, size_t valuelen, + void *user_data) +{ + on_header_callback(session, frame, name, namelen, value, valuelen, + user_data); + return NGHTTP2_ERR_PAUSE; +} + +static int on_end_headers_callback(nghttp2_session *session, + const nghttp2_frame *frame, + nghttp2_error_code error_code, + void *user_data) +{ + my_user_data *ud = (my_user_data*)user_data; + ++ud->end_headers_cb_called; + return 0; +} + static ssize_t defer_data_source_read_callback (nghttp2_session *session, int32_t stream_id, uint8_t *buf, size_t len, int *eof, @@ -639,16 +670,19 @@ void test_nghttp2_session_continue(void) nghttp2_session_callbacks callbacks; my_user_data user_data; const nghttp2_nv nv1[] = { - MAKE_NV("url", "/") + MAKE_NV(":method", "GET"), + MAKE_NV(":path", "/") }; const nghttp2_nv nv2[] = { - MAKE_NV("user-agent", "nghttp2/1.0.0") + MAKE_NV("user-agent", "nghttp2/1.0.0"), + MAKE_NV("alpha", "bravo") }; uint8_t *framedata = NULL; size_t framedatalen = 0; ssize_t framelen1, framelen2; ssize_t rv; uint8_t buffer[4096]; + uint8_t *bufp = buffer; size_t buflen; nghttp2_frame frame; nghttp2_nv *nva; @@ -657,14 +691,13 @@ void test_nghttp2_session_continue(void) nghttp2_frame_hd data_hd; nghttp2_hd_context deflater; - /* TODO TBD */ - return; - memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); callbacks.send_callback = null_send_callback; - callbacks.on_frame_recv_callback = pause_on_frame_recv_callback; + callbacks.on_frame_recv_callback = on_frame_recv_callback; callbacks.on_data_chunk_recv_callback = pause_on_data_chunk_recv_callback; callbacks.on_data_recv_callback = on_data_recv_callback; + callbacks.on_header_callback = pause_on_header_callback; + callbacks.on_end_headers_callback = on_end_headers_callback; nghttp2_session_server_new(&session, &callbacks, &user_data); @@ -675,8 +708,8 @@ void test_nghttp2_session_continue(void) nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, 1, NGHTTP2_PRI_DEFAULT, nva, nvlen); framelen1 = nghttp2_frame_pack_headers(&framedata, &framedatalen, - &frame.headers, - &deflater); + &frame.headers, + &deflater); nghttp2_frame_headers_free(&frame.headers); memcpy(buffer, framedata, framelen1); @@ -693,31 +726,69 @@ void test_nghttp2_session_continue(void) buflen = framelen1 + framelen2; /* Receive 1st HEADERS and pause */ - rv = nghttp2_session_mem_recv(session, buffer, buflen); + user_data.header_cb_called = 0; + user_data.end_headers_cb_called = 0; + rv = nghttp2_session_mem_recv(session, bufp, buflen); CU_ASSERT(rv == framelen1); + bufp += framelen1; + buflen -= framelen1; recv_frame = user_data.frame; CU_ASSERT(NGHTTP2_HEADERS == recv_frame->hd.type); CU_ASSERT(framelen1 - NGHTTP2_FRAME_HEAD_LENGTH == recv_frame->hd.length); - CU_ASSERT(1 == recv_frame->headers.nvlen); - CU_ASSERT(nghttp2_nv_equal(&nv1[0], recv_frame->headers.nva)); - rv = nghttp2_session_continue(session); - CU_ASSERT(rv == 0); + CU_ASSERT(1 == user_data.header_cb_called); + CU_ASSERT(0 == user_data.end_headers_cb_called); - /* Receive 2nd HEADERS and pause */ - rv = nghttp2_session_mem_recv(session, buffer + framelen1, - buflen - framelen1); + CU_ASSERT(nghttp2_nv_equal(&nv1[0], &user_data.nv)); + + /* get 2nd header field */ + user_data.header_cb_called = 0; + user_data.end_headers_cb_called = 0; + rv = nghttp2_session_mem_recv(session, bufp, buflen); + CU_ASSERT(0 == rv); + + CU_ASSERT(1 == user_data.header_cb_called); + CU_ASSERT(0 == user_data.end_headers_cb_called); + + CU_ASSERT(nghttp2_nv_equal(&nv1[1], &user_data.nv)); + + /* will call end_headers_callback and receive 2nd HEADERS and pause */ + user_data.header_cb_called = 0; + user_data.end_headers_cb_called = 0; + rv = nghttp2_session_mem_recv(session, bufp, buflen); CU_ASSERT(rv == framelen2); + bufp += framelen2; + buflen -= framelen2; recv_frame = user_data.frame; CU_ASSERT(NGHTTP2_HEADERS == recv_frame->hd.type); CU_ASSERT(framelen2 - NGHTTP2_FRAME_HEAD_LENGTH == recv_frame->hd.length); - CU_ASSERT(1 == recv_frame->headers.nvlen); - CU_ASSERT(nghttp2_nv_equal(&nv2[0], recv_frame->headers.nva)); - rv = nghttp2_session_continue(session); - CU_ASSERT(rv == 0); + CU_ASSERT(1 == user_data.header_cb_called); + CU_ASSERT(1 == user_data.end_headers_cb_called); + + CU_ASSERT(nghttp2_nv_equal(&nv2[0], &user_data.nv)); + + /* get 2nd header field */ + user_data.header_cb_called = 0; + user_data.end_headers_cb_called = 0; + rv = nghttp2_session_mem_recv(session, bufp, buflen); + CU_ASSERT(0 == rv); + + CU_ASSERT(1 == user_data.header_cb_called); + CU_ASSERT(0 == user_data.end_headers_cb_called); + + CU_ASSERT(nghttp2_nv_equal(&nv2[1], &user_data.nv)); + + /* No input data, just call on_end_headers_callback */ + user_data.header_cb_called = 0; + user_data.end_headers_cb_called = 0; + rv = nghttp2_session_mem_recv(session, bufp, buflen); + CU_ASSERT(0 == rv); + + CU_ASSERT(0 == user_data.header_cb_called); + CU_ASSERT(1 == user_data.end_headers_cb_called); /* Receive DATA */ data_hd.length = 16; @@ -725,14 +796,27 @@ void test_nghttp2_session_continue(void) data_hd.flags = NGHTTP2_FLAG_NONE; data_hd.stream_id = 1; nghttp2_frame_pack_frame_hd(buffer, &data_hd); + bufp = buffer; + buflen = sizeof(buffer); /* Intentionally specify larger buffer size to see pause is kicked in. */ + user_data.data_recv_cb_called = 0; rv = nghttp2_session_mem_recv(session, buffer, sizeof(buffer)); CU_ASSERT(16 + NGHTTP2_FRAME_HEAD_LENGTH == rv); + CU_ASSERT(0 == user_data.data_recv_cb_called); + /* Next nghttp2_session_mem_recv invokes on_data_recv_callback and + pause again in on_data_chunk_recv_callback since we pass same + DATA frame. */ user_data.data_recv_cb_called = 0; - rv = nghttp2_session_continue(session); - CU_ASSERT(rv == 0); + rv = nghttp2_session_mem_recv(session, buffer, sizeof(buffer)); + CU_ASSERT(16 + NGHTTP2_FRAME_HEAD_LENGTH == rv); + CU_ASSERT(1 == user_data.data_recv_cb_called); + + /* And finally call on_data_recv_callback with 0 size input */ + user_data.data_recv_cb_called = 0; + rv = nghttp2_session_mem_recv(session, NULL, 0); + CU_ASSERT(0 == rv); CU_ASSERT(1 == user_data.data_recv_cb_called); free(framedata);