From c3459c9e865f4fc7be4ecf035216e6fa1b6e2dc9 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 16 Nov 2015 21:44:36 +0900 Subject: [PATCH 01/62] nghttpx: Clarify return value of nghttp2_submit_push_promise is stream ID --- src/shrpx_http2_upstream.cc | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 42a347fb..c7e93ed9 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -1798,7 +1798,6 @@ int Http2Upstream::submit_push_promise(const std::string &scheme, const std::string &authority, const std::string &path, Downstream *downstream) { - int rv; std::vector nva; // 4 for :method, :scheme, :path and :authority nva.reserve(4 + downstream->get_request_headers().size()); @@ -1827,16 +1826,16 @@ int Http2Upstream::submit_push_promise(const std::string &scheme, } } - rv = nghttp2_submit_push_promise(session_, NGHTTP2_FLAG_NONE, - downstream->get_stream_id(), nva.data(), - nva.size(), nullptr); + auto promised_stream_id = nghttp2_submit_push_promise( + session_, NGHTTP2_FLAG_NONE, downstream->get_stream_id(), nva.data(), + nva.size(), nullptr); - if (rv < 0) { + if (promised_stream_id < 0) { if (LOG_ENABLED(INFO)) { ULOG(INFO, this) << "nghttp2_submit_push_promise() failed: " - << nghttp2_strerror(rv); + << nghttp2_strerror(promised_stream_id); } - if (nghttp2_is_fatal(rv)) { + if (nghttp2_is_fatal(promised_stream_id)) { return -1; } return 0; @@ -1847,8 +1846,8 @@ int Http2Upstream::submit_push_promise(const std::string &scheme, for (auto &nv : nva) { ss << TTY_HTTP_HD << nv.name << TTY_RST << ": " << nv.value << "\n"; } - ULOG(INFO, this) << "HTTP push request headers. promised_stream_id=" << rv - << "\n" << ss.str(); + ULOG(INFO, this) << "HTTP push request headers. promised_stream_id=" + << promised_stream_id << "\n" << ss.str(); } return 0; From 7463493259a23f95866ff15a3c69187078442fe3 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 16 Nov 2015 22:20:27 +0900 Subject: [PATCH 02/62] Use error code CANCEL to reset pushed reserved stream from remote --- lib/nghttp2_session.c | 5 ++--- tests/nghttp2_session_test.c | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index b84b6d1a..3b180865 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -4196,9 +4196,8 @@ int nghttp2_session_on_push_promise_received(nghttp2_session *session, session, frame, NGHTTP2_ERR_PROTO, "PUSH_PROMISE: stream in idle"); } } - rv = nghttp2_session_add_rst_stream(session, - frame->push_promise.promised_stream_id, - NGHTTP2_REFUSED_STREAM); + rv = nghttp2_session_add_rst_stream( + session, frame->push_promise.promised_stream_id, NGHTTP2_CANCEL); if (rv != 0) { return rv; } diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index e990e611..21ee5798 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -2632,7 +2632,7 @@ void test_nghttp2_session_on_push_promise_received(void) { item = nghttp2_session_get_next_ob_item(session); CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type); CU_ASSERT(6 == item->frame.hd.stream_id); - CU_ASSERT(NGHTTP2_REFUSED_STREAM == item->frame.rst_stream.error_code); + CU_ASSERT(NGHTTP2_CANCEL == item->frame.rst_stream.error_code); CU_ASSERT(0 == nghttp2_session_send(session)); /* Attempt to PUSH_PROMISE against non-existent stream */ @@ -2806,7 +2806,7 @@ void test_nghttp2_session_on_push_promise_received(void) { item = nghttp2_session_get_next_ob_item(session); CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type); - CU_ASSERT(NGHTTP2_REFUSED_STREAM == item->frame.rst_stream.error_code); + CU_ASSERT(NGHTTP2_CANCEL == item->frame.rst_stream.error_code); nghttp2_frame_push_promise_free(&frame.push_promise, mem); nghttp2_session_del(session); From b918f9650a7382132563a30b7f614963c3f1caf0 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 16 Nov 2015 22:47:12 +0900 Subject: [PATCH 03/62] Don't send push response if GOAWAY has been received --- lib/nghttp2_session.c | 47 +++++++++++++++++++++--------------- tests/main.c | 2 ++ tests/nghttp2_session_test.c | 27 +++++++++++++++++++++ tests/nghttp2_session_test.h | 1 + 4 files changed, 57 insertions(+), 20 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 3b180865..366a8007 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -1415,6 +1415,9 @@ static int session_predicate_response_headers_send(nghttp2_session *session, * RST_STREAM was queued for this stream. * NGHTTP2_ERR_SESSION_CLOSING * This session is closing. + * NGHTTP2_ERR_START_STREAM_NOT_ALLOWED + * New stream cannot be created because GOAWAY is already sent or + * received. */ static int session_predicate_push_response_headers_send(nghttp2_session *session, @@ -1432,6 +1435,9 @@ session_predicate_push_response_headers_send(nghttp2_session *session, if (stream->state == NGHTTP2_STREAM_CLOSING) { return NGHTTP2_ERR_STREAM_CLOSING; } + if (session->goaway_flags & NGHTTP2_GOAWAY_RECV) { + return NGHTTP2_ERR_START_STREAM_NOT_ALLOWED; + } return 0; } @@ -1795,40 +1801,41 @@ static int session_prep_frame(nghttp2_session *session, stream = nghttp2_session_get_stream(session, frame->hd.stream_id); - if (session_predicate_push_response_headers_send(session, stream) == - 0) { - frame->headers.cat = NGHTTP2_HCAT_PUSH_RESPONSE; + if (stream && stream->state == NGHTTP2_STREAM_RESERVED) { + rv = session_predicate_push_response_headers_send(session, stream); + if (rv == 0) { + frame->headers.cat = NGHTTP2_HCAT_PUSH_RESPONSE; - if (aux_data->stream_user_data) { - stream->stream_user_data = aux_data->stream_user_data; + if (aux_data->stream_user_data) { + stream->stream_user_data = aux_data->stream_user_data; + } } } else if (session_predicate_response_headers_send(session, stream) == 0) { frame->headers.cat = NGHTTP2_HCAT_RESPONSE; + rv = 0; } else { frame->headers.cat = NGHTTP2_HCAT_HEADERS; rv = session_predicate_headers_send(session, stream); + } - if (rv != 0) { - // If stream was alreay closed, - // nghttp2_session_get_stream() returns NULL, but item is - // still attached to the stream. Search stream including - // closed again. - stream = - nghttp2_session_get_stream_raw(session, frame->hd.stream_id); - if (stream && stream->item == item) { - int rv2; + if (rv != 0) { + // If stream was alreay closed, nghttp2_session_get_stream() + // returns NULL, but item is still attached to the stream. + // Search stream including closed again. + stream = nghttp2_session_get_stream_raw(session, frame->hd.stream_id); + if (stream && stream->item == item) { + int rv2; - rv2 = nghttp2_stream_detach_item(stream); + rv2 = nghttp2_stream_detach_item(stream); - if (nghttp2_is_fatal(rv2)) { - return rv2; - } + if (nghttp2_is_fatal(rv2)) { + return rv2; } - - return rv; } + + return rv; } } diff --git a/tests/main.c b/tests/main.c index cc3896c0..14b43dbf 100644 --- a/tests/main.c +++ b/tests/main.c @@ -159,6 +159,8 @@ int main(int argc _U_, char *argv[] _U_) { test_nghttp2_submit_response_with_data) || !CU_add_test(pSuite, "submit_response_without_data", test_nghttp2_submit_response_without_data) || + !CU_add_test(pSuite, "Submit_response_push_response", + test_nghttp2_submit_response_push_response) || !CU_add_test(pSuite, "submit_trailer", test_nghttp2_submit_trailer) || !CU_add_test(pSuite, "submit_headers_start_stream", test_nghttp2_submit_headers_start_stream) || diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 21ee5798..149dd81b 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -3879,6 +3879,33 @@ void test_nghttp2_submit_response_without_data(void) { nghttp2_session_del(session); } +void test_nghttp2_submit_response_push_response(void) { + nghttp2_session *session; + nghttp2_session_callbacks callbacks; + my_user_data ud; + + memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); + callbacks.send_callback = null_send_callback; + callbacks.on_frame_not_send_callback = on_frame_not_send_callback; + + nghttp2_session_server_new(&session, &callbacks, &ud); + + nghttp2_session_open_stream(session, 2, NGHTTP2_FLAG_NONE, &pri_spec_default, + NGHTTP2_STREAM_RESERVED, NULL); + + session->goaway_flags |= NGHTTP2_GOAWAY_RECV; + + CU_ASSERT(0 == + nghttp2_submit_response(session, 2, resnv, ARRLEN(resnv), NULL)); + + ud.frame_not_send_cb_called = 0; + + CU_ASSERT(0 == nghttp2_session_send(session)); + CU_ASSERT(1 == ud.frame_not_send_cb_called); + + nghttp2_session_del(session); +} + void test_nghttp2_submit_trailer(void) { nghttp2_session *session; nghttp2_session_callbacks callbacks; diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h index 80aa8edd..ce3954b3 100644 --- a/tests/nghttp2_session_test.h +++ b/tests/nghttp2_session_test.h @@ -70,6 +70,7 @@ void test_nghttp2_submit_request_with_data(void); void test_nghttp2_submit_request_without_data(void); void test_nghttp2_submit_response_with_data(void); void test_nghttp2_submit_response_without_data(void); +void test_nghttp2_submit_response_push_response(void); void test_nghttp2_submit_trailer(void); void test_nghttp2_submit_headers_start_stream(void); void test_nghttp2_submit_headers_reply(void); From b95a3c4b2851192775d14daa5f8e543666ab0606 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 16 Nov 2015 22:53:31 +0900 Subject: [PATCH 04/62] Consider to use CANCEL error code when closing streams with GOAWAY For clients, CANCEL is more appropriate for both incoming/outgoing streams. For servers, CANCEL is appropriate for its pushed stream (outgoing), but REFUSED_STREAM is more appropriate for incoming stream. --- lib/nghttp2_session.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 366a8007..de65867b 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -2200,16 +2200,19 @@ static int session_close_stream_on_goaway(nghttp2_session *session, nghttp2_stream *stream, *next_stream; nghttp2_close_stream_on_goaway_arg arg = {session, NULL, last_stream_id, incoming}; + uint32_t error_code; rv = nghttp2_map_each(&session->streams, find_stream_on_goaway_func, &arg); assert(rv == 0); + error_code = + session->server && incoming ? NGHTTP2_REFUSED_STREAM : NGHTTP2_CANCEL; + stream = arg.head; while (stream) { next_stream = stream->closed_next; stream->closed_next = NULL; - rv = nghttp2_session_close_stream(session, stream->stream_id, - NGHTTP2_REFUSED_STREAM); + rv = nghttp2_session_close_stream(session, stream->stream_id, error_code); /* stream may be deleted here */ From 1753bea6923aaf18276efb06e6bfaa59a03c3031 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 16 Nov 2015 00:12:54 +0900 Subject: [PATCH 05/62] nghttpx: Support server push from HTTP/2 backend This commits enables HTTP/2 server push from HTTP/2 backend to be relayed to HTTP/2 frontend. To use this feature, --http2-bridge or --client is required. Server push via Link header field contiues to work. --- doc/nghttpx.h2r | 12 +- src/shrpx.cc | 11 +- src/shrpx_downstream_queue.cc | 5 +- src/shrpx_downstream_queue.h | 10 +- src/shrpx_http2_session.cc | 351 +++++++++++++++++++++++++++------- src/shrpx_http2_session.h | 5 + src/shrpx_http2_upstream.cc | 75 +++++++- src/shrpx_http2_upstream.h | 8 + src/shrpx_https_upstream.cc | 16 ++ src/shrpx_https_upstream.h | 8 + src/shrpx_spdy_upstream.cc | 16 ++ src/shrpx_spdy_upstream.h | 8 + src/shrpx_upstream.h | 22 +++ 13 files changed, 458 insertions(+), 89 deletions(-) diff --git a/doc/nghttpx.h2r b/doc/nghttpx.h2r index 656baddb..8901ae7a 100644 --- a/doc/nghttpx.h2r +++ b/doc/nghttpx.h2r @@ -60,8 +60,8 @@ SIGUSR2 SERVER PUSH ----------- -nghttpx supports HTTP/2 server push in default mode. nghttpx looks -for Link header field (`RFC 5988 +nghttpx supports HTTP/2 server push in default mode with Link header +field. nghttpx looks for Link header field (`RFC 5988 `_) in response headers from backend server and extracts URI-reference with parameter ``rel=preload`` (see `preload @@ -81,6 +81,14 @@ Currently, the following restriction is applied for server push: This limitation may be loosened in the future release. +nghttpx also supports server push if both frontend and backend are +HTTP/2 (which implies :option:`--http2-bridge` or :option:`--client`). +In this case, in addition to server push via Link header field, server +push from backend is relayed to frontend HTTP/2 session. + +HTTP/2 server push will be disabled if :option:`--http2-proxy` or +:option:`--client-proxy` is used. + UNIX DOMAIN SOCKET ------------------ diff --git a/src/shrpx.cc b/src/shrpx.cc index 855df5f3..ab9e673a 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -1480,9 +1480,14 @@ HTTP/2 and SPDY: meant for debugging purpose and not intended to enhance protocol security. --no-server-push - Disable HTTP/2 server push. Server push is only - supported by default mode and HTTP/2 frontend. SPDY - frontend does not support server push. + Disable HTTP/2 server push. Server push is supported by + default mode and HTTP/2 frontend via Link header field. + It is also supported if both frontend and backend are + HTTP/2 (which implies --http2-bridge or --client mode). + In this case, server push from backend session is + relayed to frontend, and server push via Link header + field is also supported. HTTP SPDY frontend does not + support server push. Mode: (default mode) diff --git a/src/shrpx_downstream_queue.cc b/src/shrpx_downstream_queue.cc index d1430e61..02fd0356 100644 --- a/src/shrpx_downstream_queue.cc +++ b/src/shrpx_downstream_queue.cc @@ -120,7 +120,8 @@ bool remove_host_entry_if_empty(const DownstreamQueue::HostEntry &ent, } } // namespace -Downstream *DownstreamQueue::remove_and_get_blocked(Downstream *downstream) { +Downstream *DownstreamQueue::remove_and_get_blocked(Downstream *downstream, + bool next_blocked) { // Delete downstream when this function returns. auto delptr = std::unique_ptr(downstream); @@ -144,7 +145,7 @@ Downstream *DownstreamQueue::remove_and_get_blocked(Downstream *downstream) { return nullptr; } - if (ent.num_active >= conn_max_per_host_) { + if (!next_blocked || ent.num_active >= conn_max_per_host_) { return nullptr; } diff --git a/src/shrpx_downstream_queue.h b/src/shrpx_downstream_queue.h index de06d33e..755af10d 100644 --- a/src/shrpx_downstream_queue.h +++ b/src/shrpx_downstream_queue.h @@ -79,10 +79,12 @@ public: // |host|. bool can_activate(const std::string &host) const; // Removes and frees |downstream| object. If |downstream| is in - // Downstream::DISPATCH_ACTIVE, this function may return Downstream - // object with the same target host in Downstream::DISPATCH_BLOCKED - // if its connection is now not blocked by conn_max_per_host_ limit. - Downstream *remove_and_get_blocked(Downstream *downstream); + // Downstream::DISPATCH_ACTIVE, and |next_blocked| is true, this + // function may return Downstream object with the same target host + // in Downstream::DISPATCH_BLOCKED if its connection is now not + // blocked by conn_max_per_host_ limit. + Downstream *remove_and_get_blocked(Downstream *downstream, + bool next_blocked = true); Downstream *get_downstreams() const; HostEntry &find_host_entry(const std::string &host); const std::string &make_host_key(const std::string &host) const; diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index 2258903c..508ccd34 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -682,32 +682,43 @@ int on_stream_close_callback(nghttp2_session *session, int32_t stream_id, if (dconn) { auto downstream = dconn->get_downstream(); if (downstream && downstream->get_downstream_stream_id() == stream_id) { + auto upstream = downstream->get_upstream(); - if (downstream->get_upgraded() && - downstream->get_response_state() == Downstream::HEADER_COMPLETE) { - // For tunneled connection, we have to submit RST_STREAM to - // upstream *after* whole response body is sent. We just set - // MSG_COMPLETE here. Upstream will take care of that. - downstream->get_upstream()->on_downstream_body_complete(downstream); - downstream->set_response_state(Downstream::MSG_COMPLETE); - } else if (error_code == NGHTTP2_NO_ERROR) { - switch (downstream->get_response_state()) { - case Downstream::MSG_COMPLETE: - case Downstream::MSG_BAD_HEADER: - break; - default: + if (downstream->get_downstream_stream_id() % 2 == 0 && + downstream->get_request_state() == Downstream::INITIAL) { + // Downstream is canceled in backend before it is submitted in + // frontend session. + + // This will avoid to send RST_STREAM to backend + downstream->set_response_state(Downstream::MSG_RESET); + upstream->cancel_premature_downstream(downstream); + } else { + if (downstream->get_upgraded() && + downstream->get_response_state() == Downstream::HEADER_COMPLETE) { + // For tunneled connection, we have to submit RST_STREAM to + // upstream *after* whole response body is sent. We just set + // MSG_COMPLETE here. Upstream will take care of that. + downstream->get_upstream()->on_downstream_body_complete(downstream); + downstream->set_response_state(Downstream::MSG_COMPLETE); + } else if (error_code == NGHTTP2_NO_ERROR) { + switch (downstream->get_response_state()) { + case Downstream::MSG_COMPLETE: + case Downstream::MSG_BAD_HEADER: + break; + default: + downstream->set_response_state(Downstream::MSG_RESET); + } + } else if (downstream->get_response_state() != + Downstream::MSG_BAD_HEADER) { downstream->set_response_state(Downstream::MSG_RESET); } - } else if (downstream->get_response_state() != - Downstream::MSG_BAD_HEADER) { - downstream->set_response_state(Downstream::MSG_RESET); + if (downstream->get_response_state() == Downstream::MSG_RESET && + downstream->get_response_rst_stream_error_code() == + NGHTTP2_NO_ERROR) { + downstream->set_response_rst_stream_error_code(error_code); + } + call_downstream_readcb(http2session, downstream); } - if (downstream->get_response_state() == Downstream::MSG_RESET && - downstream->get_response_rst_stream_error_code() == - NGHTTP2_NO_ERROR) { - downstream->set_response_rst_stream_error_code(error_code); - } - call_downstream_readcb(http2session, downstream); // dconn may be deleted } } @@ -730,6 +741,7 @@ 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, uint8_t flags, void *user_data) { + auto http2session = static_cast(user_data); auto sd = static_cast( nghttp2_session_get_stream_user_data(session, frame->hd.stream_id)); if (!sd || !sd->dconn) { @@ -740,44 +752,80 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, return 0; } - if (frame->hd.type != NGHTTP2_HEADERS) { - return 0; - } + switch (frame->hd.type) { + case NGHTTP2_HEADERS: { + auto trailer = frame->headers.cat == NGHTTP2_HCAT_HEADERS && + !downstream->get_expect_final_response(); - auto trailer = frame->headers.cat != NGHTTP2_HCAT_RESPONSE && - !downstream->get_expect_final_response(); + if (downstream->get_response_headers_sum() + namelen + valuelen > + get_config()->header_field_buffer || + downstream->get_response_headers().size() >= + get_config()->max_header_fields) { + if (LOG_ENABLED(INFO)) { + DLOG(INFO, downstream) + << "Too large or many header field size=" + << downstream->get_response_headers_sum() + namelen + valuelen + << ", num=" << downstream->get_response_headers().size() + 1; + } - if (downstream->get_response_headers_sum() + namelen + valuelen > - get_config()->header_field_buffer || - downstream->get_response_headers().size() >= - get_config()->max_header_fields) { - if (LOG_ENABLED(INFO)) { - DLOG(INFO, downstream) - << "Too large or many header field size=" - << downstream->get_response_headers_sum() + namelen + valuelen - << ", num=" << downstream->get_response_headers().size() + 1; + if (trailer) { + // we don't care trailer part exceeds header size limit; just + // discard it. + return 0; + } + + return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; } if (trailer) { - // we don't care trailer part exceeds header size limit; just - // discard it. + // just store header fields for trailer part + downstream->add_response_trailer(name, namelen, value, valuelen, + flags & NGHTTP2_NV_FLAG_NO_INDEX, -1); return 0; } - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } + auto token = http2::lookup_token(name, namelen); - if (trailer) { - // just store header fields for trailer part - downstream->add_response_trailer(name, namelen, value, valuelen, - flags & NGHTTP2_NV_FLAG_NO_INDEX, -1); + downstream->add_response_header(name, namelen, value, valuelen, + flags & NGHTTP2_NV_FLAG_NO_INDEX, token); return 0; } + case NGHTTP2_PUSH_PROMISE: { + auto promised_stream_id = frame->push_promise.promised_stream_id; + auto promised_sd = static_cast( + nghttp2_session_get_stream_user_data(session, promised_stream_id)); + if (!promised_sd || !promised_sd->dconn) { + http2session->submit_rst_stream(promised_stream_id, NGHTTP2_CANCEL); + return 0; + } - auto token = http2::lookup_token(name, namelen); + auto promised_downstream = promised_sd->dconn->get_downstream(); + + assert(promised_downstream); + + if (promised_downstream->get_request_headers_sum() + namelen + valuelen > + get_config()->header_field_buffer || + promised_downstream->get_request_headers().size() >= + get_config()->max_header_fields) { + if (LOG_ENABLED(INFO)) { + DLOG(INFO, promised_downstream) + << "Too large or many header field size=" + << promised_downstream->get_request_headers_sum() + namelen + + valuelen << ", num=" + << promised_downstream->get_request_headers().size() + 1; + } + + return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; + } + + auto token = http2::lookup_token(name, namelen); + promised_downstream->add_request_header(name, namelen, value, valuelen, + flags & NGHTTP2_NV_FLAG_NO_INDEX, + token); + return 0; + } + } - downstream->add_response_header(name, namelen, value, valuelen, - flags & NGHTTP2_NV_FLAG_NO_INDEX, token); return 0; } } // namespace @@ -786,24 +834,52 @@ namespace { int on_begin_headers_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_RESPONSE) { + + switch (frame->hd.type) { + case NGHTTP2_HEADERS: { + if (frame->headers.cat != NGHTTP2_HCAT_RESPONSE && + frame->headers.cat != NGHTTP2_HCAT_PUSH_RESPONSE) { + return 0; + } + 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_INTERNAL_ERROR); + return 0; + } + auto downstream = sd->dconn->get_downstream(); + if (!downstream || + downstream->get_downstream_stream_id() != frame->hd.stream_id) { + http2session->submit_rst_stream(frame->hd.stream_id, + NGHTTP2_INTERNAL_ERROR); + return 0; + } return 0; } - 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_INTERNAL_ERROR); + case NGHTTP2_PUSH_PROMISE: { + auto promised_stream_id = frame->push_promise.promised_stream_id; + auto sd = static_cast( + nghttp2_session_get_stream_user_data(session, frame->hd.stream_id)); + if (!sd || !sd->dconn) { + http2session->submit_rst_stream(promised_stream_id, NGHTTP2_CANCEL); + return 0; + } + + auto downstream = sd->dconn->get_downstream(); + + assert(downstream); + assert(downstream->get_downstream_stream_id() == frame->hd.stream_id); + + if (http2session->handle_downstream_push_promise(downstream, + promised_stream_id) != 0) { + http2session->submit_rst_stream(promised_stream_id, NGHTTP2_CANCEL); + } + return 0; } - auto downstream = sd->dconn->get_downstream(); - if (!downstream || - downstream->get_downstream_stream_id() != frame->hd.stream_id) { - http2session->submit_rst_stream(frame->hd.stream_id, - NGHTTP2_INTERNAL_ERROR); - return 0; } + return 0; } } // namespace @@ -976,7 +1052,8 @@ int on_frame_recv_callback(nghttp2_session *session, const nghttp2_frame *frame, return 0; } - if (frame->headers.cat == NGHTTP2_HCAT_RESPONSE) { + if (frame->headers.cat == NGHTTP2_HCAT_RESPONSE || + frame->headers.cat == NGHTTP2_HCAT_PUSH_RESPONSE) { rv = on_response_headers(http2session, downstream, session, frame); if (rv != 0) { @@ -1044,17 +1121,47 @@ int on_frame_recv_callback(nghttp2_session *session, const nghttp2_frame *frame, http2session->connection_alive(); } return 0; - case NGHTTP2_PUSH_PROMISE: + case NGHTTP2_PUSH_PROMISE: { + auto promised_stream_id = frame->push_promise.promised_stream_id; + if (LOG_ENABLED(INFO)) { SSLOG(INFO, http2session) << "Received downstream PUSH_PROMISE stream_id=" << frame->hd.stream_id - << ", promised_stream_id=" << frame->push_promise.promised_stream_id; + << ", promised_stream_id=" << promised_stream_id; } - // We just respond with RST_STREAM. - http2session->submit_rst_stream(frame->push_promise.promised_stream_id, - NGHTTP2_REFUSED_STREAM); + + auto sd = static_cast( + nghttp2_session_get_stream_user_data(session, frame->hd.stream_id)); + if (!sd || !sd->dconn) { + http2session->submit_rst_stream(promised_stream_id, NGHTTP2_CANCEL); + return 0; + } + + auto downstream = sd->dconn->get_downstream(); + + assert(downstream); + assert(downstream->get_downstream_stream_id() == frame->hd.stream_id); + + auto promised_sd = static_cast( + nghttp2_session_get_stream_user_data(session, promised_stream_id)); + if (!promised_sd || !promised_sd->dconn) { + http2session->submit_rst_stream(promised_stream_id, NGHTTP2_CANCEL); + return 0; + } + + auto promised_downstream = promised_sd->dconn->get_downstream(); + + assert(promised_downstream); + + if (http2session->handle_downstream_push_promise_complete( + downstream, promised_downstream) != 0) { + http2session->submit_rst_stream(promised_stream_id, NGHTTP2_CANCEL); + return 0; + } + return 0; + } default: return 0; } @@ -1279,16 +1386,22 @@ int Http2Session::connection_made() { flow_control_ = true; std::array entry; - entry[0].settings_id = NGHTTP2_SETTINGS_ENABLE_PUSH; - entry[0].value = 0; - entry[1].settings_id = NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS; - entry[1].value = get_config()->http2_max_concurrent_streams; + size_t nentry = 2; + entry[0].settings_id = NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS; + entry[0].value = get_config()->http2_max_concurrent_streams; - entry[2].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE; - entry[2].value = (1 << get_config()->http2_downstream_window_bits) - 1; + entry[1].settings_id = NGHTTP2_SETTINGS_INITIAL_WINDOW_SIZE; + entry[1].value = (1 << get_config()->http2_downstream_window_bits) - 1; + + if (get_config()->no_server_push || get_config()->http2_proxy || + get_config()->client_proxy) { + entry[nentry].settings_id = NGHTTP2_SETTINGS_ENABLE_PUSH; + entry[nentry].value = 0; + ++nentry; + } rv = nghttp2_submit_settings(session_, NGHTTP2_FLAG_NONE, entry.data(), - entry.size()); + nentry); if (rv != 0) { return -1; } @@ -1771,4 +1884,96 @@ size_t Http2Session::get_group() const { return group_; } size_t Http2Session::get_index() const { return index_; } +int Http2Session::handle_downstream_push_promise(Downstream *downstream, + int32_t promised_stream_id) { + auto upstream = downstream->get_upstream(); + if (!upstream->push_enabled()) { + return -1; + } + + auto promised_downstream = + upstream->on_downstream_push_promise(downstream, promised_stream_id); + if (!promised_downstream) { + return -1; + } + + // Now we have Downstream object for pushed stream. + // promised_downstream->get_stream() still returns 0. + + auto handler = upstream->get_client_handler(); + auto worker = handler->get_worker(); + + auto promised_dconn = + make_unique(worker->get_dconn_pool(), this); + promised_dconn->set_client_handler(handler); + + auto ptr = promised_dconn.get(); + + if (promised_downstream->attach_downstream_connection( + std::move(promised_dconn)) != 0) { + return -1; + } + + auto promised_sd = make_unique(); + + nghttp2_session_set_stream_user_data(session_, promised_stream_id, + promised_sd.get()); + + ptr->attach_stream_data(promised_sd.get()); + streams_.append(promised_sd.release()); + + return 0; +} + +int Http2Session::handle_downstream_push_promise_complete( + Downstream *downstream, Downstream *promised_downstream) { + auto authority = + promised_downstream->get_request_header(http2::HD__AUTHORITY); + auto path = promised_downstream->get_request_header(http2::HD__PATH); + auto method = promised_downstream->get_request_header(http2::HD__METHOD); + auto scheme = promised_downstream->get_request_header(http2::HD__SCHEME); + + if (!authority) { + authority = promised_downstream->get_request_header(http2::HD_HOST); + } + + auto method_token = http2::lookup_method_token(method->value); + if (method_token == -1) { + if (LOG_ENABLED(INFO)) { + SSLOG(INFO, this) << "Unrecognized method: " << method->value; + } + + return -1; + } + + // TODO Rewrite authority if we enabled rewrite host. But we + // really don't know how to rewrite host. Should we use the same + // host in associated stream? + promised_downstream->set_request_http2_authority( + http2::value_to_str(authority)); + promised_downstream->set_request_method(method_token); + // libnghttp2 ensures that we don't have CONNECT method in + // PUSH_PROMISE, and guarantees that :scheme exists. + promised_downstream->set_request_http2_scheme(http2::value_to_str(scheme)); + + // For server-wide OPTIONS request, path is empty. + if (method_token != HTTP_OPTIONS || path->value != "*") { + promised_downstream->set_request_path(http2::rewrite_clean_path( + std::begin(path->value), std::end(path->value))); + } + + promised_downstream->inspect_http2_request(); + + auto upstream = promised_downstream->get_upstream(); + + promised_downstream->set_request_state(Downstream::MSG_COMPLETE); + + if (upstream->on_downstream_push_promise_complete(downstream, + promised_downstream) != 0) { + return -1; + } + + return 0; +} + } // namespace shrpx diff --git a/src/shrpx_http2_session.h b/src/shrpx_http2_session.h index 3ae3e784..e321d59e 100644 --- a/src/shrpx_http2_session.h +++ b/src/shrpx_http2_session.h @@ -156,6 +156,11 @@ public: size_t get_index() const; + int handle_downstream_push_promise(Downstream *downstream, + int32_t promised_stream_id); + int handle_downstream_push_promise_complete(Downstream *downstream, + Downstream *promised_downstream); + enum { // Disconnected DISCONNECTED, diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index c7e93ed9..bc0de69c 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -543,6 +543,13 @@ int on_frame_send_callback(nghttp2_session *session, const nghttp2_frame *frame, return 0; case NGHTTP2_PUSH_PROMISE: { auto promised_stream_id = frame->push_promise.promised_stream_id; + + if (nghttp2_session_get_stream_user_data(session, promised_stream_id)) { + // In case of push from backend, downstream object was already + // created. + return 0; + } + auto downstream = make_unique(upstream, handler->get_mcpool(), promised_stream_id, 0); @@ -1712,6 +1719,7 @@ int Http2Upstream::on_downstream_reset(bool no_retry) { } if (!downstream->request_submission_ready()) { + // pushed stream is handled here rst_stream(downstream, NGHTTP2_INTERNAL_ERROR); downstream->pop_downstream_connection(); continue; @@ -1853,15 +1861,18 @@ int Http2Upstream::submit_push_promise(const std::string &scheme, return 0; } +bool Http2Upstream::push_enabled() const { + return !(get_config()->no_server_push || + nghttp2_session_get_remote_settings( + session_, NGHTTP2_SETTINGS_ENABLE_PUSH) == 0 || + get_config()->http2_proxy || get_config()->client_proxy); +} + int Http2Upstream::initiate_push(Downstream *downstream, const char *uri, size_t len) { int rv; - if (len == 0 || get_config()->no_server_push || - nghttp2_session_get_remote_settings(session_, - NGHTTP2_SETTINGS_ENABLE_PUSH) == 0 || - get_config()->http2_proxy || get_config()->client_proxy || - (downstream->get_stream_id() % 2) == 0) { + if (len == 0 || !push_enabled() || (downstream->get_stream_id() % 2)) { return 0; } @@ -1916,4 +1927,58 @@ bool Http2Upstream::response_empty() const { return wb_.rleft() == 0; } Http2Upstream::WriteBuffer *Http2Upstream::get_response_buf() { return &wb_; } +Downstream * +Http2Upstream::on_downstream_push_promise(Downstream *downstream, + int32_t promised_stream_id) { + // promised_stream_id is for backend HTTP/2 session, not for + // frontend. + auto promised_downstream = + make_unique(this, handler_->get_mcpool(), 0, 0); + promised_downstream->set_downstream_stream_id(promised_stream_id); + + promised_downstream->disable_upstream_rtimer(); + + promised_downstream->set_request_major(2); + promised_downstream->set_request_minor(0); + + auto ptr = promised_downstream.get(); + add_pending_downstream(std::move(promised_downstream)); + downstream_queue_.mark_active(ptr); + + return ptr; +} + +int Http2Upstream::on_downstream_push_promise_complete( + Downstream *downstream, Downstream *promised_downstream) { + std::vector nva; + + auto &headers = promised_downstream->get_request_headers(); + + nva.reserve(headers.size()); + + for (auto &kv : headers) { + nva.push_back(http2::make_nv_nocopy(kv.name, kv.value, kv.no_index)); + } + + auto promised_stream_id = nghttp2_submit_push_promise( + session_, NGHTTP2_FLAG_NONE, downstream->get_stream_id(), nva.data(), + nva.size(), promised_downstream); + if (promised_stream_id < 0) { + return -1; + } + + promised_downstream->set_stream_id(promised_stream_id); + + return 0; +} + +void Http2Upstream::cancel_premature_downstream( + Downstream *promised_downstream) { + if (LOG_ENABLED(INFO)) { + ULOG(INFO, this) << "Remove premature promised stream " + << promised_downstream; + } + downstream_queue_.remove_and_get_blocked(promised_downstream, false); +} + } // namespace shrpx diff --git a/src/shrpx_http2_upstream.h b/src/shrpx_http2_upstream.h index d49455cc..e2527626 100644 --- a/src/shrpx_http2_upstream.h +++ b/src/shrpx_http2_upstream.h @@ -87,6 +87,14 @@ public: virtual void response_drain(size_t n); virtual bool response_empty() const; + virtual Downstream *on_downstream_push_promise(Downstream *downstream, + int32_t promised_stream_id); + virtual int + on_downstream_push_promise_complete(Downstream *downstream, + Downstream *promised_downstream); + virtual bool push_enabled() const; + virtual void cancel_premature_downstream(Downstream *promised_downstream); + bool get_flow_control() const; // Perform HTTP/2 upgrade from |upstream|. On success, this object // takes ownership of the |upstream|. This function returns 0 if it diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index 25184427..2f9cd831 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -1191,4 +1191,20 @@ bool HttpsUpstream::response_empty() const { return buf->rleft() == 0; } +Downstream * +HttpsUpstream::on_downstream_push_promise(Downstream *downstream, + int32_t promised_stream_id) { + return nullptr; +} + +int HttpsUpstream::on_downstream_push_promise_complete( + Downstream *downstream, Downstream *promised_downstream) { + return -1; +} + +bool HttpsUpstream::push_enabled() const { return false; } + +void HttpsUpstream::cancel_premature_downstream( + Downstream *promised_downstream) {} + } // namespace shrpx diff --git a/src/shrpx_https_upstream.h b/src/shrpx_https_upstream.h index 6f3b5742..3ab698bc 100644 --- a/src/shrpx_https_upstream.h +++ b/src/shrpx_https_upstream.h @@ -82,6 +82,14 @@ public: virtual void response_drain(size_t n); virtual bool response_empty() const; + virtual Downstream *on_downstream_push_promise(Downstream *downstream, + int32_t promised_stream_id); + virtual int + on_downstream_push_promise_complete(Downstream *downstream, + Downstream *promised_downstream); + virtual bool push_enabled() const; + virtual void cancel_premature_downstream(Downstream *promised_downstream); + void reset_current_header_length(); void log_response_headers(DefaultMemchunks *buf) const; diff --git a/src/shrpx_spdy_upstream.cc b/src/shrpx_spdy_upstream.cc index 2ce0491c..d7d2269a 100644 --- a/src/shrpx_spdy_upstream.cc +++ b/src/shrpx_spdy_upstream.cc @@ -1230,4 +1230,20 @@ bool SpdyUpstream::response_empty() const { return wb_.rleft() == 0; } SpdyUpstream::WriteBuffer *SpdyUpstream::get_response_buf() { return &wb_; } +Downstream * +SpdyUpstream::on_downstream_push_promise(Downstream *downstream, + int32_t promised_stream_id) { + return nullptr; +} + +int SpdyUpstream::on_downstream_push_promise_complete( + Downstream *downstream, Downstream *promised_downstream) { + return -1; +} + +bool SpdyUpstream::push_enabled() const { return false; } + +void SpdyUpstream::cancel_premature_downstream( + Downstream *promised_downstream) {} + } // namespace shrpx diff --git a/src/shrpx_spdy_upstream.h b/src/shrpx_spdy_upstream.h index 7d509340..9dfcc1b2 100644 --- a/src/shrpx_spdy_upstream.h +++ b/src/shrpx_spdy_upstream.h @@ -82,6 +82,14 @@ public: virtual void response_drain(size_t n); virtual bool response_empty() const; + virtual Downstream *on_downstream_push_promise(Downstream *downstream, + int32_t promised_stream_id); + virtual int + on_downstream_push_promise_complete(Downstream *downstream, + Downstream *promised_downstream); + virtual bool push_enabled() const; + virtual void cancel_premature_downstream(Downstream *promised_downstream); + bool get_flow_control() const; int consume(int32_t stream_id, size_t len); diff --git a/src/shrpx_upstream.h b/src/shrpx_upstream.h index 92fe5190..b9639e72 100644 --- a/src/shrpx_upstream.h +++ b/src/shrpx_upstream.h @@ -76,6 +76,28 @@ public: virtual int response_riovec(struct iovec *iov, int iovcnt) const = 0; virtual void response_drain(size_t n) = 0; virtual bool response_empty() const = 0; + + // Called when PUSH_PROMISE was started in downstream. The + // associated downstream is given as |downstream|. The promised + // stream ID is given as |promised_stream_id|. If upstream supports + // server push for the corresponding upstream connection, it should + // return Downstream object for pushed stream. Otherwise, returns + // nullptr. + virtual Downstream * + on_downstream_push_promise(Downstream *downstream, + int32_t promised_stream_id) = 0; + // Called when PUSH_PROMISE frame was completely received in + // downstream. The associated downstream is given as |downstream|. + // This function returns 0 if it succeeds, or -1. + virtual int + on_downstream_push_promise_complete(Downstream *downstream, + Downstream *promised_downstream) = 0; + // Returns true if server push is enabled in upstream connection. + virtual bool push_enabled() const = 0; + // Cancels promised downstream. This function is called when + // PUSH_PROMISE for |promised_downstream| is not submitted to + // upstream session. + virtual void cancel_premature_downstream(Downstream *promised_downstream) = 0; }; } // namespace shrpx From 79ee999f1be08613f50ebc890d45faefbeca8c94 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 17 Nov 2015 23:23:53 +0900 Subject: [PATCH 06/62] Update mruby to 1.2.0 --- third-party/mruby | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/third-party/mruby b/third-party/mruby index 83922d0b..22464fe5 160000 --- a/third-party/mruby +++ b/third-party/mruby @@ -1 +1 @@ -Subproject commit 83922d0bbb7de894380fa6b33e7d75061114aa59 +Subproject commit 22464fe5a0a10f2b077eaba109ce1e912e4a77de From 8c94341c18c20cd66a679b31fbce5599b5dd3914 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 18 Nov 2015 22:25:02 +0900 Subject: [PATCH 07/62] h2load: Override user-agent with -H option This commit allows user to override user-agent with -H option. We also enabled custom header support for http/1.1 requests. We also did minor optimizations (std::vector::reserve). --- src/h2load.cc | 44 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/src/h2load.cc b/src/h2load.cc index 2ef99e78..9e9c9bcb 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -1893,8 +1893,8 @@ int main(int argc, char **argv) { shared_nva.emplace_back("user-agent", user_agent); // list overridalbe headers - auto override_hdrs = - make_array(":authority", ":host", ":method", ":scheme"); + auto override_hdrs = make_array(":authority", ":host", ":method", + ":scheme", "user-agent"); for (auto &kv : config.custom_headers) { if (std::find(std::begin(override_hdrs), std::end(override_hdrs), @@ -1912,20 +1912,44 @@ int main(int argc, char **argv) { } } + auto method_it = + std::find_if(std::begin(shared_nva), std::end(shared_nva), + [](const Header &nv) { return nv.name == ":method"; }); + assert(method_it != std::end(shared_nva)); + + config.h1reqs.reserve(reqlines.size()); + config.nva.reserve(reqlines.size()); + config.nv.reserve(reqlines.size()); + for (auto &req : reqlines) { // For HTTP/1.1 - std::string h1req; - h1req = config.data_fd == -1 ? "GET" : "POST"; - h1req += " " + req; + auto h1req = (*method_it).value; + h1req += " "; + h1req += req; h1req += " HTTP/1.1\r\n"; - h1req += "Host: " + config.host + "\r\n"; - h1req += "User-Agent: " + user_agent + "\r\n"; - h1req += "Accept: */*\r\n"; + for (auto &nv : shared_nva) { + if (nv.name == ":authority") { + h1req += "Host: "; + h1req += nv.value; + h1req += "\r\n"; + continue; + } + if (nv.name[0] == ':') { + continue; + } + h1req += nv.name; + h1req += ": "; + h1req += nv.value; + h1req += "\r\n"; + } h1req += "\r\n"; - config.h1reqs.push_back(h1req); + + config.h1reqs.push_back(std::move(h1req)); // For nghttp2 std::vector nva; + // 1 for :path + nva.reserve(1 + shared_nva.size()); nva.push_back(http2::make_nv_ls(":path", req)); @@ -1937,6 +1961,8 @@ int main(int argc, char **argv) { // For spdylay std::vector cva; + // 2 for :path and :version, 1 for terminal nullptr + cva.reserve(2 * (2 + shared_nva.size()) + 1); cva.push_back(":path"); cva.push_back(req.c_str()); From 8b4f6f1778648748ab4a3a65c3f9070ed3320ca5 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 18 Nov 2015 23:21:57 +0900 Subject: [PATCH 08/62] nghttpd: Defered eviction of cached fd using timer To make use cache fd more robust manner (e.g. among several connections), eviction of cached file descriptor now takes place using timer. The timer is started when there is no handler (no connections). The timeout value is hard-coded and 2 seconds. --- src/HttpServer.cc | 62 ++++++++++++++++++++++++++++++++++++++++++++--- src/HttpServer.h | 9 ++++--- 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/src/HttpServer.cc b/src/HttpServer.cc index 2e313d85..6d22248e 100644 --- a/src/HttpServer.cc +++ b/src/HttpServer.cc @@ -175,6 +175,14 @@ namespace { void fill_callback(nghttp2_session_callbacks *callbacks, const Config *config); } // namespace +namespace { +constexpr ev_tstamp RELEASE_FD_TIMEOUT = 2.; +} // namespace + +namespace { +void release_fd_cb(struct ev_loop *loop, ev_timer *w, int revents); +} // namespace + class Sessions { public: Sessions(HttpServer *sv, struct ev_loop *loop, const Config *config, @@ -185,15 +193,24 @@ public: nghttp2_session_callbacks_new(&callbacks_); fill_callback(callbacks_, config_); + + ev_timer_init(&release_fd_timer_, release_fd_cb, 0., RELEASE_FD_TIMEOUT); + release_fd_timer_.data = this; } ~Sessions() { + ev_timer_stop(loop_, &release_fd_timer_); for (auto handler : handlers_) { delete handler; } nghttp2_session_callbacks_del(callbacks_); } void add_handler(Http2Handler *handler) { handlers_.insert(handler); } - void remove_handler(Http2Handler *handler) { handlers_.erase(handler); } + void remove_handler(Http2Handler *handler) { + handlers_.erase(handler); + if (handlers_.empty() && !fd_cache_.empty()) { + ev_timer_again(loop_, &release_fd_timer_); + } + } SSL_CTX *get_ssl_ctx() const { return ssl_ctx_; } SSL *ssl_session_new(int fd) { SSL *ssl = SSL_new(ssl_ctx_); @@ -275,12 +292,33 @@ public: return; } auto &ent = (*i).second; - if (--ent.usecount == 0) { + if (ent.usecount == 0) { + // temporary fd, close it immediately close(ent.fd); fd_cache_.erase(i); + + return; + } + + --ent.usecount; + + // We use timer to close file descriptor and delete the entry from + // cache. The timer will be started when there is no handler. + } + void release_unused_fd() { + for (auto i = std::begin(fd_cache_); i != std::end(fd_cache_);) { + auto &ent = (*i).second; + if (ent.usecount != 0) { + ++i; + continue; + } + + close(ent.fd); + i = fd_cache_.erase(i); } } const HttpServer *get_server() const { return sv_; } + bool handlers_empty() const { return handlers_.empty(); } private: std::set handlers_; @@ -291,11 +329,26 @@ private: const Config *config_; SSL_CTX *ssl_ctx_; nghttp2_session_callbacks *callbacks_; + ev_timer release_fd_timer_; int64_t next_session_id_; ev_tstamp tstamp_cached_; std::string cached_date_; }; +namespace { +void release_fd_cb(struct ev_loop *loop, ev_timer *w, int revents) { + auto sessions = static_cast(w->data); + + ev_timer_stop(loop, w); + + if (!sessions->handlers_empty()) { + return; + } + + sessions->release_unused_fd(); +} +} // namespace + Stream::Stream(Http2Handler *handler, int32_t stream_id) : handler(handler), file_ent(nullptr), body_length(0), body_offset(0), stream_id(stream_id), echo_upload(false) { @@ -979,8 +1032,9 @@ bool prepare_upload_temp_store(Stream *stream, Http2Handler *hd) { unlink(tempfn); // Ordinary request never start with "echo:". The length is 0 for // now. We will update it when we get whole request body. - stream->file_ent = sessions->cache_fd(std::string("echo:") + tempfn, - FileEntry(tempfn, 0, 0, fd, nullptr)); + auto path = std::string("echo:") + tempfn; + stream->file_ent = + sessions->cache_fd(path, FileEntry(path, 0, 0, fd, nullptr, 0)); stream->echo_upload = true; return true; } diff --git a/src/HttpServer.h b/src/HttpServer.h index de43eeec..5f701d2a 100644 --- a/src/HttpServer.h +++ b/src/HttpServer.h @@ -83,14 +83,15 @@ struct Config { class Http2Handler; struct FileEntry { + // To close fd immediately when nothing refers to this entry, give 0 + // to |usecount|. FileEntry(std::string path, int64_t length, int64_t mtime, int fd, - const std::string *content_type) - : path(std::move(path)), length(length), mtime(mtime), dlprev(nullptr), - dlnext(nullptr), content_type(content_type), fd(fd), usecount(1) {} + const std::string *content_type, int usecount = 1) + : path(std::move(path)), length(length), mtime(mtime), + content_type(content_type), fd(fd), usecount(usecount) {} std::string path; int64_t length; int64_t mtime; - FileEntry *dlprev, *dlnext; const std::string *content_type; int fd; int usecount; From ab1e8305a1dc967439acaaf3f6cbd02244464bc0 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 19 Nov 2015 00:00:05 +0900 Subject: [PATCH 09/62] asio: client: call on_error when connection is dropped When connection is dropped, we get "nonzero" error code ec in read_socket callback. We can just call on_error callback if ec is nonzero. But there is a problem when we did ordered shutdown. In this case, we also get EOF in ec, but we don't want to call on_error because it is not an error. To workaround this, we check return value of should_stop(). If it is true, then we assume that HTTP/2 session cleanly ended, and we don't call on_error. --- src/asio_client_session_impl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/asio_client_session_impl.cc b/src/asio_client_session_impl.cc index d3a57a1e..33f21137 100644 --- a/src/asio_client_session_impl.cc +++ b/src/asio_client_session_impl.cc @@ -525,7 +525,7 @@ void session_impl::do_read() { read_socket([this](const boost::system::error_code &ec, std::size_t bytes_transferred) { if (ec) { - if (ec.value() == boost::asio::error::operation_aborted) { + if (!should_stop()) { call_error_cb(ec); shutdown_socket(); } From 3bbb05f59b2d2f959a84eac3a85cb6c07e3717d9 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 20 Nov 2015 00:19:02 +0900 Subject: [PATCH 10/62] Optimize the case when only weight is changed --- lib/nghttp2_session.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index de65867b..411fa7ab 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -660,6 +660,11 @@ int nghttp2_session_reprioritize_stream( } } + if (dep_stream == stream->dep_prev && !pri_spec->exclusive) { + stream->weight = pri_spec->weight; + return 0; + } + nghttp2_stream_dep_remove_subtree(stream); /* We have to update weight after removing stream from tree */ From 4db3828fa60d6d0d4114a04ab27f7824b1f89de9 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 20 Nov 2015 00:19:17 +0900 Subject: [PATCH 11/62] Reset last_writelen to 0 when stream is removed from tree When stream is removed from tree, stream is either closed, or its remote flow control window is depleted. In the latter case, we schedule this stream as fast as possible if its remote window gets positive, since it did not sent anything in its turn. To achieve this, reset last_writelen to 0 when stream is removed from tree. --- lib/nghttp2_stream.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/nghttp2_stream.c b/lib/nghttp2_stream.c index 1c67fe40..b0cd4652 100644 --- a/lib/nghttp2_stream.c +++ b/lib/nghttp2_stream.c @@ -166,6 +166,7 @@ static void stream_obq_remove(nghttp2_stream *stream) { stream->queued = 0; stream->cycle = 0; stream->descendant_last_cycle = 0; + stream->last_writelen = 0; if (stream_subtree_active(dep_stream)) { return; From d7b0768ab88ac6b1da80c338fa173f9f13fee40e Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 20 Nov 2015 21:24:54 +0900 Subject: [PATCH 12/62] Fix bug that dep_stream->sum_dep_weight was not updated --- lib/nghttp2_session.c | 1 + tests/nghttp2_session_test.c | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 411fa7ab..da678409 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -661,6 +661,7 @@ int nghttp2_session_reprioritize_stream( } if (dep_stream == stream->dep_prev && !pri_spec->exclusive) { + dep_stream->sum_dep_weight += pri_spec->weight - stream->weight; stream->weight = pri_spec->weight; return 0; } diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 149dd81b..32e27a0b 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -3447,6 +3447,15 @@ void test_nghttp2_session_reprioritize_stream(void) { CU_ASSERT(128 == stream->weight); CU_ASSERT(dep_stream == stream->dep_prev); + /* Change weight again to test short-path case */ + pri_spec.weight = 100; + + nghttp2_session_reprioritize_stream(session, stream, &pri_spec); + + CU_ASSERT(100 == stream->weight); + CU_ASSERT(dep_stream == stream->dep_prev); + CU_ASSERT(100 == dep_stream->sum_dep_weight); + /* Test circular dependency; stream 1 is first removed and becomes root. Then stream 3 depends on it. */ nghttp2_priority_spec_init(&pri_spec, 1, 1, 0); From 63aa3466e9d69d81ebb36cf3cf4d58ffeb82a4a8 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 21 Nov 2015 12:54:04 +0900 Subject: [PATCH 13/62] nghttpd: Fix crash with CONNECT request --- src/HttpServer.cc | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/HttpServer.cc b/src/HttpServer.cc index 6d22248e..e201fd0a 100644 --- a/src/HttpServer.cc +++ b/src/HttpServer.cc @@ -1070,8 +1070,13 @@ namespace { void prepare_response(Stream *stream, Http2Handler *hd, bool allow_push = true) { int rv; - auto reqpath = - http2::get_header(stream->hdidx, http2::HD__PATH, stream->headers)->value; + auto pathhdr = + http2::get_header(stream->hdidx, http2::HD__PATH, stream->headers); + if (!pathhdr) { + prepare_status_response(stream, hd, 405); + return; + } + auto reqpath = pathhdr->value; auto ims = get_header(stream->hdidx, http2::HD_IF_MODIFIED_SINCE, stream->headers); @@ -1715,6 +1720,7 @@ enum { IDX_301, IDX_400, IDX_404, + IDX_405, }; HttpServer::HttpServer(const Config *config) : config_(config) { @@ -1723,6 +1729,7 @@ HttpServer::HttpServer(const Config *config) : config_(config) { {"301", make_status_body(301, config_->port)}, {"400", make_status_body(400, config_->port)}, {"404", make_status_body(404, config_->port)}, + {"405", make_status_body(405, config_->port)}, }; } @@ -1975,6 +1982,8 @@ const StatusPage *HttpServer::get_status_page(int status) const { return &status_pages_[IDX_400]; case 404: return &status_pages_[IDX_404]; + case 405: + return &status_pages_[IDX_405]; default: assert(0); } From 63a50b1ccf649b60b681f692b6c6d867990505c5 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 21 Nov 2015 15:07:55 +0900 Subject: [PATCH 14/62] Add nghttp2_session_check_server_session() API This is very simple API, and it returns nonzero if session is initialized as server. --- lib/includes/nghttp2/nghttp2.h | 8 ++++++++ lib/nghttp2_session.c | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 7515555c..d4ba7e3d 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -3616,6 +3616,14 @@ nghttp2_session_get_last_proc_stream_id(nghttp2_session *session); NGHTTP2_EXTERN int nghttp2_session_check_request_allowed(nghttp2_session *session); +/** + * @function + * + * Returns nonzero if |session| is initialized as server side session. + */ +NGHTTP2_EXTERN int +nghttp2_session_check_server_session(nghttp2_session *session); + /** * @function * diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index da678409..64e39da5 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -6767,3 +6767,7 @@ nghttp2_stream *nghttp2_session_find_stream(nghttp2_session *session, nghttp2_stream *nghttp2_session_get_root_stream(nghttp2_session *session) { return &session->root; } + +int nghttp2_session_check_server_session(nghttp2_session *session) { + return session->server; +} From 8481bc6e93990aea756cccd031f6f10694bea0a1 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 21 Nov 2015 15:09:06 +0900 Subject: [PATCH 15/62] python: Send RST_STREAM if remote side is not closed and response finished --- python/cnghttp2.pxd | 12 ++++++++++++ python/nghttp2.pyx | 6 ++++++ 2 files changed, 18 insertions(+) diff --git a/python/cnghttp2.pxd b/python/cnghttp2.pxd index 4a7e1100..d6ddc950 100644 --- a/python/cnghttp2.pxd +++ b/python/cnghttp2.pxd @@ -288,6 +288,18 @@ cdef extern from 'nghttp2/nghttp2.h': const char* nghttp2_strerror(int lib_error_code) + int nghttp2_session_check_server_session(nghttp2_session *session) + + ctypedef struct nghttp2_stream: + pass + + nghttp2_stream *nghttp2_session_find_stream(nghttp2_session *session, int32_t stream_id) + + ctypedef enum nghttp2_stream_proto_state: + NGHTTP2_STREAM_STATE_OPEN + + nghttp2_stream_proto_state nghttp2_stream_get_state(nghttp2_stream *stream) + int nghttp2_hd_deflate_new(nghttp2_hd_deflater **deflater_ptr, size_t deflate_hd_table_bufsize_max) diff --git a/python/nghttp2.pyx b/python/nghttp2.pyx index 7f348633..ab991d63 100644 --- a/python/nghttp2.pyx +++ b/python/nghttp2.pyx @@ -539,6 +539,12 @@ cdef ssize_t data_source_read(cnghttp2.nghttp2_session *session, if flag == DATA_EOF: data_flags[0] = cnghttp2.NGHTTP2_DATA_FLAG_EOF + if cnghttp2.nghttp2_session_check_server_session(session): + # Send RST_STREAM if remote is not closed yet + cstrm = cnghttp2.nghttp2_session_find_stream(session, stream_id) + state = cnghttp2.nghttp2_stream_get_state(cstrm) + if state == cnghttp2.NGHTTP2_STREAM_STATE_OPEN: + http2._rst_stream(stream_id) elif flag != DATA_OK: return cnghttp2.NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE From b27b002a3f0fa47a7e9a8d0dfbcd6ee8abe9b3f3 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 21 Nov 2015 15:40:05 +0900 Subject: [PATCH 16/62] python: Use nghttp2_session_get_stream_remote_close --- python/cnghttp2.pxd | 10 +--------- python/nghttp2.pyx | 7 +++---- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/python/cnghttp2.pxd b/python/cnghttp2.pxd index d6ddc950..3c829f82 100644 --- a/python/cnghttp2.pxd +++ b/python/cnghttp2.pxd @@ -290,15 +290,7 @@ cdef extern from 'nghttp2/nghttp2.h': int nghttp2_session_check_server_session(nghttp2_session *session) - ctypedef struct nghttp2_stream: - pass - - nghttp2_stream *nghttp2_session_find_stream(nghttp2_session *session, int32_t stream_id) - - ctypedef enum nghttp2_stream_proto_state: - NGHTTP2_STREAM_STATE_OPEN - - nghttp2_stream_proto_state nghttp2_stream_get_state(nghttp2_stream *stream) + int nghttp2_session_get_stream_remote_close(nghttp2_session *session, int32_t stream_id) int nghttp2_hd_deflate_new(nghttp2_hd_deflater **deflater_ptr, size_t deflate_hd_table_bufsize_max) diff --git a/python/nghttp2.pyx b/python/nghttp2.pyx index ab991d63..4ebbf178 100644 --- a/python/nghttp2.pyx +++ b/python/nghttp2.pyx @@ -541,10 +541,9 @@ cdef ssize_t data_source_read(cnghttp2.nghttp2_session *session, data_flags[0] = cnghttp2.NGHTTP2_DATA_FLAG_EOF if cnghttp2.nghttp2_session_check_server_session(session): # Send RST_STREAM if remote is not closed yet - cstrm = cnghttp2.nghttp2_session_find_stream(session, stream_id) - state = cnghttp2.nghttp2_stream_get_state(cstrm) - if state == cnghttp2.NGHTTP2_STREAM_STATE_OPEN: - http2._rst_stream(stream_id) + if cnghttp2.nghttp2_session_get_stream_remote_close( + session, stream_id) == 0: + http2._rst_stream(stream_id, cnghttp2.NGHTTP2_NO_ERROR) elif flag != DATA_OK: return cnghttp2.NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE From 3673b1303a5d187d5332aa8b84c8b8b330a08aca Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 21 Nov 2015 15:44:12 +0900 Subject: [PATCH 17/62] python: Do the same thing in 8481bc6 with HEADERS + END_STREAM flag set --- python/nghttp2.pyx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/python/nghttp2.pyx b/python/nghttp2.pyx index 4ebbf178..633cc337 100644 --- a/python/nghttp2.pyx +++ b/python/nghttp2.pyx @@ -473,6 +473,13 @@ cdef int server_on_frame_send(cnghttp2.nghttp2_session *session, if (frame.hd.flags & cnghttp2.NGHTTP2_FLAG_ACK) != 0: return 0 http2._start_settings_timer() + elif frame.hd.type == cnghttp2.NGHTTP2_HEADERS: + if (frame.hd.flags & cnghttp2.NGHTTP2_FLAG_END_STREAM) and \ + cnghttp2.nghttp2_session_check_server_session(session): + # Send RST_STREAM if remote is not closed yet + if cnghttp2.nghttp2_session_get_stream_remote_close( + session, frame.hd.stream_id) == 0: + http2._rst_stream(frame.hd.stream_id, cnghttp2.NGHTTP2_NO_ERROR) cdef int server_on_frame_not_send(cnghttp2.nghttp2_session *session, const cnghttp2.nghttp2_frame *frame, From c7304317d4c61d89162500b5dc733ed9f79a5c6d Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 20 Nov 2015 23:29:12 +0900 Subject: [PATCH 18/62] nghttpd: Check validity of cached file descriptor periodically This commit adds ability to check status of cached file descriptor to make sure that it can be reused. We inspect last modification time and number of hard links. If last modification is changed from the last validation time, or number of hard links gets 0, we don't reuse file descriptor. We also capped upper limit of the cached file descriptors. If the limit is reached, we will close file descriptor which is least recently used, and its usecount is 0. --- src/HttpServer.cc | 123 ++++++++++++++++++++++++++++++++++++---------- src/HttpServer.h | 12 +++-- 2 files changed, 106 insertions(+), 29 deletions(-) diff --git a/src/HttpServer.cc b/src/HttpServer.cc index e201fd0a..018499e1 100644 --- a/src/HttpServer.cc +++ b/src/HttpServer.cc @@ -183,6 +183,43 @@ namespace { void release_fd_cb(struct ev_loop *loop, ev_timer *w, int revents); } // namespace +namespace { +constexpr ev_tstamp FILE_ENTRY_MAX_AGE = 10.; +} // namespace + +namespace { +constexpr size_t FILE_ENTRY_EVICT_THRES = 2048; +} // namespace + +namespace { +bool need_validation_file_entry(const FileEntry *ent, ev_tstamp now) { + return ent->last_valid + FILE_ENTRY_MAX_AGE < now; +} +} // namespace + +namespace { +bool validate_file_entry(FileEntry *ent, ev_tstamp now) { + struct stat stbuf; + int rv; + + rv = fstat(ent->fd, &stbuf); + if (rv != 0) { + ent->stale = true; + return false; + } + + if (stbuf.st_nlink == 0 || ent->mtime != stbuf.st_mtime) { + ent->stale = true; + return false; + } + + ent->mtime = stbuf.st_mtime; + ent->last_valid = now; + + return true; +} +} // namespace + class Sessions { public: Sessions(HttpServer *sv, struct ev_loop *loop, const Config *config, @@ -269,13 +306,38 @@ public: return cached_date_; } FileEntry *get_cached_fd(const std::string &path) { - auto i = fd_cache_.find(path); - if (i == std::end(fd_cache_)) { + auto range = fd_cache_.equal_range(path); + if (range.first == range.second) { return nullptr; } - auto &ent = (*i).second; - ++ent.usecount; - return &ent; + + auto now = ev_now(loop_); + + for (auto it = range.first; it != range.second;) { + auto &ent = (*it).second; + if (ent.stale) { + ++it; + continue; + } + if (need_validation_file_entry(&ent, now) && + !validate_file_entry(&ent, now)) { + if (ent.usecount == 0) { + fd_cache_lru_.remove(&ent); + close(ent.fd); + it = fd_cache_.erase(it); + continue; + } + ++it; + continue; + } + + fd_cache_lru_.remove(&ent); + fd_cache_lru_.append(&ent); + + ++ent.usecount; + return &ent; + } + return nullptr; } FileEntry *cache_fd(const std::string &path, const FileEntry &ent) { #ifdef HAVE_STD_MAP_EMPLACE @@ -284,23 +346,31 @@ public: // for gcc-4.7 auto rv = fd_cache_.insert(std::make_pair(path, ent)); #endif // !HAVE_STD_MAP_EMPLACE - return &(*rv.first).second; + auto &res = (*rv).second; + res.it = rv; + fd_cache_lru_.append(&res); + + while (fd_cache_.size() > FILE_ENTRY_EVICT_THRES) { + auto ent = fd_cache_lru_.head; + if (ent->usecount) { + break; + } + fd_cache_lru_.remove(ent); + close(ent->fd); + fd_cache_.erase(ent->it); + } + + return &res; } - void release_fd(const std::string &path) { - auto i = fd_cache_.find(path); - if (i == std::end(fd_cache_)) { + void release_fd(FileEntry *target) { + --target->usecount; + + if (target->usecount == 0 && target->stale) { + fd_cache_lru_.remove(target); + close(target->fd); + fd_cache_.erase(target->it); return; } - auto &ent = (*i).second; - if (ent.usecount == 0) { - // temporary fd, close it immediately - close(ent.fd); - fd_cache_.erase(i); - - return; - } - - --ent.usecount; // We use timer to close file descriptor and delete the entry from // cache. The timer will be started when there is no handler. @@ -313,6 +383,7 @@ public: continue; } + fd_cache_lru_.remove(&ent); close(ent.fd); i = fd_cache_.erase(i); } @@ -323,7 +394,8 @@ public: private: std::set handlers_; // cache for file descriptors to read file. - std::map fd_cache_; + std::multimap fd_cache_; + DList fd_cache_lru_; HttpServer *sv_; struct ev_loop *loop_; const Config *config_; @@ -366,7 +438,7 @@ Stream::Stream(Http2Handler *handler, int32_t stream_id) Stream::~Stream() { if (file_ent != nullptr) { auto sessions = handler->get_sessions(); - sessions->release_fd(file_ent->path); + sessions->release_fd(file_ent); } auto loop = handler->get_loop(); @@ -1034,7 +1106,7 @@ bool prepare_upload_temp_store(Stream *stream, Http2Handler *hd) { // now. We will update it when we get whole request body. auto path = std::string("echo:") + tempfn; stream->file_ent = - sessions->cache_fd(path, FileEntry(path, 0, 0, fd, nullptr, 0)); + sessions->cache_fd(path, FileEntry(path, 0, 0, fd, nullptr, 0, true)); stream->echo_upload = true; return true; } @@ -1104,7 +1176,7 @@ void prepare_response(Stream *stream, Http2Handler *hd, url = util::percentDecode(std::begin(url), std::end(url)); if (!util::check_path(url)) { if (stream->file_ent) { - sessions->release_fd(stream->file_ent->path); + sessions->release_fd(stream->file_ent); stream->file_ent = nullptr; } prepare_status_response(stream, hd, 404); @@ -1186,7 +1258,8 @@ void prepare_response(Stream *stream, Http2Handler *hd, } file_ent = sessions->cache_fd( - path, FileEntry(path, buf.st_size, buf.st_mtime, file, content_type)); + path, FileEntry(path, buf.st_size, buf.st_mtime, file, content_type, + ev_now(sessions->get_loop()))); } stream->file_ent = file_ent; @@ -1710,7 +1783,7 @@ FileEntry make_status_body(int status, uint16_t port) { assert(0); } - return FileEntry(util::utos(status), nwrite, 0, fd, nullptr); + return FileEntry(util::utos(status), nwrite, 0, fd, nullptr, 0); } } // namespace diff --git a/src/HttpServer.h b/src/HttpServer.h index 5f701d2a..a81efe1d 100644 --- a/src/HttpServer.h +++ b/src/HttpServer.h @@ -83,18 +83,22 @@ struct Config { class Http2Handler; struct FileEntry { - // To close fd immediately when nothing refers to this entry, give 0 - // to |usecount|. FileEntry(std::string path, int64_t length, int64_t mtime, int fd, - const std::string *content_type, int usecount = 1) + const std::string *content_type, ev_tstamp last_valid, + bool stale = false) : path(std::move(path)), length(length), mtime(mtime), - content_type(content_type), fd(fd), usecount(usecount) {} + last_valid(last_valid), content_type(content_type), dlnext(nullptr), + dlprev(nullptr), fd(fd), usecount(1), stale(stale) {} std::string path; + std::multimap::iterator it; int64_t length; int64_t mtime; + ev_tstamp last_valid; const std::string *content_type; + FileEntry *dlnext, *dlprev; int fd; int usecount; + bool stale; }; struct Stream { From 72f2c4b2729b32cb48bdaed65df847622cb8c09f Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 21 Nov 2015 18:31:39 +0900 Subject: [PATCH 19/62] Add nghttp2_session_check_server_session.rst to APIDOCS --- doc/Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/Makefile.am b/doc/Makefile.am index 7b6caaee..4a4c37ca 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -106,6 +106,7 @@ APIDOCS= \ nghttp2_session_mem_send.rst \ nghttp2_session_recv.rst \ nghttp2_session_check_request_allowed.rst \ + nghttp2_session_check_server_session.rst \ nghttp2_session_resume_data.rst \ nghttp2_session_send.rst \ nghttp2_session_server_new.rst \ From aa317c89ea7e7d5424b70e563229d3332479aa02 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 21 Nov 2015 18:22:28 +0900 Subject: [PATCH 20/62] Add API to change stream priority without sending PRIORITY frame The added API is nghttp2_session_change_stream_priority(). This provides the same functionality to re-prioritize stream when PRIORITY frame. is received, but we do it without PRIORITY frame. This could be useful for server to change pushed stream's priority silently. --- doc/Makefile.am | 1 + lib/includes/nghttp2/nghttp2.h | 34 ++++++++++++++++++++++++++++++++++ lib/nghttp2_priority_spec.c | 8 ++++++++ lib/nghttp2_priority_spec.h | 8 ++++++++ lib/nghttp2_session.c | 21 +++++++++++++++++++++ lib/nghttp2_submit.c | 12 ++---------- 6 files changed, 74 insertions(+), 10 deletions(-) diff --git a/doc/Makefile.am b/doc/Makefile.am index 4a4c37ca..0bc4f66c 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -105,6 +105,7 @@ APIDOCS= \ nghttp2_session_mem_recv.rst \ nghttp2_session_mem_send.rst \ nghttp2_session_recv.rst \ + nghttp2_session_change_stream_priority.rst \ nghttp2_session_check_request_allowed.rst \ nghttp2_session_check_server_session.rst \ nghttp2_session_resume_data.rst \ diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index d4ba7e3d..94b07753 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -2853,6 +2853,40 @@ NGHTTP2_EXTERN int nghttp2_session_consume_stream(nghttp2_session *session, int32_t stream_id, size_t size); +/** + * @function + * + * Changes priority of existing stream denoted by |stream_id|. The + * new priority specification is |pri_spec|. + * + * The priority is changed silently and instantly, and no PRIORITY + * frame will be sent to notify the peer of this change. This + * function may be useful for server to change the priority of pushed + * stream. + * + * If |session| is initialized as server, and ``pri_spec->stream_id`` + * points to the idle stream, the idle stream is created if it does + * not exist. The created idle stream will depend on root stream + * (stream 0) with weight 16. + * + * If stream denoted by ``pri_spec->stream_id`` is not found, we use + * default priority instead of given |pri_spec|. That is make stream + * depend on root stream with weight 16. + * + * 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_INVALID_ARGUMENT` + * Attempted to depend on itself; no stream exist for the given + * |stream_id|. + */ +NGHTTP2_EXTERN int +nghttp2_session_change_stream_priority(nghttp2_session *session, + int32_t stream_id, + const nghttp2_priority_spec *pri_spec); + /** * @function * diff --git a/lib/nghttp2_priority_spec.c b/lib/nghttp2_priority_spec.c index cd254b1f..c2196e30 100644 --- a/lib/nghttp2_priority_spec.c +++ b/lib/nghttp2_priority_spec.c @@ -42,3 +42,11 @@ int nghttp2_priority_spec_check_default(const nghttp2_priority_spec *pri_spec) { return pri_spec->stream_id == 0 && pri_spec->weight == NGHTTP2_DEFAULT_WEIGHT && pri_spec->exclusive == 0; } + +void nghttp2_priority_spec_normalize_weight(nghttp2_priority_spec *pri_spec) { + if (pri_spec->weight < NGHTTP2_MIN_WEIGHT) { + pri_spec->weight = NGHTTP2_MIN_WEIGHT; + } else if (pri_spec->weight > NGHTTP2_MAX_WEIGHT) { + pri_spec->weight = NGHTTP2_MAX_WEIGHT; + } +} diff --git a/lib/nghttp2_priority_spec.h b/lib/nghttp2_priority_spec.h index a325a581..98fac210 100644 --- a/lib/nghttp2_priority_spec.h +++ b/lib/nghttp2_priority_spec.h @@ -31,4 +31,12 @@ #include +/* + * This function normalizes pri_spec->weight if it is out of range. + * If pri_spec->weight is less than NGHTTP2_MIN_WEIGHT, it is set to + * NGHTTP2_MIN_WEIGHT. If pri_spec->weight is larger than + * NGHTTP2_MAX_WEIGHT, it is set to NGHTTP2_MAX_WEIGHT. + */ +void nghttp2_priority_spec_normalize_weight(nghttp2_priority_spec *pri_spec); + #endif /* NGHTTP2_PRIORITY_SPEC_H */ diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 64e39da5..6e815ca2 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -6771,3 +6771,24 @@ nghttp2_stream *nghttp2_session_get_root_stream(nghttp2_session *session) { int nghttp2_session_check_server_session(nghttp2_session *session) { return session->server; } + +int nghttp2_session_change_stream_priority( + nghttp2_session *session, int32_t stream_id, + const nghttp2_priority_spec *pri_spec) { + nghttp2_stream *stream; + nghttp2_priority_spec pri_spec_copy; + + if (stream_id == pri_spec->stream_id) { + return NGHTTP2_ERR_INVALID_ARGUMENT; + } + + stream = nghttp2_session_get_stream_raw(session, stream_id); + if (!stream) { + return NGHTTP2_ERR_INVALID_ARGUMENT; + } + + pri_spec_copy = *pri_spec; + nghttp2_priority_spec_normalize_weight(&pri_spec_copy); + + return nghttp2_session_reprioritize_stream(session, stream, &pri_spec_copy); +} diff --git a/lib/nghttp2_submit.c b/lib/nghttp2_submit.c index 763b4038..65226363 100644 --- a/lib/nghttp2_submit.c +++ b/lib/nghttp2_submit.c @@ -117,14 +117,6 @@ fail2: return rv; } -static void adjust_priority_spec_weight(nghttp2_priority_spec *pri_spec) { - if (pri_spec->weight < NGHTTP2_MIN_WEIGHT) { - pri_spec->weight = NGHTTP2_MIN_WEIGHT; - } else if (pri_spec->weight > NGHTTP2_MAX_WEIGHT) { - pri_spec->weight = NGHTTP2_MAX_WEIGHT; - } -} - static int32_t submit_headers_shared_nva(nghttp2_session *session, uint8_t flags, int32_t stream_id, const nghttp2_priority_spec *pri_spec, @@ -141,7 +133,7 @@ static int32_t submit_headers_shared_nva(nghttp2_session *session, if (pri_spec) { copy_pri_spec = *pri_spec; - adjust_priority_spec_weight(©_pri_spec); + nghttp2_priority_spec_normalize_weight(©_pri_spec); } else { nghttp2_priority_spec_default_init(©_pri_spec); } @@ -206,7 +198,7 @@ int nghttp2_submit_priority(nghttp2_session *session, uint8_t flags _U_, copy_pri_spec = *pri_spec; - adjust_priority_spec_weight(©_pri_spec); + nghttp2_priority_spec_normalize_weight(©_pri_spec); item = nghttp2_mem_malloc(mem, sizeof(nghttp2_outbound_item)); From dff2a1995063b29f3338a0be8cfd4bb11609a1a3 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 22 Nov 2015 15:23:18 +0900 Subject: [PATCH 21/62] nghttpx: Don't send RST_STREAM to h2 backend if backend is disconnected state This avoid establishing HTTP/2 backend connection again w/o pending request. See GH-431 --- src/shrpx_http2_downstream_connection.cc | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/shrpx_http2_downstream_connection.cc b/src/shrpx_http2_downstream_connection.cc index 8a9a216f..d5674f73 100644 --- a/src/shrpx_http2_downstream_connection.cc +++ b/src/shrpx_http2_downstream_connection.cc @@ -67,14 +67,8 @@ Http2DownstreamConnection::~Http2DownstreamConnection() { error_code = NGHTTP2_INTERNAL_ERROR; } - if (downstream_->get_downstream_stream_id() != -1) { - if (LOG_ENABLED(INFO)) { - DCLOG(INFO, this) << "Submit RST_STREAM for DOWNSTREAM:" << downstream_ - << ", stream_id=" - << downstream_->get_downstream_stream_id() - << ", error_code=" << error_code; - } - + if (http2session_->get_state() == Http2Session::CONNECTED && + downstream_->get_downstream_stream_id() != -1) { submit_rst_stream(downstream_, error_code); http2session_->consume(downstream_->get_downstream_stream_id(), @@ -143,7 +137,8 @@ int Http2DownstreamConnection::submit_rst_stream(Downstream *downstream, if (LOG_ENABLED(INFO)) { DCLOG(INFO, this) << "Submit RST_STREAM for DOWNSTREAM:" << downstream << ", stream_id=" - << downstream->get_downstream_stream_id(); + << downstream->get_downstream_stream_id() + << ", error_code=" << error_code; } rv = http2session_->submit_rst_stream( downstream->get_downstream_stream_id(), error_code); From e95ad297ed7be75ced9bd37804293334c36fbfb0 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 23 Nov 2015 14:58:17 +0900 Subject: [PATCH 22/62] travis: Enable integration tests and neverbleed We added spdylay build since integration tests require it. --- .travis.yml | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1dee67b8..7239bd90 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,12 +27,26 @@ before_install: - $CC --version - if [ "$CXX" = "g++" ]; then export CXX="g++-4.9" CC="gcc-4.9"; fi - $CC --version + - go version before_script: + # First build spdylay, since integration tests require it. + # spdylay is going to be built under third-party/spdylay + - cd third-party + - git clone https://github.com/tatsuhiro-t/spdylay.git + - cd spdylay + - autoreconf -i + - ./configure --disable-src --disable-examples + - make check + - export SPDYLAY_HOME=$PWD + - cd ../.. + # Now build nghttp2 - autoreconf -i - - automake - - autoconf - git submodule update --init - - ./configure --enable-werror --with-mruby + - ./configure --enable-werror --with-mruby --with-neverbleed LIBSPDYLAY_CFLAGS="-I$SPDYLAY_HOME/lib/includes" LIBSPDYLAY_LIBS="-L$SPDYLAY_HOME/lib/.libs -lspdylay" script: - make - make check + - cd integration-tests + - export GOPATH="$PWD/integration-tests/golang" + - make itprep-local + - make it-local From 718f7a5f7e1c0772df75974ebe7f1d18ba16bedb Mon Sep 17 00:00:00 2001 From: Kit Chan Date: Mon, 23 Nov 2015 09:58:38 +0000 Subject: [PATCH 23/62] h2load goes into infinite loop when timing script file starts with 0.0 in first line --- src/h2load.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/h2load.cc b/src/h2load.cc index 9e9c9bcb..32ea3704 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -742,10 +742,14 @@ int Client::connection_made() { break; } duration = config.timings[reqidx]; + if(reqidx == 0) //if reqidx wraps around back to 0, we uses up all lines and should break + break; } - request_timeout_watcher.repeat = duration; - ev_timer_again(worker->loop, &request_timeout_watcher); + if (duration >= 1e-9) { //double check since we may have break due to reqidx wraps around back to 0 + request_timeout_watcher.repeat = duration; + ev_timer_again(worker->loop, &request_timeout_watcher); + } } signal_write(); From d62bc26b6212cf1b359b91bd1eb87831527db0f9 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 23 Nov 2015 19:48:51 +0900 Subject: [PATCH 24/62] Add test for nghttp2_session_change_stream_priority() --- tests/main.c | 2 ++ tests/nghttp2_session_test.c | 35 +++++++++++++++++++++++++++++++++++ tests/nghttp2_session_test.h | 1 + 3 files changed, 38 insertions(+) diff --git a/tests/main.c b/tests/main.c index 14b43dbf..9e0ab033 100644 --- a/tests/main.c +++ b/tests/main.c @@ -282,6 +282,8 @@ int main(int argc _U_, char *argv[] _U_) { !CU_add_test(pSuite, "session_detach_item_from_closed_stream", test_nghttp2_session_detach_item_from_closed_stream) || !CU_add_test(pSuite, "session_flooding", test_nghttp2_session_flooding) || + !CU_add_test(pSuite, "session_change_stream_priority", + test_nghttp2_session_change_stream_priority) || !CU_add_test(pSuite, "http_mandatory_headers", test_nghttp2_http_mandatory_headers) || !CU_add_test(pSuite, "http_content_length", diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 32e27a0b..ee17e9e1 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -8319,6 +8319,41 @@ void test_nghttp2_session_flooding(void) { nghttp2_bufs_free(&bufs); } +void test_nghttp2_session_change_stream_priority(void) { + nghttp2_session *session; + nghttp2_session_callbacks callbacks; + nghttp2_stream *stream1, *stream2, *stream3; + nghttp2_priority_spec pri_spec; + int rv; + + memset(&callbacks, 0, sizeof(callbacks)); + + nghttp2_session_server_new(&session, &callbacks, NULL); + + stream1 = open_stream(session, 1); + stream3 = open_stream_with_dep_weight(session, 3, 199, stream1); + stream2 = open_stream_with_dep_weight(session, 2, 101, stream3); + + nghttp2_priority_spec_init(&pri_spec, 1, 256, 0); + + rv = nghttp2_session_change_stream_priority(session, 2, &pri_spec); + + CU_ASSERT(0 == rv); + + CU_ASSERT(stream1 == stream2->dep_prev); + CU_ASSERT(256 == stream2->weight); + + /* Cannot change stream which does not exist */ + rv = nghttp2_session_change_stream_priority(session, 5, &pri_spec); + CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == rv); + + /* It is an error to depend on itself */ + rv = nghttp2_session_change_stream_priority(session, 1, &pri_spec); + CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == rv); + + nghttp2_session_del(session); +} + static void check_nghttp2_http_recv_headers_fail( nghttp2_session *session, nghttp2_hd_deflater *deflater, int32_t stream_id, int stream_state, const nghttp2_nv *nva, size_t nvlen) { diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h index ce3954b3..3c14eb3c 100644 --- a/tests/nghttp2_session_test.h +++ b/tests/nghttp2_session_test.h @@ -133,6 +133,7 @@ void test_nghttp2_session_on_begin_headers_temporal_failure(void); void test_nghttp2_session_defer_then_close(void); void test_nghttp2_session_detach_item_from_closed_stream(void); void test_nghttp2_session_flooding(void); +void test_nghttp2_session_change_stream_priority(void); void test_nghttp2_http_mandatory_headers(void); void test_nghttp2_http_content_length(void); void test_nghttp2_http_content_length_mismatch(void); From 84e23bff10e03fffcfac112d2e8092efadbbdfa2 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 23 Nov 2015 20:05:40 +0900 Subject: [PATCH 25/62] h2load: clang-format --- src/h2load.cc | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/h2load.cc b/src/h2load.cc index 32ea3704..13fe4cbd 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -742,11 +742,16 @@ int Client::connection_made() { break; } duration = config.timings[reqidx]; - if(reqidx == 0) //if reqidx wraps around back to 0, we uses up all lines and should break + if (reqidx == 0) { + // if reqidx wraps around back to 0, we uses up all lines and + // should break break; + } } - if (duration >= 1e-9) { //double check since we may have break due to reqidx wraps around back to 0 + if (duration >= 1e-9) { + // double check since we may have break due to reqidx wraps + // around back to 0 request_timeout_watcher.repeat = duration; ev_timer_again(worker->loop, &request_timeout_watcher); } From 8f970dec0ec75bb0ca23261c479dc54f4d61d3a0 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 23 Nov 2015 21:05:25 +0900 Subject: [PATCH 26/62] Update doc --- lib/nghttp2_session.c | 7 +++++++ lib/nghttp2_stream.c | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 6e815ca2..d969d9fd 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -661,6 +661,13 @@ int nghttp2_session_reprioritize_stream( } if (dep_stream == stream->dep_prev && !pri_spec->exclusive) { + /* This is minor optimization when just weight is changed. + Currently, we don't reschedule stream in this case, since we + don't retain enough information to do that + (descendant_last_cycle we used to schedule it). This means new + weight is only applied in the next scheduling, and if weight is + drastically increased, library is not responding very quickly. + If this is really an issue, we will do workaround for this. */ dep_stream->sum_dep_weight += pri_spec->weight - stream->weight; stream->weight = pri_spec->weight; return 0; diff --git a/lib/nghttp2_stream.c b/lib/nghttp2_stream.c index b0cd4652..f83265b7 100644 --- a/lib/nghttp2_stream.c +++ b/lib/nghttp2_stream.c @@ -207,6 +207,11 @@ void nghttp2_stream_reschedule(nghttp2_stream *stream) { dep_stream->descendant_last_cycle = 0; stream->cycle = 0; } else { + /* We update descendant_last_cycle here, and we don't do it when + no data is written for stream. This effectively means that + we treat these streams as if they are not scheduled at all. + This does not cause disruption in scheduling machinery. It + just makes new streams scheduled a bit early. */ dep_stream->descendant_last_cycle = stream->cycle; stream->cycle = From c44ee44cc3047812b8811d5dcea9f2021781f579 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 23 Nov 2015 21:25:12 +0900 Subject: [PATCH 27/62] nghttpd: Fix compile error due to incomplete FileEntry --- src/HttpServer.cc | 39 ++++++++++++++++++++------------------- src/HttpServer.h | 2 +- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/HttpServer.cc b/src/HttpServer.cc index 018499e1..ded5b6fd 100644 --- a/src/HttpServer.cc +++ b/src/HttpServer.cc @@ -315,15 +315,15 @@ public: for (auto it = range.first; it != range.second;) { auto &ent = (*it).second; - if (ent.stale) { + if (ent->stale) { ++it; continue; } - if (need_validation_file_entry(&ent, now) && - !validate_file_entry(&ent, now)) { - if (ent.usecount == 0) { - fd_cache_lru_.remove(&ent); - close(ent.fd); + if (need_validation_file_entry(ent.get(), now) && + !validate_file_entry(ent.get(), now)) { + if (ent->usecount == 0) { + fd_cache_lru_.remove(ent.get()); + close(ent->fd); it = fd_cache_.erase(it); continue; } @@ -331,24 +331,25 @@ public: continue; } - fd_cache_lru_.remove(&ent); - fd_cache_lru_.append(&ent); + fd_cache_lru_.remove(ent.get()); + fd_cache_lru_.append(ent.get()); - ++ent.usecount; - return &ent; + ++ent->usecount; + return ent.get(); } return nullptr; } FileEntry *cache_fd(const std::string &path, const FileEntry &ent) { #ifdef HAVE_STD_MAP_EMPLACE - auto rv = fd_cache_.emplace(path, ent); + auto rv = fd_cache_.emplace(path, make_unique(ent)); #else // !HAVE_STD_MAP_EMPLACE // for gcc-4.7 - auto rv = fd_cache_.insert(std::make_pair(path, ent)); + auto rv = + fd_cache_.insert(std::make_pair(path, make_unique(ent))); #endif // !HAVE_STD_MAP_EMPLACE auto &res = (*rv).second; - res.it = rv; - fd_cache_lru_.append(&res); + res->it = rv; + fd_cache_lru_.append(res.get()); while (fd_cache_.size() > FILE_ENTRY_EVICT_THRES) { auto ent = fd_cache_lru_.head; @@ -360,7 +361,7 @@ public: fd_cache_.erase(ent->it); } - return &res; + return res.get(); } void release_fd(FileEntry *target) { --target->usecount; @@ -378,13 +379,13 @@ public: void release_unused_fd() { for (auto i = std::begin(fd_cache_); i != std::end(fd_cache_);) { auto &ent = (*i).second; - if (ent.usecount != 0) { + if (ent->usecount != 0) { ++i; continue; } - fd_cache_lru_.remove(&ent); - close(ent.fd); + fd_cache_lru_.remove(ent.get()); + close(ent->fd); i = fd_cache_.erase(i); } } @@ -394,7 +395,7 @@ public: private: std::set handlers_; // cache for file descriptors to read file. - std::multimap fd_cache_; + std::multimap> fd_cache_; DList fd_cache_lru_; HttpServer *sv_; struct ev_loop *loop_; diff --git a/src/HttpServer.h b/src/HttpServer.h index a81efe1d..3c1dc73e 100644 --- a/src/HttpServer.h +++ b/src/HttpServer.h @@ -90,7 +90,7 @@ struct FileEntry { last_valid(last_valid), content_type(content_type), dlnext(nullptr), dlprev(nullptr), fd(fd), usecount(1), stale(stale) {} std::string path; - std::multimap::iterator it; + std::multimap>::iterator it; int64_t length; int64_t mtime; ev_tstamp last_valid; From b53b1381b7679276432cd14a21e4dddfef066b7d Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 24 Nov 2015 22:30:12 +0900 Subject: [PATCH 28/62] Fix bug that nghttp2_session_find_stream(session, 0) returned NULL Previously, nghttp2_session_find_stream(session, 0) returned NULL despite the fact that documentation said that it should return root stream. Now it is corrected, and it returns root stream as documented. --- lib/nghttp2_session.c | 4 ++++ tests/main.c | 2 ++ tests/nghttp2_session_test.c | 28 ++++++++++++++++++++++++++++ tests/nghttp2_session_test.h | 1 + 4 files changed, 35 insertions(+) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index d969d9fd..7f1e883e 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -6768,6 +6768,10 @@ int32_t nghttp2_session_get_last_proc_stream_id(nghttp2_session *session) { nghttp2_stream *nghttp2_session_find_stream(nghttp2_session *session, int32_t stream_id) { + if (stream_id == 0) { + return &session->root; + } + return nghttp2_session_get_stream_raw(session, stream_id); } diff --git a/tests/main.c b/tests/main.c index 9e0ab033..004dd293 100644 --- a/tests/main.c +++ b/tests/main.c @@ -251,6 +251,8 @@ int main(int argc _U_, char *argv[] _U_) { test_nghttp2_session_stream_get_state) || !CU_add_test(pSuite, "session_stream_get_something", test_nghttp2_session_stream_get_something) || + !CU_add_test(pSuite, "session_find_stream", + test_nghttp2_session_find_stream) || !CU_add_test(pSuite, "session_keep_closed_stream", test_nghttp2_session_keep_closed_stream) || !CU_add_test(pSuite, "session_keep_idle_stream", diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index ee17e9e1..3b85e743 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -7439,6 +7439,34 @@ void test_nghttp2_session_stream_get_something(void) { nghttp2_session_del(session); } +void test_nghttp2_session_find_stream(void) { + nghttp2_session *session; + nghttp2_session_callbacks callbacks; + nghttp2_stream *stream; + + memset(&callbacks, 0, sizeof(callbacks)); + + nghttp2_session_server_new(&session, &callbacks, NULL); + + open_stream(session, 1); + + stream = nghttp2_session_find_stream(session, 1); + + CU_ASSERT(NULL != stream); + CU_ASSERT(1 == stream->stream_id); + + stream = nghttp2_session_find_stream(session, 0); + + CU_ASSERT(&session->root == stream); + CU_ASSERT(0 == stream->stream_id); + + stream = nghttp2_session_find_stream(session, 2); + + CU_ASSERT(NULL == stream); + + nghttp2_session_del(session); +} + void test_nghttp2_session_keep_closed_stream(void) { nghttp2_session *session; nghttp2_session_callbacks callbacks; diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h index 3c14eb3c..f7421180 100644 --- a/tests/nghttp2_session_test.h +++ b/tests/nghttp2_session_test.h @@ -117,6 +117,7 @@ void test_nghttp2_session_stream_attach_item(void); void test_nghttp2_session_stream_attach_item_subtree(void); void test_nghttp2_session_stream_get_state(void); void test_nghttp2_session_stream_get_something(void); +void test_nghttp2_session_find_stream(void); void test_nghttp2_session_keep_closed_stream(void); void test_nghttp2_session_keep_idle_stream(void); void test_nghttp2_session_detach_idle_stream(void); From b08d5b1975656f705585015cdf0ad72d06b73f56 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 24 Nov 2015 22:33:06 +0900 Subject: [PATCH 29/62] Explicitly treat stream_id 0 as error in nghttp2_session_change_stream_priority --- lib/includes/nghttp2/nghttp2.h | 4 ++-- lib/nghttp2_session.c | 2 +- tests/nghttp2_session_test.c | 4 ++++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 94b07753..47ca79a5 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -2879,8 +2879,8 @@ NGHTTP2_EXTERN int nghttp2_session_consume_stream(nghttp2_session *session, * :enum:`NGHTTP2_ERR_NOMEM` * Out of memory. * :enum:`NGHTTP2_ERR_INVALID_ARGUMENT` - * Attempted to depend on itself; no stream exist for the given - * |stream_id|. + * Attempted to depend on itself; or no stream exist for the given + * |stream_id|; or |stream_id| is 0 */ NGHTTP2_EXTERN int nghttp2_session_change_stream_priority(nghttp2_session *session, diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 7f1e883e..473b3807 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -6789,7 +6789,7 @@ int nghttp2_session_change_stream_priority( nghttp2_stream *stream; nghttp2_priority_spec pri_spec_copy; - if (stream_id == pri_spec->stream_id) { + if (stream_id == 0 || stream_id == pri_spec->stream_id) { return NGHTTP2_ERR_INVALID_ARGUMENT; } diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 3b85e743..45f61d33 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -8379,6 +8379,10 @@ void test_nghttp2_session_change_stream_priority(void) { rv = nghttp2_session_change_stream_priority(session, 1, &pri_spec); CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == rv); + /* It is an error to change priority of root stream (0) */ + rv = nghttp2_session_change_stream_priority(session, 0, &pri_spec); + CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == rv); + nghttp2_session_del(session); } From 9088e4a16a902196cf79b404015d11baa9a2caa5 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 24 Nov 2015 23:55:19 +0900 Subject: [PATCH 30/62] Bump up version number to 1.5.0-DEV --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 69c50ddd..887b173e 100644 --- a/configure.ac +++ b/configure.ac @@ -25,7 +25,7 @@ dnl Do not change user variables! dnl http://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html AC_PREREQ(2.61) -AC_INIT([nghttp2], [1.4.1-DEV], [t-tujikawa@users.sourceforge.net]) +AC_INIT([nghttp2], [1.5.0-DEV], [t-tujikawa@users.sourceforge.net]) AC_CONFIG_AUX_DIR([.]) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_HEADERS([config.h]) From 3048bb9d90c9f384100c528436579f2201652451 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 25 Nov 2015 22:18:54 +0900 Subject: [PATCH 31/62] Fix test: stream_id is not incremented --- tests/nghttp2_session_test.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 45f61d33..da4d3af4 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -7657,13 +7657,12 @@ void test_nghttp2_session_large_dep_tree(void) { nghttp2_session_server_new(&session, &callbacks, NULL); stream_id = 1; - for (i = 0; i < 250; ++i) { + for (i = 0; i < 250; ++i, stream_id += 2) { dep_stream = open_stream_with_dep(session, stream_id, dep_stream); - stream_id += 2; } stream_id = 1; - for (i = 0; i < 250; ++i) { + for (i = 0; i < 250; ++i, stream_id += 2) { stream = nghttp2_session_get_stream(session, stream_id); CU_ASSERT(nghttp2_stream_dep_find_ancestor(stream, &session->root)); CU_ASSERT(nghttp2_stream_in_dep_tree(stream)); From faad0418681bb5d19e3fe61f509d10382c493aa5 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 25 Nov 2015 22:26:56 +0900 Subject: [PATCH 32/62] Use seq to break a tie for stream weight Because of the nature of heap based priority queue, and our compare function, streams with the same weight and same parent are handled in the reverse order they pushed to the queue. To allow stream pushed earlier to be served first, secondary key "seq" is introduced to break a tie. "seq" is monotonically increased integer per parent stream, and it is assigned to stream when it is pused to the queue, and gets incremented. --- lib/nghttp2_stream.c | 14 ++++++++++---- lib/nghttp2_stream.h | 5 +++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/nghttp2_stream.c b/lib/nghttp2_stream.c index f83265b7..54219a4d 100644 --- a/lib/nghttp2_stream.c +++ b/lib/nghttp2_stream.c @@ -30,13 +30,14 @@ #include "nghttp2_session.h" #include "nghttp2_helper.h" -static int stream_weight_less(const void *lhsx, const void *rhsx) { +static int stream_less(const void *lhsx, const void *rhsx) { const nghttp2_stream *lhs, *rhs; lhs = nghttp2_struct_of(lhsx, nghttp2_stream, pq_entry); rhs = nghttp2_struct_of(rhsx, nghttp2_stream, pq_entry); - return lhs->cycle < rhs->cycle; + return lhs->cycle < rhs->cycle || + (lhs->cycle == rhs->cycle && lhs->seq < rhs->seq); } void nghttp2_stream_init(nghttp2_stream *stream, int32_t stream_id, @@ -45,7 +46,7 @@ void nghttp2_stream_init(nghttp2_stream *stream, int32_t stream_id, int32_t local_initial_window_size, void *stream_user_data, nghttp2_mem *mem) { nghttp2_map_entry_init(&stream->map_entry, (key_type)stream_id); - nghttp2_pq_init(&stream->obq, stream_weight_less, mem); + nghttp2_pq_init(&stream->obq, stream_less, mem); stream->stream_id = stream_id; stream->flags = flags; @@ -79,6 +80,8 @@ void nghttp2_stream_init(nghttp2_stream *stream, int32_t stream_id, stream->queued = 0; stream->descendant_last_cycle = 0; stream->cycle = 0; + stream->descendant_next_seq = 0; + stream->seq = 0; stream->last_writelen = 0; } @@ -124,6 +127,7 @@ static int stream_obq_push(nghttp2_stream *dep_stream, nghttp2_stream *stream) { stream = dep_stream, dep_stream = dep_stream->dep_prev) { stream->cycle = stream_next_cycle(stream, dep_stream->descendant_last_cycle); + stream->seq = dep_stream->descendant_next_seq++; DEBUGF(fprintf(stderr, "stream: stream=%d obq push cycle=%ld\n", stream->stream_id, stream->cycle)); @@ -214,10 +218,12 @@ void nghttp2_stream_reschedule(nghttp2_stream *stream) { just makes new streams scheduled a bit early. */ dep_stream->descendant_last_cycle = stream->cycle; + nghttp2_pq_remove(&dep_stream->obq, &stream->pq_entry); + stream->cycle = stream_next_cycle(stream, dep_stream->descendant_last_cycle); + stream->seq = dep_stream->descendant_next_seq++; - nghttp2_pq_remove(&dep_stream->obq, &stream->pq_entry); nghttp2_pq_push(&dep_stream->obq, &stream->pq_entry); } diff --git a/lib/nghttp2_stream.h b/lib/nghttp2_stream.h index f73febbb..3417daeb 100644 --- a/lib/nghttp2_stream.h +++ b/lib/nghttp2_stream.h @@ -206,6 +206,11 @@ struct nghttp2_stream { uint64_t descendant_last_cycle; /* Next scheduled time to sent item */ uint64_t cycle; + /* Next seq used for direct descendant streams */ + uint64_t descendant_next_seq; + /* Secondary key for prioritization to break a tie for cycle. This + value is monotonically increased for single parent stream. */ + uint64_t seq; /* Last written length of frame payload */ size_t last_writelen; /* This flag is used to reduce excessive queuing of WINDOW_UPDATE to From 8e06e3737506dbcd680e0f18824057e9636c7b58 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 26 Nov 2015 21:26:00 +0900 Subject: [PATCH 33/62] h2load: Fix crash when dealing connection: close with HTTP/1.1 server --- src/h2load.cc | 20 ++++++++++++++++---- src/h2load_http1_session.cc | 10 +++++++++- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/h2load.cc b/src/h2load.cc index 13fe4cbd..30d53685 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -360,12 +360,23 @@ void Client::fail() { if (new_connection_requested) { new_connection_requested = false; + if (req_started < req_todo) { + // At the moment, we don't have a facility to re-start request + // already in in-flight. Make them fail. + auto req_abandoned = req_started - req_done; - // Keep using current address - if (connect() == 0) { - return; + worker->stats.req_failed += req_abandoned; + worker->stats.req_error += req_abandoned; + worker->stats.req_done += req_abandoned; + + req_done = req_started; + + // Keep using current address + if (connect() == 0) { + return; + } + std::cerr << "client could not connect to host" << std::endl; } - std::cerr << "client could not connect to host" << std::endl; } process_abandoned_streams(); @@ -377,6 +388,7 @@ void Client::disconnect() { ev_timer_stop(worker->loop, &request_timeout_watcher); streams.clear(); session.reset(); + wb.reset(); state = CLIENT_IDLE; ev_io_stop(worker->loop, &wev); ev_io_stop(worker->loop, &rev); diff --git a/src/h2load_http1_session.cc b/src/h2load_http1_session.cc index 728233d3..0825540d 100644 --- a/src/h2load_http1_session.cc +++ b/src/h2load_http1_session.cc @@ -51,7 +51,15 @@ Http1Session::~Http1Session() {} namespace { // HTTP response message begin -int htp_msg_begincb(http_parser *htp) { return 0; } +int htp_msg_begincb(http_parser *htp) { + auto session = static_cast(htp->data); + + if (session->stream_resp_counter_ >= session->stream_req_counter_) { + return -1; + } + + return 0; +} } // namespace namespace { From bd041bcbb0c349d550da577fa837ebf2c70f67f2 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 26 Nov 2015 21:33:27 +0900 Subject: [PATCH 34/62] h2load: Add --h1 option to force http/1.1 for both http and https URI --- src/h2load.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/h2load.cc b/src/h2load.cc index 30d53685..26d75ac6 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -1490,6 +1490,9 @@ Options: only and any white spaces are treated as a part of protocol string. Default: )" << DEFAULT_NPN_LIST << R"( + --h1 Short hand for --npn-list=http/1.1 + --no-tls-proto=http/1.1, which effectively force + http/1.1 for both http and https URI. -v, --verbose Output debug information. --version Display version information and exit. @@ -1537,6 +1540,7 @@ int main(int argc, char **argv) { {"base-uri", required_argument, nullptr, 'B'}, {"npn-list", required_argument, &flag, 4}, {"rate-period", required_argument, &flag, 5}, + {"h1", no_argument, &flag, 6}, {nullptr, 0, nullptr, 0}}; int option_index = 0; auto c = getopt_long(argc, argv, "hvW:c:d:m:n:p:t:w:H:i:r:T:N:B:", @@ -1703,6 +1707,11 @@ int main(int argc, char **argv) { exit(EXIT_FAILURE); } break; + case 6: + // --h1 + config.npn_list = util::parse_config_str_list("http/1.1"); + config.no_tls_proto = Config::PROTO_HTTP1_1; + break; } break; default: From ba9e96b8296fbe7e980016d16f4f0cead95b81d2 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 26 Nov 2015 22:04:31 +0900 Subject: [PATCH 35/62] h2load: Avoid copy of h1 request --- src/h2load_http1_session.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/h2load_http1_session.cc b/src/h2load_http1_session.cc index 0825540d..aa43f90d 100644 --- a/src/h2load_http1_session.cc +++ b/src/h2load_http1_session.cc @@ -152,7 +152,7 @@ void Http1Session::on_connect() { client_->signal_write(); } int Http1Session::submit_request(RequestStat *req_stat) { auto config = client_->worker->config; - auto req = config->h1reqs[client_->reqidx]; + const auto &req = config->h1reqs[client_->reqidx]; client_->reqidx++; if (client_->reqidx == config->h1reqs.size()) { From 1e74ee2d548bc64c50223cc9b438679fb2e959db Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 26 Nov 2015 22:21:58 +0900 Subject: [PATCH 36/62] Bump up version number to 1.5.0, LT revision to 17:0:3 --- configure.ac | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/configure.ac b/configure.ac index 887b173e..062d5cb9 100644 --- a/configure.ac +++ b/configure.ac @@ -25,7 +25,7 @@ dnl Do not change user variables! dnl http://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html AC_PREREQ(2.61) -AC_INIT([nghttp2], [1.5.0-DEV], [t-tujikawa@users.sourceforge.net]) +AC_INIT([nghttp2], [1.5.0], [t-tujikawa@users.sourceforge.net]) AC_CONFIG_AUX_DIR([.]) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_HEADERS([config.h]) @@ -46,9 +46,9 @@ m4_ifdef([AM_SILENT_RULES], [AM_SILENT_RULES([yes])]) dnl See versioning rule: dnl http://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html -AC_SUBST(LT_CURRENT, 16) +AC_SUBST(LT_CURRENT, 17) AC_SUBST(LT_REVISION, 0) -AC_SUBST(LT_AGE, 2) +AC_SUBST(LT_AGE, 3) major=`echo $PACKAGE_VERSION |cut -d. -f1 | sed -e "s/[^0-9]//g"` minor=`echo $PACKAGE_VERSION |cut -d. -f2 | sed -e "s/[^0-9]//g"` From 6444f0c5cd902a360d9d41774577b54912dcff93 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 26 Nov 2015 22:25:22 +0900 Subject: [PATCH 37/62] Update man pages --- doc/h2load.1 | 9 ++++++++- doc/h2load.1.rst | 6 ++++++ doc/nghttp.1 | 2 +- doc/nghttpd.1 | 2 +- doc/nghttpx.1 | 25 +++++++++++++++++++------ doc/nghttpx.1.rst | 23 ++++++++++++++++++----- 6 files changed, 53 insertions(+), 14 deletions(-) diff --git a/doc/h2load.1 b/doc/h2load.1 index 4e639968..797dace2 100644 --- a/doc/h2load.1 +++ b/doc/h2load.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "H2LOAD" "1" "November 12, 2015" "1.4.1-DEV" "nghttp2" +.TH "H2LOAD" "1" "November 26, 2015" "1.5.0" "nghttp2" .SH NAME h2load \- HTTP/2 benchmarking tool . @@ -227,6 +227,13 @@ Default: \fBh2,h2\-16,h2\-14,spdy/3.1,spdy/3,spdy/2,http/1.1\fP .UNINDENT .INDENT 0.0 .TP +.B \-\-h1 +Short hand for \fI\%\-\-npn\-list\fP=http/1.1 +\fI\%\-\-no\-tls\-proto\fP=http/1.1, which effectively force +http/1.1 for both http and https URI. +.UNINDENT +.INDENT 0.0 +.TP .B \-v, \-\-verbose Output debug information. .UNINDENT diff --git a/doc/h2load.1.rst b/doc/h2load.1.rst index 5a9f8321..448c4276 100644 --- a/doc/h2load.1.rst +++ b/doc/h2load.1.rst @@ -188,6 +188,12 @@ OPTIONS Default: ``h2,h2-16,h2-14,spdy/3.1,spdy/3,spdy/2,http/1.1`` +.. option:: --h1 + + Short hand for :option:`--npn-list`\=http/1.1 + :option:`--no-tls-proto`\=http/1.1, which effectively force + http/1.1 for both http and https URI. + .. option:: -v, --verbose Output debug information. diff --git a/doc/nghttp.1 b/doc/nghttp.1 index 1029cb57..a5781f59 100644 --- a/doc/nghttp.1 +++ b/doc/nghttp.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTP" "1" "November 12, 2015" "1.4.1-DEV" "nghttp2" +.TH "NGHTTP" "1" "November 26, 2015" "1.5.0" "nghttp2" .SH NAME nghttp \- HTTP/2 client . diff --git a/doc/nghttpd.1 b/doc/nghttpd.1 index 06ac32d6..99be2733 100644 --- a/doc/nghttpd.1 +++ b/doc/nghttpd.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTPD" "1" "November 12, 2015" "1.4.1-DEV" "nghttp2" +.TH "NGHTTPD" "1" "November 26, 2015" "1.5.0" "nghttp2" .SH NAME nghttpd \- HTTP/2 server . diff --git a/doc/nghttpx.1 b/doc/nghttpx.1 index 8c0a577f..9c22cd4b 100644 --- a/doc/nghttpx.1 +++ b/doc/nghttpx.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTPX" "1" "November 12, 2015" "1.4.1-DEV" "nghttp2" +.TH "NGHTTPX" "1" "November 26, 2015" "1.5.0" "nghttp2" .SH NAME nghttpx \- HTTP/2 proxy . @@ -685,9 +685,14 @@ protocol security. .INDENT 0.0 .TP .B \-\-no\-server\-push -Disable HTTP/2 server push. Server push is only -supported by default mode and HTTP/2 frontend. SPDY -frontend does not support server push. +Disable HTTP/2 server push. Server push is supported by +default mode and HTTP/2 frontend via Link header field. +It is also supported if both frontend and backend are +HTTP/2 (which implies \fI\%\-\-http2\-bridge\fP or \fI\%\-\-client\fP mode). +In this case, server push from backend session is +relayed to frontend, and server push via Link header +field is also supported. HTTP SPDY frontend does not +support server push. .UNINDENT .SS Mode .INDENT 0.0 @@ -1053,8 +1058,8 @@ than master process. .UNINDENT .SH SERVER PUSH .sp -nghttpx supports HTTP/2 server push in default mode. nghttpx looks -for Link header field (\fI\%RFC 5988\fP) in response headers from +nghttpx supports HTTP/2 server push in default mode with Link header +field. nghttpx looks for Link header field (\fI\%RFC 5988\fP) in response headers from backend server and extracts URI\-reference with parameter \fBrel=preload\fP (see \fI\%preload\fP) and pushes those URIs to the frontend client. Here is a sample Link @@ -1079,6 +1084,14 @@ associated stream\(aqs status code must be 200. .UNINDENT .sp This limitation may be loosened in the future release. +.sp +nghttpx also supports server push if both frontend and backend are +HTTP/2 (which implies \fI\%\-\-http2\-bridge\fP or \fI\%\-\-client\fP). +In this case, in addition to server push via Link header field, server +push from backend is relayed to frontend HTTP/2 session. +.sp +HTTP/2 server push will be disabled if \fI\%\-\-http2\-proxy\fP or +\fI\%\-\-client\-proxy\fP is used. .SH UNIX DOMAIN SOCKET .sp nghttpx supports UNIX domain socket with a filename for both frontend diff --git a/doc/nghttpx.1.rst b/doc/nghttpx.1.rst index 33b7b4f2..a644406c 100644 --- a/doc/nghttpx.1.rst +++ b/doc/nghttpx.1.rst @@ -613,9 +613,14 @@ HTTP/2 and SPDY .. option:: --no-server-push - Disable HTTP/2 server push. Server push is only - supported by default mode and HTTP/2 frontend. SPDY - frontend does not support server push. + Disable HTTP/2 server push. Server push is supported by + default mode and HTTP/2 frontend via Link header field. + It is also supported if both frontend and backend are + HTTP/2 (which implies :option:`--http2-bridge` or :option:`\--client` mode). + In this case, server push from backend session is + relayed to frontend, and server push via Link header + field is also supported. HTTP SPDY frontend does not + support server push. Mode @@ -954,8 +959,8 @@ SIGUSR2 SERVER PUSH ----------- -nghttpx supports HTTP/2 server push in default mode. nghttpx looks -for Link header field (`RFC 5988 +nghttpx supports HTTP/2 server push in default mode with Link header +field. nghttpx looks for Link header field (`RFC 5988 `_) in response headers from backend server and extracts URI-reference with parameter ``rel=preload`` (see `preload @@ -975,6 +980,14 @@ Currently, the following restriction is applied for server push: This limitation may be loosened in the future release. +nghttpx also supports server push if both frontend and backend are +HTTP/2 (which implies :option:`--http2-bridge` or :option:`--client`). +In this case, in addition to server push via Link header field, server +push from backend is relayed to frontend HTTP/2 session. + +HTTP/2 server push will be disabled if :option:`--http2-proxy` or +:option:`--client-proxy` is used. + UNIX DOMAIN SOCKET ------------------ From 0d52097213e634a56a1d25023ff1a52fe130c0fc Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 26 Nov 2015 22:25:34 +0900 Subject: [PATCH 38/62] Update bash_completion --- doc/bash_completion/h2load | 2 +- doc/bash_completion/nghttpd | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/bash_completion/h2load b/doc/bash_completion/h2load index f63fc4a4..9746d3a4 100644 --- a/doc/bash_completion/h2load +++ b/doc/bash_completion/h2load @@ -8,7 +8,7 @@ _h2load() _get_comp_words_by_ref cur prev case $cur in -*) - COMPREPLY=( $( compgen -W '--threads --connection-window-bits --rate-period --input-file --npn-list --help --requests --timing-script-file --data --verbose --base-uri --ciphers --connection-active-timeout --rate --connection-inactivity-timeout --window-bits --clients --no-tls-proto --version --header --max-concurrent-streams ' -- "$cur" ) ) + COMPREPLY=( $( compgen -W '--connection-window-bits --clients --verbose --ciphers --rate --no-tls-proto --requests --base-uri --h1 --threads --npn-list --rate-period --data --version --connection-inactivity-timeout --timing-script-file --max-concurrent-streams --connection-active-timeout --input-file --header --window-bits --help ' -- "$cur" ) ) ;; *) _filedir diff --git a/doc/bash_completion/nghttpd b/doc/bash_completion/nghttpd index 6de3d267..7bbef245 100644 --- a/doc/bash_completion/nghttpd +++ b/doc/bash_completion/nghttpd @@ -8,7 +8,7 @@ _nghttpd() _get_comp_words_by_ref cur prev case $cur in -*) - COMPREPLY=( $( compgen -W '--error-gzip --push --header-table-size --trailer --htdocs --address --padding --verbose --version --help --hexdump --dh-param-file --daemon --verify-client --echo-upload --workers --no-tls --color --early-response --max-concurrent-streams ' -- "$cur" ) ) + COMPREPLY=( $( compgen -W '--mime-types-file --error-gzip --push --header-table-size --trailer --htdocs --address --padding --verbose --version --help --hexdump --dh-param-file --daemon --verify-client --echo-upload --workers --no-tls --color --early-response --max-concurrent-streams ' -- "$cur" ) ) ;; *) _filedir From c87a062dba95be8b0155a06ae8afc913bb7dbc92 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 26 Nov 2015 22:28:45 +0900 Subject: [PATCH 39/62] Bump up version number to v1.5.0-DEV --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 062d5cb9..6bc25f4d 100644 --- a/configure.ac +++ b/configure.ac @@ -25,7 +25,7 @@ dnl Do not change user variables! dnl http://www.gnu.org/software/automake/manual/html_node/Flag-Variables-Ordering.html AC_PREREQ(2.61) -AC_INIT([nghttp2], [1.5.0], [t-tujikawa@users.sourceforge.net]) +AC_INIT([nghttp2], [1.5.1-DEV], [t-tujikawa@users.sourceforge.net]) AC_CONFIG_AUX_DIR([.]) AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_HEADERS([config.h]) From 3d1d54e2ced302f2ded012e154436fae3b855f98 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 27 Nov 2015 21:13:44 +0900 Subject: [PATCH 40/62] Remove dead code --- lib/nghttp2_session.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 473b3807..a02f3c35 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -3361,8 +3361,6 @@ int nghttp2_session_end_headers_received(nghttp2_session *session, nghttp2_stream *stream) { int rv; if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { - if (!nghttp2_session_is_my_stream_id(session, frame->hd.stream_id)) { - } nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_RD); rv = nghttp2_session_close_stream_if_shut_rdwr(session, stream); if (nghttp2_is_fatal(rv)) { From f23e34fa3cbb575300df6bc7c8854d9d07012477 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 27 Nov 2015 22:50:13 +0900 Subject: [PATCH 41/62] Handle response in nghttp2_on_begin_frame_callback Previously, nghttp2_session_end_request_headers_received assumes stream is still writable (in other words, local endpoint has not sent END_STREAM). But this assumption is false, because application can send response in nghttp2_on_begin_frame_callback. Probably, this assumption was made before the callback was introduced. This commit addresses this issue. Since all nghttp2_session_end_*_headers_received functions are identical, we refactored them into one function. --- lib/nghttp2_session.c | 87 +++++------------------------------- lib/nghttp2_session.h | 12 ----- tests/main.c | 2 + tests/nghttp2_session_test.c | 79 ++++++++++++++++++++++++++++++++ tests/nghttp2_session_test.h | 1 + 5 files changed, 94 insertions(+), 87 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index a02f3c35..be2d3d38 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -3292,9 +3292,7 @@ static int inflate_header_block(nghttp2_session *session, nghttp2_frame *frame, } /* - * 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. + * Call this function when HEADERS frame was completely received. * * This function returns 0 if it succeeds, or one of negative error * codes: @@ -3304,69 +3302,20 @@ static int inflate_header_block(nghttp2_session *session, nghttp2_frame *frame, * NGHTTP2_ERR_NOMEM * Out of memory. */ -int nghttp2_session_end_request_headers_received(nghttp2_session *session _U_, - nghttp2_frame *frame, - nghttp2_stream *stream) { - if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { - nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_RD); - } - /* Here we assume that stream is not shutdown in NGHTTP2_SHUT_WR */ - 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. - */ -int nghttp2_session_end_response_headers_received(nghttp2_session *session, - nghttp2_frame *frame, - nghttp2_stream *stream) { +static int session_end_stream_headers_received(nghttp2_session *session, + nghttp2_frame *frame, + nghttp2_stream *stream) { int rv; - if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { - /* This is the last frame of this stream, so disallow - further receptions. */ - nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_RD); - rv = nghttp2_session_close_stream_if_shut_rdwr(session, stream); - if (nghttp2_is_fatal(rv)) { - return rv; - } + if ((frame->hd.flags & NGHTTP2_FLAG_END_STREAM) == 0) { + return 0; } - 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. - */ -int nghttp2_session_end_headers_received(nghttp2_session *session, - nghttp2_frame *frame, - nghttp2_stream *stream) { - int rv; - if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { - nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_RD); - rv = nghttp2_session_close_stream_if_shut_rdwr(session, stream); - if (nghttp2_is_fatal(rv)) { - return rv; - } + nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_RD); + rv = nghttp2_session_close_stream_if_shut_rdwr(session, stream); + if (nghttp2_is_fatal(rv)) { + return rv; } + return 0; } @@ -3447,19 +3396,7 @@ static int session_after_header_block_received(nghttp2_session *session) { return 0; } - switch (frame->headers.cat) { - case NGHTTP2_HCAT_REQUEST: - return nghttp2_session_end_request_headers_received(session, frame, stream); - case NGHTTP2_HCAT_RESPONSE: - case NGHTTP2_HCAT_PUSH_RESPONSE: - return nghttp2_session_end_response_headers_received(session, frame, - stream); - case NGHTTP2_HCAT_HEADERS: - return nghttp2_session_end_headers_received(session, frame, stream); - default: - assert(0); - } - return 0; + return session_end_stream_headers_received(session, frame, stream); } int nghttp2_session_on_request_headers_received(nghttp2_session *session, diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h index f62ccb1e..179799a5 100644 --- a/lib/nghttp2_session.h +++ b/lib/nghttp2_session.h @@ -560,18 +560,6 @@ int nghttp2_session_adjust_idle_stream(nghttp2_session *session); int nghttp2_session_close_stream_if_shut_rdwr(nghttp2_session *session, nghttp2_stream *stream); -int nghttp2_session_end_request_headers_received(nghttp2_session *session, - nghttp2_frame *frame, - nghttp2_stream *stream); - -int nghttp2_session_end_response_headers_received(nghttp2_session *session, - nghttp2_frame *frame, - nghttp2_stream *stream); - -int nghttp2_session_end_headers_received(nghttp2_session *session, - nghttp2_frame *frame, - nghttp2_stream *stream); - int nghttp2_session_on_request_headers_received(nghttp2_session *session, nghttp2_frame *frame); diff --git a/tests/main.c b/tests/main.c index 004dd293..72573040 100644 --- a/tests/main.c +++ b/tests/main.c @@ -87,6 +87,8 @@ int main(int argc _U_, char *argv[] _U_) { test_nghttp2_session_recv_continuation) || !CU_add_test(pSuite, "session_recv_headers_with_priority", test_nghttp2_session_recv_headers_with_priority) || + !CU_add_test(pSuite, "session_recv_headers_early_response", + test_nghttp2_session_recv_headers_early_response) || !CU_add_test(pSuite, "session_recv_premature_headers", test_nghttp2_session_recv_premature_headers) || !CU_add_test(pSuite, "session_recv_unknown_frame", diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index da4d3af4..7a0df3e8 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -1356,6 +1356,85 @@ void test_nghttp2_session_recv_headers_with_priority(void) { nghttp2_session_del(session); } +static int response_on_begin_frame_callback(nghttp2_session *session, + const nghttp2_frame_hd *hd, + void *user_data _U_) { + int rv; + + if (hd->type != NGHTTP2_HEADERS) { + return 0; + } + + rv = nghttp2_submit_response(session, hd->stream_id, resnv, ARRLEN(resnv), + NULL); + + CU_ASSERT(0 == rv); + + return 0; +} + +void test_nghttp2_session_recv_headers_early_response(void) { + nghttp2_session *session; + nghttp2_session_callbacks callbacks; + nghttp2_bufs bufs; + nghttp2_buf *buf; + nghttp2_hd_deflater deflater; + nghttp2_mem *mem; + nghttp2_nv *nva; + size_t nvlen; + nghttp2_frame frame; + ssize_t rv; + nghttp2_stream *stream; + + mem = nghttp2_mem_default(); + frame_pack_bufs_init(&bufs); + + memset(&callbacks, 0, sizeof(nghttp2_session_callbacks)); + callbacks.send_callback = null_send_callback; + callbacks.on_begin_frame_callback = response_on_begin_frame_callback; + + nghttp2_session_server_new(&session, &callbacks, NULL); + + nghttp2_hd_deflate_init(&deflater, mem); + + nvlen = ARRLEN(reqnv); + nghttp2_nv_array_copy(&nva, reqnv, nvlen, mem); + nghttp2_frame_headers_init(&frame.headers, + NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_END_STREAM, + 1, NGHTTP2_HCAT_REQUEST, NULL, nva, nvlen); + + rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater); + + CU_ASSERT(0 == rv); + + nghttp2_frame_headers_free(&frame.headers, mem); + + buf = &bufs.head->buf; + + /* Only receive 9 bytes headers, and invoke + on_begin_frame_callback */ + rv = nghttp2_session_mem_recv(session, buf->pos, 9); + + CU_ASSERT(9 == rv); + + rv = nghttp2_session_send(session); + + CU_ASSERT(0 == rv); + + rv = + nghttp2_session_mem_recv(session, buf->pos + 9, nghttp2_buf_len(buf) - 9); + + CU_ASSERT((ssize_t)nghttp2_buf_len(buf) - 9 == rv); + + stream = nghttp2_session_get_stream_raw(session, 1); + + CU_ASSERT(stream->flags & NGHTTP2_STREAM_FLAG_CLOSED); + + nghttp2_hd_deflate_free(&deflater); + nghttp2_session_del(session); + nghttp2_bufs_free(&bufs); +} + void test_nghttp2_session_recv_premature_headers(void) { nghttp2_session *session; nghttp2_session_callbacks callbacks; diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h index f7421180..d079cbf0 100644 --- a/tests/nghttp2_session_test.h +++ b/tests/nghttp2_session_test.h @@ -33,6 +33,7 @@ void test_nghttp2_session_recv_data(void); void test_nghttp2_session_recv_data_no_auto_flow_control(void); void test_nghttp2_session_recv_continuation(void); void test_nghttp2_session_recv_headers_with_priority(void); +void test_nghttp2_session_recv_headers_early_response(void); void test_nghttp2_session_recv_premature_headers(void); void test_nghttp2_session_recv_unknown_frame(void); void test_nghttp2_session_recv_unexpected_continuation(void); From a70445e12230217bd7f93bf12b8da67121b3c38f Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 27 Nov 2015 22:54:55 +0900 Subject: [PATCH 42/62] Retain stream in reserved state on client side Application may use it using nghttp2_stream_* functions, and traverse its dependency. --- lib/nghttp2_session.c | 9 --------- tests/nghttp2_session_test.c | 2 +- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index be2d3d38..7c6207d7 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -973,15 +973,6 @@ nghttp2_stream *nghttp2_session_open_stream(nghttp2_session *session, } } - /* We don't have to track dependency of received reserved stream */ - if (stream->shut_flags & NGHTTP2_SHUT_WR) { - return stream; - } - - /* TODO Client does not have to track dependencies of streams except - for those which have upload data. Currently, we just track - everything. */ - if (pri_spec->stream_id == 0) { dep_stream = &session->root; } diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 7a0df3e8..5a6610be 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -4886,7 +4886,7 @@ void test_nghttp2_session_open_stream(void) { NGHTTP2_STREAM_RESERVED, NULL); CU_ASSERT(0 == session->num_incoming_streams); CU_ASSERT(0 == session->num_outgoing_streams); - CU_ASSERT(NULL == stream->dep_prev); + CU_ASSERT(&session->root == stream->dep_prev); CU_ASSERT(NGHTTP2_DEFAULT_WEIGHT == stream->weight); CU_ASSERT(NGHTTP2_SHUT_WR == stream->shut_flags); From 863493766d9f46300b75fc6f369dc73ccaf6423e Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 27 Nov 2015 23:51:26 +0900 Subject: [PATCH 43/62] Fix in_attr_char(); cleanup const char array iteration --- src/util.cc | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/util.cc b/src/util.cc index 1a040cf5..a4897868 100644 --- a/src/util.cc +++ b/src/util.cc @@ -68,14 +68,15 @@ const char DEFAULT_STRIP_CHARSET[] = "\r\n\t "; const char UPPER_XDIGITS[] = "0123456789ABCDEF"; bool inRFC3986UnreservedChars(const char c) { - static const char unreserved[] = {'-', '.', '_', '~'}; + static constexpr const char unreserved[] = {'-', '.', '_', '~'}; return isAlpha(c) || isDigit(c) || - std::find(&unreserved[0], &unreserved[4], c) != &unreserved[4]; + std::find(std::begin(unreserved), std::end(unreserved), c) != + std::end(unreserved); } bool in_rfc3986_sub_delims(const char c) { - static const char sub_delims[] = {'!', '$', '&', '\'', '(', ')', - '*', '+', ',', ';', '='}; + static constexpr const char sub_delims[] = {'!', '$', '&', '\'', '(', ')', + '*', '+', ',', ';', '='}; return std::find(std::begin(sub_delims), std::end(sub_delims), c) != std::end(sub_delims); } @@ -117,18 +118,17 @@ std::string percent_encode_path(const std::string &s) { } bool in_token(char c) { - static const char extra[] = {'!', '#', '$', '%', '&', '\'', '*', '+', - '-', '.', '^', '_', '`', '|', '~'}; - + static constexpr const char extra[] = {'!', '#', '$', '%', '&', + '\'', '*', '+', '-', '.', + '^', '_', '`', '|', '~'}; return isAlpha(c) || isDigit(c) || - std::find(&extra[0], &extra[sizeof(extra)], c) != - &extra[sizeof(extra)]; + std::find(std::begin(extra), std::end(extra), c) != std::end(extra); } bool in_attr_char(char c) { - static const char bad[] = {'*', '\'', '%'}; + static constexpr const char bad[] = {'*', '\'', '%'}; return util::in_token(c) && - std::find(std::begin(bad), std::end(bad) - 1, c) == std::end(bad) - 1; + std::find(std::begin(bad), std::end(bad), c) == std::end(bad); } std::string percent_encode_token(const std::string &target) { From c0858d8c1af4804ef8f3412d82c01af074dada94 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 28 Nov 2015 00:03:16 +0900 Subject: [PATCH 44/62] src: Minor optimization for appending single character --- src/HttpServer.cc | 2 +- src/asio_client_session_impl.cc | 6 +++--- src/asio_server_serve_mux.cc | 2 +- src/h2load.cc | 4 ++-- src/http2.cc | 12 ++++++------ src/nghttp.cc | 4 ++-- src/shrpx_config.cc | 2 +- src/shrpx_http.cc | 2 +- src/shrpx_http2_session.cc | 2 +- src/shrpx_log.cc | 6 +++--- src/util.cc | 14 +++++++------- 11 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/HttpServer.cc b/src/HttpServer.cc index ded5b6fd..93f8c917 100644 --- a/src/HttpServer.cc +++ b/src/HttpServer.cc @@ -1228,7 +1228,7 @@ void prepare_response(Stream *stream, Http2Handler *hd, close(file); if (query_pos == std::string::npos) { - reqpath += "/"; + reqpath += '/'; } else { reqpath.insert(query_pos, "/"); } diff --git a/src/asio_client_session_impl.cc b/src/asio_client_session_impl.cc index 33f21137..3be7b20d 100644 --- a/src/asio_client_session_impl.cc +++ b/src/asio_client_session_impl.cc @@ -420,10 +420,10 @@ const request *session_impl::submit(boost::system::error_code &ec, if (util::ipv6_numeric_addr(uref.host.c_str())) { uref.host = "[" + uref.host; - uref.host += "]"; + uref.host += ']'; } if (u.field_set & (1 << UF_PORT)) { - uref.host += ":"; + uref.host += ':'; uref.host += util::utos(u.port); } @@ -435,7 +435,7 @@ const request *session_impl::submit(boost::system::error_code &ec, auto path = uref.raw_path; if (u.field_set & (1 << UF_QUERY)) { - path += "?"; + path += '?'; path += uref.raw_query; } diff --git a/src/asio_server_serve_mux.cc b/src/asio_server_serve_mux.cc index 829568ba..d2b43925 100644 --- a/src/asio_server_serve_mux.cc +++ b/src/asio_server_serve_mux.cc @@ -83,7 +83,7 @@ request_cb serve_mux::handler(request_impl &req) const { auto new_uri = util::percent_encode_path(clean_path); auto &uref = req.uri(); if (!uref.raw_query.empty()) { - new_uri += "?"; + new_uri += '?'; new_uri += uref.raw_query; } diff --git a/src/h2load.cc b/src/h2load.cc index 26d75ac6..c868906a 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -1177,7 +1177,7 @@ std::string get_reqline(const char *uri, const http_parser_url &u) { } if (util::has_uri_field(u, UF_QUERY)) { - reqline += "?"; + reqline += '?'; reqline += util::get_uri_field(uri, u, UF_QUERY); } @@ -1954,7 +1954,7 @@ int main(int argc, char **argv) { for (auto &req : reqlines) { // For HTTP/1.1 auto h1req = (*method_it).value; - h1req += " "; + h1req += ' '; h1req += req; h1req += " HTTP/1.1\r\n"; for (auto &nv : shared_nva) { diff --git a/src/http2.cc b/src/http2.cc index acac147a..6faaeddf 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -471,12 +471,12 @@ std::string rewrite_location_uri(const std::string &uri, } if (u.field_set & (1 << UF_QUERY)) { field = &u.field_data[UF_QUERY]; - res += "?"; + res += '?'; res.append(&uri[field->off], field->len); } if (u.field_set & (1 << UF_FRAGMENT)) { field = &u.field_data[UF_FRAGMENT]; - res += "#"; + res += '#'; res.append(&uri[field->off], field->len); } return res; @@ -1185,12 +1185,12 @@ std::string path_join(const char *base_path, size_t base_pathlen, } if (rel_querylen == 0) { if (base_querylen) { - res += "?"; + res += '?'; res.append(base_query, base_querylen); } return res; } - res += "?"; + res += '?'; res.append(rel_query, rel_querylen); return res; } @@ -1242,7 +1242,7 @@ std::string path_join(const char *base_path, size_t base_pathlen, ; } if (rel_querylen) { - res += "?"; + res += '?'; res.append(rel_query, rel_querylen); } return res; @@ -1500,7 +1500,7 @@ int construct_push_component(std::string &scheme, std::string &authority, if (u.field_set & (1 << UF_HOST)) { http2::copy_url_component(authority, &u, UF_HOST, uri); if (u.field_set & (1 << UF_PORT)) { - authority += ":"; + authority += ':'; authority += util::utos(u.port); } } diff --git a/src/nghttp.cc b/src/nghttp.cc index e48d6592..057eac01 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -168,7 +168,7 @@ std::string Request::make_reqpath() const { ? util::get_uri_field(uri.c_str(), u, UF_PATH) : "/"; if (util::has_uri_field(u, UF_QUERY)) { - path += "?"; + path += '?'; path.append(uri.c_str() + u.field_data[UF_QUERY].off, u.field_data[UF_QUERY].len); } @@ -828,7 +828,7 @@ int HttpClient::on_upgrade_connect() { reqvec[0]->method = "GET"; } else { req = (*meth).value; - req += " "; + req += ' '; reqvec[0]->method = (*meth).value; } req += reqvec[0]->make_reqpath(); diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index a2e35abf..b379b7ac 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -596,7 +596,7 @@ void parse_mapping(const DownstreamAddr &addr, const char *src) { // This effectively makes empty pattern to "/". pattern.assign(raw_pattern.first, raw_pattern.second); util::inp_strlower(pattern); - pattern += "/"; + pattern += '/'; } else { pattern.assign(raw_pattern.first, slash); util::inp_strlower(pattern); diff --git a/src/shrpx_http.cc b/src/shrpx_http.cc index c3f1aa7b..5f2f4b9e 100644 --- a/src/shrpx_http.cc +++ b/src/shrpx_http.cc @@ -55,7 +55,7 @@ std::string create_via_header_value(int major, int minor) { std::string hdrs; hdrs += static_cast(major + '0'); if (major < 2) { - hdrs += "."; + hdrs += '.'; hdrs += static_cast(minor + '0'); } hdrs += " nghttpx"; diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index 508ccd34..dd6ed02d 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -520,7 +520,7 @@ int Http2Session::downstream_connect_proxy() { std::string req = "CONNECT "; req += downstream_addr.hostport.get(); if (downstream_addr.port == 80 || downstream_addr.port == 443) { - req += ":"; + req += ':'; req += util::utos(downstream_addr.port); } req += " HTTP/1.1\r\nHost: "; diff --git a/src/shrpx_log.cc b/src/shrpx_log.cc index 59095b51..6055f3e9 100644 --- a/src/shrpx_log.cc +++ b/src/shrpx_log.cc @@ -297,7 +297,7 @@ void upstream_accesslog(const std::vector &lfv, if (frac.size() < 3) { frac = std::string(3 - frac.size(), '0') + frac; } - sec += "."; + sec += '.'; sec += frac; std::tie(p, avail) = copy(sec, avail, p); @@ -415,12 +415,12 @@ void log_chld(pid_t pid, int rstatus, const char *msg) { auto s = strsignal(sig); if (s) { signalstr += s; - signalstr += "("; + signalstr += '('; } else { signalstr += "UNKNOWN("; } signalstr += util::utos(sig); - signalstr += ")"; + signalstr += ')'; } LOG(NOTICE) << msg << ": [" << pid << "] exited " diff --git a/src/util.cc b/src/util.cc index a4897868..5f0a7276 100644 --- a/src/util.cc +++ b/src/util.cc @@ -89,7 +89,7 @@ std::string percentEncode(const unsigned char *target, size_t len) { if (inRFC3986UnreservedChars(c)) { dest += c; } else { - dest += "%"; + dest += '%'; dest += UPPER_XDIGITS[c >> 4]; dest += UPPER_XDIGITS[(c & 0x0f)]; } @@ -110,7 +110,7 @@ std::string percent_encode_path(const std::string &s) { continue; } - dest += "%"; + dest += '%'; dest += UPPER_XDIGITS[(c >> 4) & 0x0f]; dest += UPPER_XDIGITS[(c & 0x0f)]; } @@ -141,7 +141,7 @@ std::string percent_encode_token(const std::string &target) { if (c != '%' && in_token(c)) { dest += c; } else { - dest += "%"; + dest += '%'; dest += UPPER_XDIGITS[c >> 4]; dest += UPPER_XDIGITS[(c & 0x0f)]; } @@ -712,7 +712,7 @@ std::string ascii_dump(const uint8_t *data, size_t len) { if (c >= 0x20 && c < 0x7f) { res += c; } else { - res += "."; + res += '.'; } } @@ -1103,17 +1103,17 @@ std::string make_hostport(const char *host, uint16_t port) { std::string hostport; if (ipv6) { - hostport += "["; + hostport += '['; } hostport += host; if (ipv6) { - hostport += "]"; + hostport += ']'; } if (port != 80 && port != 443) { - hostport += ":"; + hostport += ':'; hostport += utos(port); } From 1ba28bef1f558330cbde38d30fc333edf977934e Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 28 Nov 2015 00:14:11 +0900 Subject: [PATCH 45/62] util: Remove unused functions; rename regacy camel-case function names --- src/HttpServer.cc | 2 +- src/asio_common.cc | 2 +- src/asio_common.h | 2 +- src/http2.h | 2 +- src/nghttp.cc | 2 +- src/shrpx_config.cc | 2 +- src/util.cc | 17 ++++--- src/util.h | 106 ++------------------------------------------ src/util_test.cc | 6 +-- 9 files changed, 21 insertions(+), 120 deletions(-) diff --git a/src/HttpServer.cc b/src/HttpServer.cc index 93f8c917..14cb0395 100644 --- a/src/HttpServer.cc +++ b/src/HttpServer.cc @@ -1174,7 +1174,7 @@ void prepare_response(Stream *stream, Http2Handler *hd, auto sessions = hd->get_sessions(); - url = util::percentDecode(std::begin(url), std::end(url)); + url = util::percent_decode(std::begin(url), std::end(url)); if (!util::check_path(url)) { if (stream->file_ent) { sessions->release_fd(stream->file_ent); diff --git a/src/asio_common.cc b/src/asio_common.cc index 0319169e..bbcb7e6a 100644 --- a/src/asio_common.cc +++ b/src/asio_common.cc @@ -134,7 +134,7 @@ generator_cb file_generator_from_fd(int fd) { bool check_path(const std::string &path) { return util::check_path(path); } std::string percent_decode(const std::string &s) { - return util::percentDecode(std::begin(s), std::end(s)); + return util::percent_decode(std::begin(s), std::end(s)); } std::string http_date(int64_t t) { return util::http_date(t); } diff --git a/src/asio_common.h b/src/asio_common.h index b2384649..7eacfa51 100644 --- a/src/asio_common.h +++ b/src/asio_common.h @@ -55,7 +55,7 @@ void split_path(uri_ref &dst, InputIt first, InputIt last) { } else { query_first = path_last + 1; } - dst.path = util::percentDecode(first, path_last); + dst.path = util::percent_decode(first, path_last); dst.raw_path.assign(first, path_last); dst.raw_query.assign(query_first, last); } diff --git a/src/http2.h b/src/http2.h index dedee285..7fa42b37 100644 --- a/src/http2.h +++ b/src/http2.h @@ -343,7 +343,7 @@ std::string normalize_path(InputIt first, InputIt last) { if (util::isHexDigit(*(first + 1)) && util::isHexDigit(*(first + 2))) { auto c = (util::hex_to_uint(*(first + 1)) << 4) + util::hex_to_uint(*(first + 2)); - if (util::inRFC3986UnreservedChars(c)) { + if (util::in_rfc3986_unreserved_chars(c)) { result += c; first += 3; continue; diff --git a/src/nghttp.cc b/src/nghttp.cc index 057eac01..2a392f67 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -199,7 +199,7 @@ std::string decode_host(std::string host) { auto zone_id_src = (*(zone_start + 1) == '2' && *(zone_start + 2) == '5') ? zone_start + 3 : zone_start + 1; - auto zone_id = util::percentDecode(zone_id_src, std::end(host)); + auto zone_id = util::percent_decode(zone_id_src, std::end(host)); host.erase(zone_start + 1, std::end(host)); host += zone_id; return host; diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index b379b7ac..c6b88721 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -1664,7 +1664,7 @@ int parse_config(const char *opt, const char *optarg, // Surprisingly, u.field_set & UF_USERINFO is nonzero even if // userinfo component is empty string. if (!val.empty()) { - val = util::percentDecode(val.begin(), val.end()); + val = util::percent_decode(std::begin(val), std::end(val)); mod_config()->downstream_http_proxy_userinfo = strcopy(val); } } diff --git a/src/util.cc b/src/util.cc index 5f0a7276..8dd84b1e 100644 --- a/src/util.cc +++ b/src/util.cc @@ -63,11 +63,9 @@ namespace nghttp2 { namespace util { -const char DEFAULT_STRIP_CHARSET[] = "\r\n\t "; - const char UPPER_XDIGITS[] = "0123456789ABCDEF"; -bool inRFC3986UnreservedChars(const char c) { +bool in_rfc3986_unreserved_chars(const char c) { static constexpr const char unreserved[] = {'-', '.', '_', '~'}; return isAlpha(c) || isDigit(c) || std::find(std::begin(unreserved), std::end(unreserved), c) != @@ -81,12 +79,12 @@ bool in_rfc3986_sub_delims(const char c) { std::end(sub_delims); } -std::string percentEncode(const unsigned char *target, size_t len) { +std::string percent_encode(const unsigned char *target, size_t len) { std::string dest; for (size_t i = 0; i < len; ++i) { unsigned char c = target[i]; - if (inRFC3986UnreservedChars(c)) { + if (in_rfc3986_unreserved_chars(c)) { dest += c; } else { dest += '%'; @@ -97,15 +95,16 @@ std::string percentEncode(const unsigned char *target, size_t len) { return dest; } -std::string percentEncode(const std::string &target) { - return percentEncode(reinterpret_cast(target.c_str()), - target.size()); +std::string percent_encode(const std::string &target) { + return percent_encode(reinterpret_cast(target.c_str()), + target.size()); } std::string percent_encode_path(const std::string &s) { std::string dest; for (auto c : s) { - if (inRFC3986UnreservedChars(c) || in_rfc3986_sub_delims(c) || c == '/') { + if (in_rfc3986_unreserved_chars(c) || in_rfc3986_sub_delims(c) || + c == '/') { dest += c; continue; } diff --git a/src/util.h b/src/util.h index 8eb88cdd..b2e62171 100644 --- a/src/util.h +++ b/src/util.h @@ -64,104 +64,6 @@ constexpr const char NGHTTP2_H1_1[] = "http/1.1"; namespace util { -extern const char DEFAULT_STRIP_CHARSET[]; - -template -std::pair -stripIter(InputIterator first, InputIterator last, - const char *chars = DEFAULT_STRIP_CHARSET) { - for (; first != last && strchr(chars, *first) != 0; ++first) - ; - if (first == last) { - return std::make_pair(first, last); - } - InputIterator left = last - 1; - for (; left != first && strchr(chars, *left) != 0; --left) - ; - return std::make_pair(first, left + 1); -} - -template -OutputIterator splitIter(InputIterator first, InputIterator last, - OutputIterator out, char delim, bool doStrip = false, - bool allowEmpty = false) { - for (InputIterator i = first; i != last;) { - InputIterator j = std::find(i, last, delim); - std::pair p(i, j); - if (doStrip) { - p = stripIter(i, j); - } - if (allowEmpty || p.first != p.second) { - *out++ = p; - } - i = j; - if (j != last) { - ++i; - } - } - if (allowEmpty && (first == last || *(last - 1) == delim)) { - *out++ = std::make_pair(last, last); - } - return out; -} - -template -OutputIterator split(InputIterator first, InputIterator last, - OutputIterator out, char delim, bool doStrip = false, - bool allowEmpty = false) { - for (InputIterator i = first; i != last;) { - InputIterator j = std::find(i, last, delim); - std::pair p(i, j); - if (doStrip) { - p = stripIter(i, j); - } - if (allowEmpty || p.first != p.second) { - *out++ = std::string(p.first, p.second); - } - i = j; - if (j != last) { - ++i; - } - } - if (allowEmpty && (first == last || *(last - 1) == delim)) { - *out++ = std::string(last, last); - } - return out; -} - -template -std::string strjoin(InputIterator first, InputIterator last, - const DelimiterType &delim) { - std::string result; - if (first == last) { - return result; - } - InputIterator beforeLast = last - 1; - for (; first != beforeLast; ++first) { - result += *first; - result += delim; - } - result += *beforeLast; - return result; -} - -template -std::string joinPath(InputIterator first, InputIterator last) { - std::vector elements; - for (; first != last; ++first) { - if (*first == "..") { - if (!elements.empty()) { - elements.pop_back(); - } - } else if (*first == ".") { - // do nothing - } else { - elements.push_back(*first); - } - } - return strjoin(elements.begin(), elements.end(), "/"); -} - inline bool isAlpha(const char c) { return ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z'); } @@ -172,7 +74,7 @@ inline bool isHexDigit(const char c) { return isDigit(c) || ('A' <= c && c <= 'F') || ('a' <= c && c <= 'f'); } -bool inRFC3986UnreservedChars(const char c); +bool in_rfc3986_unreserved_chars(const char c); bool in_rfc3986_sub_delims(const char c); @@ -185,15 +87,15 @@ bool in_attr_char(char c); // if isHexDigit(c) is false. uint32_t hex_to_uint(char c); -std::string percentEncode(const unsigned char *target, size_t len); +std::string percent_encode(const unsigned char *target, size_t len); -std::string percentEncode(const std::string &target); +std::string percent_encode(const std::string &target); // percent-encode path component of URI |s|. std::string percent_encode_path(const std::string &s); template -std::string percentDecode(InputIt first, InputIt last) { +std::string percent_decode(InputIt first, InputIt last) { std::string result; for (; first != last; ++first) { if (*first == '%') { diff --git a/src/util_test.cc b/src/util_test.cc index da2485a9..77542017 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -147,15 +147,15 @@ void test_util_percent_encode_path(void) { void test_util_percent_decode(void) { { std::string s = "%66%6F%6f%62%61%72"; - CU_ASSERT("foobar" == util::percentDecode(std::begin(s), std::end(s))); + CU_ASSERT("foobar" == util::percent_decode(std::begin(s), std::end(s))); } { std::string s = "%66%6"; - CU_ASSERT("f%6" == util::percentDecode(std::begin(s), std::end(s))); + CU_ASSERT("f%6" == util::percent_decode(std::begin(s), std::end(s))); } { std::string s = "%66%"; - CU_ASSERT("f%" == util::percentDecode(std::begin(s), std::end(s))); + CU_ASSERT("f%" == util::percent_decode(std::begin(s), std::end(s))); } } From de247f7d332fef45b22b49108af20d8ed635c161 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 28 Nov 2015 00:31:42 +0900 Subject: [PATCH 46/62] src: Rename startsWith as starts_with --- src/asio_server_serve_mux.cc | 2 +- src/http2.cc | 4 ++-- src/shrpx.cc | 10 +++++----- src/shrpx_config.cc | 6 +++--- src/shrpx_ssl.cc | 4 ++-- src/util.cc | 6 +++--- src/util.h | 26 +++++++++++++------------- src/util_test.cc | 16 ++++++++-------- 8 files changed, 37 insertions(+), 37 deletions(-) diff --git a/src/asio_server_serve_mux.cc b/src/asio_server_serve_mux.cc index d2b43925..b635ee4d 100644 --- a/src/asio_server_serve_mux.cc +++ b/src/asio_server_serve_mux.cc @@ -108,7 +108,7 @@ bool path_match(const std::string &pattern, const std::string &path) { if (pattern.back() != '/') { return pattern == path; } - return util::startsWith(path, pattern); + return util::starts_with(path, pattern); } } // namespace diff --git a/src/http2.cc b/src/http2.cc index 6faaeddf..671b1a92 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -454,8 +454,8 @@ std::string rewrite_location_uri(const std::string &uri, return ""; } auto field = &u.field_data[UF_HOST]; - if (!util::startsWith(std::begin(match_host), std::end(match_host), - &uri[field->off], &uri[field->off] + field->len) || + if (!util::starts_with(std::begin(match_host), std::end(match_host), + &uri[field->off], &uri[field->off] + field->len) || (match_host.size() != field->len && match_host[field->len] != ':')) { return ""; } diff --git a/src/shrpx.cc b/src/shrpx.cc index ab9e673a..a83062f3 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -325,11 +325,11 @@ void exec_binary(SignalServer *ssv) { } for (size_t i = 0; i < envlen; ++i) { - if (util::startsWith(environ[i], ENV_LISTENER4_FD) || - util::startsWith(environ[i], ENV_LISTENER6_FD) || - util::startsWith(environ[i], ENV_PORT) || - util::startsWith(environ[i], ENV_UNIX_FD) || - util::startsWith(environ[i], ENV_UNIX_PATH)) { + if (util::starts_with(environ[i], ENV_LISTENER4_FD) || + util::starts_with(environ[i], ENV_LISTENER6_FD) || + util::starts_with(environ[i], ENV_PORT) || + util::starts_with(environ[i], ENV_UNIX_FD) || + util::starts_with(environ[i], ENV_UNIX_PATH)) { continue; } diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index c6b88721..c9b229eb 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -519,7 +519,7 @@ std::vector parse_log_format(const char *optarg) { auto type = log_var_lookup_token(var_name, var_namelen); if (type == SHRPX_LOGF_NONE) { - if (util::istartsWith(var_name, var_namelen, "http_")) { + if (util::istarts_with(var_name, var_namelen, "http_")) { if (util::streq("host", var_name + str_size("http_"), var_namelen - str_size("http_"))) { // Special handling of host header field. We will use @@ -1342,7 +1342,7 @@ int parse_config(const char *opt, const char *optarg, pat_delim = optarg + optarglen; } DownstreamAddr addr; - if (util::istartsWith(optarg, SHRPX_UNIX_PATH_PREFIX)) { + if (util::istarts_with(optarg, SHRPX_UNIX_PATH_PREFIX)) { auto path = optarg + str_size(SHRPX_UNIX_PATH_PREFIX); addr.host = strcopy(path, pat_delim); addr.host_unix = true; @@ -1368,7 +1368,7 @@ int parse_config(const char *opt, const char *optarg, return 0; } case SHRPX_OPTID_FRONTEND: { - if (util::istartsWith(optarg, SHRPX_UNIX_PATH_PREFIX)) { + if (util::istarts_with(optarg, SHRPX_UNIX_PATH_PREFIX)) { auto path = optarg + str_size(SHRPX_UNIX_PATH_PREFIX); mod_config()->host = strcopy(path); mod_config()->port = 0; diff --git a/src/shrpx_ssl.cc b/src/shrpx_ssl.cc index 8c4b4166..b3cf2537 100644 --- a/src/shrpx_ssl.cc +++ b/src/shrpx_ssl.cc @@ -792,7 +792,7 @@ bool tls_hostname_match(const char *pattern, const char *hostname) { // Don't attempt to match a presented identifier where the wildcard // character is embedded within an A-label. if (ptLeftLabelEnd == 0 || strchr(ptLeftLabelEnd + 1, '.') == 0 || - ptLeftLabelEnd < ptWildcard || util::istartsWith(pattern, "xn--")) { + ptLeftLabelEnd < ptWildcard || util::istarts_with(pattern, "xn--")) { wildcardEnabled = false; } if (!wildcardEnabled) { @@ -807,7 +807,7 @@ bool tls_hostname_match(const char *pattern, const char *hostname) { if (hnLeftLabelEnd - hostname < ptLeftLabelEnd - pattern) { return false; } - return util::istartsWith(hostname, hnLeftLabelEnd, pattern, ptWildcard) && + return util::istarts_with(hostname, hnLeftLabelEnd, pattern, ptWildcard) && util::iendsWith(hostname, hnLeftLabelEnd, ptWildcard + 1, ptLeftLabelEnd); } diff --git a/src/util.cc b/src/util.cc index 8dd84b1e..5d884b3c 100644 --- a/src/util.cc +++ b/src/util.cc @@ -353,7 +353,7 @@ void streq_advance(const char **ap, const char **bp) { } } // namespace -bool istartsWith(const char *a, const char *b) { +bool istarts_with(const char *a, const char *b) { if (!a || !b) { return false; } @@ -509,8 +509,8 @@ void show_candidates(const char *unkopt, option *options) { for (size_t i = 0; options[i].name != nullptr; ++i) { auto optnamelen = strlen(options[i].name); // Use cost 0 for prefix match - if (istartsWith(options[i].name, options[i].name + optnamelen, unkopt, - unkopt + unkoptlen)) { + if (istarts_with(options[i].name, options[i].name + optnamelen, unkopt, + unkopt + unkoptlen)) { if (optnamelen == static_cast(unkoptlen)) { // Exact match, then we don't show any condidates. return; diff --git a/src/util.h b/src/util.h index b2e62171..7fcc7817 100644 --- a/src/util.h +++ b/src/util.h @@ -169,20 +169,20 @@ inline char lowcase(char c) { } template -bool startsWith(InputIterator1 first1, InputIterator1 last1, - InputIterator2 first2, InputIterator2 last2) { +bool starts_with(InputIterator1 first1, InputIterator1 last1, + InputIterator2 first2, InputIterator2 last2) { if (last1 - first1 < last2 - first2) { return false; } return std::equal(first2, last2, first1); } -inline bool startsWith(const std::string &a, const std::string &b) { - return startsWith(std::begin(a), std::end(a), std::begin(b), std::end(b)); +inline bool starts_with(const std::string &a, const std::string &b) { + return starts_with(std::begin(a), std::end(a), std::begin(b), std::end(b)); } -inline bool startsWith(const char *a, const char *b) { - return startsWith(a, a + strlen(a), b, b + strlen(b)); +inline bool starts_with(const char *a, const char *b) { + return starts_with(a, a + strlen(a), b, b + strlen(b)); } struct CaseCmp { @@ -192,24 +192,24 @@ struct CaseCmp { }; template -bool istartsWith(InputIterator1 first1, InputIterator1 last1, - InputIterator2 first2, InputIterator2 last2) { +bool istarts_with(InputIterator1 first1, InputIterator1 last1, + InputIterator2 first2, InputIterator2 last2) { if (last1 - first1 < last2 - first2) { return false; } return std::equal(first2, last2, first1, CaseCmp()); } -inline bool istartsWith(const std::string &a, const std::string &b) { - return istartsWith(std::begin(a), std::end(a), std::begin(b), std::end(b)); +inline bool istarts_with(const std::string &a, const std::string &b) { + return istarts_with(std::begin(a), std::end(a), std::begin(b), std::end(b)); } template -bool istartsWith(InputIt a, size_t an, const char *b) { - return istartsWith(a, a + an, b, b + strlen(b)); +bool istarts_with(InputIt a, size_t an, const char *b) { + return istarts_with(a, a + an, b, b + strlen(b)); } -bool istartsWith(const char *a, const char *b); +bool istarts_with(const char *a, const char *b); template bool endsWith(InputIterator1 first1, InputIterator1 last1, diff --git a/src/util_test.cc b/src/util_test.cc index 77542017..19cdbf90 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -344,15 +344,15 @@ void test_util_format_duration(void) { } void test_util_starts_with(void) { - CU_ASSERT(util::startsWith("foo", "foo")); - CU_ASSERT(util::startsWith("fooo", "foo")); - CU_ASSERT(util::startsWith("ofoo", "")); - CU_ASSERT(!util::startsWith("ofoo", "foo")); + CU_ASSERT(util::starts_with("foo", "foo")); + CU_ASSERT(util::starts_with("fooo", "foo")); + CU_ASSERT(util::starts_with("ofoo", "")); + CU_ASSERT(!util::starts_with("ofoo", "foo")); - CU_ASSERT(util::istartsWith("FOO", "fOO")); - CU_ASSERT(util::startsWith("ofoo", "")); - CU_ASSERT(util::istartsWith("fOOo", "Foo")); - CU_ASSERT(!util::istartsWith("ofoo", "foo")); + CU_ASSERT(util::istarts_with("FOO", "fOO")); + CU_ASSERT(util::starts_with("ofoo", "")); + CU_ASSERT(util::istarts_with("fOOo", "Foo")); + CU_ASSERT(!util::istarts_with("ofoo", "foo")); } void test_util_ends_with(void) { From d867fe64e3325b4f7af2b761de1a1ce2372d5cdf Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 28 Nov 2015 00:36:43 +0900 Subject: [PATCH 47/62] src: Rename endsWith as ends_with --- src/shrpx_ssl.cc | 4 ++-- src/util.cc | 6 +++--- src/util.h | 16 ++++++++-------- src/util_test.cc | 16 ++++++++-------- 4 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/shrpx_ssl.cc b/src/shrpx_ssl.cc index b3cf2537..0a7ca6ac 100644 --- a/src/shrpx_ssl.cc +++ b/src/shrpx_ssl.cc @@ -808,8 +808,8 @@ bool tls_hostname_match(const char *pattern, const char *hostname) { return false; } return util::istarts_with(hostname, hnLeftLabelEnd, pattern, ptWildcard) && - util::iendsWith(hostname, hnLeftLabelEnd, ptWildcard + 1, - ptLeftLabelEnd); + util::iends_with(hostname, hnLeftLabelEnd, ptWildcard + 1, + ptLeftLabelEnd); } } // namespace diff --git a/src/util.cc b/src/util.cc index 5d884b3c..2962e152 100644 --- a/src/util.cc +++ b/src/util.cc @@ -521,8 +521,8 @@ void show_candidates(const char *unkopt, option *options) { } // Use cost 0 for suffix match, but match at least 3 characters if (unkoptlen >= 3 && - iendsWith(options[i].name, options[i].name + optnamelen, unkopt, - unkopt + unkoptlen)) { + iends_with(options[i].name, options[i].name + optnamelen, unkopt, + unkopt + unkoptlen)) { cands.emplace_back(0, options[i].name); continue; } @@ -754,7 +754,7 @@ bool check_path(const std::string &path) { path.find('\\') == std::string::npos && path.find("/../") == std::string::npos && path.find("/./") == std::string::npos && - !util::endsWith(path, "/..") && !util::endsWith(path, "/."); + !util::ends_with(path, "/..") && !util::ends_with(path, "/."); } int64_t to_time64(const timeval &tv) { diff --git a/src/util.h b/src/util.h index 7fcc7817..a4468bcb 100644 --- a/src/util.h +++ b/src/util.h @@ -212,29 +212,29 @@ bool istarts_with(InputIt a, size_t an, const char *b) { bool istarts_with(const char *a, const char *b); template -bool endsWith(InputIterator1 first1, InputIterator1 last1, - InputIterator2 first2, InputIterator2 last2) { +bool ends_with(InputIterator1 first1, InputIterator1 last1, + InputIterator2 first2, InputIterator2 last2) { if (last1 - first1 < last2 - first2) { return false; } return std::equal(first2, last2, last1 - (last2 - first2)); } -inline bool endsWith(const std::string &a, const std::string &b) { - return endsWith(std::begin(a), std::end(a), std::begin(b), std::end(b)); +inline bool ends_with(const std::string &a, const std::string &b) { + return ends_with(std::begin(a), std::end(a), std::begin(b), std::end(b)); } template -bool iendsWith(InputIterator1 first1, InputIterator1 last1, - InputIterator2 first2, InputIterator2 last2) { +bool iends_with(InputIterator1 first1, InputIterator1 last1, + InputIterator2 first2, InputIterator2 last2) { if (last1 - first1 < last2 - first2) { return false; } return std::equal(first2, last2, last1 - (last2 - first2), CaseCmp()); } -inline bool iendsWith(const std::string &a, const std::string &b) { - return iendsWith(std::begin(a), std::end(a), std::begin(b), std::end(b)); +inline bool iends_with(const std::string &a, const std::string &b) { + return iends_with(std::begin(a), std::end(a), std::begin(b), std::end(b)); } int strcompare(const char *a, const uint8_t *b, size_t n); diff --git a/src/util_test.cc b/src/util_test.cc index 19cdbf90..adc079f6 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -356,15 +356,15 @@ void test_util_starts_with(void) { } void test_util_ends_with(void) { - CU_ASSERT(util::endsWith("foo", "foo")); - CU_ASSERT(util::endsWith("foo", "")); - CU_ASSERT(util::endsWith("ofoo", "foo")); - CU_ASSERT(!util::endsWith("ofoo", "fo")); + CU_ASSERT(util::ends_with("foo", "foo")); + CU_ASSERT(util::ends_with("foo", "")); + CU_ASSERT(util::ends_with("ofoo", "foo")); + CU_ASSERT(!util::ends_with("ofoo", "fo")); - CU_ASSERT(util::iendsWith("fOo", "Foo")); - CU_ASSERT(util::iendsWith("foo", "")); - CU_ASSERT(util::iendsWith("oFoo", "fOO")); - CU_ASSERT(!util::iendsWith("ofoo", "fo")); + CU_ASSERT(util::iends_with("fOo", "Foo")); + CU_ASSERT(util::iends_with("foo", "")); + CU_ASSERT(util::iends_with("oFoo", "fOO")); + CU_ASSERT(!util::iends_with("ofoo", "fo")); } void test_util_parse_http_date(void) { From ba9e912cf6548ed0cbb970e9e2afa3a8c6067877 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 28 Nov 2015 00:41:39 +0900 Subject: [PATCH 48/62] src: Rename isAlpha, isDigit, and isHexDigit as is_... --- src/http2.h | 3 ++- src/shrpx_client_handler.cc | 4 ++-- src/shrpx_config.cc | 2 +- src/util.cc | 4 ++-- src/util.h | 14 +++++++------- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/http2.h b/src/http2.h index 7fa42b37..aaf076d6 100644 --- a/src/http2.h +++ b/src/http2.h @@ -340,7 +340,8 @@ std::string normalize_path(InputIt first, InputIt last) { } else { for (; first < last - 2;) { if (*first == '%') { - if (util::isHexDigit(*(first + 1)) && util::isHexDigit(*(first + 2))) { + if (util::is_hex_digit(*(first + 1)) && + util::is_hex_digit(*(first + 2))) { auto c = (util::hex_to_uint(*(first + 1)) << 4) + util::hex_to_uint(*(first + 2)); if (util::in_rfc3986_unreserved_chars(c)) { diff --git a/src/shrpx_client_handler.cc b/src/shrpx_client_handler.cc index 47818894..8ace9d4a 100644 --- a/src/shrpx_client_handler.cc +++ b/src/shrpx_client_handler.cc @@ -859,13 +859,13 @@ ssize_t parse_proxy_line_port(const uint8_t *first, const uint8_t *last) { } if (*p == '0') { - if (p + 1 != last && util::isDigit(*(p + 1))) { + if (p + 1 != last && util::is_digit(*(p + 1))) { return -1; } return 1; } - for (; p != last && util::isDigit(*p); ++p) { + for (; p != last && util::is_digit(*p); ++p) { port *= 10; port += *p - '0'; diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index c9b229eb..565d3118 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -471,7 +471,7 @@ LogFragmentType log_var_lookup_token(const char *name, size_t namelen) { namespace { bool var_token(char c) { - return util::isAlpha(c) || util::isDigit(c) || c == '_'; + return util::is_alpha(c) || util::is_digit(c) || c == '_'; } } // namespace diff --git a/src/util.cc b/src/util.cc index 2962e152..90ace89a 100644 --- a/src/util.cc +++ b/src/util.cc @@ -67,7 +67,7 @@ const char UPPER_XDIGITS[] = "0123456789ABCDEF"; bool in_rfc3986_unreserved_chars(const char c) { static constexpr const char unreserved[] = {'-', '.', '_', '~'}; - return isAlpha(c) || isDigit(c) || + return is_alpha(c) || is_digit(c) || std::find(std::begin(unreserved), std::end(unreserved), c) != std::end(unreserved); } @@ -120,7 +120,7 @@ bool in_token(char c) { static constexpr const char extra[] = {'!', '#', '$', '%', '&', '\'', '*', '+', '-', '.', '^', '_', '`', '|', '~'}; - return isAlpha(c) || isDigit(c) || + return is_alpha(c) || is_digit(c) || std::find(std::begin(extra), std::end(extra), c) != std::end(extra); } diff --git a/src/util.h b/src/util.h index a4468bcb..9319e7cd 100644 --- a/src/util.h +++ b/src/util.h @@ -64,14 +64,14 @@ constexpr const char NGHTTP2_H1_1[] = "http/1.1"; namespace util { -inline bool isAlpha(const char c) { +inline bool is_alpha(const char c) { return ('A' <= c && c <= 'Z') || ('a' <= c && c <= 'z'); } -inline bool isDigit(const char c) { return '0' <= c && c <= '9'; } +inline bool is_digit(const char c) { return '0' <= c && c <= '9'; } -inline bool isHexDigit(const char c) { - return isDigit(c) || ('A' <= c && c <= 'F') || ('a' <= c && c <= 'f'); +inline bool is_hex_digit(const char c) { + return is_digit(c) || ('A' <= c && c <= 'F') || ('a' <= c && c <= 'f'); } bool in_rfc3986_unreserved_chars(const char c); @@ -84,7 +84,7 @@ bool in_token(char c); bool in_attr_char(char c); // Returns integer corresponding to hex notation |c|. It is undefined -// if isHexDigit(c) is false. +// if is_hex_digit(c) is false. uint32_t hex_to_uint(char c); std::string percent_encode(const unsigned char *target, size_t len); @@ -99,8 +99,8 @@ std::string percent_decode(InputIt first, InputIt last) { std::string result; for (; first != last; ++first) { if (*first == '%') { - if (first + 1 != last && first + 2 != last && isHexDigit(*(first + 1)) && - isHexDigit(*(first + 2))) { + if (first + 1 != last && first + 2 != last && + is_hex_digit(*(first + 1)) && is_hex_digit(*(first + 2))) { result += (hex_to_uint(*(first + 1)) << 4) + hex_to_uint(*(first + 2)); first += 2; continue; From aacac613af5cb0d275394830ac9cce8c457161f3 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 28 Nov 2015 00:50:29 +0900 Subject: [PATCH 49/62] Assert dep_stream is non-null to shut up scan-build --- lib/nghttp2_session.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 7c6207d7..38b2878b 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -660,6 +660,8 @@ int nghttp2_session_reprioritize_stream( } } + assert(dep_stream); + if (dep_stream == stream->dep_prev && !pri_spec->exclusive) { /* This is minor optimization when just weight is changed. Currently, we don't reschedule stream in this case, since we From 12b2e0a2b35d00f97a1fcf721081ed3a1829fa26 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 28 Nov 2015 15:04:39 +0900 Subject: [PATCH 50/62] Add nghttp2_session_create_idle_stream() API See GH-436 --- doc/Makefile.am | 1 + lib/includes/nghttp2/nghttp2.h | 45 +++++++++++++++++++++ lib/nghttp2_session.c | 29 ++++++++++++++ tests/nghttp2_session_test.c | 71 ++++++++++++++++++++++++++++++++++ tests/nghttp2_session_test.h | 1 + 5 files changed, 147 insertions(+) diff --git a/doc/Makefile.am b/doc/Makefile.am index 0bc4f66c..0e6a9abc 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -86,6 +86,7 @@ APIDOCS= \ nghttp2_session_consume.rst \ nghttp2_session_consume_connection.rst \ nghttp2_session_consume_stream.rst \ + nghttp2_session_create_idle_stream.rst \ nghttp2_session_del.rst \ nghttp2_session_find_stream.rst \ nghttp2_session_get_effective_local_window_size.rst \ diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 47ca79a5..1729f27c 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -2887,6 +2887,51 @@ nghttp2_session_change_stream_priority(nghttp2_session *session, int32_t stream_id, const nghttp2_priority_spec *pri_spec); +/** + * @function + * + * Creates idle stream with the given |stream_id|, and priority + * |pri_spec|. + * + * The stream creation is done without sending PRIORITY frame, which + * means that peer does not know about the existence of this idle + * stream in the local endpoint. + * + * RFC 7540 does not disallow the use of creation of idle stream with + * odd or even stream ID regardless of client or server. So this + * function can create odd or even stream ID regardless of client or + * server. But probably it is a bit safer to use the stream ID the + * local endpoint can initiate (in other words, use odd stream ID for + * client, and even stream ID for server), to avoid potential + * collision from peer's instruction. Also we can use + * `nghttp2_session_set_next_stream_id()` to avoid to open created + * idle streams accidentally if we follow this recommendation. + * + * If |session| is initialized as server, and ``pri_spec->stream_id`` + * points to the idle stream, the idle stream is created if it does + * not exist. The created idle stream will depend on root stream + * (stream 0) with weight 16. + * + * Otherwise, if stream denoted by ``pri_spec->stream_id`` is not + * found, we use default priority instead of given |pri_spec|. That + * is make stream depend on root stream with weight 16. + * + * 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_INVALID_ARGUMENT` + * Attempted to depend on itself; or stream denoted by |stream_id| + * already exists; or |stream_id| cannot be used to create idle + * stream (in other words, local endpoint has already opened + * stream ID greater than or equal to the given stream ID; or + * |stream_id| is 0 + */ +NGHTTP2_EXTERN int +nghttp2_session_create_idle_stream(nghttp2_session *session, int32_t stream_id, + const nghttp2_priority_spec *pri_spec); + /** * @function * diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 38b2878b..fd007c93 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -6731,3 +6731,32 @@ int nghttp2_session_change_stream_priority( return nghttp2_session_reprioritize_stream(session, stream, &pri_spec_copy); } + +int nghttp2_session_create_idle_stream(nghttp2_session *session, + int32_t stream_id, + const nghttp2_priority_spec *pri_spec) { + nghttp2_stream *stream; + nghttp2_priority_spec pri_spec_copy; + + if (stream_id == 0 || stream_id == pri_spec->stream_id || + !session_detect_idle_stream(session, stream_id)) { + return NGHTTP2_ERR_INVALID_ARGUMENT; + } + + stream = nghttp2_session_get_stream_raw(session, stream_id); + if (stream) { + return NGHTTP2_ERR_INVALID_ARGUMENT; + } + + pri_spec_copy = *pri_spec; + nghttp2_priority_spec_normalize_weight(&pri_spec_copy); + + stream = + nghttp2_session_open_stream(session, stream_id, NGHTTP2_STREAM_FLAG_NONE, + &pri_spec_copy, NGHTTP2_STREAM_IDLE, NULL); + if (!stream) { + return NGHTTP2_ERR_NOMEM; + } + + return 0; +} diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 5a6610be..aae1cb25 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -8464,6 +8464,77 @@ void test_nghttp2_session_change_stream_priority(void) { nghttp2_session_del(session); } +void test_nghttp2_session_create_idle_stream(void) { + nghttp2_session *session; + nghttp2_session_callbacks callbacks; + nghttp2_stream *stream2, *stream4, *stream8, *stream10; + nghttp2_priority_spec pri_spec; + int rv; + + memset(&callbacks, 0, sizeof(callbacks)); + + nghttp2_session_server_new(&session, &callbacks, NULL); + + stream2 = open_stream(session, 2); + + nghttp2_priority_spec_init(&pri_spec, 2, 111, 1); + + rv = nghttp2_session_create_idle_stream(session, 4, &pri_spec); + + CU_ASSERT(0 == rv); + + stream4 = nghttp2_session_get_stream_raw(session, 4); + + CU_ASSERT(4 == stream4->stream_id); + CU_ASSERT(111 == stream4->weight); + CU_ASSERT(stream2 == stream4->dep_prev); + CU_ASSERT(stream4 == stream2->dep_next); + + /* If pri_spec->stream_id does not exist, and it is idle stream, it + is created too */ + nghttp2_priority_spec_init(&pri_spec, 8, 109, 0); + + rv = nghttp2_session_create_idle_stream(session, 8, &pri_spec); + + CU_ASSERT(0 == rv); + + stream8 = nghttp2_session_get_stream_raw(session, 8); + stream10 = nghttp2_session_get_stream_raw(session, 10); + + CU_ASSERT(8 == stream8->stream_id); + CU_ASSERT(109 == stream8->weight); + CU_ASSERT(10 == stream10->stream_id); + CU_ASSERT(16 == stream10->weight); + CU_ASSERT(stream10 == stream8->dep_prev); + CU_ASSERT(&session->root == stream10->dep_prev); + + /* It is an error to attempt to create already existing idle + stream */ + rv = nghttp2_session_create_idle_stream(session, 4, &pri_spec); + + CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == rv); + + /* It is an error to depend on itself */ + pri_spec.stream_id = 6; + + rv = nghttp2_session_create_idle_stream(session, 6, &pri_spec); + CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == rv); + + /* It is an error to create root stream (0) as idle stream */ + rv = nghttp2_session_create_idle_stream(session, 0, &pri_spec); + CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == rv); + + /* It is an error to create non-idle stream */ + session->next_stream_id = 20; + pri_spec.stream_id = 2; + + rv = nghttp2_session_create_idle_stream(session, 18, &pri_spec); + + CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == rv); + + nghttp2_session_del(session); +} + static void check_nghttp2_http_recv_headers_fail( nghttp2_session *session, nghttp2_hd_deflater *deflater, int32_t stream_id, int stream_state, const nghttp2_nv *nva, size_t nvlen) { diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h index d079cbf0..1834fe95 100644 --- a/tests/nghttp2_session_test.h +++ b/tests/nghttp2_session_test.h @@ -136,6 +136,7 @@ void test_nghttp2_session_defer_then_close(void); void test_nghttp2_session_detach_item_from_closed_stream(void); void test_nghttp2_session_flooding(void); void test_nghttp2_session_change_stream_priority(void); +void test_nghttp2_session_create_idle_stream(void); void test_nghttp2_http_mandatory_headers(void); void test_nghttp2_http_content_length(void); void test_nghttp2_http_content_length_mismatch(void); From 2754d9e2bb23ca04e2a009d657525f2bbab74f95 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 28 Nov 2015 15:14:31 +0900 Subject: [PATCH 51/62] Update doc --- lib/includes/nghttp2/nghttp2.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 1729f27c..9914aab4 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -2869,9 +2869,9 @@ NGHTTP2_EXTERN int nghttp2_session_consume_stream(nghttp2_session *session, * not exist. The created idle stream will depend on root stream * (stream 0) with weight 16. * - * If stream denoted by ``pri_spec->stream_id`` is not found, we use - * default priority instead of given |pri_spec|. That is make stream - * depend on root stream with weight 16. + * Otherwise, if stream denoted by ``pri_spec->stream_id`` is not + * found, we use default priority instead of given |pri_spec|. That + * is make stream depend on root stream with weight 16. * * This function returns 0 if it succeeds, or one of the following * negative error codes: From e1e7840b2cfeaa87e5504329ac9fdb1ee8f9e6a6 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 29 Nov 2015 19:08:17 +0900 Subject: [PATCH 52/62] doc: Fix broken layout in ascii art --- doc/nghttpx.h2r | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/nghttpx.h2r b/doc/nghttpx.h2r index 8901ae7a..abbc4c9a 100644 --- a/doc/nghttpx.h2r +++ b/doc/nghttpx.h2r @@ -149,7 +149,9 @@ has to deploy key generator program to update keys frequently (e.g., every 1 hour). The example key generator tlsticketupdate.go is available under contrib directory in nghttp2 archive. The memcached entry key is ``nghttpx:tls-ticket-key``. The data format stored in -memcached is the binary format described below:: +memcached is the binary format described below: + +.. code-block:: text +--------------+-------+----------------+ | VERSION (4) |LEN (2)|KEY(48 or 80) ... From e01d213636c7da5601d8c5af7e94a3a3d43229ec Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 29 Nov 2015 19:12:53 +0900 Subject: [PATCH 53/62] Update man pages --- doc/h2load.1 | 2 +- doc/nghttp.1 | 2 +- doc/nghttpd.1 | 2 +- doc/nghttpx.1 | 2 +- doc/nghttpx.1.rst | 4 +++- 5 files changed, 7 insertions(+), 5 deletions(-) diff --git a/doc/h2load.1 b/doc/h2load.1 index 797dace2..241d6505 100644 --- a/doc/h2load.1 +++ b/doc/h2load.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "H2LOAD" "1" "November 26, 2015" "1.5.0" "nghttp2" +.TH "H2LOAD" "1" "November 29, 2015" "1.5.1-DEV" "nghttp2" .SH NAME h2load \- HTTP/2 benchmarking tool . diff --git a/doc/nghttp.1 b/doc/nghttp.1 index a5781f59..b246da69 100644 --- a/doc/nghttp.1 +++ b/doc/nghttp.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTP" "1" "November 26, 2015" "1.5.0" "nghttp2" +.TH "NGHTTP" "1" "November 29, 2015" "1.5.1-DEV" "nghttp2" .SH NAME nghttp \- HTTP/2 client . diff --git a/doc/nghttpd.1 b/doc/nghttpd.1 index 99be2733..ad8d93da 100644 --- a/doc/nghttpd.1 +++ b/doc/nghttpd.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTPD" "1" "November 26, 2015" "1.5.0" "nghttp2" +.TH "NGHTTPD" "1" "November 29, 2015" "1.5.1-DEV" "nghttp2" .SH NAME nghttpd \- HTTP/2 server . diff --git a/doc/nghttpx.1 b/doc/nghttpx.1 index 9c22cd4b..a0982c49 100644 --- a/doc/nghttpx.1 +++ b/doc/nghttpx.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTPX" "1" "November 26, 2015" "1.5.0" "nghttp2" +.TH "NGHTTPX" "1" "November 29, 2015" "1.5.1-DEV" "nghttp2" .SH NAME nghttpx \- HTTP/2 proxy . diff --git a/doc/nghttpx.1.rst b/doc/nghttpx.1.rst index a644406c..e87377cd 100644 --- a/doc/nghttpx.1.rst +++ b/doc/nghttpx.1.rst @@ -1048,7 +1048,9 @@ has to deploy key generator program to update keys frequently (e.g., every 1 hour). The example key generator tlsticketupdate.go is available under contrib directory in nghttp2 archive. The memcached entry key is ``nghttpx:tls-ticket-key``. The data format stored in -memcached is the binary format described below:: +memcached is the binary format described below: + +.. code-block:: text +--------------+-------+----------------+ | VERSION (4) |LEN (2)|KEY(48 or 80) ... From cbad05e0de1ff001154ed447d271ec171bcba862 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 1 Dec 2015 21:21:50 +0900 Subject: [PATCH 54/62] src: Fix compile error with gcc-4.7 --- src/util.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/util.cc b/src/util.cc index 90ace89a..40066588 100644 --- a/src/util.cc +++ b/src/util.cc @@ -1244,8 +1244,13 @@ int read_mime_types(std::map &res, break; } ext_end = std::find_if(ext_start, std::end(line), delim_pred); +#ifdef HAVE_STD_MAP_EMPLACE res.emplace(std::string(ext_start, ext_end), std::string(std::begin(line), type_end)); +#else // !HAVE_STD_MAP_EMPLACE + res.insert(std::make_pair(std::string(ext_start, ext_end), + std::string(std::begin(line), type_end))); +#endif // !HAVE_STD_MAP_EMPLACE } } From d1d1c83e56a542fdb9e0e4f0f74d26ed292bad71 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 1 Dec 2015 22:29:30 +0900 Subject: [PATCH 55/62] h2load: Fix broken connection times --- src/h2load.cc | 92 +++++++++++++++++++++++++++++++++------------------ src/h2load.h | 31 +++++++++++------ 2 files changed, 79 insertions(+), 44 deletions(-) diff --git a/src/h2load.cc b/src/h2load.cc index c868906a..dcd4e4e6 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -71,6 +71,12 @@ using namespace nghttp2; namespace h2load { +namespace { +bool recorded(const std::chrono::steady_clock::time_point &t) { + return std::chrono::steady_clock::duration::zero() != t.time_since_epoch(); +} +} // namespace + Config::Config() : data_length(-1), addrs(nullptr), nreqs(1), nclients(1), nthreads(1), max_concurrent_streams(-1), window_bits(30), connection_window_bits(30), @@ -92,11 +98,11 @@ Config config; RequestStat::RequestStat() : data_offset(0), completed(false) {} -Stats::Stats(size_t req_todo) +Stats::Stats(size_t req_todo, size_t nclients) : req_todo(0), req_started(0), req_done(0), req_success(0), req_status_success(0), req_failed(0), req_error(0), req_timedout(0), bytes_total(0), bytes_head(0), bytes_head_decomp(0), bytes_body(0), - status(), req_stats(req_todo) {} + status(), req_stats(req_todo), client_stats(nclients) {} Stream::Stream() : status_success(-1) {} @@ -149,7 +155,8 @@ void rate_period_timeout_w_cb(struct ev_loop *loop, ev_timer *w, int revents) { ++req_todo; --worker->nreqs_rem; } - worker->clients.push_back(make_unique(worker, req_todo)); + worker->clients.push_back( + make_unique(worker->next_client_id++, worker, req_todo)); auto &client = worker->clients.back(); if (client->connect() != 0) { std::cerr << "client could not connect to host" << std::endl; @@ -232,11 +239,11 @@ void client_request_timeout_cb(struct ev_loop *loop, ev_timer *w, int revents) { } } // namespace -Client::Client(Worker *worker, size_t req_todo) +Client::Client(uint32_t id, Worker *worker, size_t req_todo) : worker(worker), ssl(nullptr), next_addr(config.addrs), - current_addr(nullptr), reqidx(0), state(CLIENT_IDLE), - first_byte_received(false), req_todo(req_todo), req_started(0), - req_done(0), fd(-1), new_connection_requested(false) { + current_addr(nullptr), reqidx(0), state(CLIENT_IDLE), req_todo(req_todo), + req_started(0), req_done(0), id(id), fd(-1), + new_connection_requested(false) { ev_io_init(&wev, writecb, 0, EV_WRITE); ev_io_init(&rev, readcb, 0, EV_READ); @@ -302,7 +309,8 @@ int Client::make_socket(addrinfo *addr) { int Client::connect() { int rv; - record_start_time(&worker->stats); + clear_connect_times(); + record_connect_start_time(); if (worker->config->conn_inactivity_timeout > 0.) { ev_timer_again(worker->loop, &conn_inactivity_watcher); @@ -732,7 +740,7 @@ int Client::connection_made() { session->on_connect(); - record_connect_time(&worker->stats); + record_connect_time(); if (!config.timing_script) { auto nreq = @@ -971,21 +979,31 @@ void Client::record_request_time(RequestStat *req_stat) { req_stat->request_time = std::chrono::steady_clock::now(); } -void Client::record_start_time(Stats *stat) { - stat->start_times.push_back(std::chrono::steady_clock::now()); +void Client::record_connect_start_time() { + auto &cstat = worker->stats.client_stats[id]; + cstat.connect_start_time = std::chrono::steady_clock::now(); } -void Client::record_connect_time(Stats *stat) { - stat->connect_times.push_back(std::chrono::steady_clock::now()); +void Client::record_connect_time() { + auto &cstat = worker->stats.client_stats[id]; + cstat.connect_time = std::chrono::steady_clock::now(); } void Client::record_ttfb() { - if (first_byte_received) { + auto &cstat = worker->stats.client_stats[id]; + if (recorded(cstat.ttfb)) { return; } - first_byte_received = true; - worker->stats.ttfbs.push_back(std::chrono::steady_clock::now()); + cstat.ttfb = std::chrono::steady_clock::now(); +} + +void Client::clear_connect_times() { + auto &cstat = worker->stats.client_stats[id]; + + cstat.connect_start_time = std::chrono::steady_clock::time_point(); + cstat.connect_time = std::chrono::steady_clock::time_point(); + cstat.ttfb = std::chrono::steady_clock::time_point(); } void Client::signal_write() { ev_io_start(worker->loop, &wev); } @@ -994,10 +1012,11 @@ void Client::try_new_connection() { new_connection_requested = true; } Worker::Worker(uint32_t id, SSL_CTX *ssl_ctx, size_t req_todo, size_t nclients, size_t rate, Config *config) - : stats(req_todo), loop(ev_loop_new(0)), ssl_ctx(ssl_ctx), config(config), - id(id), tls_info_report_done(false), app_info_report_done(false), - nconns_made(0), nclients(nclients), nreqs_per_client(req_todo / nclients), - nreqs_rem(req_todo % nclients), rate(rate) { + : stats(req_todo, nclients), loop(ev_loop_new(0)), ssl_ctx(ssl_ctx), + config(config), id(id), tls_info_report_done(false), + app_info_report_done(false), nconns_made(0), nclients(nclients), + nreqs_per_client(req_todo / nclients), nreqs_rem(req_todo % nclients), + rate(rate), next_client_id(0) { stats.req_todo = req_todo; progress_interval = std::max(static_cast(1), req_todo / 10); @@ -1013,7 +1032,7 @@ Worker::Worker(uint32_t id, SSL_CTX *ssl_ctx, size_t req_todo, size_t nclients, ++req_todo; --nreqs_rem; } - clients.push_back(make_unique(this, req_todo)); + clients.push_back(make_unique(next_client_id++, this, req_todo)); } } } @@ -1099,17 +1118,17 @@ TimeStat compute_time_stat(const std::vector &samples) { namespace { TimeStats process_time_stats(const std::vector> &workers) { - size_t nrequest_times = 0, nttfb_times = 0; + size_t nrequest_times = 0; for (const auto &w : workers) { nrequest_times += w->stats.req_stats.size(); - nttfb_times += w->stats.ttfbs.size(); } std::vector request_times; request_times.reserve(nrequest_times); + std::vector connect_times, ttfb_times; - connect_times.reserve(nttfb_times); - ttfb_times.reserve(nttfb_times); + connect_times.reserve(config.nclients); + ttfb_times.reserve(config.nclients); for (const auto &w : workers) { for (const auto &req_stat : w->stats.req_stats) { @@ -1122,18 +1141,25 @@ process_time_stats(const std::vector> &workers) { } const auto &stat = w->stats; - // rule out cases where we started but didn't connect or get the - // first byte (errors). We will get connect event before FFTB. - assert(stat.start_times.size() >= stat.ttfbs.size()); - assert(stat.connect_times.size() >= stat.ttfbs.size()); - for (size_t i = 0; i < stat.ttfbs.size(); ++i) { + + for (const auto &cstat : stat.client_stats) { + // We will get connect event before FFTB. + if (!recorded(cstat.connect_start_time) || + !recorded(cstat.connect_time)) { + continue; + } + connect_times.push_back( std::chrono::duration_cast( - stat.connect_times[i] - stat.start_times[i])); + cstat.connect_time - cstat.connect_start_time)); + + if (!recorded(cstat.ttfb)) { + continue; + } ttfb_times.push_back( std::chrono::duration_cast( - stat.ttfbs[i] - stat.start_times[i])); + cstat.ttfb - cstat.connect_start_time)); } } @@ -2114,7 +2140,7 @@ int main(int argc, char **argv) { auto duration = std::chrono::duration_cast(end - start); - Stats stats(0); + Stats stats(0, 0); for (const auto &w : workers) { const auto &s = w->stats; diff --git a/src/h2load.h b/src/h2load.h index fe6c99ff..3b7c16cb 100644 --- a/src/h2load.h +++ b/src/h2load.h @@ -124,6 +124,15 @@ struct RequestStat { bool completed; }; +struct ClientStat { + // time connect starts + std::chrono::steady_clock::time_point connect_start_time; + // time to connect + std::chrono::steady_clock::time_point connect_time; + // time to first byte (TTFB) + std::chrono::steady_clock::time_point ttfb; +}; + template struct TimeStat { // min, max, mean and sd (standard deviation) Duration min, max, mean, sd; @@ -143,7 +152,7 @@ struct TimeStats { enum TimeStatType { STAT_REQUEST, STAT_CONNECT, STAT_FIRST_BYTE }; struct Stats { - Stats(size_t req_todo); + Stats(size_t req_todo, size_t nclients); // The total number of requests size_t req_todo; // The number of requests issued so far @@ -179,12 +188,8 @@ struct Stats { std::array status; // The statistics per request std::vector req_stats; - // time connect starts - std::vector start_times; - // time to connect - std::vector connect_times; - // time to first byte (TTFB) - std::vector ttfbs; + // THe statistics per client + std::vector client_stats; }; enum ClientState { CLIENT_IDLE, CLIENT_CONNECTED }; @@ -210,6 +215,8 @@ struct Worker { size_t nreqs_rem; size_t rate; ev_timer timeout_watcher; + // The next client ID this worker assigns + uint32_t next_client_id; Worker(uint32_t id, SSL_CTX *ssl_ctx, size_t nreq_todo, size_t nclients, size_t rate, Config *config); @@ -240,13 +247,14 @@ struct Client { addrinfo *current_addr; size_t reqidx; ClientState state; - bool first_byte_received; // The number of requests this client has to issue. size_t req_todo; // The number of requests this client has issued so far. size_t req_started; // The number of requests this client has done so far. size_t req_done; + // The client id per worker + uint32_t id; int fd; Buffer<64_k> wb; ev_timer conn_active_watcher; @@ -256,7 +264,7 @@ struct Client { enum { ERR_CONNECT_FAIL = -100 }; - Client(Worker *worker, size_t req_todo); + Client(uint32_t id, Worker *worker, size_t req_todo); ~Client(); int make_socket(addrinfo *addr); int connect(); @@ -302,9 +310,10 @@ struct Client { bool final = false); void record_request_time(RequestStat *req_stat); - void record_start_time(Stats *stat); - void record_connect_time(Stats *stat); + void record_connect_start_time(); + void record_connect_time(); void record_ttfb(); + void clear_connect_times(); void signal_write(); }; From 6beaf4d9f325fa1766a046b0cf2cd69bf1fca3e0 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 1 Dec 2015 23:49:28 +0900 Subject: [PATCH 56/62] h2load: Add req/s min, max, mean and sd for clients --- doc/h2load.h2r | 15 +++++++++ src/h2load.cc | 88 ++++++++++++++++++++++++++++++++++-------------- src/h2load.h | 30 ++++++++++++----- src/util.cc | 14 ++++++++ src/util.h | 3 ++ src/util_test.cc | 9 +++++ 6 files changed, 125 insertions(+), 34 deletions(-) diff --git a/doc/h2load.h2r b/doc/h2load.h2r index 60826931..e022aa8d 100644 --- a/doc/h2load.h2r +++ b/doc/h2load.h2r @@ -86,6 +86,21 @@ time for 1st byte (of (decrypted in case of TLS) application data) deviation range (mean +/- sd) against total number of successful connections. +req/s (client) + min + The minimum request per second among all clients. + max + The maximum request per second among all clients. + mean + The mean request per second among all clients. + sd + The standard deviation of request per second among all clients. + server. + +/- sd + The fraction of the number of connections within standard + deviation range (mean +/- sd) against total number of successful + connections. + FLOW CONTROL ------------ diff --git a/src/h2load.cc b/src/h2load.cc index dcd4e4e6..3f642eb6 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -309,6 +309,7 @@ int Client::make_socket(addrinfo *addr) { int Client::connect() { int rv; + record_client_start_time(); clear_connect_times(); record_connect_start_time(); @@ -391,6 +392,8 @@ void Client::fail() { } void Client::disconnect() { + record_client_end_time(); + ev_timer_stop(worker->loop, &conn_inactivity_watcher); ev_timer_stop(worker->loop, &conn_active_watcher); ev_timer_stop(worker->loop, &request_timeout_watcher); @@ -610,6 +613,8 @@ void Client::on_stream_close(int32_t stream_id, bool success, if (success) { req_stat->completed = true; ++worker->stats.req_success; + auto &cstat = worker->stats.client_stats[id]; + ++cstat.req_success; } ++worker->stats.req_done; ++req_done; @@ -1006,6 +1011,26 @@ void Client::clear_connect_times() { cstat.ttfb = std::chrono::steady_clock::time_point(); } +void Client::record_client_start_time() { + auto &cstat = worker->stats.client_stats[id]; + + // Record start time only once at the very first connection is going + // to be made. + if (recorded(cstat.client_start_time)) { + return; + } + + cstat.client_start_time = std::chrono::steady_clock::now(); +} + +void Client::record_client_end_time() { + auto &cstat = worker->stats.client_stats[id]; + + // Unlike client_start_time, we overwrite client_end_time. This + // handles multiple connect/disconnect for HTTP/1.1 benchmark. + cstat.client_end_time = std::chrono::steady_clock::now(); +} + void Client::signal_write() { ev_io_start(worker->loop, &wev); } void Client::try_new_connection() { new_connection_requested = true; } @@ -1065,9 +1090,7 @@ void Worker::run() { namespace { // Returns percentage of number of samples within mean +/- sd. -template -double within_sd(const std::vector &samples, const Duration &mean, - const Duration &sd) { +double within_sd(const std::vector &samples, double mean, double sd) { if (samples.size() == 0) { return 0.0; } @@ -1075,7 +1098,7 @@ double within_sd(const std::vector &samples, const Duration &mean, auto upper = mean + sd; auto m = std::count_if( std::begin(samples), std::end(samples), - [&lower, &upper](const Duration &t) { return lower <= t && t <= upper; }); + [&lower, &upper](double t) { return lower <= t && t <= upper; }); return (m / static_cast(samples.size())) * 100; } } // namespace @@ -1083,32 +1106,31 @@ double within_sd(const std::vector &samples, const Duration &mean, namespace { // Computes statistics using |samples|. The min, max, mean, sd, and // percentage of number of samples within mean +/- sd are computed. -template -TimeStat compute_time_stat(const std::vector &samples) { +SDStat compute_time_stat(const std::vector &samples) { if (samples.empty()) { - return {Duration::zero(), Duration::zero(), Duration::zero(), - Duration::zero(), 0.0}; + return {0.0, 0.0, 0.0, 0.0, 0.0}; } // standard deviation calculated using Rapid calculation method: // http://en.wikipedia.org/wiki/Standard_deviation#Rapid_calculation_methods double a = 0, q = 0; size_t n = 0; - int64_t sum = 0; - auto res = TimeStat{Duration::max(), Duration::min()}; + double sum = 0; + auto res = SDStat{std::numeric_limits::max(), + std::numeric_limits::min()}; for (const auto &t : samples) { ++n; res.min = std::min(res.min, t); res.max = std::max(res.max, t); - sum += t.count(); + sum += t; - auto na = a + (t.count() - a) / n; - q += (t.count() - a) * (t.count() - na); + auto na = a + (t - a) / n; + q += (t - a) * (t - na); a = na; } assert(n > 0); - res.mean = Duration(sum / n); - res.sd = Duration(static_cast(sqrt(q / n))); + res.mean = sum / n; + res.sd = sqrt(q / n); res.within_sd = within_sd(samples, res.mean, res.sd); return res; @@ -1116,19 +1138,20 @@ TimeStat compute_time_stat(const std::vector &samples) { } // namespace namespace { -TimeStats +SDStats process_time_stats(const std::vector> &workers) { size_t nrequest_times = 0; for (const auto &w : workers) { nrequest_times += w->stats.req_stats.size(); } - std::vector request_times; + std::vector request_times; request_times.reserve(nrequest_times); - std::vector connect_times, ttfb_times; + std::vector connect_times, ttfb_times, rps_values; connect_times.reserve(config.nclients); ttfb_times.reserve(config.nclients); + rps_values.reserve(config.nclients); for (const auto &w : workers) { for (const auto &req_stat : w->stats.req_stats) { @@ -1136,13 +1159,22 @@ process_time_stats(const std::vector> &workers) { continue; } request_times.push_back( - std::chrono::duration_cast( - req_stat.stream_close_time - req_stat.request_time)); + std::chrono::duration_cast>( + req_stat.stream_close_time - req_stat.request_time).count()); } const auto &stat = w->stats; for (const auto &cstat : stat.client_stats) { + if (recorded(cstat.client_start_time) && + recorded(cstat.client_end_time)) { + auto t = std::chrono::duration_cast>( + cstat.client_end_time - cstat.client_start_time).count(); + if (t > 1e-9) { + rps_values.push_back(cstat.req_success / t); + } + } + // We will get connect event before FFTB. if (!recorded(cstat.connect_start_time) || !recorded(cstat.connect_time)) { @@ -1150,21 +1182,21 @@ process_time_stats(const std::vector> &workers) { } connect_times.push_back( - std::chrono::duration_cast( - cstat.connect_time - cstat.connect_start_time)); + std::chrono::duration_cast>( + cstat.connect_time - cstat.connect_start_time).count()); if (!recorded(cstat.ttfb)) { continue; } ttfb_times.push_back( - std::chrono::duration_cast( - cstat.ttfb - cstat.connect_start_time)); + std::chrono::duration_cast>( + cstat.ttfb - cstat.connect_start_time).count()); } } return {compute_time_stat(request_times), compute_time_stat(connect_times), - compute_time_stat(ttfb_times)}; + compute_time_stat(ttfb_times), compute_time_stat(rps_values)}; } } // namespace @@ -2220,7 +2252,11 @@ time for request: )" << std::setw(10) << util::format_duration(ts.request.min) << util::format_duration(ts.ttfb.max) << " " << std::setw(10) << util::format_duration(ts.ttfb.mean) << " " << std::setw(10) << util::format_duration(ts.ttfb.sd) << std::setw(9) - << util::dtos(ts.ttfb.within_sd) << "%" << std::endl; + << util::dtos(ts.ttfb.within_sd) << "%" + << "\nreq/s (client) : " << std::setw(10) << ts.rps.min << " " + << std::setw(10) << ts.rps.max << " " << std::setw(10) + << ts.rps.mean << " " << std::setw(10) << ts.rps.sd << std::setw(9) + << util::dtos(ts.rps.within_sd) << "%" << std::endl; SSL_CTX_free(ssl_ctx); diff --git a/src/h2load.h b/src/h2load.h index 3b7c16cb..a7e07ea4 100644 --- a/src/h2load.h +++ b/src/h2load.h @@ -125,6 +125,18 @@ struct RequestStat { }; struct ClientStat { + // time client started (i.e., first connect starts) + std::chrono::steady_clock::time_point client_start_time; + // time client end (i.e., client somehow processed all requests it + // is responsible for, and disconnected) + std::chrono::steady_clock::time_point client_end_time; + // The number of requests completed successfull, but not necessarily + // means successful HTTP status code. + size_t req_success; + + // The following 3 numbers are overwritten each time when connection + // is made. + // time connect starts std::chrono::steady_clock::time_point connect_start_time; // time to connect @@ -133,24 +145,24 @@ struct ClientStat { std::chrono::steady_clock::time_point ttfb; }; -template struct TimeStat { +struct SDStat { // min, max, mean and sd (standard deviation) - Duration min, max, mean, sd; + double min, max, mean, sd; // percentage of samples inside mean -/+ sd double within_sd; }; -struct TimeStats { +struct SDStats { // time for request - TimeStat request; + SDStat request; // time for connect - TimeStat connect; + SDStat connect; // time to first byte (TTFB) - TimeStat ttfb; + SDStat ttfb; + // request per second for each client + SDStat rps; }; -enum TimeStatType { STAT_REQUEST, STAT_CONNECT, STAT_FIRST_BYTE }; - struct Stats { Stats(size_t req_todo, size_t nclients); // The total number of requests @@ -314,6 +326,8 @@ struct Client { void record_connect_time(); void record_ttfb(); void clear_connect_times(); + void record_client_start_time(); + void record_client_end_time(); void signal_write(); }; diff --git a/src/util.cc b/src/util.cc index 40066588..a2d649ef 100644 --- a/src/util.cc +++ b/src/util.cc @@ -1092,6 +1092,20 @@ std::string format_duration(const std::chrono::microseconds &u) { return dtos(static_cast(t) / d) + unit; } +std::string format_duration(double t) { + const char *unit = "us"; + if (t >= 1.) { + unit = "s"; + } else if (t >= 0.001) { + t *= 1000.; + unit = "ms"; + } else { + t *= 1000000.; + return utos(static_cast(t)) + unit; + } + return dtos(t) + unit; +} + std::string dtos(double n) { auto f = utos(static_cast(round(100. * n)) % 100); return utos(static_cast(n)) + "." + (f.size() == 1 ? "0" : "") + f; diff --git a/src/util.h b/src/util.h index 9319e7cd..8efd7407 100644 --- a/src/util.h +++ b/src/util.h @@ -585,6 +585,9 @@ std::string duration_str(double t); // fractional digits follow. std::string format_duration(const std::chrono::microseconds &u); +// Just like above, but this takes |t| as seconds. +std::string format_duration(double t); + // Creates "host:port" string using given |host| and |port|. If // |host| is numeric IPv6 address (e.g., ::1), it is enclosed by "[" // and "]". If |port| is 80 or 443, port part is omitted. diff --git a/src/util_test.cc b/src/util_test.cc index adc079f6..9732ff15 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -341,6 +341,15 @@ void test_util_format_duration(void) { util::format_duration(std::chrono::microseconds(1000000))); CU_ASSERT("1.05s" == util::format_duration(std::chrono::microseconds(1050000))); + + CU_ASSERT("0us" == util::format_duration(0.)); + CU_ASSERT("999us" == util::format_duration(0.000999)); + CU_ASSERT("1.00ms" == util::format_duration(0.001)); + CU_ASSERT("1.09ms" == util::format_duration(0.00109)); + CU_ASSERT("1.01ms" == util::format_duration(0.001009)); + CU_ASSERT("999.99ms" == util::format_duration(0.99999)); + CU_ASSERT("1.00s" == util::format_duration(1.)); + CU_ASSERT("1.05s" == util::format_duration(1.05)); } void test_util_starts_with(void) { From 2288ee8060b8243b96a4ce95be54ed2e91ff57a2 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 2 Dec 2015 21:16:30 +0900 Subject: [PATCH 57/62] Create stream object for pushed resource during nghttp2_submit_push_promise() Previously, stream object for pushed resource was not created during nghttp2_submit_push_promise(). It was created just before nghttp2_before_frame_send_callback was called for that PUSH_PROMISE frame. This means that application could not call nghttp2_submit_response for the pushed resource before nghttp2_before_frame_send_callback was called. This could be solved by callback chaining, but for web server with back pressure from backend stream, it is a bit unnecessarily hard to use. This commit changes nghttp2_submit_push_promise() behaviour so that stream object is created during that call. It makes application call nghttp2_submit_response right after successful nghttp2_submit_push_promise call. --- lib/includes/nghttp2/nghttp2.h | 14 +++++++--- lib/nghttp2_session.c | 51 +++++++++++++++++++--------------- lib/nghttp2_session.h | 2 +- tests/nghttp2_session_test.c | 24 ++++++++-------- 4 files changed, 52 insertions(+), 39 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 9914aab4..9dcd1157 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -3573,14 +3573,20 @@ NGHTTP2_EXTERN int nghttp2_submit_settings(nghttp2_session *session, * :enum:`NGHTTP2_ERR_INVALID_ARGUMENT` * The |stream_id| is 0; The |stream_id| does not designate stream * that peer initiated. + * :enum:`NGHTTP2_ERR_STREAM_CLOSED` + * The stream was alreay closed; or the |stream_id| is invalid. * * .. warning:: * * This function returns assigned promised stream ID if it succeeds. - * But that stream is not opened yet. The application must not - * submit frame to that stream ID before - * :type:`nghttp2_before_frame_send_callback` is called for this - * frame. + * As of 1.16.0, stream object for pushed resource is created when + * this function succeeds. In that case, the application can submit + * push response for the promised frame. + * + * In 1.15.0 or prior versions, pushed stream is not opened yet when + * this function succeeds. The application must not submit frame to + * that stream ID before :type:`nghttp2_before_frame_send_callback` + * is called for this frame. * */ NGHTTP2_EXTERN int32_t diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index fd007c93..7483c9b3 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -750,6 +750,31 @@ int nghttp2_session_add_item(nghttp2_session *session, nghttp2_outbound_queue_push(&session->ob_reg, item); item->queued = 1; break; + case NGHTTP2_PUSH_PROMISE: { + nghttp2_headers_aux_data *aux_data; + nghttp2_priority_spec pri_spec; + + aux_data = &item->aux_data.headers; + + if (!stream) { + return NGHTTP2_ERR_STREAM_CLOSED; + } + + nghttp2_priority_spec_init(&pri_spec, stream->stream_id, + NGHTTP2_DEFAULT_WEIGHT, 0); + + if (!nghttp2_session_open_stream( + session, frame->push_promise.promised_stream_id, + NGHTTP2_STREAM_FLAG_NONE, &pri_spec, NGHTTP2_STREAM_RESERVED, + aux_data->stream_user_data)) { + return NGHTTP2_ERR_NOMEM; + } + + nghttp2_outbound_queue_push(&session->ob_reg, item); + item->queued = 1; + + break; + } case NGHTTP2_WINDOW_UPDATE: if (stream) { stream->window_update_queued = 1; @@ -1903,30 +1928,8 @@ static int session_prep_frame(nghttp2_session *session, } case NGHTTP2_PUSH_PROMISE: { nghttp2_stream *stream; - nghttp2_headers_aux_data *aux_data; - nghttp2_priority_spec pri_spec; size_t estimated_payloadlen; - aux_data = &item->aux_data.headers; - - stream = nghttp2_session_get_stream(session, frame->hd.stream_id); - - /* stream could be NULL if associated stream was already - closed. */ - if (stream) { - nghttp2_priority_spec_init(&pri_spec, stream->stream_id, - NGHTTP2_DEFAULT_WEIGHT, 0); - } else { - nghttp2_priority_spec_default_init(&pri_spec); - } - - if (!nghttp2_session_open_stream( - session, frame->push_promise.promised_stream_id, - NGHTTP2_STREAM_FLAG_NONE, &pri_spec, NGHTTP2_STREAM_RESERVED, - aux_data->stream_user_data)) { - return NGHTTP2_ERR_NOMEM; - } - estimated_payloadlen = session_estimate_headers_payload( session, frame->push_promise.nva, frame->push_promise.nvlen, 0); @@ -1934,6 +1937,10 @@ static int session_prep_frame(nghttp2_session *session, return NGHTTP2_ERR_FRAME_SIZE_ERROR; } + /* stream could be NULL if associated stream was already + closed. */ + stream = nghttp2_session_get_stream(session, frame->hd.stream_id); + /* predicte should fail if stream is NULL. */ rv = session_predicate_push_promise_send(session, stream); if (rv != 0) { diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h index 179799a5..34c06e53 100644 --- a/lib/nghttp2_session.h +++ b/lib/nghttp2_session.h @@ -334,7 +334,7 @@ int nghttp2_session_is_my_stream_id(nghttp2_session *session, * NGHTTP2_ERR_NOMEM * Out of memory. * NGHTTP2_ERR_STREAM_CLOSED - * Stream already closed (DATA frame only) + * Stream already closed (DATA and PUSH_PROMISE frame only) */ int nghttp2_session_add_item(nghttp2_session *session, nghttp2_outbound_item *item); diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index aae1cb25..36f36d05 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -4554,28 +4554,28 @@ void test_nghttp2_submit_push_promise(void) { CU_ASSERT(2 == nghttp2_submit_push_promise(session, NGHTTP2_FLAG_NONE, 1, reqnv, ARRLEN(reqnv), &ud)); + stream = nghttp2_session_get_stream(session, 2); + + CU_ASSERT(NULL != stream); + CU_ASSERT(NGHTTP2_STREAM_RESERVED == stream->state); + CU_ASSERT(&ud == nghttp2_session_get_stream_user_data(session, 2)); + ud.frame_send_cb_called = 0; ud.sent_frame_type = 0; + CU_ASSERT(0 == nghttp2_session_send(session)); CU_ASSERT(1 == ud.frame_send_cb_called); CU_ASSERT(NGHTTP2_PUSH_PROMISE == ud.sent_frame_type); + stream = nghttp2_session_get_stream(session, 2); + CU_ASSERT(NGHTTP2_STREAM_RESERVED == stream->state); CU_ASSERT(&ud == nghttp2_session_get_stream_user_data(session, 2)); /* submit PUSH_PROMISE while associated stream is not opened */ - CU_ASSERT(4 == nghttp2_submit_push_promise(session, NGHTTP2_FLAG_NONE, 3, - reqnv, ARRLEN(reqnv), &ud)); - - ud.frame_not_send_cb_called = 0; - - CU_ASSERT(0 == nghttp2_session_send(session)); - CU_ASSERT(1 == ud.frame_not_send_cb_called); - CU_ASSERT(NGHTTP2_PUSH_PROMISE == ud.not_sent_frame_type); - - stream = nghttp2_session_get_stream(session, 4); - - CU_ASSERT(NULL == stream); + CU_ASSERT(NGHTTP2_ERR_STREAM_CLOSED == + nghttp2_submit_push_promise(session, NGHTTP2_FLAG_NONE, 3, reqnv, + ARRLEN(reqnv), &ud)); nghttp2_session_del(session); } From 93d8636fb007ee07558313b6bbfd560b982f4504 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 3 Dec 2015 22:48:41 +0900 Subject: [PATCH 58/62] Keep incoming streams only at server side We should only keep incoming closed streams because we only keep at most max concurrent streams, which only applied to incoming streams. --- lib/nghttp2_session.c | 10 +++++++--- lib/nghttp2_session.h | 10 ++++++---- tests/nghttp2_session_test.c | 19 ++++++++++++++++++- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 7483c9b3..938f27c9 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -1023,6 +1023,7 @@ int nghttp2_session_close_stream(nghttp2_session *session, int32_t stream_id, int rv; nghttp2_stream *stream; nghttp2_mem *mem; + int is_my_stream_id; mem = &session->mem; stream = nghttp2_session_get_stream(session, stream_id); @@ -1070,14 +1071,16 @@ int nghttp2_session_close_stream(nghttp2_session *session, int32_t stream_id, } } + is_my_stream_id = nghttp2_session_is_my_stream_id(session, stream_id); + /* pushed streams which is not opened yet is not counted toward max concurrent limits */ if ((stream->flags & NGHTTP2_STREAM_FLAG_PUSH)) { - if (!nghttp2_session_is_my_stream_id(session, stream_id)) { + if (!is_my_stream_id) { --session->num_incoming_reserved_streams; } } else { - if (nghttp2_session_is_my_stream_id(session, stream_id)) { + if (is_my_stream_id) { --session->num_outgoing_streams; } else { --session->num_incoming_streams; @@ -1087,7 +1090,8 @@ int nghttp2_session_close_stream(nghttp2_session *session, int32_t stream_id, /* Closes both directions just in case they are not closed yet */ stream->flags |= NGHTTP2_STREAM_FLAG_CLOSED; - if (session->server && nghttp2_stream_in_dep_tree(stream)) { + if (session->server && !is_my_stream_id && + nghttp2_stream_in_dep_tree(stream)) { /* On server side, retain stream at most MAX_CONCURRENT_STREAMS combined with the current active incoming streams to make dependency tree work better. */ diff --git a/lib/nghttp2_session.h b/lib/nghttp2_session.h index 34c06e53..dd69b5e3 100644 --- a/lib/nghttp2_session.h +++ b/lib/nghttp2_session.h @@ -190,11 +190,13 @@ struct nghttp2_session { updated when one frame was written. */ uint64_t last_cycle; void *user_data; - /* Points to the latest closed stream. NULL if there is no closed - stream. Only used when session is initialized as server. */ + /* Points to the latest incoming closed stream. NULL if there is no + closed stream. Only used when session is initialized as + server. */ nghttp2_stream *closed_stream_head; - /* Points to the oldest closed stream. NULL if there is no closed - stream. Only used when session is initialized as server. */ + /* Points to the oldest incoming closed stream. NULL if there is no + closed stream. Only used when session is initialized as + server. */ nghttp2_stream *closed_stream_tail; /* Points to the latest idle stream. NULL if there is no idle stream. Only used when session is initialized as server .*/ diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 36f36d05..1c896535 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -7378,7 +7378,8 @@ void test_nghttp2_session_stream_get_state(void) { stream = nghttp2_session_find_stream(session, 2); - CU_ASSERT(NGHTTP2_STREAM_STATE_CLOSED == nghttp2_stream_get_state(stream)); + /* At server, pushed stream object is not retained after closed */ + CU_ASSERT(NULL == stream); /* Push stream 4 associated to stream 5 */ rv = nghttp2_submit_push_promise(session, NGHTTP2_FLAG_NONE, 5, reqnv, @@ -7599,6 +7600,22 @@ void test_nghttp2_session_keep_closed_stream(void) { CU_ASSERT(NULL == session->closed_stream_tail); CU_ASSERT(NULL == session->closed_stream_head); + nghttp2_session_close_stream(session, 3, NGHTTP2_NO_ERROR); + + CU_ASSERT(1 == session->num_closed_streams); + CU_ASSERT(3 == session->closed_stream_head->stream_id); + + /* server initiated stream is not counted to max concurrent limit */ + open_stream(session, 2); + + CU_ASSERT(1 == session->num_closed_streams); + CU_ASSERT(3 == session->closed_stream_head->stream_id); + + nghttp2_session_close_stream(session, 2, NGHTTP2_NO_ERROR); + + CU_ASSERT(1 == session->num_closed_streams); + CU_ASSERT(3 == session->closed_stream_head->stream_id); + nghttp2_session_del(session); } From 478a423bcf2aaa2c9a79d2beb2b2e8c5d282d65d Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 3 Dec 2015 22:53:02 +0900 Subject: [PATCH 59/62] Reduce nghttp2_stream size --- lib/nghttp2_stream.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/nghttp2_stream.h b/lib/nghttp2_stream.h index 3417daeb..e2dddefa 100644 --- a/lib/nghttp2_stream.h +++ b/lib/nghttp2_stream.h @@ -146,6 +146,15 @@ struct nghttp2_stream { int64_t content_length; /* Received body so far */ int64_t recv_content_length; + /* Base last_cycle for direct descendent streams. */ + uint64_t descendant_last_cycle; + /* Next scheduled time to sent item */ + uint64_t cycle; + /* Next seq used for direct descendant streams */ + uint64_t descendant_next_seq; + /* Secondary key for prioritization to break a tie for cycle. This + value is monotonically increased for single parent stream. */ + uint64_t seq; /* pointers to form dependency tree. If multiple streams depend on a stream, only one stream (left most) has non-NULL dep_prev which points to the stream it depends on. The remaining streams are @@ -164,6 +173,8 @@ struct nghttp2_stream { void *stream_user_data; /* Item to send */ nghttp2_outbound_item *item; + /* Last written length of frame payload */ + size_t last_writelen; /* stream ID */ int32_t stream_id; /* Current remote window size. This value is computed against the @@ -202,17 +213,6 @@ struct nghttp2_stream { then its ancestors, except for root, are also queued. This invariant may break in fatal error condition. */ uint8_t queued; - /* Base last_cycle for direct descendent streams. */ - uint64_t descendant_last_cycle; - /* Next scheduled time to sent item */ - uint64_t cycle; - /* Next seq used for direct descendant streams */ - uint64_t descendant_next_seq; - /* Secondary key for prioritization to break a tie for cycle. This - value is monotonically increased for single parent stream. */ - uint64_t seq; - /* Last written length of frame payload */ - size_t last_writelen; /* This flag is used to reduce excessive queuing of WINDOW_UPDATE to this stream. The nonzero does not necessarily mean WINDOW_UPDATE is not queued. */ From a151a44caf92d8bc7ecca8d8ec4780fa6206be96 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 3 Dec 2015 23:40:37 +0900 Subject: [PATCH 60/62] Set max number of outgoing concurrent streams to 100 by default Instead of using nonsensical large value for max outgoing concurrent streams, use more sensible value, 100. --- lib/includes/nghttp2/nghttp2.h | 11 ++++++++++- lib/nghttp2_session.c | 3 +++ src/nghttp.cc | 3 +-- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 9dcd1157..5b7d8a28 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -609,7 +609,16 @@ typedef enum { /** * @macro - * Default maximum concurrent streams. + * + * Default maximum number of incoming concurrent streams. Use + * `nghttp2_submit_settings()` with + * :enum:`NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS` to change the + * maximum number of incoming concurrent streams. + * + * .. note:: + * + * The maximum number of outgoing concurrent streams is 100 by + * default. */ #define NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS ((1U << 31) - 1) diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 938f27c9..191e2720 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -372,6 +372,9 @@ static int session_new(nghttp2_session **session_ptr, (*session_ptr)->max_incoming_reserved_streams = NGHTTP2_MAX_INCOMING_RESERVED_STREAMS; + /* Limit max outgoing concurrent streams to sensible value */ + (*session_ptr)->remote_settings.max_concurrent_streams = 100; + if (option) { if ((option->opt_set_mask & NGHTTP2_OPT_NO_AUTO_WINDOW_UPDATE) && option->no_auto_window_update) { diff --git a/src/nghttp.cc b/src/nghttp.cc index 2a392f67..403f0fdd 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -95,8 +95,7 @@ constexpr auto anchors = std::array{{ Config::Config() : header_table_size(-1), min_header_table_size(std::numeric_limits::max()), padding(0), - max_concurrent_streams(100), - peer_max_concurrent_streams(NGHTTP2_INITIAL_MAX_CONCURRENT_STREAMS), + max_concurrent_streams(100), peer_max_concurrent_streams(100), weight(NGHTTP2_DEFAULT_WEIGHT), multiply(1), timeout(0.), window_bits(-1), connection_window_bits(-1), verbose(0), null_out(false), remote_name(false), get_assets(false), stat(false), upgrade(false), From 90b5a5856b84216e6f6d8ae88ee0db5e012e61bb Mon Sep 17 00:00:00 2001 From: Sunpoet Po-Chuan Hsieh Date: Fri, 4 Dec 2015 17:38:04 +0800 Subject: [PATCH 61/62] Fix build when OpenSSL 1.0.2 is not available --- src/asio_common.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/asio_common.cc b/src/asio_common.cc index bbcb7e6a..c8d148fa 100644 --- a/src/asio_common.cc +++ b/src/asio_common.cc @@ -177,9 +177,11 @@ bool tls_h2_negotiated(ssl_socket &socket) { unsigned int next_proto_len = 0; SSL_get0_next_proto_negotiated(ssl, &next_proto, &next_proto_len); +#if OPENSSL_VERSION_NUMBER >= 0x10002000L if (next_proto == nullptr) { SSL_get0_alpn_selected(ssl, &next_proto, &next_proto_len); } +#endif // OPENSSL_VERSION_NUMBER >= 0x10002000L if (next_proto == nullptr) { return false; From 7ca9ead36b43f1d0f5334d709061b7cf92fc154b Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 4 Dec 2015 23:08:37 +0900 Subject: [PATCH 62/62] nghttp: Update doc for default value of -M --- src/nghttp.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nghttp.cc b/src/nghttp.cc index 403f0fdd..2788c104 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -2448,7 +2448,7 @@ Options: -M, --peer-max-concurrent-streams= Use as SETTINGS_MAX_CONCURRENT_STREAMS value of remote endpoint as if it is received in SETTINGS frame. - The default is large enough as it is seen as unlimited. + Default: 100 -c, --header-table-size= Specify decoder header table size. If this option is used multiple times, and the minimum value among the