From e8de437d5c18fa162da3d395223217dff11a37a0 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 7 May 2014 23:24:07 +0900 Subject: [PATCH] Return new stream ID from nghttp2_submit_{request, headers, push_promise} Previously stream ID was assigned just before HEADERS or PUSH_PROMISE was serialized and nghttp2_submit_{request, headers, push_promise} did not return stream ID. The application has to check assigned stream ID using before_frame_send_callback. Now it is apparent that priority is meant to DATA transfer only. Also application can reorder the requests if it wants. Therefore we can assign stream ID in nghttp2_submit_* functions and return stream ID from them. With this change, now application does not have to check stream ID using before_frame_send_callback and its code will be simplified. --- examples/client.c | 40 +++----- examples/libevent-client.c | 35 ++----- lib/includes/nghttp2/nghttp2.h | 76 +++++++-------- lib/nghttp2_frame.c | 3 +- lib/nghttp2_frame.h | 1 + lib/nghttp2_session.c | 60 +----------- lib/nghttp2_submit.c | 83 +++++++++++++---- src/HttpServer.cc | 61 ++++++------ src/h2load_http2_session.cc | 24 ++--- src/nghttp.cc | 164 +++++++++++++-------------------- src/shrpx_http2_session.cc | 42 ++------- tests/nghttp2_frame_test.c | 6 +- tests/nghttp2_session_test.c | 113 +++++++++++++---------- 13 files changed, 293 insertions(+), 415 deletions(-) diff --git a/examples/client.c b/examples/client.c index 959bea39..b8fc7217 100644 --- a/examples/client.c +++ b/examples/client.c @@ -197,29 +197,6 @@ static ssize_t recv_callback(nghttp2_session *session, return rv; } -/* - * The implementation of nghttp2_before_frame_send_callback type. We - * use this function to get stream ID of the request. This is because - * stream ID is not known when we submit the request - * (nghttp2_submit_request). - */ -static int before_frame_send_callback(nghttp2_session *session, - const nghttp2_frame *frame, - void *user_data) -{ - if(frame->hd.type == NGHTTP2_HEADERS && - frame->headers.cat == NGHTTP2_HCAT_REQUEST) { - struct Request *req; - int32_t stream_id = frame->hd.stream_id; - req = nghttp2_session_get_stream_user_data(session, stream_id); - if(req && req->stream_id == -1) { - req->stream_id = stream_id; - printf("[INFO] Stream ID = %d\n", stream_id); - } - } - return 0; -} - static int on_frame_send_callback(nghttp2_session *session, const nghttp2_frame *frame, void *user_data) { @@ -335,7 +312,6 @@ static void setup_nghttp2_callbacks(nghttp2_session_callbacks *callbacks) memset(callbacks, 0, sizeof(nghttp2_session_callbacks)); callbacks->send_callback = send_callback; callbacks->recv_callback = recv_callback; - callbacks->before_frame_send_callback = before_frame_send_callback; callbacks->on_frame_send_callback = on_frame_send_callback; callbacks->on_frame_recv_callback = on_frame_recv_callback; callbacks->on_stream_close_callback = on_stream_close_callback; @@ -470,7 +446,7 @@ static void ctl_poll(struct pollfd *pollfd, struct Connection *connection) */ static void submit_request(struct Connection *connection, struct Request *req) { - int rv; + int32_t stream_id; const nghttp2_nv nva[] = { /* Make sure that the last item is NULL */ MAKE_NV(":method", "GET"), @@ -480,11 +456,17 @@ static void submit_request(struct Connection *connection, struct Request *req) MAKE_NV("accept", "*/*"), MAKE_NV("user-agent", "nghttp2/"NGHTTP2_VERSION) }; - rv = nghttp2_submit_request(connection->session, NULL, - nva, sizeof(nva)/sizeof(nva[0]), NULL, req); - if(rv != 0) { - diec("nghttp2_submit_request", rv); + + stream_id = nghttp2_submit_request(connection->session, NULL, + nva, sizeof(nva)/sizeof(nva[0]), + NULL, req); + + if(stream_id < 0) { + diec("nghttp2_submit_request", stream_id); } + + req->stream_id = stream_id; + printf("[INFO] Stream ID = %d\n", stream_id); } /* diff --git a/examples/libevent-client.c b/examples/libevent-client.c index 251d7ecf..e423bd6e 100644 --- a/examples/libevent-client.c +++ b/examples/libevent-client.c @@ -188,28 +188,6 @@ static ssize_t send_callback(nghttp2_session *session, return length; } -/* nghttp2_before_frame_send_callback: Called when nghttp2 library is - about to send a frame. We use this callback to get stream ID of new - stream. Since HEADERS in HTTP/2 has several roles, we check that - it is a HTTP request HEADERS. */ -static int before_frame_send_callback -(nghttp2_session *session, const nghttp2_frame *frame, void *user_data) -{ - http2_session_data *session_data = (http2_session_data*)user_data; - http2_stream_data *stream_data; - - if(frame->hd.type == NGHTTP2_HEADERS && - frame->headers.cat == NGHTTP2_HCAT_REQUEST) { - stream_data = - (http2_stream_data*)nghttp2_session_get_stream_user_data - (session, frame->hd.stream_id); - if(stream_data == session_data->stream_data) { - stream_data->stream_id = frame->hd.stream_id; - } - } - return 0; -} - /* nghttp2_on_header_callback: Called when nghttp2 library emits single header name/value pair. */ static int on_header_callback(nghttp2_session *session, @@ -357,7 +335,6 @@ static void initialize_nghttp2_session(http2_session_data *session_data) memset(&callbacks, 0, sizeof(callbacks)); callbacks.send_callback = send_callback; - callbacks.before_frame_send_callback = before_frame_send_callback; callbacks.on_frame_recv_callback = on_frame_recv_callback; callbacks.on_data_chunk_recv_callback = on_data_chunk_recv_callback; callbacks.on_stream_close_callback = on_stream_close_callback; @@ -394,7 +371,7 @@ static void send_client_connection_header(http2_session_data *session_data) /* Send HTTP request to the remote peer */ static void submit_request(http2_session_data *session_data) { - int rv; + int32_t stream_id; http2_stream_data *stream_data = session_data->stream_data; const char *uri = stream_data->uri; const struct http_parser_url *u = stream_data->u; @@ -407,11 +384,13 @@ static void submit_request(http2_session_data *session_data) }; fprintf(stderr, "Request headers:\n"); print_headers(stderr, hdrs, ARRLEN(hdrs)); - rv = nghttp2_submit_request(session_data->session, NULL, - hdrs, ARRLEN(hdrs), NULL, stream_data); - if(rv != 0) { - errx(1, "Could not submit HTTP request: %s", nghttp2_strerror(rv)); + stream_id = nghttp2_submit_request(session_data->session, NULL, + hdrs, ARRLEN(hdrs), NULL, stream_data); + if(stream_id < 0) { + errx(1, "Could not submit HTTP request: %s", nghttp2_strerror(stream_id)); } + + stream_data->stream_id = stream_id; } /* Serialize the frame and send (or buffer) the data to diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index b370df8e..62c4176b 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -2231,29 +2231,20 @@ int nghttp2_priority_spec_check_default(const nghttp2_priority_spec *pri_spec); * arbitrary pointer, which can be retrieved later by * `nghttp2_session_get_stream_user_data()`. * - * Since the library reorders the frames and tries to send the highest - * prioritized one first and the HTTP/2 specification requires the - * stream ID must be strictly increasing, the stream ID of this - * request cannot be known until it is about to sent. To know the - * stream ID of the request, the application can use - * :member:`nghttp2_session_callbacks.before_frame_send_callback`. - * This callback is called just before the frame is sent. For HEADERS - * frame, the argument frame has the stream ID assigned. Also since - * the stream is already opened, - * `nghttp2_session_get_stream_user_data()` can be used to get - * |stream_user_data| to identify which HEADERS we are processing. - * - * This function returns 0 if it succeeds, or one of the following - * negative error codes: + * This function returns assigned stream ID if it succeeds, or one of + * the following negative error codes: * * :enum:`NGHTTP2_ERR_NOMEM` * Out of memory. + * :enum:`NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE` + * No stream ID is available because maximum stream ID was + * reached. */ -int nghttp2_submit_request(nghttp2_session *session, - const nghttp2_priority_spec *pri_spec, - const nghttp2_nv *nva, size_t nvlen, - const nghttp2_data_provider *data_prd, - void *stream_user_data); +int32_t nghttp2_submit_request(nghttp2_session *session, + const nghttp2_priority_spec *pri_spec, + const nghttp2_nv *nva, size_t nvlen, + const nghttp2_data_provider *data_prd, + void *stream_user_data); /** * @function @@ -2314,8 +2305,8 @@ int nghttp2_submit_response(nghttp2_session *session, * * If the |stream_id| is -1, this frame is assumed as request (i.e., * request HEADERS frame which opens new stream). In this case, the - * actual stream ID is assigned just before the frame is sent. For - * response, specify stream ID in |stream_id|. + * assigned stream ID will be returned. Otherwise, specify stream ID + * in |stream_id|. * * The |pri_spec| is priority specification of this request. ``NULL`` * means the default priority (see @@ -2348,17 +2339,21 @@ int nghttp2_submit_response(nghttp2_session *session, * specify flags directly. For usual HTTP request, * `nghttp2_submit_request()` is useful. * - * This function returns 0 if it succeeds, or one of the following - * negative error codes: + * This function returns newly assigned stream ID if it succeeds and + * |stream_id| is -1. Otherwise, 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_STREAM_ID_NOT_AVAILABLE` + * No stream ID is available because maximum stream ID was + * reached. */ -int nghttp2_submit_headers(nghttp2_session *session, uint8_t flags, - int32_t stream_id, - const nghttp2_priority_spec *pri_spec, - const nghttp2_nv *nva, size_t nvlen, - void *stream_user_data); +int32_t nghttp2_submit_headers(nghttp2_session *session, uint8_t flags, + int32_t stream_id, + const nghttp2_priority_spec *pri_spec, + const nghttp2_nv *nva, size_t nvlen, + void *stream_user_data); /** * @function @@ -2500,31 +2495,24 @@ int nghttp2_submit_settings(nghttp2_session *session, uint8_t flags, * access it in :type:`nghttp2_before_frame_send_callback` and * :type:`nghttp2_on_frame_send_callback` of this frame. * - * Since the library reorders the frames and tries to send the highest - * prioritized one first and the HTTP/2 specification requires the - * stream ID must be strictly increasing, the promised stream ID - * cannot be known until it is about to sent. To know the promised - * stream ID, the application can use - * :member:`nghttp2_session_callbacks.before_frame_send_callback`. - * This callback is called just before the frame is sent. For - * PUSH_PROMISE frame, the argument frame has the promised stream ID - * assigned. - * * The client side is not allowed to use this function. * - * This function returns 0 if it succeeds, or one of the following - * negative error codes: + * This function returns assigned promised stream ID if it succeeds, + * or one of the following negative error codes: * * :enum:`NGHTTP2_ERR_NOMEM` * Out of memory. * :enum:`NGHTTP2_ERR_PROTO` * This function was invoked when |session| is initialized as * client. + * :enum:`NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE` + * No stream ID is available because maximum stream ID was + * reached. */ -int nghttp2_submit_push_promise(nghttp2_session *session, uint8_t flags, - int32_t stream_id, - const nghttp2_nv *nva, size_t nvlen, - void *promised_stream_user_data); +int32_t nghttp2_submit_push_promise(nghttp2_session *session, uint8_t flags, + int32_t stream_id, + const nghttp2_nv *nva, size_t nvlen, + void *promised_stream_user_data); /** * @function diff --git a/lib/nghttp2_frame.c b/lib/nghttp2_frame.c index 99ee71d5..8c85f0eb 100644 --- a/lib/nghttp2_frame.c +++ b/lib/nghttp2_frame.c @@ -66,6 +66,7 @@ static void nghttp2_frame_set_hd(nghttp2_frame_hd *hd, uint16_t length, void nghttp2_frame_headers_init(nghttp2_headers *frame, uint8_t flags, int32_t stream_id, + nghttp2_headers_category cat, const nghttp2_priority_spec *pri_spec, nghttp2_nv *nva, size_t nvlen) { @@ -73,7 +74,7 @@ void nghttp2_frame_headers_init(nghttp2_headers *frame, frame->padlen = 0; frame->nva = nva; frame->nvlen = nvlen; - frame->cat = NGHTTP2_HCAT_REQUEST; + frame->cat = cat; if(pri_spec) { frame->pri_spec = *pri_spec; diff --git a/lib/nghttp2_frame.h b/lib/nghttp2_frame.h index 2a168113..a1ee6f23 100644 --- a/lib/nghttp2_frame.h +++ b/lib/nghttp2_frame.h @@ -436,6 +436,7 @@ int nghttp2_frame_pack_blocked(nghttp2_bufs *bufs, nghttp2_blocked *frame); */ void nghttp2_frame_headers_init(nghttp2_headers *frame, uint8_t flags, int32_t stream_id, + nghttp2_headers_category cat, const nghttp2_priority_spec *pri_spec, nghttp2_nv *nva, size_t nvlen); diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index b1e401b0..a4007a8c 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -560,40 +560,13 @@ int nghttp2_session_add_frame(nghttp2_session *session, if(frame_cat == NGHTTP2_CAT_CTRL) { nghttp2_frame *frame = (nghttp2_frame*)abs_frame; nghttp2_stream *stream; - nghttp2_stream *dep_stream; stream = nghttp2_session_get_stream(session, frame->hd.stream_id); switch(frame->hd.type) { case NGHTTP2_HEADERS: - if(frame->hd.stream_id == -1) { - /* Initial HEADERS, which will open stream */ + item->weight = NGHTTP2_MAX_WEIGHT; - /* TODO If we always frame.headers.pri_spec filled in, we - don't have to check flags */ - if(frame->hd.flags & NGHTTP2_FLAG_PRIORITY) { - if(frame->headers.pri_spec.stream_id == 0) { - item->weight = frame->headers.pri_spec.weight; - } else { - dep_stream = nghttp2_session_get_stream - (session, frame->headers.pri_spec.stream_id); - - if(dep_stream) { - item->weight = nghttp2_stream_dep_distributed_effective_weight - (dep_stream, frame->headers.pri_spec.weight); - } else { - item->weight = frame->headers.pri_spec.weight; - } - } - } else { - item->weight = NGHTTP2_DEFAULT_WEIGHT; - } - - } else if(stream) { - /* Otherwise, the frame must have stream ID. We use its - effective_weight. */ - item->weight = stream->effective_weight; - } break; case NGHTTP2_PRIORITY: break; @@ -607,18 +580,13 @@ int nghttp2_session_add_frame(nghttp2_session *session, stream->state = NGHTTP2_STREAM_CLOSING; } } - break; case NGHTTP2_SETTINGS: item->weight = NGHTTP2_OB_SETTINGS_WEIGHT; break; case NGHTTP2_PUSH_PROMISE: - /* Use priority of associated stream */ - if(stream) { - item->weight = stream->effective_weight; - } - + item->weight = NGHTTP2_MAX_WEIGHT; break; case NGHTTP2_PING: /* Ping has highest priority. */ @@ -637,7 +605,7 @@ int nghttp2_session_add_frame(nghttp2_session *session, } if(frame->hd.type == NGHTTP2_HEADERS && - (frame->hd.stream_id == -1 || + (frame->headers.cat == NGHTTP2_HCAT_REQUEST || (stream && stream->state == NGHTTP2_STREAM_RESERVED))) { /* We push request HEADERS and push response HEADERS to dedicated queue because their transmission is affected by @@ -996,9 +964,6 @@ static int nghttp2_predicate_stream_for_send(nghttp2_stream *stream) * NGHTTP2_ERR_START_STREAM_NOT_ALLOWED * New stream cannot be created because GOAWAY is already sent or * received. - * NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE - * Stream ID has reached the maximum value. Therefore no stream ID - * is available. */ static int nghttp2_session_predicate_request_headers_send (nghttp2_session *session, nghttp2_headers *frame) @@ -1008,10 +973,6 @@ static int nghttp2_session_predicate_request_headers_send HEADERS. */ return NGHTTP2_ERR_START_STREAM_NOT_ALLOWED; } - /* All 32bit signed stream IDs are spent. */ - if(session->next_stream_id > INT32_MAX) { - return NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE; - } return 0; } @@ -1176,9 +1137,6 @@ static int nghttp2_session_predicate_priority_send * NGHTTP2_ERR_START_STREAM_NOT_ALLOWED * New stream cannot be created because GOAWAY is already sent or * received. - * NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE - * Stream ID has reached the maximum value. Therefore no stream ID - * is available. * NGHTTP2_ERR_PROTO * The client side attempts to send PUSH_PROMISE, or the server * sends PUSH_PROMISE for the stream not initiated by the client. @@ -1220,10 +1178,6 @@ static int nghttp2_session_predicate_push_promise_send stream ID */ return NGHTTP2_ERR_START_STREAM_NOT_ALLOWED; } - /* All 32bit signed stream IDs are spent. */ - if(session->next_stream_id > INT32_MAX) { - return NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE; - } return 0; } @@ -1598,19 +1552,16 @@ static int nghttp2_session_prep_frame(nghttp2_session *session, aux_data = (nghttp2_headers_aux_data*)item->aux_data; - if(frame->hd.stream_id == -1) { + if(frame->headers.cat == NGHTTP2_HCAT_REQUEST) { nghttp2_priority_spec pri_spec_default; nghttp2_stream *stream; /* initial HEADERS, which opens stream */ - frame->headers.cat = NGHTTP2_HCAT_REQUEST; rv = nghttp2_session_predicate_request_headers_send(session, &frame->headers); if(rv != 0) { return rv; } - frame->hd.stream_id = session->next_stream_id; - session->next_stream_id += 2; /* We first open strea with default priority. This is because priority may be adjusted in callback. */ @@ -1733,8 +1684,7 @@ static int nghttp2_session_prep_frame(nghttp2_session *session, if(rv != 0) { return rv; } - frame->push_promise.promised_stream_id = session->next_stream_id; - session->next_stream_id += 2; + framerv = nghttp2_frame_pack_push_promise(&session->aob.framebufs, &frame->push_promise, &session->hd_deflater); diff --git a/lib/nghttp2_submit.c b/lib/nghttp2_submit.c index 61c4cb44..5769edde 100644 --- a/lib/nghttp2_submit.c +++ b/lib/nghttp2_submit.c @@ -35,7 +35,7 @@ /* This function takes ownership of |nva_copy|. Regardless of the return value, the caller must not free |nva_copy| after this function returns. */ -static int nghttp2_submit_headers_shared +static int32_t nghttp2_submit_headers_shared (nghttp2_session *session, uint8_t flags, int32_t stream_id, @@ -50,6 +50,7 @@ static int nghttp2_submit_headers_shared nghttp2_frame *frame = NULL; nghttp2_data_provider *data_prd_copy = NULL; nghttp2_headers_aux_data *aux_data = NULL; + nghttp2_headers_category hcat; if(data_prd != NULL && data_prd->read_callback != NULL) { data_prd_copy = malloc(sizeof(nghttp2_data_provider)); @@ -80,8 +81,24 @@ static int nghttp2_submit_headers_shared NGHTTP2_FLAG_PRIORITY)) | NGHTTP2_FLAG_END_HEADERS; - nghttp2_frame_headers_init(&frame->headers, flags_copy, stream_id, pri_spec, - nva_copy, nvlen); + if(stream_id == -1) { + if(session->next_stream_id > INT32_MAX) { + rv = NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE; + goto fail; + } + + stream_id = session->next_stream_id; + session->next_stream_id += 2; + + hcat = NGHTTP2_HCAT_REQUEST; + } else { + /* More specific categorization will be done later. */ + hcat = NGHTTP2_HCAT_HEADERS; + } + + nghttp2_frame_headers_init(&frame->headers, flags_copy, stream_id, + hcat, pri_spec, nva_copy, nvlen); + rv = nghttp2_session_add_frame(session, NGHTTP2_CAT_CTRL, frame, aux_data); @@ -90,7 +107,13 @@ static int nghttp2_submit_headers_shared nghttp2_frame_headers_free(&frame->headers); goto fail2; } + + if(hcat == NGHTTP2_HCAT_REQUEST) { + return stream_id; + } + return 0; + fail: /* nghttp2_frame_headers_init() takes ownership of nva_copy. */ nghttp2_nv_array_del(nva_copy); @@ -110,7 +133,7 @@ static void adjust_priority_spec_weight(nghttp2_priority_spec *pri_spec) } } -static int nghttp2_submit_headers_shared_nva +static int32_t nghttp2_submit_headers_shared_nva (nghttp2_session *session, uint8_t flags, int32_t stream_id, @@ -141,11 +164,11 @@ static int nghttp2_submit_headers_shared_nva stream_user_data); } -int nghttp2_submit_headers(nghttp2_session *session, uint8_t flags, - int32_t stream_id, - const nghttp2_priority_spec *pri_spec, - const nghttp2_nv *nva, size_t nvlen, - void *stream_user_data) +int32_t nghttp2_submit_headers(nghttp2_session *session, uint8_t flags, + int32_t stream_id, + const nghttp2_priority_spec *pri_spec, + const nghttp2_nv *nva, size_t nvlen, + void *stream_user_data) { flags &= NGHTTP2_FLAG_END_STREAM; @@ -227,15 +250,16 @@ int nghttp2_submit_settings(nghttp2_session *session, uint8_t flags, return nghttp2_session_add_settings(session, NGHTTP2_FLAG_NONE, iv, niv); } -int nghttp2_submit_push_promise(nghttp2_session *session, uint8_t flags, - int32_t stream_id, - const nghttp2_nv *nva, size_t nvlen, - void *promised_stream_user_data) +int32_t nghttp2_submit_push_promise(nghttp2_session *session, uint8_t flags, + int32_t stream_id, + const nghttp2_nv *nva, size_t nvlen, + void *promised_stream_user_data) { nghttp2_frame *frame; nghttp2_nv *nva_copy; uint8_t flags_copy; nghttp2_headers_aux_data *aux_data = NULL; + int32_t promised_stream_id; int rv; if(!session->server) { @@ -261,16 +285,35 @@ int nghttp2_submit_push_promise(nghttp2_session *session, uint8_t flags, free(frame); return rv; } + flags_copy = NGHTTP2_FLAG_END_HEADERS; + + /* All 32bit signed stream IDs are spent. */ + if(session->next_stream_id > INT32_MAX) { + free(aux_data); + free(frame); + + return NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE; + } + + promised_stream_id = session->next_stream_id; + session->next_stream_id += 2; + nghttp2_frame_push_promise_init(&frame->push_promise, flags_copy, - stream_id, -1, nva_copy, nvlen); + stream_id, promised_stream_id, + nva_copy, nvlen); + rv = nghttp2_session_add_frame(session, NGHTTP2_CAT_CTRL, frame, aux_data); + if(rv != 0) { nghttp2_frame_push_promise_free(&frame->push_promise); free(aux_data); free(frame); + + return rv; } - return 0; + + return promised_stream_id; } int nghttp2_submit_window_update(nghttp2_session *session, uint8_t flags, @@ -404,11 +447,11 @@ static uint8_t set_request_flags(const nghttp2_priority_spec *pri_spec, return flags; } -int nghttp2_submit_request(nghttp2_session *session, - const nghttp2_priority_spec *pri_spec, - const nghttp2_nv *nva, size_t nvlen, - const nghttp2_data_provider *data_prd, - void *stream_user_data) +int32_t nghttp2_submit_request(nghttp2_session *session, + const nghttp2_priority_spec *pri_spec, + const nghttp2_nv *nva, size_t nvlen, + const nghttp2_data_provider *data_prd, + void *stream_user_data) { uint8_t flags; diff --git a/src/HttpServer.cc b/src/HttpServer.cc index d824b9ac..24aaf581 100644 --- a/src/HttpServer.cc +++ b/src/HttpServer.cc @@ -89,6 +89,17 @@ void print_session_id(int64_t id) } } // namespace +namespace { +void append_nv(Stream *stream, const std::vector& nva) +{ + for(auto& nv : nva) { + http2::split_add_header(stream->headers, + nv.name, nv.namelen, nv.value, nv.valuelen, + nv.flags & NGHTTP2_NV_FLAG_NO_INDEX); + } +} +} // namespace + Config::Config() : stream_read_timeout{60, 0}, stream_write_timeout{60, 0}, @@ -851,9 +862,22 @@ int Http2Handler::submit_push_promise(Stream *stream, http2::make_nv_ll(":scheme", "https"), http2::make_nv_ls(":authority", (*itr).value) }; - return nghttp2_submit_push_promise(session_, NGHTTP2_FLAG_END_HEADERS, - stream->stream_id, nva.data(), nva.size(), - nullptr); + + auto promised_stream_id = + nghttp2_submit_push_promise(session_, NGHTTP2_FLAG_END_HEADERS, + stream->stream_id, nva.data(), nva.size(), + nullptr); + + if(promised_stream_id < 0) { + return promised_stream_id; + } + + auto promised_stream = util::make_unique(this, promised_stream_id); + + append_nv(promised_stream.get(), http2::sort_nva(nva.data(), nva.size())); + add_stream(promised_stream_id, std::move(promised_stream)); + + return 0; } int Http2Handler::submit_rst_stream(Stream *stream, @@ -1121,17 +1145,6 @@ void prepare_response(Stream *stream, Http2Handler *hd, bool allow_push = true) } } // namespace -namespace { -void append_nv(Stream *stream, const std::vector& nva) -{ - for(auto& nv : nva) { - http2::split_add_header(stream->headers, - nv.name, nv.namelen, nv.value, nv.valuelen, - nv.flags & NGHTTP2_NV_FLAG_NO_INDEX); - } -} -} // namespace - namespace { const char *REQUIRED_HEADERS[] = { ":method", ":path", ":scheme", nullptr @@ -1299,25 +1312,6 @@ int hd_on_frame_recv_callback } } // namespace -namespace { -int hd_before_frame_send_callback -(nghttp2_session *session, const nghttp2_frame *frame, void *user_data) -{ - auto hd = static_cast(user_data); - - if(frame->hd.type == NGHTTP2_PUSH_PROMISE) { - auto stream_id = frame->push_promise.promised_stream_id; - auto stream = util::make_unique(hd, stream_id); - auto nva = http2::sort_nva(frame->push_promise.nva, - frame->push_promise.nvlen); - - append_nv(stream.get(), nva); - hd->add_stream(stream_id, std::move(stream)); - } - return 0; -} -} // namespace - namespace { int hd_on_frame_send_callback (nghttp2_session *session, const nghttp2_frame *frame, @@ -1432,7 +1426,6 @@ void fill_callback(nghttp2_session_callbacks& callbacks, const Config *config) memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); callbacks.on_stream_close_callback = on_stream_close_callback; callbacks.on_frame_recv_callback = hd_on_frame_recv_callback; - callbacks.before_frame_send_callback = hd_before_frame_send_callback; callbacks.on_frame_send_callback = hd_on_frame_send_callback; if(config->verbose) { callbacks.on_invalid_frame_recv_callback = diff --git a/src/h2load_http2_session.cc b/src/h2load_http2_session.cc index 12857305..57329a0e 100644 --- a/src/h2load_http2_session.cc +++ b/src/h2load_http2_session.cc @@ -43,19 +43,6 @@ Http2Session::~Http2Session() nghttp2_session_del(session_); } -namespace { -int before_frame_send_callback -(nghttp2_session *session, const nghttp2_frame *frame, void *user_data) -{ - auto client = static_cast(user_data); - if(frame->hd.type == NGHTTP2_HEADERS && - frame->headers.cat == NGHTTP2_HCAT_REQUEST) { - client->on_request(frame->hd.stream_id); - } - return 0; -} -} // namespace - namespace { int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, @@ -116,7 +103,6 @@ void Http2Session::on_connect() int rv; nghttp2_session_callbacks callbacks = {0}; - callbacks.before_frame_send_callback = before_frame_send_callback; callbacks.on_frame_recv_callback = on_frame_recv_callback; callbacks.on_data_chunk_recv_callback = on_data_chunk_recv_callback; callbacks.on_stream_close_callback = on_stream_close_callback; @@ -149,7 +135,6 @@ void Http2Session::on_connect() void Http2Session::submit_request() { - int rv; auto config = client_->worker->config; auto& nva = config->nva[client_->reqidx++]; @@ -157,9 +142,12 @@ void Http2Session::submit_request() client_->reqidx = 0; } - rv = nghttp2_submit_request(session_, nullptr, nva.data(), nva.size(), - nullptr, nullptr); - assert(rv == 0); + auto stream_id = nghttp2_submit_request(session_, nullptr, + nva.data(), nva.size(), + nullptr, nullptr); + assert(stream_id > 0); + + client_->on_request(stream_id); } ssize_t Http2Session::on_read() diff --git a/src/nghttp.cc b/src/nghttp.cc index 96ca2522..cc6acb43 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -196,7 +196,7 @@ struct Request { Request(const std::string& uri, const http_parser_url &u, const nghttp2_data_provider *data_prd, int64_t data_length, const nghttp2_priority_spec& pri_spec, - std::shared_ptr dep, int level = 0) + std::shared_ptr dep, int pri = 0, int level = 0) : uri(uri), u(u), dep(std::move(dep)), @@ -208,7 +208,7 @@ struct Request { data_prd(data_prd), stream_id(-1), level(level), - pri(0) + pri(pri) {} ~Request() @@ -402,11 +402,6 @@ int submit_request Request *req); } // namespace -namespace { -void check_stream_id(nghttp2_session *session, int32_t stream_id, - void *user_data); -} // namespace - namespace { void settings_timeout_cb(evutil_socket_t fd, short what, void *arg); } // namespace @@ -735,7 +730,8 @@ struct HttpClient { return -1; } if(stream_user_data) { - check_stream_id(session, 1, this); + stream_user_data->stream_id = 1; + on_request(stream_user_data); } } // Send connection header here @@ -869,7 +865,7 @@ struct HttpClient { int64_t data_length, const nghttp2_priority_spec& pri_spec, std::shared_ptr dep, - int level = 0) + int pri = 0, int level = 0) { http_parser_url u; memset(&u, 0, sizeof(u)); @@ -886,7 +882,7 @@ struct HttpClient { reqvec.push_back(util::make_unique(uri, u, data_prd, data_length, pri_spec, std::move(dep), - level)); + pri, level)); return true; } } @@ -926,6 +922,44 @@ struct HttpClient { return true; } + + void on_request(Request *req) + { + streams[req->stream_id] = req; + req->record_request_time(); + + if(req->pri == 0 && req->dep) { + assert(req->dep->deps.empty()); + + req->dep->deps.push_back(std::vector{req}); + + return; + } + + if(req->stream_id % 2 == 0) { + return; + } + + auto itr = std::begin(req->dep->deps); + for(; itr != std::end(req->dep->deps); ++itr) { + if((*itr)[0]->pri == req->pri) { + (*itr).push_back(req); + + break; + } + + if((*itr)[0]->pri > req->pri) { + auto v = std::vector{req}; + req->dep->deps.insert(itr, std::move(v)); + + break; + } + } + + if(itr == std::end(req->dep->deps)) { + req->dep->deps.push_back(std::vector{req}); + } + } }; } // namespace @@ -1030,13 +1064,18 @@ int submit_request nva.push_back(http2::make_nv(kv.name, kv.value, kv.no_index)); } - auto rv = nghttp2_submit_request(client->session, &req->pri_spec, - nva.data(), nva.size(), req->data_prd, req); - if(rv != 0) { + auto stream_id = nghttp2_submit_request(client->session, &req->pri_spec, + nva.data(), nva.size(), + req->data_prd, req); + if(stream_id < 0) { std::cerr << "nghttp2_submit_request() returned error: " - << nghttp2_strerror(rv) << std::endl; + << nghttp2_strerror(stream_id) << std::endl; return -1; } + + req->stream_id = stream_id; + client->on_request(req); + return 0; } } // namespace @@ -1052,6 +1091,8 @@ void update_html_parser(HttpClient *client, Request *req, for(auto& p : req->html_parser->get_links()) { auto uri = strip_fragment(p.first.c_str()); + auto pri = p.second; + http_parser_url u; memset(&u, 0, sizeof(u)); if(http_parser_parse_url(uri.c_str(), uri.size(), 0, &u) == 0 && @@ -1059,18 +1100,10 @@ void update_html_parser(HttpClient *client, Request *req, util::fieldeq(uri.c_str(), u, req->uri.c_str(), req->u, UF_HOST) && util::porteq(uri.c_str(), u, req->uri.c_str(), req->u)) { // No POST data for assets - - nghttp2_priority_spec pri_spec; - - // We adjust the priority using separate PRIORITY frame after - // stream ID becomes known. - nghttp2_priority_spec_default_init(&pri_spec); + auto pri_spec = req->resolve_dep(pri); if ( client->add_request(uri, nullptr, 0, pri_spec, req->dep, - req->level+1) ) { - auto& req = client->reqvec.back(); - - req->pri = p.second; + pri, req->level+1) ) { submit_request(client, config.headers, client->reqvec.back().get()); @@ -1181,52 +1214,6 @@ int on_data_chunk_recv_callback } } // namespace -namespace { -void check_stream_id(nghttp2_session *session, int32_t stream_id, - void *user_data) -{ - auto client = get_session(user_data); - auto req = (Request*)nghttp2_session_get_stream_user_data(session, - stream_id); - assert(req); - req->stream_id = stream_id; - client->streams[stream_id] = req; - req->record_request_time(); - - if(req->pri == 0 && req->dep) { - assert(req->dep->deps.empty()); - - req->dep->deps.push_back(std::vector{req}); - - return; - } - - if(stream_id % 2 == 0) { - return; - } - - auto itr = std::begin(req->dep->deps); - for(; itr != std::end(req->dep->deps); ++itr) { - if((*itr)[0]->pri == req->pri) { - (*itr).push_back(req); - - break; - } - - if((*itr)[0]->pri > req->pri) { - auto v = std::vector{req}; - req->dep->deps.insert(itr, std::move(v)); - - break; - } - } - - if(itr == std::end(req->dep->deps)) { - req->dep->deps.push_back(std::vector{req}); - } -} -} // namespace - namespace { void settings_timeout_cb(evutil_socket_t fd, short what, void *arg) { @@ -1240,35 +1227,6 @@ void settings_timeout_cb(evutil_socket_t fd, short what, void *arg) } } // namespace -namespace { -int adjust_priority_callback -(nghttp2_session *session, const nghttp2_frame *frame, - nghttp2_priority_spec *pri_spec, void *user_data) -{ - auto req = (Request*)nghttp2_session_get_stream_user_data - (session, frame->hd.stream_id); - auto pri_spec_adjusted = req->resolve_dep(req->pri); - - if(!nghttp2_priority_spec_check_default(&pri_spec_adjusted)) { - *pri_spec = pri_spec_adjusted; - } - - return 0; -} -} // namespace - -namespace { -int before_frame_send_callback -(nghttp2_session *session, const nghttp2_frame *frame, void *user_data) -{ - if(frame->hd.type == NGHTTP2_HEADERS && - frame->headers.cat == NGHTTP2_HCAT_REQUEST) { - check_stream_id(session, frame->hd.stream_id, user_data); - } - return 0; -} -} // namespace - namespace { ssize_t select_padding_callback (nghttp2_session *session, const nghttp2_frame *frame, size_t max_payload, @@ -1323,9 +1281,13 @@ int on_begin_headers_callback(nghttp2_session *session, auto req = util::make_unique("", u, nullptr, 0, pri_spec, nullptr); + req->stream_id = stream_id; + nghttp2_session_set_stream_user_data(session, stream_id, req.get()); + + client->on_request(req.get()); client->reqvec.push_back(std::move(req)); - check_stream_id(session, stream_id, user_data); + break; } } @@ -1868,7 +1830,6 @@ int run(char **uris, int n) memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); callbacks.on_stream_close_callback = on_stream_close_callback; callbacks.on_frame_recv_callback = on_frame_recv_callback2; - callbacks.before_frame_send_callback = before_frame_send_callback; if(config.verbose) { callbacks.on_frame_send_callback = verbose_on_frame_send_callback; callbacks.on_invalid_frame_recv_callback = @@ -1882,7 +1843,6 @@ int run(char **uris, int n) if(config.padding) { callbacks.select_padding_callback = select_padding_callback; } - callbacks.adjust_priority_callback = adjust_priority_callback; std::string prev_scheme; std::string prev_host; diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index 599049bc..0f52a7f0 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -588,16 +588,18 @@ int Http2Session::submit_request(Http2DownstreamConnection *dconn, assert(state_ == CONNECTED); auto sd = util::make_unique(); // TODO Specify nullptr to pri_spec for now - int rv = nghttp2_submit_request(session_, nullptr, nva, nvlen, - data_prd, sd.get()); - if(rv == 0) { - dconn->attach_stream_data(sd.get()); - streams_.insert(sd.release()); - } else { + auto stream_id = nghttp2_submit_request(session_, nullptr, nva, nvlen, + data_prd, sd.get()); + if(stream_id < 0) { SSLOG(FATAL, this) << "nghttp2_submit_request() failed: " - << nghttp2_strerror(rv); + << nghttp2_strerror(stream_id); return -1; } + + dconn->attach_stream_data(sd.get()); + dconn->get_downstream()->set_downstream_stream_id(stream_id); + streams_.insert(sd.release()); + return 0; } @@ -1082,31 +1084,6 @@ int on_data_chunk_recv_callback(nghttp2_session *session, } } // namespace -namespace { -int before_frame_send_callback(nghttp2_session *session, - const nghttp2_frame *frame, - void *user_data) -{ - auto http2session = static_cast(user_data); - if(frame->hd.type == NGHTTP2_HEADERS && - frame->headers.cat == NGHTTP2_HCAT_REQUEST) { - auto sd = static_cast - (nghttp2_session_get_stream_user_data(session, frame->hd.stream_id)); - if(!sd || !sd->dconn) { - http2session->submit_rst_stream(frame->hd.stream_id, NGHTTP2_CANCEL); - return 0; - } - auto downstream = sd->dconn->get_downstream(); - if(downstream) { - downstream->set_downstream_stream_id(frame->hd.stream_id); - } else { - http2session->submit_rst_stream(frame->hd.stream_id, NGHTTP2_CANCEL); - } - } - return 0; -} -} // namespace - namespace { int on_frame_send_callback(nghttp2_session* session, const nghttp2_frame *frame, void *user_data) @@ -1205,7 +1182,6 @@ int Http2Session::on_connect() callbacks.on_stream_close_callback = on_stream_close_callback; callbacks.on_frame_recv_callback = on_frame_recv_callback; callbacks.on_data_chunk_recv_callback = on_data_chunk_recv_callback; - callbacks.before_frame_send_callback = before_frame_send_callback; callbacks.on_frame_send_callback = on_frame_send_callback; callbacks.on_frame_not_send_callback = on_frame_not_send_callback; callbacks.on_unknown_frame_recv_callback = on_unknown_frame_recv_callback; diff --git a/tests/nghttp2_frame_test.c b/tests/nghttp2_frame_test.c index 7f6d2141..4f461f0c 100644 --- a/tests/nghttp2_frame_test.c +++ b/tests/nghttp2_frame_test.c @@ -97,7 +97,8 @@ void test_nghttp2_frame_pack_headers() nghttp2_frame_headers_init(&frame, NGHTTP2_FLAG_END_STREAM | NGHTTP2_FLAG_END_HEADERS, - 1000000007, &pri_spec, nva, nvlen); + 1000000007, NGHTTP2_HCAT_REQUEST, + &pri_spec, nva, nvlen); rv = nghttp2_frame_pack_headers(&bufs, &frame, &deflater); nghttp2_bufs_rewind(&bufs); @@ -194,7 +195,8 @@ void test_nghttp2_frame_pack_headers_frame_too_large(void) nghttp2_hd_deflate_init(&deflater); nghttp2_frame_headers_init(&frame, NGHTTP2_FLAG_END_STREAM|NGHTTP2_FLAG_END_HEADERS, - 1000000007, NULL, nva, nvlen); + 1000000007, NGHTTP2_HCAT_REQUEST, + NULL, nva, nvlen); rv = nghttp2_frame_pack_headers(&bufs, &frame, &deflater); CU_ASSERT(NGHTTP2_ERR_HEADER_COMP == rv); diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index d3fa96e4..7e5fb472 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -411,7 +411,7 @@ void test_nghttp2_session_recv(void) nvlen = nghttp2_nv_array_copy(&nva, nv, ARRLEN(nv)); nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, - 1, NULL, nva, nvlen); + 1, NGHTTP2_HCAT_HEADERS, NULL, nva, nvlen); rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater); CU_ASSERT(0 == rv); @@ -437,7 +437,7 @@ void test_nghttp2_session_recv(void) /* Received HEADERS without header block, which is valid */ nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, - 5, NULL, NULL, 0); + 5, NGHTTP2_HCAT_HEADERS, NULL, NULL, 0); rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater); CU_ASSERT(0 == rv); @@ -510,7 +510,7 @@ void test_nghttp2_session_recv_invalid_stream_id(void) nghttp2_hd_deflate_init(&deflater); nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, 2, - NULL, NULL, 0); + NGHTTP2_HCAT_HEADERS, NULL, NULL, 0); rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater); CU_ASSERT(0 == rv); @@ -556,7 +556,7 @@ void test_nghttp2_session_recv_invalid_frame(void) nghttp2_hd_deflate_init(&deflater); nvlen = nghttp2_nv_array_copy(&nva, nv, ARRLEN(nv)); nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, 1, - NULL, nva, nvlen); + NGHTTP2_HCAT_HEADERS, NULL, nva, nvlen); rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater); CU_ASSERT(0 == rv); @@ -809,7 +809,7 @@ void test_nghttp2_session_recv_continuation(void) /* Make 1 HEADERS and insert CONTINUATION header */ nvlen = nghttp2_nv_array_copy(&nva, nv1, ARRLEN(nv1)); nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_NONE, 1, - NULL, nva, nvlen); + NGHTTP2_HCAT_HEADERS, NULL, nva, nvlen); rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater); CU_ASSERT(0 == rv); @@ -871,7 +871,7 @@ void test_nghttp2_session_recv_continuation(void) /* HEADERS without END_HEADERS flag */ nvlen = nghttp2_nv_array_copy(&nva, nv1, ARRLEN(nv1)); nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_NONE, 1, - NULL, nva, nvlen); + NGHTTP2_HCAT_HEADERS, NULL, nva, nvlen); nghttp2_bufs_reset(&bufs); rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater); @@ -952,7 +952,7 @@ void test_nghttp2_session_recv_headers_with_priority(void) nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_PRIORITY, - 3, &pri_spec, nva, nvlen); + 3, NGHTTP2_HCAT_HEADERS, &pri_spec, nva, nvlen); rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater); @@ -986,7 +986,7 @@ void test_nghttp2_session_recv_headers_with_priority(void) nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_PRIORITY, - 5, &pri_spec, nva, nvlen); + 5, NGHTTP2_HCAT_HEADERS, &pri_spec, nva, nvlen); rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater); @@ -1032,7 +1032,7 @@ void test_nghttp2_session_recv_headers_with_priority(void) nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_PRIORITY, - 1, &pri_spec, nva, nvlen); + 1, NGHTTP2_HCAT_HEADERS, &pri_spec, nva, nvlen); rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater); @@ -1095,7 +1095,7 @@ void test_nghttp2_session_recv_premature_headers(void) nvlen = nghttp2_nv_array_copy(&nva, nv1, ARRLEN(nv1)); nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, 1, - NULL, nva, nvlen); + NGHTTP2_HCAT_HEADERS, NULL, nva, nvlen); rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater); CU_ASSERT(0 == rv); @@ -1259,7 +1259,7 @@ void test_nghttp2_session_continue(void) /* Make 2 HEADERS frames */ nvlen = nghttp2_nv_array_copy(&nva, nv1, ARRLEN(nv1)); nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, 1, - NULL, nva, nvlen); + NGHTTP2_HCAT_HEADERS, NULL, nva, nvlen); rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater); CU_ASSERT(0 == rv); @@ -1275,7 +1275,7 @@ void test_nghttp2_session_continue(void) nvlen = nghttp2_nv_array_copy(&nva, nv2, ARRLEN(nv2)); nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, 3, - NULL, nva, nvlen); + NGHTTP2_HCAT_HEADERS, NULL, nva, nvlen); nghttp2_bufs_reset(&bufs); rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater); @@ -1439,7 +1439,10 @@ void test_nghttp2_session_add_frame(void) nghttp2_frame_headers_init(&frame->headers, NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_PRIORITY, - -1, NULL, nva, nvlen); + session->next_stream_id, + NGHTTP2_HCAT_REQUEST, NULL, nva, nvlen); + + session->next_stream_id += 2; CU_ASSERT(0 == nghttp2_session_add_frame(session, NGHTTP2_CAT_CTRL, frame, aux_data)); @@ -1477,7 +1480,8 @@ void test_nghttp2_session_on_request_headers_received(void) nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_PRIORITY, - stream_id, &pri_spec, NULL, 0); + stream_id, NGHTTP2_HCAT_REQUEST, &pri_spec, + NULL, 0); user_data.begin_headers_cb_called = 0; user_data.invalid_frame_recv_cb_called = 0; @@ -1495,7 +1499,7 @@ void test_nghttp2_session_on_request_headers_received(void) nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_PRIORITY, - 3, NULL, NULL, 0); + 3, NGHTTP2_HCAT_HEADERS, NULL, NULL, 0); user_data.invalid_frame_recv_cb_called = 0; CU_ASSERT(NGHTTP2_ERR_IGN_HEADER_BLOCK == nghttp2_session_on_request_headers_received(session, &frame)); @@ -1511,7 +1515,7 @@ void test_nghttp2_session_on_request_headers_received(void) nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_PRIORITY, - 3, NULL, NULL, 0); + 3, NGHTTP2_HCAT_HEADERS, NULL, NULL, 0); user_data.invalid_frame_recv_cb_called = 0; CU_ASSERT(NGHTTP2_ERR_IGN_HEADER_BLOCK == nghttp2_session_on_request_headers_received(session, &frame)); @@ -1528,7 +1532,7 @@ void test_nghttp2_session_on_request_headers_received(void) nvlen = nghttp2_nv_array_copy(&nva, malformed_nva, ARRLEN(malformed_nva)); nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_PRIORITY, - 1, NULL, nva, nvlen); + 1, NGHTTP2_HCAT_HEADERS, NULL, nva, nvlen); user_data.begin_headers_cb_called = 0; user_data.invalid_frame_recv_cb_called = 0; CU_ASSERT(0 == nghttp2_session_on_request_headers_received(session, &frame)); @@ -1557,7 +1561,7 @@ void test_nghttp2_session_on_response_headers_received(void) &pri_spec_default, NGHTTP2_STREAM_OPENING, NULL); nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, 1, - NULL, NULL, 0); + NGHTTP2_HCAT_HEADERS, NULL, NULL, 0); user_data.begin_headers_cb_called = 0; user_data.invalid_frame_recv_cb_called = 0; @@ -1589,7 +1593,7 @@ void test_nghttp2_session_on_headers_received(void) NGHTTP2_STREAM_OPENED, NULL); nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_WR); nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, 1, - NULL, NULL, 0); + NGHTTP2_HCAT_HEADERS, NULL, NULL, 0); user_data.begin_headers_cb_called = 0; user_data.invalid_frame_recv_cb_called = 0; @@ -1661,7 +1665,7 @@ void test_nghttp2_session_on_push_response_headers_received(void) &pri_spec_default, NGHTTP2_STREAM_RESERVED, NULL); nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, 2, - NULL, NULL, 0); + NGHTTP2_HCAT_HEADERS, NULL, NULL, 0); /* nghttp2_session_on_push_response_headers_received assumes stream's state is NGHTTP2_STREAM_RESERVED and session->server is 0. */ @@ -2324,8 +2328,12 @@ void test_nghttp2_session_send_headers_start_stream(void) callbacks.send_callback = null_send_callback; nghttp2_session_client_new(&session, &callbacks, NULL); - nghttp2_frame_headers_init(&frame->headers, NGHTTP2_FLAG_END_HEADERS, -1, - NULL, NULL, 0); + + nghttp2_frame_headers_init(&frame->headers, NGHTTP2_FLAG_END_HEADERS, + session->next_stream_id, + NGHTTP2_HCAT_REQUEST, NULL, NULL, 0); + session->next_stream_id += 2; + nghttp2_session_add_frame(session, NGHTTP2_CAT_CTRL, frame, aux_data); CU_ASSERT(0 == nghttp2_session_send(session)); stream = nghttp2_session_get_stream(session, 1); @@ -2349,7 +2357,7 @@ void test_nghttp2_session_send_headers_reply(void) &pri_spec_default, NGHTTP2_STREAM_OPENING, NULL); nghttp2_frame_headers_init(&frame->headers, NGHTTP2_FLAG_END_HEADERS, 2, - NULL, NULL, 0); + NGHTTP2_HCAT_HEADERS, NULL, NULL, 0); nghttp2_session_add_frame(session, NGHTTP2_CAT_CTRL, frame, NULL); CU_ASSERT(0 == nghttp2_session_send(session)); stream = nghttp2_session_get_stream(session, 2); @@ -2386,8 +2394,11 @@ void test_nghttp2_session_send_headers_header_comp_error(void) nghttp2_session_client_new(&session, &callbacks, NULL); nvlen = nghttp2_nv_array_copy(&nva, nv, nnv); - nghttp2_frame_headers_init(&frame->headers, NGHTTP2_FLAG_END_HEADERS, -1, - NULL, nva, nvlen); + nghttp2_frame_headers_init(&frame->headers, NGHTTP2_FLAG_END_HEADERS, + session->next_stream_id, + NGHTTP2_HCAT_REQUEST, NULL, nva, nvlen); + + session->next_stream_id += 2; nghttp2_session_add_frame(session, NGHTTP2_CAT_CTRL, frame, NULL); CU_ASSERT(0 == nghttp2_session_send(session)); @@ -2416,7 +2427,7 @@ void test_nghttp2_session_send_headers_push_reply(void) &pri_spec_default, NGHTTP2_STREAM_RESERVED, NULL); nghttp2_frame_headers_init(&frame->headers, NGHTTP2_FLAG_END_HEADERS, 2, - NULL, NULL, 0); + NGHTTP2_HCAT_HEADERS, NULL, NULL, 0); nghttp2_session_add_frame(session, NGHTTP2_CAT_CTRL, frame, NULL); CU_ASSERT(0 == session->num_outgoing_streams); CU_ASSERT(0 == nghttp2_session_send(session)); @@ -2468,8 +2479,12 @@ void test_nghttp2_session_send_push_promise(void) &pri_spec_default, NGHTTP2_STREAM_OPENING, NULL); nghttp2_frame_push_promise_init(&frame->push_promise, - NGHTTP2_FLAG_END_HEADERS, 1, -1, + NGHTTP2_FLAG_END_HEADERS, 1, + session->next_stream_id, NULL, 0); + + session->next_stream_id += 2; + nghttp2_session_add_frame(session, NGHTTP2_CAT_CTRL, frame, NULL); CU_ASSERT(0 == nghttp2_session_send(session)); @@ -2747,7 +2762,7 @@ void test_nghttp2_submit_request_with_data(void) data_prd.read_callback = fixed_length_data_source_read_callback; ud.data_source_length = 64*1024 - 1; CU_ASSERT(0 == nghttp2_session_client_new(&session, &callbacks, &ud)); - CU_ASSERT(0 == nghttp2_submit_request(session, NULL, + CU_ASSERT(1 == nghttp2_submit_request(session, NULL, nva, ARRLEN(nva), &data_prd, NULL)); item = nghttp2_session_get_next_ob_item(session); CU_ASSERT(nvnameeq(":version", &OB_CTRL(item)->headers.nva[0])); @@ -2783,7 +2798,7 @@ void test_nghttp2_submit_request_without_data(void) CU_ASSERT(0 == nghttp2_session_client_new(&session, &callbacks, &ud)); nghttp2_hd_inflate_init(&inflater); - CU_ASSERT(0 == nghttp2_submit_request(session, NULL, + CU_ASSERT(1 == nghttp2_submit_request(session, NULL, nva, ARRLEN(nva), &data_prd, NULL)); item = nghttp2_session_get_next_ob_item(session); CU_ASSERT(nvnameeq(":version", &OB_CTRL(item)->headers.nva[0])); @@ -2895,7 +2910,7 @@ void test_nghttp2_submit_headers_start_stream(void) memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); CU_ASSERT(0 == nghttp2_session_client_new(&session, &callbacks, NULL)); - CU_ASSERT(0 == nghttp2_submit_headers(session, + CU_ASSERT(1 == nghttp2_submit_headers(session, NGHTTP2_FLAG_END_STREAM, -1, NULL, nv, ARRLEN(nv), NULL)); @@ -3110,7 +3125,7 @@ void test_nghttp2_submit_headers_continuation(void) callbacks.on_frame_send_callback = on_frame_send_callback; CU_ASSERT(0 == nghttp2_session_client_new(&session, &callbacks, &ud)); - CU_ASSERT(0 == nghttp2_submit_headers(session, + CU_ASSERT(1 == nghttp2_submit_headers(session, NGHTTP2_FLAG_END_STREAM, -1, NULL, nv, ARRLEN(nv), NULL)); @@ -3363,7 +3378,7 @@ void test_nghttp2_submit_push_promise(void) nghttp2_session_open_stream(session, 1, NGHTTP2_STREAM_FLAG_NONE, &pri_spec_default, NGHTTP2_STREAM_OPENING, NULL); - CU_ASSERT(0 == nghttp2_submit_push_promise(session, NGHTTP2_FLAG_NONE, 1, + CU_ASSERT(2 == nghttp2_submit_push_promise(session, NGHTTP2_FLAG_NONE, 1, nv, ARRLEN(nv), &ud)); ud.frame_send_cb_called = 0; @@ -3606,7 +3621,7 @@ void test_nghttp2_submit_invalid_nv(void) CU_ASSERT(0 == nghttp2_session_server_new(&session, &callbacks, NULL)); /* nghttp2_submit_request */ - CU_ASSERT(0 == + CU_ASSERT(0 < nghttp2_submit_request(session, NULL, empty_name_nv, ARRLEN(empty_name_nv), NULL, NULL)); @@ -3618,14 +3633,14 @@ void test_nghttp2_submit_invalid_nv(void) NULL)); /* nghttp2_submit_headers */ - CU_ASSERT(0 == + CU_ASSERT(0 < nghttp2_submit_headers(session, NGHTTP2_FLAG_NONE, -1, NULL, empty_name_nv, ARRLEN(empty_name_nv), NULL)); /* nghttp2_submit_push_promise */ - CU_ASSERT(0 == + CU_ASSERT(0 < nghttp2_submit_push_promise(session, NGHTTP2_FLAG_NONE, 2, empty_name_nv, ARRLEN(empty_name_nv), NULL)); @@ -3862,7 +3877,7 @@ void test_nghttp2_session_max_concurrent_streams(void) /* Check un-ACKed SETTINGS_MAX_CONCURRENT_STREAMS */ nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, 3, - NULL, NULL, 0); + NGHTTP2_HCAT_HEADERS, NULL, NULL, 0); session->pending_local_max_concurrent_stream = 1; CU_ASSERT(NGHTTP2_ERR_IGN_HEADER_BLOCK == @@ -4412,19 +4427,19 @@ void test_nghttp2_session_on_ctrl_not_send(void) CU_ASSERT(nghttp2_session_client_new(&session, &callbacks, &user_data) == 0); /* Maximum Stream ID is reached */ session->next_stream_id = (1u << 31)+1; - CU_ASSERT(0 == nghttp2_submit_headers(session, NGHTTP2_FLAG_END_STREAM, -1, - NULL, NULL, 0, NULL)); - CU_ASSERT(0 == nghttp2_session_send(session)); - CU_ASSERT(1 == user_data.frame_not_send_cb_called); - CU_ASSERT(NGHTTP2_HEADERS == user_data.not_sent_frame_type); - CU_ASSERT(NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE == user_data.not_sent_error); + CU_ASSERT(NGHTTP2_ERR_STREAM_ID_NOT_AVAILABLE == + nghttp2_submit_headers(session, NGHTTP2_FLAG_END_STREAM, -1, + NULL, NULL, 0, NULL)); user_data.frame_not_send_cb_called = 0; /* Send GOAWAY */ CU_ASSERT(0 == nghttp2_submit_goaway(session, NGHTTP2_FLAG_NONE, NGHTTP2_NO_ERROR, NULL, 0)); - CU_ASSERT(0 == nghttp2_submit_headers(session, NGHTTP2_FLAG_END_STREAM, -1, - NULL, NULL, 0, NULL)); + + session->next_stream_id = 9; + + CU_ASSERT(0 < nghttp2_submit_headers(session, NGHTTP2_FLAG_END_STREAM, -1, + NULL, NULL, 0, NULL)); CU_ASSERT(0 == nghttp2_session_send(session)); CU_ASSERT(1 == user_data.frame_not_send_cb_called); CU_ASSERT(NGHTTP2_HEADERS == user_data.not_sent_frame_type); @@ -4740,7 +4755,7 @@ void test_nghttp2_session_pack_headers_with_padding(void) ud.padding_boundary = 16385; - CU_ASSERT(0 == + CU_ASSERT(1 == nghttp2_submit_request(session, NULL, nva, ARRLEN(nva), NULL, NULL)); CU_ASSERT(0 == nghttp2_session_send(session)); @@ -4753,7 +4768,7 @@ void test_nghttp2_session_pack_headers_with_padding(void) CU_ASSERT(NULL == nghttp2_session_get_next_ob_item(sv_session)); /* Check PUSH_PROMISE */ - CU_ASSERT(0 == + CU_ASSERT(2 == nghttp2_submit_push_promise(sv_session, NGHTTP2_FLAG_NONE, 1, nva, ARRLEN(nva), NULL)); acc.length = 0; @@ -4802,7 +4817,7 @@ void test_nghttp2_session_pack_headers_with_padding2(void) ud.padding_boundary = 16385; - CU_ASSERT(0 == + CU_ASSERT(1 == nghttp2_submit_request(session, NULL, nva, ARRLEN(nva), NULL, NULL)); CU_ASSERT(0 == nghttp2_session_send(session)); @@ -4852,7 +4867,7 @@ void test_nghttp2_session_pack_headers_with_padding3(void) ud.padding_boundary = 16385; - CU_ASSERT(0 == + CU_ASSERT(1 == nghttp2_submit_request(session, NULL, nva, ARRLEN(nva), NULL, NULL)); CU_ASSERT(0 == nghttp2_session_send(session)); @@ -4891,7 +4906,7 @@ void test_nghttp2_session_pack_headers_with_padding4(void) ud.padding_boundary = 16385; - CU_ASSERT(0 == + CU_ASSERT(1 == nghttp2_submit_request(session, NULL, &nv, 1, NULL, NULL)); CU_ASSERT(0 == nghttp2_session_send(session));