From be0d0e29956baed4384ddfac303c1e00de92f7cb Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 30 Oct 2015 22:48:27 +0900 Subject: [PATCH 01/23] Rename nghttp2_session_request_allowed as nghttp2_session_check_request_allowed --- doc/Makefile.am | 2 +- lib/includes/nghttp2/nghttp2.h | 3 ++- lib/nghttp2_session.c | 2 +- src/h2load_http2_session.cc | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/doc/Makefile.am b/doc/Makefile.am index 07ebb3c8..d3055c38 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -105,7 +105,7 @@ APIDOCS= \ nghttp2_session_mem_recv.rst \ nghttp2_session_mem_send.rst \ nghttp2_session_recv.rst \ - nghttp2_session_request_allowed.rst \ + nghttp2_session_check_request_allowed.rst \ nghttp2_session_resume_data.rst \ nghttp2_session_send.rst \ nghttp2_session_server_new.rst \ diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 09fa5d04..9aecc1dc 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -3494,7 +3494,8 @@ NGHTTP2_EXTERN int32_t * may return error. Or, request is failed to sent, and * :type:`nghttp2_on_stream_close_callback` is called. */ -NGHTTP2_EXTERN int nghttp2_session_request_allowed(nghttp2_session *session); +NGHTTP2_EXTERN int +nghttp2_session_check_request_allowed(nghttp2_session *session); /** * @function diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index bb103502..4b5e6a22 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -1322,7 +1322,7 @@ static int session_predicate_for_stream_send(nghttp2_session *session, return 0; } -int nghttp2_session_request_allowed(nghttp2_session *session) { +int nghttp2_session_check_request_allowed(nghttp2_session *session) { return !session->server && session->next_stream_id <= INT32_MAX && (session->goaway_flags & (NGHTTP2_GOAWAY_TERM_ON_SEND | NGHTTP2_GOAWAY_RECV)) == 0; diff --git a/src/h2load_http2_session.cc b/src/h2load_http2_session.cc index 43d2ac6b..00ff92fe 100644 --- a/src/h2load_http2_session.cc +++ b/src/h2load_http2_session.cc @@ -210,7 +210,7 @@ void Http2Session::on_connect() { } int Http2Session::submit_request(RequestStat *req_stat) { - if (nghttp2_session_request_allowed(session_) == 0) { + if (nghttp2_session_check_request_allowed(session_) == 0) { return -1; } From 58d636abdb76d5634825f7c4f492d26e2cd63cce Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 2 Nov 2015 23:38:29 +0900 Subject: [PATCH 02/23] Revert 918f8cca36c4349a3b03f834201f3c4e181b32d9 For first scheduling, we might ignore weight. This is OK since weight is just a distribution of resource, rather than strict priority ordering. --- lib/nghttp2_stream.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/nghttp2_stream.c b/lib/nghttp2_stream.c index 3a5e4ad0..1c67fe40 100644 --- a/lib/nghttp2_stream.c +++ b/lib/nghttp2_stream.c @@ -114,8 +114,7 @@ static int stream_subtree_active(nghttp2_stream *stream) { */ static uint64_t stream_next_cycle(nghttp2_stream *stream, uint64_t last_cycle) { return last_cycle + - (stream->last_writelen + 1) * NGHTTP2_MAX_WEIGHT / - (uint32_t)stream->weight; + stream->last_writelen * NGHTTP2_MAX_WEIGHT / (uint32_t)stream->weight; } static int stream_obq_push(nghttp2_stream *dep_stream, nghttp2_stream *stream) { From 0dc7fee7133d1cd2c7d5031c6d26cbba801d22e8 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 4 Nov 2015 01:03:35 +0900 Subject: [PATCH 03/23] h2load: Print "space savings" to measure header compression efficiency --- doc/h2load.h2r | 12 ++++++++---- src/h2load.cc | 16 ++++++++++++---- src/h2load.h | 6 +++++- src/h2load_http1_session.cc | 2 ++ src/h2load_http2_session.cc | 5 ++++- src/h2load_spdy_session.cc | 13 ++++++++++--- 6 files changed, 41 insertions(+), 13 deletions(-) diff --git a/doc/h2load.h2r b/doc/h2load.h2r index 14aa763e..86e57e08 100644 --- a/doc/h2load.h2r +++ b/doc/h2load.h2r @@ -33,10 +33,14 @@ traffic requests were made via TLS, this value is the number of decrpyted bytes. headers - The number of response header bytes from the server without - decompression. For HTTP/2, this is the sum of the payload of - HEADERS frame. For SPDY, this is the sum of the payload of - SYN_REPLY frame. + The number of response header bytes from the server without + decompression. The ``space savings`` shows efficiency of header + compression. Let ``decompressed(headers)`` to the number of bytes + used for header fields after decompression. The ``space savings`` + is calculated by (1 - ``headers`` / ``decompressed(headers)``) * + 100. For HTTP/1.1, this is usually 0.00%, since it does not have + header compression. For HTTP/2 and SPDY, it shows some insightful + numbers. data The number of response body bytes received from the server. diff --git a/src/h2load.cc b/src/h2load.cc index 0658d623..61e1112a 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -95,8 +95,8 @@ RequestStat::RequestStat() : data_offset(0), completed(false) {} Stats::Stats(size_t req_todo) : 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_body(0), status(), - req_stats(req_todo) {} + bytes_total(0), bytes_head(0), bytes_head_decomp(0), bytes_body(0), + status(), req_stats(req_todo) {} Stream::Stream() : status_success(-1) {} @@ -2073,6 +2073,7 @@ int main(int argc, char **argv) { stats.req_error += s.req_error; stats.bytes_total += s.bytes_total; stats.bytes_head += s.bytes_head; + stats.bytes_head_decomp += s.bytes_head_decomp; stats.bytes_body += s.bytes_body; for (size_t i = 0; i < stats.status.size(); ++i) { @@ -2102,7 +2103,13 @@ int main(int argc, char **argv) { bps = stats.bytes_total / secd.count(); } - std::cout << R"( + double header_space_savings = 0.; + if (stats.bytes_head_decomp > 0) { + header_space_savings = + 1. - static_cast(stats.bytes_head) / stats.bytes_head_decomp; + } + + std::cout << std::fixed << std::setprecision(2) << R"( finished in )" << util::format_duration(duration) << ", " << rps << " req/s, " << util::utos_with_funit(bps) << R"(B/s requests: )" << stats.req_todo << " total, " << stats.req_started @@ -2113,7 +2120,8 @@ requests: )" << stats.req_todo << " total, " << stats.req_started status codes: )" << stats.status[2] << " 2xx, " << stats.status[3] << " 3xx, " << stats.status[4] << " 4xx, " << stats.status[5] << R"( 5xx traffic: )" << stats.bytes_total << " bytes total, " << stats.bytes_head - << " bytes headers, " << stats.bytes_body << R"( bytes data + << " bytes headers (space savings " << header_space_savings * 100 + << "%), " << stats.bytes_body << R"( bytes data min max mean sd +/- sd time for request: )" << std::setw(10) << util::format_duration(ts.request.min) << " " << std::setw(10) << util::format_duration(ts.request.max) diff --git a/src/h2load.h b/src/h2load.h index 0b5f747b..fe6c99ff 100644 --- a/src/h2load.h +++ b/src/h2load.h @@ -166,8 +166,12 @@ struct Stats { // The number of bytes received on the "wire". If SSL/TLS is used, // this is the number of decrypted bytes the application received. int64_t bytes_total; - // The number of bytes received in HEADERS frame payload. + // The number of bytes received for header fields. This is + // compressed version. int64_t bytes_head; + // The number of bytes received for header fields after they are + // decompressed. + int64_t bytes_head_decomp; // The number of bytes received in DATA frame. int64_t bytes_body; // The number of each HTTP status category, status[i] is status code diff --git a/src/h2load_http1_session.cc b/src/h2load_http1_session.cc index 30691ed8..728233d3 100644 --- a/src/h2load_http1_session.cc +++ b/src/h2load_http1_session.cc @@ -99,6 +99,7 @@ int htp_hdr_keycb(http_parser *htp, const char *data, size_t len) { auto client = session->get_client(); client->worker->stats.bytes_head += len; + client->worker->stats.bytes_head_decomp += len; return 0; } } // namespace @@ -109,6 +110,7 @@ int htp_hdr_valcb(http_parser *htp, const char *data, size_t len) { auto client = session->get_client(); client->worker->stats.bytes_head += len; + client->worker->stats.bytes_head_decomp += len; return 0; } } // namespace diff --git a/src/h2load_http2_session.cc b/src/h2load_http2_session.cc index 00ff92fe..0bbbe8e8 100644 --- a/src/h2load_http2_session.cc +++ b/src/h2load_http2_session.cc @@ -51,6 +51,7 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, return 0; } client->on_header(frame->hd.stream_id, name, namelen, value, valuelen); + client->worker->stats.bytes_head_decomp += namelen + valuelen; return 0; } } // namespace @@ -63,7 +64,9 @@ int on_frame_recv_callback(nghttp2_session *session, const nghttp2_frame *frame, frame->headers.cat != NGHTTP2_HCAT_RESPONSE) { return 0; } - client->worker->stats.bytes_head += frame->hd.length; + client->worker->stats.bytes_head += + frame->hd.length - frame->headers.padlen - + ((frame->hd.flags & NGHTTP2_FLAG_PRIORITY) ? 5 : 0); if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { client->record_ttfb(); } diff --git a/src/h2load_spdy_session.cc b/src/h2load_spdy_session.cc index a7d52de7..a3b85591 100644 --- a/src/h2load_spdy_session.cc +++ b/src/h2load_spdy_session.cc @@ -65,11 +65,18 @@ void on_ctrl_recv_callback(spdylay_session *session, spdylay_frame_type type, for (auto p = frame->syn_reply.nv; *p; p += 2) { auto name = *p; auto value = *(p + 1); + auto namelen = strlen(name); + auto valuelen = strlen(value); client->on_header(frame->syn_reply.stream_id, - reinterpret_cast(name), strlen(name), - reinterpret_cast(value), strlen(value)); + reinterpret_cast(name), namelen, + reinterpret_cast(value), valuelen); + client->worker->stats.bytes_head_decomp += namelen + valuelen; } - client->worker->stats.bytes_head += frame->syn_reply.hd.length; + + // Strictly speaking, we have to subtract 2 (unused field) if SPDY + // version is 2. But it is already deprecated, and we don't do + // extra work for it. + client->worker->stats.bytes_head += frame->syn_reply.hd.length - 4; if (frame->syn_stream.hd.flags & SPDYLAY_CTRL_FLAG_FIN) { client->record_ttfb(); From cb73fa1d3b45e111dd8ab6da4ab028c9c54f25a4 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 5 Nov 2015 21:26:38 +0900 Subject: [PATCH 04/23] h2load: Return SSL_TLSEXT_ERR_NOACK if there is protocol list overlap in NPN --- src/h2load.cc | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/h2load.cc b/src/h2load.cc index 61e1112a..2ef99e78 100644 --- a/src/h2load.cc +++ b/src/h2load.cc @@ -1171,12 +1171,11 @@ int client_select_next_proto_cb(SSL *ssl, unsigned char **out, if (util::select_protocol(const_cast(out), outlen, in, inlen, config.npn_list)) { return SSL_TLSEXT_ERR_OK; - } else if (inlen == 0) { - std::cout - << "Server does not support NPN. Fallback behaviour may be activated." - << std::endl; } - return SSL_TLSEXT_ERR_OK; + + // OpenSSL will terminate handshake with fatal alert if we return + // NOACK. So there is no way to fallback. + return SSL_TLSEXT_ERR_NOACK; } } // namespace From 43b230685fc7376535c62e07327acc240d499984 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 5 Nov 2015 00:44:14 +0900 Subject: [PATCH 05/23] Introduce NGHTTP2_NV_FLAG_NO_COPY_NAME and NGHTTP2_NV_FLAG_NO_COPY_VALUE --- lib/includes/nghttp2/nghttp2.h | 84 +++++++++++++++++++++++++++++----- lib/nghttp2_frame.c | 56 +++++++++++++++-------- 2 files changed, 111 insertions(+), 29 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 9aecc1dc..b4d79429 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -430,7 +430,19 @@ typedef enum { * Header Field never Indexed" representation must be used in HPACK * encoding). Other implementation calls this bit as "sensitive". */ - NGHTTP2_NV_FLAG_NO_INDEX = 0x01 + NGHTTP2_NV_FLAG_NO_INDEX = 0x01, + /** + * This flag is set solely by application. If this flag is set, the + * library does not make a copy of header field name. This could + * improve performance. + */ + NGHTTP2_NV_FLAG_NO_COPY_NAME = 0x02, + /** + * This flag is set solely by application. If this flag is set, the + * library does not make a copy of header field value. This could + * improve performance. + */ + NGHTTP2_NV_FLAG_NO_COPY_VALUE = 0x04 } nghttp2_nv_flag; /** @@ -442,17 +454,27 @@ typedef struct { /** * The |name| byte string. If this struct is presented from library * (e.g., :type:`nghttp2_on_frame_recv_callback`), |name| is - * guaranteed to be NULL-terminated. When application is - * constructing this struct, |name| is not required to be + * guaranteed to be NULL-terminated. For some callbacks + * (:type:`nghttp2_before_frame_send_callback`, + * :type:`nghttp2_on_frame_send_callback`, and + * :type:`nghttp2_on_frame_not_send_callback`), it may not be + * NULL-terminated if header field is passed from application with + * the flag :enum:`NGHTTP2_NV_FLAG_NO_COPY_NAME`). When application + * is constructing this struct, |name| is not required to be * NULL-terminated. */ uint8_t *name; /** * The |value| byte string. If this struct is presented from * library (e.g., :type:`nghttp2_on_frame_recv_callback`), |value| - * is guaranteed to be NULL-terminated. When application is - * constructing this struct, |value| is not required to be - * NULL-terminated. + * is guaranteed to be NULL-terminated. For some callbacks + * (:type:`nghttp2_before_frame_send_callback`, + * :type:`nghttp2_on_frame_send_callback`, and + * :type:`nghttp2_on_frame_not_send_callback`), it may not be + * NULL-terminated if header field is passed from application with + * the flag :enum:`NGHTTP2_NV_FLAG_NO_COPY_VALUE`). When + * application is constructing this struct, |value| is not required + * to be NULL-terminated. */ uint8_t *value; /** @@ -2960,7 +2982,15 @@ nghttp2_priority_spec_check_default(const nghttp2_priority_spec *pri_spec); * * This function creates copies of all name/value pairs in |nva|. It * also lower-cases all names in |nva|. The order of elements in - * |nva| is preserved. + * |nva| is preserved. For header fields with + * :enum:`NGHTTP2_NV_FLAG_NO_COPY_NAME` and + * :enum:`NGHTTP2_NV_FLAG_NO_COPY_VALUE` are set, header field name + * and value are not copied respectively. With + * :enum:`NGHTTP2_NV_FLAG_NO_COPY_NAME`, application is responsible to + * pass header field name in lowercase. The application should + * maintain the references to them until + * :type:`nghttp2_on_frame_send_callback` or + * :type:`nghttp2_on_frame_not_send_callback` is called. * * HTTP/2 specification has requirement about header fields in the * request HEADERS. See the specification for more details. @@ -3016,7 +3046,15 @@ NGHTTP2_EXTERN int32_t * * This function creates copies of all name/value pairs in |nva|. It * also lower-cases all names in |nva|. The order of elements in - * |nva| is preserved. + * |nva| is preserved. For header fields with + * :enum:`NGHTTP2_NV_FLAG_NO_COPY_NAME` and + * :enum:`NGHTTP2_NV_FLAG_NO_COPY_VALUE` are set, header field name + * and value are not copied respectively. With + * :enum:`NGHTTP2_NV_FLAG_NO_COPY_NAME`, application is responsible to + * pass header field name in lowercase. The application should + * maintain the references to them until + * :type:`nghttp2_on_frame_send_callback` or + * :type:`nghttp2_on_frame_not_send_callback` is called. * * HTTP/2 specification has requirement about header fields in the * response HEADERS. See the specification for more details. @@ -3073,7 +3111,15 @@ nghttp2_submit_response(nghttp2_session *session, int32_t stream_id, * * This function creates copies of all name/value pairs in |nva|. It * also lower-cases all names in |nva|. The order of elements in - * |nva| is preserved. + * |nva| is preserved. For header fields with + * :enum:`NGHTTP2_NV_FLAG_NO_COPY_NAME` and + * :enum:`NGHTTP2_NV_FLAG_NO_COPY_VALUE` are set, header field name + * and value are not copied respectively. With + * :enum:`NGHTTP2_NV_FLAG_NO_COPY_NAME`, application is responsible to + * pass header field name in lowercase. The application should + * maintain the references to them until + * :type:`nghttp2_on_frame_send_callback` or + * :type:`nghttp2_on_frame_not_send_callback` is called. * * For server, trailer must be followed by response HEADERS or * response DATA. The library does not check that response HEADERS @@ -3149,7 +3195,15 @@ NGHTTP2_EXTERN int nghttp2_submit_trailer(nghttp2_session *session, * * This function creates copies of all name/value pairs in |nva|. It * also lower-cases all names in |nva|. The order of elements in - * |nva| is preserved. + * |nva| is preserved. For header fields with + * :enum:`NGHTTP2_NV_FLAG_NO_COPY_NAME` and + * :enum:`NGHTTP2_NV_FLAG_NO_COPY_VALUE` are set, header field name + * and value are not copied respectively. With + * :enum:`NGHTTP2_NV_FLAG_NO_COPY_NAME`, application is responsible to + * pass header field name in lowercase. The application should + * maintain the references to them until + * :type:`nghttp2_on_frame_send_callback` or + * :type:`nghttp2_on_frame_not_send_callback` is called. * * The |stream_user_data| is a pointer to an arbitrary data which is * associated to the stream this frame will open. Therefore it is @@ -3347,7 +3401,15 @@ NGHTTP2_EXTERN int nghttp2_submit_settings(nghttp2_session *session, * * This function creates copies of all name/value pairs in |nva|. It * also lower-cases all names in |nva|. The order of elements in - * |nva| is preserved. + * |nva| is preserved. For header fields with + * :enum:`NGHTTP2_NV_FLAG_NO_COPY_NAME` and + * :enum:`NGHTTP2_NV_FLAG_NO_COPY_VALUE` are set, header field name + * and value are not copied respectively. With + * :enum:`NGHTTP2_NV_FLAG_NO_COPY_NAME`, application is responsible to + * pass header field name in lowercase. The application should + * maintain the references to them until + * :type:`nghttp2_on_frame_send_callback` or + * :type:`nghttp2_on_frame_not_send_callback` is called. * * The |promised_stream_user_data| is a pointer to an arbitrary data * which is associated to the promised stream this frame will open and diff --git a/lib/nghttp2_frame.c b/lib/nghttp2_frame.c index e324b9c9..30b4f396 100644 --- a/lib/nghttp2_frame.c +++ b/lib/nghttp2_frame.c @@ -741,21 +741,26 @@ void nghttp2_nv_array_sort(nghttp2_nv *nva, size_t nvlen) { int nghttp2_nv_array_copy(nghttp2_nv **nva_ptr, const nghttp2_nv *nva, size_t nvlen, nghttp2_mem *mem) { size_t i; - uint8_t *data; + uint8_t *data = NULL; size_t buflen = 0; nghttp2_nv *p; - for (i = 0; i < nvlen; ++i) { - /* + 2 for null-termination */ - buflen += nva[i].namelen + nva[i].valuelen + 2; - } - if (nvlen == 0) { *nva_ptr = NULL; return 0; } + for (i = 0; i < nvlen; ++i) { + /* + 1 for null-termination */ + if ((nva[i].flags & NGHTTP2_NV_FLAG_NO_COPY_NAME) == 0) { + buflen += nva[i].namelen + 1; + } + if ((nva[i].flags & NGHTTP2_NV_FLAG_NO_COPY_VALUE) == 0) { + buflen += nva[i].valuelen + 1; + } + } + buflen += sizeof(nghttp2_nv) * nvlen; *nva_ptr = nghttp2_mem_malloc(mem, buflen); @@ -765,22 +770,37 @@ int nghttp2_nv_array_copy(nghttp2_nv **nva_ptr, const nghttp2_nv *nva, } p = *nva_ptr; - data = (uint8_t *)(*nva_ptr) + sizeof(nghttp2_nv) * nvlen; + + if (buflen > sizeof(nghttp2_nv) * nvlen) { + data = (uint8_t *)(*nva_ptr) + sizeof(nghttp2_nv) * nvlen; + } for (i = 0; i < nvlen; ++i) { p->flags = nva[i].flags; - memcpy(data, nva[i].name, nva[i].namelen); - p->name = data; - p->namelen = nva[i].namelen; - data[p->namelen] = '\0'; - nghttp2_downcase(p->name, p->namelen); - data += nva[i].namelen + 1; - memcpy(data, nva[i].value, nva[i].valuelen); - p->value = data; - p->valuelen = nva[i].valuelen; - data[p->valuelen] = '\0'; - data += nva[i].valuelen + 1; + if (nva[i].flags & NGHTTP2_NV_FLAG_NO_COPY_NAME) { + p->name = nva[i].name; + p->namelen = nva[i].namelen; + } else { + memcpy(data, nva[i].name, nva[i].namelen); + p->name = data; + p->namelen = nva[i].namelen; + data[p->namelen] = '\0'; + nghttp2_downcase(p->name, p->namelen); + data += nva[i].namelen + 1; + } + + if (nva[i].flags & NGHTTP2_NV_FLAG_NO_COPY_VALUE) { + p->value = nva[i].value; + p->valuelen = nva[i].valuelen; + } else { + memcpy(data, nva[i].value, nva[i].valuelen); + p->value = data; + p->valuelen = nva[i].valuelen; + data[p->valuelen] = '\0'; + data += nva[i].valuelen + 1; + } + ++p; } return 0; From ac4194653382dec203b831e77378295ccca2cf08 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 5 Nov 2015 22:48:54 +0900 Subject: [PATCH 06/23] nghttpx: Use NGHTTP2_NV_FLAG_NO_COPY_NAME and NGHTTP2_NV_FLAG_NO_COPY_VALUE For both HTTP/2 frontend and backend. Also adds http2::stringify_status to optimize status code serialization. --- src/http2.cc | 140 ++++++++++++++++++++++- src/http2.h | 30 ++++- src/http2_test.cc | 18 ++- src/shrpx_config.cc | 7 +- src/shrpx_downstream.cc | 37 +++++- src/shrpx_downstream.h | 8 +- src/shrpx_downstream_test.cc | 18 ++- src/shrpx_http2_downstream_connection.cc | 35 +++--- src/shrpx_http2_upstream.cc | 63 ++++++---- 9 files changed, 299 insertions(+), 57 deletions(-) diff --git a/src/http2.cc b/src/http2.cc index e978a58f..acac147a 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -132,6 +132,108 @@ std::string get_status_string(unsigned int status_code) { } } +const char *stringify_status(unsigned int status_code) { + switch (status_code) { + case 100: + return "100"; + case 101: + return "101"; + case 200: + return "200"; + case 201: + return "201"; + case 202: + return "202"; + case 203: + return "203"; + case 204: + return "204"; + case 205: + return "205"; + case 206: + return "206"; + case 300: + return "300"; + case 301: + return "301"; + case 302: + return "302"; + case 303: + return "303"; + case 304: + return "304"; + case 305: + return "305"; + // case 306: return "306"; + case 307: + return "307"; + case 308: + return "308"; + case 400: + return "400"; + case 401: + return "401"; + case 402: + return "402"; + case 403: + return "403"; + case 404: + return "404"; + case 405: + return "405"; + case 406: + return "406"; + case 407: + return "407"; + case 408: + return "408"; + case 409: + return "409"; + case 410: + return "410"; + case 411: + return "411"; + case 412: + return "412"; + case 413: + return "413"; + case 414: + return "414"; + case 415: + return "415"; + case 416: + return "416"; + case 417: + return "417"; + case 421: + return "421"; + case 426: + return "426"; + case 428: + return "428"; + case 429: + return "429"; + case 431: + return "431"; + case 500: + return "500"; + case 501: + return "501"; + case 502: + return "502"; + case 503: + return "503"; + case 504: + return "504"; + case 505: + return "505"; + case 511: + return "511"; + default: + return nullptr; + } +} + void capitalize(DefaultMemchunks *buf, const std::string &s) { buf->append(util::upcase(s[0])); for (size_t i = 1; i < s.size(); ++i) { @@ -207,17 +309,34 @@ bool non_empty_value(const Headers::value_type *nv) { return nv && !nv->value.empty(); } -nghttp2_nv make_nv(const std::string &name, const std::string &value, - bool no_index) { +namespace { +nghttp2_nv make_nv_internal(const std::string &name, const std::string &value, + bool no_index, uint8_t nv_flags) { uint8_t flags; - flags = no_index ? NGHTTP2_NV_FLAG_NO_INDEX : NGHTTP2_NV_FLAG_NONE; + flags = + nv_flags | (no_index ? NGHTTP2_NV_FLAG_NO_INDEX : NGHTTP2_NV_FLAG_NONE); return {(uint8_t *)name.c_str(), (uint8_t *)value.c_str(), name.size(), value.size(), flags}; } +} // namespace -void copy_headers_to_nva(std::vector &nva, const Headers &headers) { +nghttp2_nv make_nv(const std::string &name, const std::string &value, + bool no_index) { + return make_nv_internal(name, value, no_index, NGHTTP2_NV_FLAG_NONE); +} + +nghttp2_nv make_nv_nocopy(const std::string &name, const std::string &value, + bool no_index) { + return make_nv_internal(name, value, no_index, + NGHTTP2_NV_FLAG_NO_COPY_NAME | + NGHTTP2_NV_FLAG_NO_COPY_VALUE); +} + +namespace { +void copy_headers_to_nva_internal(std::vector &nva, + const Headers &headers, uint8_t nv_flags) { for (auto &kv : headers) { if (kv.name.empty() || kv.name[0] == ':') { continue; @@ -238,9 +357,20 @@ void copy_headers_to_nva(std::vector &nva, const Headers &headers) { case HD_X_FORWARDED_PROTO: continue; } - nva.push_back(make_nv(kv.name, kv.value, kv.no_index)); + nva.push_back(make_nv_internal(kv.name, kv.value, kv.no_index, nv_flags)); } } +} // namespace + +void copy_headers_to_nva(std::vector &nva, const Headers &headers) { + copy_headers_to_nva_internal(nva, headers, NGHTTP2_NV_FLAG_NONE); +} + +void copy_headers_to_nva_nocopy(std::vector &nva, + const Headers &headers) { + copy_headers_to_nva_internal(nva, headers, NGHTTP2_NV_FLAG_NO_COPY_NAME | + NGHTTP2_NV_FLAG_NO_COPY_VALUE); +} void build_http1_headers_from_headers(DefaultMemchunks *buf, const Headers &headers) { diff --git a/src/http2.h b/src/http2.h index e313f79f..2669dd49 100644 --- a/src/http2.h +++ b/src/http2.h @@ -70,6 +70,10 @@ namespace http2 { std::string get_status_string(unsigned int status_code); +// Returns string version of |status_code|. This function can handle +// only predefined status code. Otherwise, returns nullptr. +const char *stringify_status(unsigned int status_code); + void capitalize(DefaultMemchunks *buf, const std::string &s); // Returns true if |value| is LWS @@ -110,18 +114,27 @@ bool non_empty_value(const Headers::value_type *nv); nghttp2_nv make_nv(const std::string &name, const std::string &value, bool no_index = false); +nghttp2_nv make_nv_nocopy(const std::string &name, const std::string &value, + bool no_index = false); + // Create nghttp2_nv from string literal |name| and |value|. template constexpr nghttp2_nv make_nv_ll(const char (&name)[N], const char (&value)[M]) { return {(uint8_t *)name, (uint8_t *)value, N - 1, M - 1, - NGHTTP2_NV_FLAG_NONE}; + NGHTTP2_NV_FLAG_NO_COPY_NAME | NGHTTP2_NV_FLAG_NO_COPY_VALUE}; } // Create nghttp2_nv from string literal |name| and c-string |value|. template nghttp2_nv make_nv_lc(const char (&name)[N], const char *value) { return {(uint8_t *)name, (uint8_t *)value, N - 1, strlen(value), - NGHTTP2_NV_FLAG_NONE}; + NGHTTP2_NV_FLAG_NO_COPY_NAME}; +} + +template +nghttp2_nv make_nv_lc_nocopy(const char (&name)[N], const char *value) { + return {(uint8_t *)name, (uint8_t *)value, N - 1, strlen(value), + NGHTTP2_NV_FLAG_NO_COPY_NAME | NGHTTP2_NV_FLAG_NO_COPY_VALUE}; } // Create nghttp2_nv from string literal |name| and std::string @@ -129,7 +142,13 @@ nghttp2_nv make_nv_lc(const char (&name)[N], const char *value) { template nghttp2_nv make_nv_ls(const char (&name)[N], const std::string &value) { return {(uint8_t *)name, (uint8_t *)value.c_str(), N - 1, value.size(), - NGHTTP2_NV_FLAG_NONE}; + NGHTTP2_NV_FLAG_NO_COPY_NAME}; +} + +template +nghttp2_nv make_nv_ls_nocopy(const char (&name)[N], const std::string &value) { + return {(uint8_t *)name, (uint8_t *)value.c_str(), N - 1, value.size(), + NGHTTP2_NV_FLAG_NO_COPY_NAME | NGHTTP2_NV_FLAG_NO_COPY_VALUE}; } // Appends headers in |headers| to |nv|. |headers| must be indexed @@ -138,6 +157,11 @@ nghttp2_nv make_nv_ls(const char (&name)[N], const std::string &value) { // which require special handling (i.e. via), are not copied. void copy_headers_to_nva(std::vector &nva, const Headers &headers); +// Just like copy_headers_to_nva(), but this adds +// NGHTTP2_NV_FLAG_NO_COPY_NAME and NGHTTP2_NV_FLAG_NO_COPY_VALUE. +void copy_headers_to_nva_nocopy(std::vector &nva, + const Headers &headers); + // Appends HTTP/1.1 style header lines to |buf| from headers in // |headers|. |headers| must be indexed before this call (its // element's token field is assigned). Certain headers, which diff --git a/src/http2_test.cc b/src/http2_test.cc index ae4325b8..8f3aab35 100644 --- a/src/http2_test.cc +++ b/src/http2_test.cc @@ -151,10 +151,26 @@ auto headers = } // namespace void test_http2_copy_headers_to_nva(void) { + auto ans = std::vector{0, 1, 4, 5, 6, 7, 12}; std::vector nva; + + http2::copy_headers_to_nva_nocopy(nva, headers); + CU_ASSERT(7 == nva.size()); + for (size_t i = 0; i < ans.size(); ++i) { + check_nv(headers[ans[i]], &nva[i]); + + if (ans[i] == 0) { + CU_ASSERT((NGHTTP2_NV_FLAG_NO_COPY_NAME | NGHTTP2_NV_FLAG_NO_COPY_VALUE | + NGHTTP2_NV_FLAG_NO_INDEX) == nva[i].flags); + } else { + CU_ASSERT((NGHTTP2_NV_FLAG_NO_COPY_NAME | + NGHTTP2_NV_FLAG_NO_COPY_VALUE) == nva[i].flags); + } + } + + nva.clear(); http2::copy_headers_to_nva(nva, headers); CU_ASSERT(7 == nva.size()); - auto ans = std::vector{0, 1, 4, 5, 6, 7, 12}; for (size_t i = 0; i < ans.size(); ++i) { check_nv(headers[ans[i]], &nva[i]); diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index 247b2368..a2e35abf 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -288,7 +288,12 @@ std::pair parse_header(const char *optarg) { for (; *value == '\t' || *value == ' '; ++value) ; - return {std::string(optarg, colon), std::string(value, strlen(value))}; + auto p = std::make_pair(std::string(optarg, colon), + std::string(value, strlen(value))); + util::inp_strlower(p.first); + util::inp_strlower(p.second); + + return p; } template diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index f4d5498c..65f3e970 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -277,8 +277,33 @@ void Downstream::assemble_request_cookie() { } } -Headers Downstream::crumble_request_cookie() { - Headers cookie_hdrs; +size_t Downstream::count_crumble_request_cookie() { + size_t n = 0; + for (auto &kv : request_headers_) { + if (kv.name.size() != 6 || kv.name[5] != 'e' || + !util::streq_l("cooki", kv.name.c_str(), 5)) { + continue; + } + size_t last = kv.value.size(); + + for (size_t j = 0; j < last;) { + j = kv.value.find_first_not_of("\t ;", j); + if (j == std::string::npos) { + break; + } + + j = kv.value.find(';', j); + if (j == std::string::npos) { + j = last; + } + + ++n; + } + } + return n; +} + +void Downstream::crumble_request_cookie(std::vector &nva) { for (auto &kv : request_headers_) { if (kv.name.size() != 6 || kv.name[5] != 'e' || !util::streq_l("cooki", kv.name.c_str(), 5)) { @@ -298,11 +323,13 @@ Headers Downstream::crumble_request_cookie() { j = last; } - cookie_hdrs.push_back( - Header("cookie", kv.value.substr(first, j - first), kv.no_index)); + nva.push_back({(uint8_t *)"cookie", (uint8_t *)kv.value.c_str() + first, + str_size("cookie"), j - first, + (uint8_t)(NGHTTP2_NV_FLAG_NO_COPY_NAME | + NGHTTP2_NV_FLAG_NO_COPY_VALUE | + (kv.no_index ? NGHTTP2_NV_FLAG_NO_INDEX : 0))}); } } - return cookie_hdrs; } const std::string &Downstream::get_assembled_request_cookie() const { diff --git a/src/shrpx_downstream.h b/src/shrpx_downstream.h index 6960a236..ad659637 100644 --- a/src/shrpx_downstream.h +++ b/src/shrpx_downstream.h @@ -97,9 +97,11 @@ public: // downstream request API const Headers &get_request_headers() const; Headers &get_request_headers(); - // Crumbles (split cookie by ";") in request_headers_ and returns - // them. Headers::no_index is inherited. - Headers crumble_request_cookie(); + // Count number of crumbled cookies + size_t count_crumble_request_cookie(); + // Crumbles (split cookie by ";") in request_headers_ and adds them + // in |nva|. Headers::no_index is inherited. + void crumble_request_cookie(std::vector &nva); void assemble_request_cookie(); const std::string &get_assembled_request_cookie() const; // Lower the request header field names and indexes request headers. diff --git a/src/shrpx_downstream_test.cc b/src/shrpx_downstream_test.cc index 2e7a6960..871cac70 100644 --- a/src/shrpx_downstream_test.cc +++ b/src/shrpx_downstream_test.cc @@ -108,13 +108,29 @@ void test_downstream_crumble_request_cookie(void) { reinterpret_cast(val), strlen(val), true, -1); d.add_request_header("cookie", ";delta"); d.add_request_header("cookie", "echo"); - auto cookies = d.crumble_request_cookie(); + + std::vector nva; + d.crumble_request_cookie(nva); + + auto num_cookies = d.count_crumble_request_cookie(); + + CU_ASSERT(5 == nva.size()); + CU_ASSERT(5 == num_cookies); + + Headers cookies; + std::transform(std::begin(nva), std::end(nva), std::back_inserter(cookies), + [](const nghttp2_nv &nv) { + return Header(std::string(nv.name, nv.name + nv.namelen), + std::string(nv.value, nv.value + nv.valuelen), + nv.flags & NGHTTP2_NV_FLAG_NO_INDEX); + }); Headers ans = {{"cookie", "alpha"}, {"cookie", "bravo"}, {"cookie", "charlie"}, {"cookie", "delta"}, {"cookie", "echo"}}; + CU_ASSERT(ans == cookies); CU_ASSERT(cookies[0].no_index); CU_ASSERT(cookies[1].no_index); diff --git a/src/shrpx_http2_downstream_connection.cc b/src/shrpx_http2_downstream_connection.cc index 86e200f6..8a9a216f 100644 --- a/src/shrpx_http2_downstream_connection.cc +++ b/src/shrpx_http2_downstream_connection.cc @@ -205,6 +205,8 @@ ssize_t http2_data_read_callback(nghttp2_session *session, int32_t stream_id, if (!trailers.empty()) { std::vector nva; nva.reserve(trailers.size()); + // We cannot use nocopy version, since nva may be touched after + // Downstream object is deleted. http2::copy_headers_to_nva(nva, trailers); if (!nva.empty()) { rv = nghttp2_submit_trailer(session, stream_id, nva.data(), nva.size()); @@ -273,17 +275,13 @@ int Http2DownstreamConnection::push_request_headers() { authority = req_authority.c_str(); } - if (!authority) { - authority = downstream_hostport; - } - downstream_->set_request_downstream_host(authority); auto nheader = downstream_->get_request_headers().size(); - Headers cookies; + size_t num_cookies = 0; if (!get_config()->http2_no_cookie_crumbling) { - cookies = downstream_->crumble_request_cookie(); + num_cookies = downstream_->count_crumble_request_cookie(); } // 8 means: @@ -296,29 +294,30 @@ int Http2DownstreamConnection::push_request_headers() { // 7. x-forwarded-proto (optional) // 8. te (optional) auto nva = std::vector(); - nva.reserve(nheader + 8 + cookies.size() + + nva.reserve(nheader + 8 + num_cookies + get_config()->add_request_headers.size()); - nva.push_back(http2::make_nv_lc(":method", http2::to_method_string(method))); + nva.push_back( + http2::make_nv_lc_nocopy(":method", http2::to_method_string(method))); auto &scheme = downstream_->get_request_http2_scheme(); - nva.push_back(http2::make_nv_lc(":authority", authority)); + nva.push_back(http2::make_nv_lc_nocopy(":authority", authority)); if (method != HTTP_CONNECT) { assert(!scheme.empty()); - nva.push_back(http2::make_nv_ls(":scheme", scheme)); + nva.push_back(http2::make_nv_ls_nocopy(":scheme", scheme)); auto &path = downstream_->get_request_path(); if (method == HTTP_OPTIONS && path.empty()) { nva.push_back(http2::make_nv_ll(":path", "*")); } else { - nva.push_back(http2::make_nv_ls(":path", path)); + nva.push_back(http2::make_nv_ls_nocopy(":path", path)); } } - http2::copy_headers_to_nva(nva, downstream_->get_request_headers()); + http2::copy_headers_to_nva_nocopy(nva, downstream_->get_request_headers()); bool chunked_encoding = false; auto transfer_encoding = @@ -328,8 +327,8 @@ int Http2DownstreamConnection::push_request_headers() { chunked_encoding = true; } - for (auto &nv : cookies) { - nva.push_back(http2::make_nv(nv.name, nv.value, nv.no_index)); + if (!get_config()->http2_no_cookie_crumbling) { + downstream_->crumble_request_cookie(nva); } std::string xff_value; @@ -343,20 +342,20 @@ int Http2DownstreamConnection::push_request_headers() { downstream_->get_upstream()->get_client_handler()->get_ipaddr(); nva.push_back(http2::make_nv_ls("x-forwarded-for", xff_value)); } else if (xff && !get_config()->strip_incoming_x_forwarded_for) { - nva.push_back(http2::make_nv_ls("x-forwarded-for", (*xff).value)); + nva.push_back(http2::make_nv_ls_nocopy("x-forwarded-for", (*xff).value)); } if (!get_config()->http2_proxy && !get_config()->client_proxy && downstream_->get_request_method() != HTTP_CONNECT) { // We use same protocol with :scheme header field - nva.push_back(http2::make_nv_ls("x-forwarded-proto", scheme)); + nva.push_back(http2::make_nv_ls_nocopy("x-forwarded-proto", scheme)); } std::string via_value; auto via = downstream_->get_request_header(http2::HD_VIA); if (get_config()->no_via) { if (via) { - nva.push_back(http2::make_nv_ls("via", (*via).value)); + nva.push_back(http2::make_nv_ls_nocopy("via", (*via).value)); } } else { if (via) { @@ -377,7 +376,7 @@ int Http2DownstreamConnection::push_request_headers() { } for (auto &p : get_config()->add_request_headers) { - nva.push_back(http2::make_nv(p.first, p.second)); + nva.push_back(http2::make_nv_nocopy(p.first, p.second)); } if (LOG_ENABLED(INFO)) { diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 529f4b81..97d297c3 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -1259,7 +1259,7 @@ ssize_t downstream_data_read_callback(nghttp2_session *session, if (!trailers.empty()) { std::vector nva; nva.reserve(trailers.size()); - http2::copy_headers_to_nva(nva, trailers); + http2::copy_headers_to_nva_nocopy(nva, trailers); if (!nva.empty()) { rv = nghttp2_submit_trailer(session, stream_id, nva.data(), nva.size()); @@ -1295,13 +1295,20 @@ int Http2Upstream::send_reply(Downstream *downstream, const uint8_t *body, data_prd_ptr = &data_prd; } - auto status_code_str = util::utos(downstream->get_response_http_status()); auto &headers = downstream->get_response_headers(); auto nva = std::vector(); // 2 for :status and server nva.reserve(2 + headers.size()); - nva.push_back(http2::make_nv_ls(":status", status_code_str)); + std::string status_code_str; + auto response_status_const = + http2::stringify_status(downstream->get_response_http_status()); + if (response_status_const) { + nva.push_back(http2::make_nv_lc_nocopy(":status", response_status_const)); + } else { + status_code_str = util::utos(downstream->get_response_http_status()); + nva.push_back(http2::make_nv_ls(":status", status_code_str)); + } for (auto &kv : headers) { if (kv.name.empty() || kv.name[0] == ':') { @@ -1316,11 +1323,12 @@ int Http2Upstream::send_reply(Downstream *downstream, const uint8_t *body, case http2::HD_UPGRADE: continue; } - nva.push_back(http2::make_nv(kv.name, kv.value, kv.no_index)); + nva.push_back(http2::make_nv_nocopy(kv.name, kv.value, kv.no_index)); } if (!downstream->get_response_header(http2::HD_SERVER)) { - nva.push_back(http2::make_nv_lc("server", get_config()->server_name)); + nva.push_back( + http2::make_nv_lc_nocopy("server", get_config()->server_name)); } rv = nghttp2_submit_response(session_, downstream->get_stream_id(), @@ -1356,14 +1364,20 @@ int Http2Upstream::error_reply(Downstream *downstream, auto lgconf = log_config(); lgconf->update_tstamp(std::chrono::system_clock::now()); + auto response_status_const = http2::stringify_status(status_code); auto content_length = util::utos(html.size()); - auto status_code_str = util::utos(status_code); - auto nva = - make_array(http2::make_nv_ls(":status", status_code_str), - http2::make_nv_ll("content-type", "text/html; charset=UTF-8"), - http2::make_nv_lc("server", get_config()->server_name), - http2::make_nv_ls("content-length", content_length), - http2::make_nv_ls("date", lgconf->time_http_str)); + + std::string status_code_str; + + auto nva = make_array( + response_status_const + ? http2::make_nv_lc_nocopy(":status", response_status_const) + : http2::make_nv_ls(":status", + (status_code_str = util::utos(status_code))), + http2::make_nv_ll("content-type", "text/html; charset=UTF-8"), + http2::make_nv_lc_nocopy("server", get_config()->server_name), + http2::make_nv_ls("content-length", content_length), + http2::make_nv_ls("date", lgconf->time_http_str)); rv = nghttp2_submit_response(session_, downstream->get_stream_id(), nva.data(), nva.size(), &data_prd); @@ -1445,10 +1459,18 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream) { // field. nva.reserve(nheader + 4 + get_config()->add_response_headers.size()); std::string via_value; - auto response_status = util::utos(downstream->get_response_http_status()); - nva.push_back(http2::make_nv_ls(":status", response_status)); + std::string response_status; - http2::copy_headers_to_nva(nva, downstream->get_response_headers()); + auto response_status_const = + http2::stringify_status(downstream->get_response_http_status()); + if (response_status_const) { + nva.push_back(http2::make_nv_lc_nocopy(":status", response_status_const)); + } else { + response_status = util::utos(downstream->get_response_http_status()); + nva.push_back(http2::make_nv_ls(":status", response_status)); + } + + http2::copy_headers_to_nva_nocopy(nva, downstream->get_response_headers()); if (downstream->get_non_final_response()) { if (LOG_ENABLED(INFO)) { @@ -1470,18 +1492,19 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream) { } if (!get_config()->http2_proxy && !get_config()->client_proxy) { - nva.push_back(http2::make_nv_lc("server", get_config()->server_name)); + nva.push_back( + http2::make_nv_lc_nocopy("server", get_config()->server_name)); } else { auto server = downstream->get_response_header(http2::HD_SERVER); if (server) { - nva.push_back(http2::make_nv_ls("server", (*server).value)); + nva.push_back(http2::make_nv_ls_nocopy("server", (*server).value)); } } auto via = downstream->get_response_header(http2::HD_VIA); if (get_config()->no_via) { if (via) { - nva.push_back(http2::make_nv_ls("via", (*via).value)); + nva.push_back(http2::make_nv_ls_nocopy("via", (*via).value)); } } else { if (via) { @@ -1494,7 +1517,7 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream) { } for (auto &p : get_config()->add_response_headers) { - nva.push_back(http2::make_nv(p.first, p.second)); + nva.push_back(http2::make_nv_nocopy(p.first, p.second)); } if (downstream->get_stream_id() % 2 == 0) { @@ -1797,7 +1820,7 @@ int Http2Upstream::submit_push_promise(const std::string &scheme, case http2::HD_CACHE_CONTROL: case http2::HD_HOST: case http2::HD_USER_AGENT: - nva.push_back(http2::make_nv(kv.name, kv.value, kv.no_index)); + nva.push_back(http2::make_nv_nocopy(kv.name, kv.value, kv.no_index)); break; } } From 7755c2827ca9a4892c6881e1e85a7d8e4687fa4b Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 5 Nov 2015 23:47:22 +0900 Subject: [PATCH 07/23] nghttpx: Reserve headers vector --- src/shrpx_downstream.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index 65f3e970..eb284aa7 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -150,6 +150,9 @@ Downstream::Downstream(Upstream *upstream, MemchunkPool *mcpool, http2::init_hdidx(request_hdidx_); http2::init_hdidx(response_hdidx_); + + request_headers_.reserve(16); + response_headers_.reserve(32); } Downstream::~Downstream() { From dfbbb0812411be4f1828cd9d75fcccfa7ec6bdb8 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 6 Nov 2015 20:07:40 +0900 Subject: [PATCH 08/23] Silence warning with scan-build --- lib/nghttp2_frame.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/nghttp2_frame.c b/lib/nghttp2_frame.c index 30b4f396..78fdf073 100644 --- a/lib/nghttp2_frame.c +++ b/lib/nghttp2_frame.c @@ -770,10 +770,7 @@ int nghttp2_nv_array_copy(nghttp2_nv **nva_ptr, const nghttp2_nv *nva, } p = *nva_ptr; - - if (buflen > sizeof(nghttp2_nv) * nvlen) { - data = (uint8_t *)(*nva_ptr) + sizeof(nghttp2_nv) * nvlen; - } + data = (uint8_t *)(*nva_ptr) + sizeof(nghttp2_nv) * nvlen; for (i = 0; i < nvlen; ++i) { p->flags = nva[i].flags; From 5e7e479c6c5e8c903256adba0847b5c7872f52b7 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 7 Nov 2015 10:56:40 +0900 Subject: [PATCH 09/23] Workaround HTTP upgrade with HEAD request By default, we check the length of response body matches content-length. For HEAD request, this is not necessarily true, so we sniff request method, and if it is HEAD, make sure that response body length is 0. But this does not work for HTTP Upgrade, since nghttp2_session_upgrade() has no parameter to tell the request method was HEAD. This commit disables this response body length validation for the stream upgraded by HTTP Upgrade. We will add new version of nghttp2_session_upgrade with the parameter to pass the request method information so that we can handle this situation properly. --- lib/nghttp2_http.c | 3 ++- lib/nghttp2_session.c | 10 +++++++++ lib/nghttp2_stream.h | 12 ++++++----- tests/main.c | 2 ++ tests/nghttp2_session_test.c | 42 ++++++++++++++++++++++++++++++++++++ tests/nghttp2_session_test.h | 1 + 6 files changed, 64 insertions(+), 6 deletions(-) diff --git a/lib/nghttp2_http.c b/lib/nghttp2_http.c index 5dfd44fc..2abcd50f 100644 --- a/lib/nghttp2_http.c +++ b/lib/nghttp2_http.c @@ -381,7 +381,8 @@ int nghttp2_http_on_response_headers(nghttp2_stream *stream) { if (!expect_response_body(stream)) { stream->content_length = 0; - } else if (stream->http_flags & NGHTTP2_HTTP_FLAG_METH_CONNECT) { + } else if (stream->http_flags & (NGHTTP2_HTTP_FLAG_METH_CONNECT | + NGHTTP2_HTTP_FLAG_METH_UPGRADE_WORKAROUND)) { stream->content_length = -1; } diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 4b5e6a22..07ba9608 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -6559,6 +6559,16 @@ int nghttp2_session_upgrade(nghttp2_session *session, nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_WR); session->next_stream_id += 2; } + /* We have no information about request header fields when Upgrade + was happened. So we don't know the request method here. If + request method is HEAD, we have a trouble because we may have + nonzero content-length header field in response headers, and we + will going to check it against the actual DATA frames, but we may + get mismatch because HEAD response body must be empty. We will + add new version of nghttp2_session_upgrade with the parameter to + pass the request method information so that we can handle this + situation properly. */ + stream->http_flags |= NGHTTP2_HTTP_FLAG_METH_UPGRADE_WORKAROUND; return 0; } diff --git a/lib/nghttp2_stream.h b/lib/nghttp2_stream.h index c8aca99a..f73febbb 100644 --- a/lib/nghttp2_stream.h +++ b/lib/nghttp2_stream.h @@ -116,19 +116,21 @@ typedef enum { NGHTTP2_HTTP_FLAG_METH_CONNECT = 1 << 7, NGHTTP2_HTTP_FLAG_METH_HEAD = 1 << 8, NGHTTP2_HTTP_FLAG_METH_OPTIONS = 1 << 9, + NGHTTP2_HTTP_FLAG_METH_UPGRADE_WORKAROUND = 1 << 10, NGHTTP2_HTTP_FLAG_METH_ALL = NGHTTP2_HTTP_FLAG_METH_CONNECT | NGHTTP2_HTTP_FLAG_METH_HEAD | - NGHTTP2_HTTP_FLAG_METH_OPTIONS, + NGHTTP2_HTTP_FLAG_METH_OPTIONS | + NGHTTP2_HTTP_FLAG_METH_UPGRADE_WORKAROUND, /* :path category */ /* path starts with "/" */ - NGHTTP2_HTTP_FLAG_PATH_REGULAR = 1 << 10, + NGHTTP2_HTTP_FLAG_PATH_REGULAR = 1 << 11, /* path "*" */ - NGHTTP2_HTTP_FLAG_PATH_ASTERISK = 1 << 11, + NGHTTP2_HTTP_FLAG_PATH_ASTERISK = 1 << 12, /* scheme */ /* "http" or "https" scheme */ - NGHTTP2_HTTP_FLAG_SCHEME_HTTP = 1 << 12, + NGHTTP2_HTTP_FLAG_SCHEME_HTTP = 1 << 13, /* set if final response is expected */ - NGHTTP2_HTTP_FLAG_EXPECT_FINAL_RESPONSE = 1 << 13 + NGHTTP2_HTTP_FLAG_EXPECT_FINAL_RESPONSE = 1 << 14 } nghttp2_http_flag; struct nghttp2_stream { diff --git a/tests/main.c b/tests/main.c index 64710066..2887d071 100644 --- a/tests/main.c +++ b/tests/main.c @@ -298,6 +298,8 @@ int main(int argc _U_, char *argv[] _U_) { test_nghttp2_http_record_request_method) || !CU_add_test(pSuite, "http_push_promise", test_nghttp2_http_push_promise) || + !CU_add_test(pSuite, "http_head_method_upgrade_workaround", + test_nghttp2_http_head_method_upgrade_workaround) || !CU_add_test(pSuite, "frame_pack_headers", test_nghttp2_frame_pack_headers) || !CU_add_test(pSuite, "frame_pack_headers_frame_too_large", diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index b3441f5c..463f2e10 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -9246,3 +9246,45 @@ void test_nghttp2_http_push_promise(void) { nghttp2_session_del(session); nghttp2_bufs_free(&bufs); } + +void test_nghttp2_http_head_method_upgrade_workaround(void) { + nghttp2_session *session; + nghttp2_session_callbacks callbacks; + const nghttp2_nv resnv[] = {MAKE_NV(":status", "200"), + MAKE_NV("content-length", "1000000007")}; + nghttp2_bufs bufs; + nghttp2_hd_deflater deflater; + nghttp2_mem *mem; + 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; + + nghttp2_session_client_new(&session, &callbacks, NULL); + + nghttp2_hd_deflate_init(&deflater, mem); + + nghttp2_session_upgrade(session, NULL, 0, NULL); + + rv = pack_headers(&bufs, &deflater, 1, NGHTTP2_FLAG_END_HEADERS, resnv, + ARRLEN(resnv), mem); + + CU_ASSERT(0 == rv); + + rv = nghttp2_session_mem_recv(session, bufs.head->buf.pos, + nghttp2_buf_len(&bufs.head->buf)); + + CU_ASSERT((ssize_t)nghttp2_buf_len(&bufs.head->buf) == rv); + + stream = nghttp2_session_get_stream(session, 1); + + CU_ASSERT(-1 == stream->content_length); + + nghttp2_hd_deflate_free(&deflater); + nghttp2_session_del(session); + nghttp2_bufs_free(&bufs); +} diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h index ce08b5f9..979dbc46 100644 --- a/tests/nghttp2_session_test.h +++ b/tests/nghttp2_session_test.h @@ -141,5 +141,6 @@ void test_nghttp2_http_ignore_regular_header(void); void test_nghttp2_http_ignore_content_length(void); void test_nghttp2_http_record_request_method(void); void test_nghttp2_http_push_promise(void); +void test_nghttp2_http_head_method_upgrade_workaround(void); #endif /* NGHTTP2_SESSION_TEST_H */ From 6958d0b55d30728efcc92baa83e643f66078759b Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 7 Nov 2015 11:14:28 +0900 Subject: [PATCH 10/23] nghttp: Use method given in -H with ":method" in HTTP Upgrade --- src/nghttp.cc | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/nghttp.cc b/src/nghttp.cc index 2d14367a..cf0bef2b 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -813,7 +813,16 @@ int HttpClient::on_upgrade_connect() { // If the request contains upload data, use OPTIONS * to upgrade req = "OPTIONS *"; } else { - req = "GET "; + auto meth = std::find_if( + std::begin(config.headers), std::end(config.headers), + [](const Header &kv) { return util::strieq_l(":method", kv.name); }); + + if (meth == std::end(config.headers)) { + req = "GET "; + } else { + req = (*meth).value; + req += " "; + } req += reqvec[0]->make_reqpath(); } From af4c3cb2cfe773f6358c40b113a340b4d3c8673e Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 7 Nov 2015 11:34:05 +0900 Subject: [PATCH 11/23] Fix travis gcc build --- tests/nghttp2_session_test.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 463f2e10..632c606a 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -9250,8 +9250,8 @@ void test_nghttp2_http_push_promise(void) { void test_nghttp2_http_head_method_upgrade_workaround(void) { nghttp2_session *session; nghttp2_session_callbacks callbacks; - const nghttp2_nv resnv[] = {MAKE_NV(":status", "200"), - MAKE_NV("content-length", "1000000007")}; + const nghttp2_nv cl_resnv[] = {MAKE_NV(":status", "200"), + MAKE_NV("content-length", "1000000007")}; nghttp2_bufs bufs; nghttp2_hd_deflater deflater; nghttp2_mem *mem; @@ -9270,8 +9270,8 @@ void test_nghttp2_http_head_method_upgrade_workaround(void) { nghttp2_session_upgrade(session, NULL, 0, NULL); - rv = pack_headers(&bufs, &deflater, 1, NGHTTP2_FLAG_END_HEADERS, resnv, - ARRLEN(resnv), mem); + rv = pack_headers(&bufs, &deflater, 1, NGHTTP2_FLAG_END_HEADERS, cl_resnv, + ARRLEN(cl_resnv), mem); CU_ASSERT(0 == rv); From 269a10008177d359e978d6a256d00dedb8837b52 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 7 Nov 2015 11:30:15 +0900 Subject: [PATCH 12/23] Add nghttp2_session_upgrade2(), deprecate nghttp2_session_upgrade() To validate actual response body length against the value declared in content-length response header field, we first check request method. If request method is HEAD, respose body must be 0 regardless of the value in content-length. nghttp2_session_upgrade() has no parameter to indicate the request method is HEAD, so we failed to validate response body if HEAD is used with HTTP Upgrade. New nghttp2_session_upgrade2() accepts new parameter to indicate that request method is HEAD or not to fix this issue. Although, this issue affects client side only, we deprecate nghttp2_session_upgrade() in favor of nghttp2_session_upgrade2() for both client and server side. --- doc/Makefile.am | 1 + lib/includes/nghttp2/nghttp2.h | 56 ++++++++++++++++++++++++++++++++ lib/nghttp2_session.c | 58 +++++++++++++++++++++++++++++----- tests/main.c | 2 +- tests/nghttp2_session_test.c | 26 +++++++-------- tests/nghttp2_session_test.h | 2 +- 6 files changed, 122 insertions(+), 23 deletions(-) diff --git a/doc/Makefile.am b/doc/Makefile.am index d3055c38..7b6caaee 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -116,6 +116,7 @@ APIDOCS= \ nghttp2_session_terminate_session.rst \ nghttp2_session_terminate_session2.rst \ nghttp2_session_upgrade.rst \ + nghttp2_session_upgrade2.rst \ nghttp2_session_want_read.rst \ nghttp2_session_want_write.rst \ nghttp2_stream_get_first_child.rst \ diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index b4d79429..82749969 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -2858,6 +2858,17 @@ NGHTTP2_EXTERN int nghttp2_session_consume_stream(nghttp2_session *session, * be called from both client and server, but the behavior is very * different in each other. * + * .. warning:: + * + * This function is deprecated in favor of + * `nghttp2_session_upgrade2()`, because this function lacks the + * parameter to tell the library the request method used in the + * original HTTP request. This information is required for client + * to validate actual response body length against content-length + * header field (see `nghttp2_option_set_no_http_messaging()`). If + * HEAD is used in request, the length of response body must be 0 + * regardless of value included in content-length header field. + * * If called from client side, the |settings_payload| must be the * value sent in ``HTTP2-Settings`` header field and must be decoded * by base64url decoder. The |settings_payloadlen| is the length of @@ -2892,6 +2903,51 @@ NGHTTP2_EXTERN int nghttp2_session_upgrade(nghttp2_session *session, size_t settings_payloadlen, void *stream_user_data); +/** + * @function + * + * Performs post-process of HTTP Upgrade request. This function can + * be called from both client and server, but the behavior is very + * different in each other. + * + * If called from client side, the |settings_payload| must be the + * value sent in ``HTTP2-Settings`` header field and must be decoded + * by base64url decoder. The |settings_payloadlen| is the length of + * |settings_payload|. The |settings_payload| is unpacked and its + * setting values will be submitted using `nghttp2_submit_settings()`. + * This means that the client application code does not need to submit + * SETTINGS by itself. The stream with stream ID=1 is opened and the + * |stream_user_data| is used for its stream_user_data. The opened + * stream becomes half-closed (local) state. + * + * If called from server side, the |settings_payload| must be the + * value received in ``HTTP2-Settings`` header field and must be + * decoded by base64url decoder. The |settings_payloadlen| is the + * length of |settings_payload|. It is treated as if the SETTINGS + * frame with that payload is received. Thus, callback functions for + * the reception of SETTINGS frame will be invoked. The stream with + * stream ID=1 is opened. The |stream_user_data| is ignored. The + * opened stream becomes half-closed (remote). + * + * If the request method is HEAD, pass nonzero value to + * |head_request|. Otherwise, pass 0. + * + * 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` + * The |settings_payload| is badly formed. + * :enum:`NGHTTP2_ERR_PROTO` + * The stream ID 1 is already used or closed; or is not available. + */ +NGHTTP2_EXTERN int nghttp2_session_upgrade2(nghttp2_session *session, + const uint8_t *settings_payload, + size_t settings_payloadlen, + int head_request, + void *stream_user_data); + /** * @function * diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 07ba9608..dfb29a0d 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -6502,10 +6502,10 @@ uint32_t nghttp2_session_get_remote_settings(nghttp2_session *session, assert(0); } -int nghttp2_session_upgrade(nghttp2_session *session, - const uint8_t *settings_payload, - size_t settings_payloadlen, - void *stream_user_data) { +static int nghttp2_session_upgrade_internal(nghttp2_session *session, + const uint8_t *settings_payload, + size_t settings_payloadlen, + void *stream_user_data) { nghttp2_stream *stream; nghttp2_frame frame; nghttp2_settings_entry *iv; @@ -6559,19 +6559,61 @@ int nghttp2_session_upgrade(nghttp2_session *session, nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_WR); session->next_stream_id += 2; } + return 0; +} + +int nghttp2_session_upgrade(nghttp2_session *session, + const uint8_t *settings_payload, + size_t settings_payloadlen, + void *stream_user_data) { + int rv; + nghttp2_stream *stream; + + rv = nghttp2_session_upgrade_internal(session, settings_payload, + settings_payloadlen, stream_user_data); + if (rv != 0) { + return rv; + } + + stream = nghttp2_session_get_stream(session, 1); + assert(stream); + /* We have no information about request header fields when Upgrade was happened. So we don't know the request method here. If request method is HEAD, we have a trouble because we may have nonzero content-length header field in response headers, and we will going to check it against the actual DATA frames, but we may - get mismatch because HEAD response body must be empty. We will - add new version of nghttp2_session_upgrade with the parameter to - pass the request method information so that we can handle this - situation properly. */ + get mismatch because HEAD response body must be empty. Because + of this reason, nghttp2_session_upgrade() was deprecated in favor + of nghttp2_session_upgrade2(), which has |head_request| parameter + to indicate that request method is HEAD or not. */ stream->http_flags |= NGHTTP2_HTTP_FLAG_METH_UPGRADE_WORKAROUND; return 0; } +int nghttp2_session_upgrade2(nghttp2_session *session, + const uint8_t *settings_payload, + size_t settings_payloadlen, int head_request, + void *stream_user_data) { + int rv; + nghttp2_stream *stream; + + rv = nghttp2_session_upgrade_internal(session, settings_payload, + settings_payloadlen, stream_user_data); + if (rv != 0) { + return rv; + } + + stream = nghttp2_session_get_stream(session, 1); + assert(stream); + + if (head_request) { + stream->http_flags |= NGHTTP2_HTTP_FLAG_METH_HEAD; + } + + return 0; +} + int nghttp2_session_get_stream_local_close(nghttp2_session *session, int32_t stream_id) { nghttp2_stream *stream; diff --git a/tests/main.c b/tests/main.c index 2887d071..cc3896c0 100644 --- a/tests/main.c +++ b/tests/main.c @@ -138,7 +138,7 @@ int main(int argc _U_, char *argv[] _U_) { test_nghttp2_session_send_push_promise) || !CU_add_test(pSuite, "session_is_my_stream_id", test_nghttp2_session_is_my_stream_id) || - !CU_add_test(pSuite, "session_upgrade", test_nghttp2_session_upgrade) || + !CU_add_test(pSuite, "session_upgrade2", test_nghttp2_session_upgrade2) || !CU_add_test(pSuite, "session_reprioritize_stream", test_nghttp2_session_reprioritize_stream) || !CU_add_test( diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 632c606a..758ada16 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -3331,7 +3331,7 @@ void test_nghttp2_session_is_my_stream_id(void) { nghttp2_session_del(session); } -void test_nghttp2_session_upgrade(void) { +void test_nghttp2_session_upgrade2(void) { nghttp2_session *session; nghttp2_session_callbacks callbacks; uint8_t settings_payload[128]; @@ -3351,8 +3351,8 @@ void test_nghttp2_session_upgrade(void) { /* Check client side */ nghttp2_session_client_new(&session, &callbacks, NULL); - CU_ASSERT(0 == nghttp2_session_upgrade(session, settings_payload, - settings_payloadlen, &callbacks)); + CU_ASSERT(0 == nghttp2_session_upgrade2(session, settings_payload, + settings_payloadlen, 0, &callbacks)); stream = nghttp2_session_get_stream(session, 1); CU_ASSERT(stream != NULL); CU_ASSERT(&callbacks == stream->stream_user_data); @@ -3367,16 +3367,16 @@ void test_nghttp2_session_upgrade(void) { item->frame.settings.iv[1].settings_id); CU_ASSERT(4095 == item->frame.settings.iv[1].value); - /* Call nghttp2_session_upgrade() again is error */ + /* Call nghttp2_session_upgrade2() again is error */ CU_ASSERT(NGHTTP2_ERR_PROTO == - nghttp2_session_upgrade(session, settings_payload, - settings_payloadlen, &callbacks)); + nghttp2_session_upgrade2(session, settings_payload, + settings_payloadlen, 0, &callbacks)); nghttp2_session_del(session); /* Check server side */ nghttp2_session_server_new(&session, &callbacks, NULL); - CU_ASSERT(0 == nghttp2_session_upgrade(session, settings_payload, - settings_payloadlen, &callbacks)); + CU_ASSERT(0 == nghttp2_session_upgrade2(session, settings_payload, + settings_payloadlen, 0, &callbacks)); stream = nghttp2_session_get_stream(session, 1); CU_ASSERT(stream != NULL); CU_ASSERT(NULL == stream->stream_user_data); @@ -3384,10 +3384,10 @@ void test_nghttp2_session_upgrade(void) { CU_ASSERT(NULL == nghttp2_session_get_next_ob_item(session)); CU_ASSERT(1 == session->remote_settings.max_concurrent_streams); CU_ASSERT(4095 == session->remote_settings.initial_window_size); - /* Call nghttp2_session_upgrade() again is error */ + /* Call nghttp2_session_upgrade2() again is error */ CU_ASSERT(NGHTTP2_ERR_PROTO == - nghttp2_session_upgrade(session, settings_payload, - settings_payloadlen, &callbacks)); + nghttp2_session_upgrade2(session, settings_payload, + settings_payloadlen, 0, &callbacks)); nghttp2_session_del(session); /* Empty SETTINGS is OK */ @@ -3395,8 +3395,8 @@ void test_nghttp2_session_upgrade(void) { settings_payload, sizeof(settings_payload), NULL, 0); nghttp2_session_client_new(&session, &callbacks, NULL); - CU_ASSERT(0 == nghttp2_session_upgrade(session, settings_payload, - settings_payloadlen, NULL)); + CU_ASSERT(0 == nghttp2_session_upgrade2(session, settings_payload, + settings_payloadlen, 0, NULL)); nghttp2_session_del(session); } diff --git a/tests/nghttp2_session_test.h b/tests/nghttp2_session_test.h index 979dbc46..80aa8edd 100644 --- a/tests/nghttp2_session_test.h +++ b/tests/nghttp2_session_test.h @@ -59,7 +59,7 @@ void test_nghttp2_session_send_headers_push_reply(void); void test_nghttp2_session_send_rst_stream(void); void test_nghttp2_session_send_push_promise(void); void test_nghttp2_session_is_my_stream_id(void); -void test_nghttp2_session_upgrade(void); +void test_nghttp2_session_upgrade2(void); void test_nghttp2_session_reprioritize_stream(void); void test_nghttp2_session_reprioritize_stream_with_idle_stream_dep(void); void test_nghttp2_submit_data(void); From 558934cee5e8b0557e089c8da08f9e973af6d033 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 7 Nov 2015 12:16:22 +0900 Subject: [PATCH 13/23] src: Use nghttp2_session_upgrade2 --- src/nghttp.cc | 16 +++++++++++++--- src/shrpx_http2_upstream.cc | 5 +++-- src/util.h | 4 ++++ 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/nghttp.cc b/src/nghttp.cc index cf0bef2b..5d58471b 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -815,7 +815,7 @@ int HttpClient::on_upgrade_connect() { } else { auto meth = std::find_if( std::begin(config.headers), std::end(config.headers), - [](const Header &kv) { return util::strieq_l(":method", kv.name); }); + [](const Header &kv) { return util::streq_l(":method", kv.name); }); if (meth == std::end(config.headers)) { req = "GET "; @@ -988,8 +988,18 @@ int HttpClient::connection_made() { if (!reqvec[0]->data_prd) { stream_user_data = reqvec[0].get(); } - rv = nghttp2_session_upgrade(session, settings_payload.data(), - settings_payloadlen, stream_user_data); + // If HEAD is used, that is only when user specified it with -H + // option. + auto head_request = + stream_user_data && + std::find_if(std::begin(config.headers), std::end(config.headers), + [](const Header &kv) { + return util::streq_l(":method", kv.name) && + util::streq_l("HEAD", kv.value); + }) != std::end(config.headers); + rv = nghttp2_session_upgrade2(session, settings_payload.data(), + settings_payloadlen, head_request, + stream_user_data); if (rv != 0) { std::cerr << "[ERROR] nghttp2_session_upgrade() returned error: " << nghttp2_strerror(rv) << std::endl; diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 97d297c3..05d0f70a 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -108,9 +108,10 @@ int Http2Upstream::upgrade_upstream(HttpsUpstream *http) { auto settings_payload = base64::decode(std::begin(http2_settings), std::end(http2_settings)); - rv = nghttp2_session_upgrade( + rv = nghttp2_session_upgrade2( session_, reinterpret_cast(settings_payload.c_str()), - settings_payload.size(), nullptr); + settings_payload.size(), + http->get_downstream()->get_request_method() == HTTP_HEAD, nullptr); if (rv != 0) { if (LOG_ENABLED(INFO)) { ULOG(INFO, this) << "nghttp2_session_upgrade() returned error: " diff --git a/src/util.h b/src/util.h index 60662926..3a835e75 100644 --- a/src/util.h +++ b/src/util.h @@ -404,6 +404,10 @@ bool streq_l(const char (&a)[N], InputIt b, size_t blen) { return streq(a, N - 1, b, blen); } +template bool streq_l(const char (&a)[N], const std::string &b) { + return streq(a, N - 1, std::begin(b), b.size()); +} + bool strifind(const char *a, const char *b); template void inp_strlower(InputIt first, InputIt last) { From eaefce379238bad53dacde06d437b81c987d3af8 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 7 Nov 2015 12:47:26 +0900 Subject: [PATCH 14/23] nghttp: Record request method to output it in har correctly --- src/nghttp.cc | 49 +++++++++++++++++++++++-------------------------- src/nghttp.h | 1 + 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/src/nghttp.cc b/src/nghttp.cc index 5d58471b..b58c5d04 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -392,6 +392,11 @@ int submit_request(HttpClient *client, const Headers &headers, Request *req) { nva.push_back(http2::make_nv(kv.name, kv.value, kv.no_index)); } + auto method = http2::get_header(build_headers, ":method"); + assert(method); + + req->method = method->value; + std::string trailer_names; if (!config.trailer.empty()) { trailer_names = config.trailer[0].name; @@ -808,6 +813,7 @@ int HttpClient::on_upgrade_connect() { base64::encode(std::begin(settings_payload), std::begin(settings_payload) + settings_payloadlen); util::to_token68(token68); + std::string req; if (reqvec[0]->data_prd) { // If the request contains upload data, use OPTIONS * to upgrade @@ -819,25 +825,30 @@ int HttpClient::on_upgrade_connect() { if (meth == std::end(config.headers)) { req = "GET "; + reqvec[0]->method = "GET"; } else { req = (*meth).value; req += " "; + reqvec[0]->method = (*meth).value; } req += reqvec[0]->make_reqpath(); } - auto headers = Headers{{"Host", hostport}, - {"Connection", "Upgrade, HTTP2-Settings"}, - {"Upgrade", NGHTTP2_CLEARTEXT_PROTO_VERSION_ID}, - {"HTTP2-Settings", token68}, - {"Accept", "*/*"}, - {"User-Agent", "nghttp2/" NGHTTP2_VERSION}}; + auto headers = Headers{{"host", hostport}, + {"connection", "Upgrade, HTTP2-Settings"}, + {"upgrade", NGHTTP2_CLEARTEXT_PROTO_VERSION_ID}, + {"http2-settings", token68}, + {"accept", "*/*"}, + {"user-agent", "nghttp2/" NGHTTP2_VERSION}}; auto initial_headerslen = headers.size(); for (auto &kv : config.headers) { size_t i; + if (kv.name.empty() || kv.name[0] == ':') { + continue; + } for (i = 0; i < initial_headerslen; ++i) { - if (util::strieq(kv.name, headers[i].name)) { + if (kv.name == headers[i].name) { headers[i].value = kv.value; break; } @@ -845,9 +856,7 @@ int HttpClient::on_upgrade_connect() { if (i < initial_headerslen) { continue; } - if (kv.name.size() != 0 && kv.name[0] != ':') { - headers.emplace_back(kv.name, kv.value, kv.no_index); - } + headers.emplace_back(kv.name, kv.value, kv.no_index); } req += " HTTP/1.1\r\n"; @@ -867,9 +876,10 @@ int HttpClient::on_upgrade_connect() { std::cout << " HTTP Upgrade request\n" << req << std::endl; } - // record request time if this is GET request if (!reqvec[0]->data_prd) { + // record request time if this is a part of real request. reqvec[0]->record_request_start_time(); + reqvec[0]->req_nva = std::move(headers); } on_writefn = &HttpClient::noop; @@ -990,13 +1000,7 @@ int HttpClient::connection_made() { } // If HEAD is used, that is only when user specified it with -H // option. - auto head_request = - stream_user_data && - std::find_if(std::begin(config.headers), std::end(config.headers), - [](const Header &kv) { - return util::streq_l(":method", kv.name) && - util::streq_l("HEAD", kv.value); - }) != std::end(config.headers); + auto head_request = stream_user_data && stream_user_data->method == "HEAD"; rv = nghttp2_session_upgrade2(session, settings_payload.data(), settings_payloadlen, head_request, stream_user_data); @@ -1397,13 +1401,6 @@ void HttpClient::output_har(FILE *outfile) { auto request = json_object(); json_object_set_new(entry, "request", request); - auto method_ptr = http2::get_header(req->req_nva, ":method"); - - const char *method = "GET"; - if (method_ptr) { - method = (*method_ptr).value.c_str(); - } - auto req_headers = json_array(); json_object_set_new(request, "headers", req_headers); @@ -1415,7 +1412,7 @@ void HttpClient::output_har(FILE *outfile) { json_object_set_new(hd, "value", json_string(nv.value.c_str())); } - json_object_set_new(request, "method", json_string(method)); + json_object_set_new(request, "method", json_string(req->method.c_str())); json_object_set_new(request, "url", json_string(req->uri.c_str())); json_object_set_new(request, "httpVersion", json_string("HTTP/2.0")); json_object_set_new(request, "cookies", json_array()); diff --git a/src/nghttp.h b/src/nghttp.h index e6e054ac..ffc07007 100644 --- a/src/nghttp.h +++ b/src/nghttp.h @@ -137,6 +137,7 @@ struct Request { Headers res_nva; Headers req_nva; + std::string method; // URI without fragment std::string uri; http_parser_url u; From a4012b594b8660a49acb3c0e636ffc9b3c6f5705 Mon Sep 17 00:00:00 2001 From: Tomasz Buchert Date: Sat, 7 Nov 2015 12:45:57 +0100 Subject: [PATCH 15/23] added apparmor profile --- contrib/usr.sbin.nghttpx | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 contrib/usr.sbin.nghttpx diff --git a/contrib/usr.sbin.nghttpx b/contrib/usr.sbin.nghttpx new file mode 100644 index 00000000..891ff52c --- /dev/null +++ b/contrib/usr.sbin.nghttpx @@ -0,0 +1,16 @@ +#include + +/usr/sbin/nghttpx { + #include + #include + #include + + capability setgid, + capability setuid, + + /usr/sbin/nghttpx rmix, # allow to run itself + /etc/nghttpx/nghttpx.conf r, # allow to read the config file + /etc/ssl/** r, # give access to ssl keys + + /{,var/}run/nghttpx.pid lw, # allow to store a pid file +} From b89f1f5869c1bf289e0c166897c289afb20ccc31 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 7 Nov 2015 10:32:08 +0900 Subject: [PATCH 16/23] asio: ALPN support --- src/asio_client_session_tls_impl.cc | 8 ++++++ src/asio_client_tls_context.cc | 6 ++++ src/asio_common.cc | 43 +++++++++++++++++++++++++++++ src/asio_common.h | 8 ++++++ src/asio_server.cc | 11 ++++++-- src/asio_server_tls_context.cc | 18 ++++++++++++ src/includes/nghttp2/asio_http2.h | 31 ++++++++++++++------- 7 files changed, 113 insertions(+), 12 deletions(-) diff --git a/src/asio_client_session_tls_impl.cc b/src/asio_client_session_tls_impl.cc index cde2ab31..8db75c17 100644 --- a/src/asio_client_session_tls_impl.cc +++ b/src/asio_client_session_tls_impl.cc @@ -23,6 +23,7 @@ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ #include "asio_client_session_tls_impl.h" +#include "asio_common.h" namespace nghttp2 { namespace asio_http2 { @@ -59,6 +60,13 @@ void session_tls_impl::start_connect(tcp::resolver::iterator endpoint_it) { not_connected(ec); return; } + + if (!tls_h2_negotiated(socket_)) { + not_connected( + make_error_code(NGHTTP2_ASIO_ERR_TLS_NO_APP_PROTO_NEGOTIATED)); + return; + } + connected(endpoint_it); }); }); diff --git a/src/asio_client_tls_context.cc b/src/asio_client_tls_context.cc index 55138341..75830013 100644 --- a/src/asio_client_tls_context.cc +++ b/src/asio_client_tls_context.cc @@ -56,6 +56,12 @@ configure_tls_context(boost::system::error_code &ec, SSL_CTX_set_next_proto_select_cb(ctx, client_select_next_proto_cb, nullptr); +#if OPENSSL_VERSION_NUMBER >= 0x10002000L + auto proto_list = util::get_default_alpn(); + + SSL_CTX_set_alpn_protos(ctx, proto_list.data(), proto_list.size()); +#endif // OPENSSL_VERSION_NUMBER >= 0x10002000L + return ec; } diff --git a/src/asio_common.cc b/src/asio_common.cc index c3f98e7c..f8567a5f 100644 --- a/src/asio_common.cc +++ b/src/asio_common.cc @@ -48,6 +48,31 @@ boost::system::error_code make_error_code(nghttp2_error ev) { return boost::system::error_code(static_cast(ev), nghttp2_category()); } +class nghttp2_asio_category_impl : public boost::system::error_category { +public: + const char *name() const noexcept { return "nghttp2_asio"; } + std::string message(int ev) const { + switch (ev) { + case NGHTTP2_ASIO_ERR_NO_ERROR: + return "no error"; + case NGHTTP2_ASIO_ERR_TLS_NO_APP_PROTO_NEGOTIATED: + return "tls: no application protocol negotiated"; + default: + return "unknown"; + } + } +}; + +const boost::system::error_category &nghttp2_asio_category() noexcept { + static nghttp2_asio_category_impl cat; + return cat; +} + +boost::system::error_code make_error_code(nghttp2_asio_error ev) { + return boost::system::error_code(static_cast(ev), + nghttp2_asio_category()); +} + generator_cb string_generator(std::string data) { auto strio = std::make_shared>(std::move(data), data.size()); @@ -144,5 +169,23 @@ boost::system::error_code host_service_from_uri(boost::system::error_code &ec, return ec; } +bool tls_h2_negotiated(ssl_socket &socket) { + auto ssl = socket.native_handle(); + + const unsigned char *next_proto = nullptr; + unsigned int next_proto_len = 0; + + SSL_get0_next_proto_negotiated(ssl, &next_proto, &next_proto_len); + if (next_proto == nullptr) { + SSL_get0_alpn_selected(ssl, &next_proto, &next_proto_len); + } + + if (next_proto == nullptr) { + return false; + } + + return util::check_h2_is_selected(next_proto, next_proto_len); +} + } // namespace asio_http2 } // namespace nghttp2 diff --git a/src/asio_common.h b/src/asio_common.h index 8f59a0d1..b2384649 100644 --- a/src/asio_common.h +++ b/src/asio_common.h @@ -39,6 +39,8 @@ namespace asio_http2 { boost::system::error_code make_error_code(nghttp2_error ev); +boost::system::error_code make_error_code(nghttp2_asio_error ev); + generator_cb string_generator(std::string data); // Returns generator_cb, which just returns NGHTTP2_ERR_DEFERRED @@ -58,6 +60,12 @@ void split_path(uri_ref &dst, InputIt first, InputIt last) { dst.raw_query.assign(query_first, last); } +using boost::asio::ip::tcp; + +using ssl_socket = boost::asio::ssl::stream; + +bool tls_h2_negotiated(ssl_socket &socket); + } // namespace asio_http2 } // namespace nghttp2 diff --git a/src/asio_server.cc b/src/asio_server.cc index 310e672c..11c6c31d 100644 --- a/src/asio_server.cc +++ b/src/asio_server.cc @@ -37,6 +37,7 @@ #include "asio_server.h" #include "asio_server_connection.h" +#include "asio_common.h" #include "util.h" namespace nghttp2 { @@ -130,9 +131,15 @@ void server::start_accept(boost::asio::ssl::context &tls_context, new_connection->socket().async_handshake( boost::asio::ssl::stream_base::server, [new_connection](const boost::system::error_code &e) { - if (!e) { - new_connection->start(); + if (e) { + return; } + + if (!tls_h2_negotiated(new_connection->socket())) { + return; + } + + new_connection->start(); }); } diff --git a/src/asio_server_tls_context.cc b/src/asio_server_tls_context.cc index 4790a956..53fc9883 100644 --- a/src/asio_server_tls_context.cc +++ b/src/asio_server_tls_context.cc @@ -42,6 +42,19 @@ std::vector &get_alpn_token() { } } // namespace +#if OPENSSL_VERSION_NUMBER >= 0x10002000L +namespace { +int alpn_select_proto_cb(SSL *ssl, const unsigned char **out, + unsigned char *outlen, const unsigned char *in, + unsigned int inlen, void *arg) { + if (!util::select_h2(out, outlen, in, inlen)) { + return SSL_TLSEXT_ERR_NOACK; + } + return SSL_TLSEXT_ERR_OK; +} +} // namespace +#endif // OPENSSL_VERSION_NUMBER >= 0x10002000L + boost::system::error_code configure_tls_context_easy(boost::system::error_code &ec, boost::asio::ssl::context &tls_context) { @@ -81,6 +94,11 @@ configure_tls_context_easy(boost::system::error_code &ec, }, nullptr); +#if OPENSSL_VERSION_NUMBER >= 0x10002000L + // ALPN selection callback + SSL_CTX_set_alpn_select_cb(ctx, alpn_select_proto_cb, nullptr); +#endif // OPENSSL_VERSION_NUMBER >= 0x10002000L + return ec; } diff --git a/src/includes/nghttp2/asio_http2.h b/src/includes/nghttp2/asio_http2.h index a54a9e8e..a95d7b95 100644 --- a/src/includes/nghttp2/asio_http2.h +++ b/src/includes/nghttp2/asio_http2.h @@ -38,16 +38,6 @@ #include -namespace boost { -namespace system { - -template <> struct is_error_code_enum { - BOOST_STATIC_CONSTANT(bool, value = true); -}; - -} // namespace system -} // namespace boost - namespace nghttp2 { namespace asio_http2 { @@ -132,8 +122,29 @@ boost::system::error_code host_service_from_uri(boost::system::error_code &ec, std::string &service, const std::string &uri); +enum nghttp2_asio_error { + NGHTTP2_ASIO_ERR_NO_ERROR = 0, + NGHTTP2_ASIO_ERR_TLS_NO_APP_PROTO_NEGOTIATED = 1, +}; + } // namespace asio_http2 } // namespace nghttp2 +namespace boost { + +namespace system { + +template <> struct is_error_code_enum { + BOOST_STATIC_CONSTANT(bool, value = true); +}; + +template <> struct is_error_code_enum { + BOOST_STATIC_CONSTANT(bool, value = true); +}; + +} // namespace system + +} // namespace boost + #endif // ASIO_HTTP2_H From 9b18e47671eb8a71aa655e9f5842947252c73194 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sun, 8 Nov 2015 00:19:56 +0900 Subject: [PATCH 17/23] nghttpx: Use --backend-tls-sni-field to verify certificate hostname --- src/shrpx_ssl.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/shrpx_ssl.cc b/src/shrpx_ssl.cc index 5d72cbbf..d204e2d6 100644 --- a/src/shrpx_ssl.cc +++ b/src/shrpx_ssl.cc @@ -930,7 +930,10 @@ int check_cert(SSL *ssl, const DownstreamAddr *addr) { std::vector dns_names; std::vector ip_addrs; get_altnames(cert, dns_names, ip_addrs, common_name); - if (verify_hostname(addr->host.get(), &addr->addr, dns_names, ip_addrs, + auto hostname = get_config()->backend_tls_sni_name + ? get_config()->backend_tls_sni_name.get() + : addr->host.get(); + if (verify_hostname(hostname, &addr->addr, dns_names, ip_addrs, common_name) != 0) { LOG(ERROR) << "Certificate verification failed: hostname does not match"; return -1; From a93e0016ffb93961576de87d902af792b197159d Mon Sep 17 00:00:00 2001 From: Syohei YOSHIDA Date: Mon, 9 Nov 2015 11:40:35 +0900 Subject: [PATCH 18/23] Correct misspellings --- doc/README.rst | 2 +- doc/h2load.1 | 2 +- doc/h2load.1.rst | 2 +- doc/h2load.h2r | 2 +- doc/nghttp.1 | 2 +- doc/nghttp.1.rst | 2 +- doc/nghttpx.1 | 6 +++--- doc/nghttpx.1.rst | 6 +++--- doc/nghttpx.h2r | 4 ++-- 9 files changed, 14 insertions(+), 14 deletions(-) diff --git a/doc/README.rst b/doc/README.rst index 35bfe7ed..549e5506 100644 --- a/doc/README.rst +++ b/doc/README.rst @@ -22,7 +22,7 @@ starts with ``/**``. In other words, it only processes the comment block starting ``/**``. The comment block must end with ``*/``. The ``mkapiref.py`` requires that which type of the object this comment block refers to. To specify the type of the object, the next line -must contain the so-caled action keyword. Currently, the following +must contain the so-called action keyword. Currently, the following action keywords are supported: ``@function``, ``@functypedef``, ``@enum``, ``@struct`` and ``@union``. The following sections describes each action keyword. diff --git a/doc/h2load.1 b/doc/h2load.1 index 9c5633cc..04234013 100644 --- a/doc/h2load.1 +++ b/doc/h2load.1 @@ -288,7 +288,7 @@ The number of status code h2load received. .TP .B total The number of bytes received from the server "on the wire". If -requests were made via TLS, this value is the number of decrpyted +requests were made via TLS, this value is the number of decrypted bytes. .TP .B headers diff --git a/doc/h2load.1.rst b/doc/h2load.1.rst index 0be0dff9..6087c141 100644 --- a/doc/h2load.1.rst +++ b/doc/h2load.1.rst @@ -239,7 +239,7 @@ status codes traffic total The number of bytes received from the server "on the wire". If - requests were made via TLS, this value is the number of decrpyted + requests were made via TLS, this value is the number of decrypted bytes. headers The number of response header bytes from the server without diff --git a/doc/h2load.h2r b/doc/h2load.h2r index 86e57e08..60826931 100644 --- a/doc/h2load.h2r +++ b/doc/h2load.h2r @@ -30,7 +30,7 @@ status codes traffic total The number of bytes received from the server "on the wire". If - requests were made via TLS, this value is the number of decrpyted + requests were made via TLS, this value is the number of decrypted bytes. headers The number of response header bytes from the server without diff --git a/doc/nghttp.1 b/doc/nghttp.1 index 74c28cd3..023257a3 100644 --- a/doc/nghttp.1 +++ b/doc/nghttp.1 @@ -58,7 +58,7 @@ Discard downloaded data. .TP .B \-O, \-\-remote\-name Save download data in the current directory. The -filename is dereived from URI. If URI ends with \(aq\fI/\fP\(aq, +filename is derived from URI. If URI ends with \(aq\fI/\fP\(aq, \(aqindex.html\(aq is used as a filename. Not implemented yet. .UNINDENT diff --git a/doc/nghttp.1.rst b/doc/nghttp.1.rst index dcd3a537..c2ece836 100644 --- a/doc/nghttp.1.rst +++ b/doc/nghttp.1.rst @@ -36,7 +36,7 @@ OPTIONS .. option:: -O, --remote-name Save download data in the current directory. The - filename is dereived from URI. If URI ends with '*/*', + filename is derived from URI. If URI ends with '*/*', 'index.html' is used as a filename. Not implemented yet. diff --git a/doc/nghttpx.1 b/doc/nghttpx.1 index f9bad533..c5d8ab94 100644 --- a/doc/nghttpx.1 +++ b/doc/nghttpx.1 @@ -267,7 +267,7 @@ is done for each pattern group. Set maximum number of backend concurrent HTTP/1 connections per origin host. This option is meaningful when \fI\%\-s\fP option is used. The origin host is determined -by authority portion of requset URI (or :authority +by authority portion of request URI (or :authority header field for HTTP/2). To limit the number of connections per frontend for default mode, use \fI\%\-\-backend\-http1\-connections\-per\-frontend\fP\&. @@ -1377,7 +1377,7 @@ Clear all existing response header fields. Return custom response \fIbody\fP to a client. When this method is called in request phase hook, the request is not forwarded to the backend, and response phase hook for this request will -not be invoked. When this method is called in resonse phase +not be invoked. When this method is called in response phase hook, response from backend server is canceled and discarded. The status code and response header fields should be set before using this method. To set status code, use :rb:meth To @@ -1395,7 +1395,7 @@ It is an error to call this method twice for a given request. .UNINDENT .SS MRUBY EXAMPLES .sp -Modify requet path: +Modify request path: .INDENT 0.0 .INDENT 3.5 .sp diff --git a/doc/nghttpx.1.rst b/doc/nghttpx.1.rst index c932ace1..33b7b4f2 100644 --- a/doc/nghttpx.1.rst +++ b/doc/nghttpx.1.rst @@ -236,7 +236,7 @@ Performance Set maximum number of backend concurrent HTTP/1 connections per origin host. This option is meaningful when :option:`-s` option is used. The origin host is determined - by authority portion of requset URI (or :authority + by authority portion of request URI (or :authority header field for HTTP/2). To limit the number of connections per frontend for default mode, use :option:`--backend-http1-connections-per-frontend`\. @@ -1248,7 +1248,7 @@ respectively. Return custom response *body* to a client. When this method is called in request phase hook, the request is not forwarded to the backend, and response phase hook for this request will - not be invoked. When this method is called in resonse phase + not be invoked. When this method is called in response phase hook, response from backend server is canceled and discarded. The status code and response header fields should be set before using this method. To set status code, use :rb:meth To @@ -1266,7 +1266,7 @@ respectively. MRUBY EXAMPLES ~~~~~~~~~~~~~~ -Modify requet path: +Modify request path: .. code-block:: ruby diff --git a/doc/nghttpx.h2r b/doc/nghttpx.h2r index 4cacbad8..656baddb 100644 --- a/doc/nghttpx.h2r +++ b/doc/nghttpx.h2r @@ -354,7 +354,7 @@ respectively. Return custom response *body* to a client. When this method is called in request phase hook, the request is not forwarded to the backend, and response phase hook for this request will - not be invoked. When this method is called in resonse phase + not be invoked. When this method is called in response phase hook, response from backend server is canceled and discarded. The status code and response header fields should be set before using this method. To set status code, use :rb:meth To @@ -372,7 +372,7 @@ respectively. MRUBY EXAMPLES ~~~~~~~~~~~~~~ -Modify requet path: +Modify request path: .. code-block:: ruby From dbbed6414612e0ade16295d00b4ae17684ee75b5 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 9 Nov 2015 21:35:53 +0900 Subject: [PATCH 19/23] nghttpx: Fix bug that causes connection failure with backend proxy URI This is a regression when we introduced SSL/TLS session resumption in HTTP/2 backend. Before the introduction of session resumption, conn_.tls.ssl is always nullptr when connection is made to proxy. But we have to keep conn_.tls.ssl to enable session resumption, so our code breaks when it is reused. This commit fixes this issue. See GH-421 --- src/shrpx_http2_session.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index f9bb0136..6f255cf9 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -1569,13 +1569,6 @@ int Http2Session::connected() { conn_.rlimit.startw(); - if (conn_.tls.ssl) { - read_ = &Http2Session::tls_handshake; - write_ = &Http2Session::tls_handshake; - - return do_write(); - } - read_ = &Http2Session::read_clear; write_ = &Http2Session::write_clear; @@ -1583,6 +1576,13 @@ int Http2Session::connected() { return do_write(); } + if (conn_.tls.ssl) { + read_ = &Http2Session::tls_handshake; + write_ = &Http2Session::tls_handshake; + + return do_write(); + } + if (connection_made() != 0) { state_ = CONNECT_FAILING; return -1; From c711a804116ead69b558d6c2926dcab645e109b2 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Mon, 9 Nov 2015 21:43:25 +0900 Subject: [PATCH 20/23] src: Fix typo Apply typo fixes to the original source files in the previous commit which were done in generated text. --- src/nghttp.cc | 2 +- src/shrpx.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nghttp.cc b/src/nghttp.cc index b58c5d04..625f0905 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -2398,7 +2398,7 @@ Options: Discard downloaded data. -O, --remote-name Save download data in the current directory. The - filename is dereived from URI. If URI ends with '/', + filename is derived from URI. If URI ends with '/', 'index.html' is used as a filename. Not implemented yet. -t, --timeout= diff --git a/src/shrpx.cc b/src/shrpx.cc index f95f6417..855df5f3 100644 --- a/src/shrpx.cc +++ b/src/shrpx.cc @@ -1213,7 +1213,7 @@ Performance: Set maximum number of backend concurrent HTTP/1 connections per origin host. This option is meaningful when -s option is used. The origin host is determined - by authority portion of requset URI (or :authority + by authority portion of request URI (or :authority header field for HTTP/2). To limit the number of connections per frontend for default mode, use --backend-http1-connections-per-frontend. From 7c613386db87401b947a2e5457177dd223bafeee Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 12 Nov 2015 23:14:45 +0900 Subject: [PATCH 21/23] Update man pages --- doc/h2load.1 | 14 +++++++++----- doc/h2load.1.rst | 12 ++++++++---- doc/nghttp.1 | 4 ++-- doc/nghttp.1.rst | 2 +- doc/nghttpd.1 | 10 +++++++++- doc/nghttpd.1.rst | 7 +++++++ doc/nghttpx.1 | 2 +- 7 files changed, 37 insertions(+), 14 deletions(-) diff --git a/doc/h2load.1 b/doc/h2load.1 index 04234013..4e639968 100644 --- a/doc/h2load.1 +++ b/doc/h2load.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "H2LOAD" "1" "October 25, 2015" "1.4.0" "nghttp2" +.TH "H2LOAD" "1" "November 12, 2015" "1.4.1-DEV" "nghttp2" .SH NAME h2load \- HTTP/2 benchmarking tool . @@ -292,10 +292,14 @@ requests were made via TLS, this value is the number of decrypted bytes. .TP .B headers -The number of response header bytes from the server without -decompression. For HTTP/2, this is the sum of the payload of -HEADERS frame. For SPDY, this is the sum of the payload of -SYN_REPLY frame. +The number of response header bytes from the server without +decompression. The \fBspace savings\fP shows efficiency of header +compression. Let \fBdecompressed(headers)\fP to the number of bytes +used for header fields after decompression. The \fBspace savings\fP +is calculated by (1 \- \fBheaders\fP / \fBdecompressed(headers)\fP) * +100. For HTTP/1.1, this is usually 0.00%, since it does not have +header compression. For HTTP/2 and SPDY, it shows some insightful +numbers. .TP .B data The number of response body bytes received from the server. diff --git a/doc/h2load.1.rst b/doc/h2load.1.rst index 6087c141..5a9f8321 100644 --- a/doc/h2load.1.rst +++ b/doc/h2load.1.rst @@ -242,10 +242,14 @@ traffic requests were made via TLS, this value is the number of decrypted bytes. headers - The number of response header bytes from the server without - decompression. For HTTP/2, this is the sum of the payload of - HEADERS frame. For SPDY, this is the sum of the payload of - SYN_REPLY frame. + The number of response header bytes from the server without + decompression. The ``space savings`` shows efficiency of header + compression. Let ``decompressed(headers)`` to the number of bytes + used for header fields after decompression. The ``space savings`` + is calculated by (1 - ``headers`` / ``decompressed(headers)``) * + 100. For HTTP/1.1, this is usually 0.00%, since it does not have + header compression. For HTTP/2 and SPDY, it shows some insightful + numbers. data The number of response body bytes received from the server. diff --git a/doc/nghttp.1 b/doc/nghttp.1 index 023257a3..1029cb57 100644 --- a/doc/nghttp.1 +++ b/doc/nghttp.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTP" "1" "October 25, 2015" "1.4.0" "nghttp2" +.TH "NGHTTP" "1" "November 12, 2015" "1.4.1-DEV" "nghttp2" .SH NAME nghttp \- HTTP/2 client . @@ -58,7 +58,7 @@ Discard downloaded data. .TP .B \-O, \-\-remote\-name Save download data in the current directory. The -filename is derived from URI. If URI ends with \(aq\fI/\fP\(aq, +filename is derived from URI. If URI ends with \(aq\fI/\fP\(aq, \(aqindex.html\(aq is used as a filename. Not implemented yet. .UNINDENT diff --git a/doc/nghttp.1.rst b/doc/nghttp.1.rst index c2ece836..128eeab6 100644 --- a/doc/nghttp.1.rst +++ b/doc/nghttp.1.rst @@ -36,7 +36,7 @@ OPTIONS .. option:: -O, --remote-name Save download data in the current directory. The - filename is derived from URI. If URI ends with '*/*', + filename is derived from URI. If URI ends with '*/*', 'index.html' is used as a filename. Not implemented yet. diff --git a/doc/nghttpd.1 b/doc/nghttpd.1 index 91dc8ace..06ac32d6 100644 --- a/doc/nghttpd.1 +++ b/doc/nghttpd.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTPD" "1" "October 25, 2015" "1.4.0" "nghttp2" +.TH "NGHTTPD" "1" "November 12, 2015" "1.4.1-DEV" "nghttp2" .SH NAME nghttpd \- HTTP/2 server . @@ -172,6 +172,14 @@ Send back uploaded content if method is POST or PUT. .UNINDENT .INDENT 0.0 .TP +.B \-\-mime\-types\-file= +Path to file that contains MIME media types and the +extensions that represent them. +.sp +Default: \fB/etc/mime.types\fP +.UNINDENT +.INDENT 0.0 +.TP .B \-\-version Display version information and exit. .UNINDENT diff --git a/doc/nghttpd.1.rst b/doc/nghttpd.1.rst index 29ba9724..6b33b0ea 100644 --- a/doc/nghttpd.1.rst +++ b/doc/nghttpd.1.rst @@ -132,6 +132,13 @@ OPTIONS Send back uploaded content if method is POST or PUT. +.. option:: --mime-types-file= + + Path to file that contains MIME media types and the + extensions that represent them. + + Default: ``/etc/mime.types`` + .. option:: --version Display version information and exit. diff --git a/doc/nghttpx.1 b/doc/nghttpx.1 index c5d8ab94..8c0a577f 100644 --- a/doc/nghttpx.1 +++ b/doc/nghttpx.1 @@ -1,6 +1,6 @@ .\" Man page generated from reStructuredText. . -.TH "NGHTTPX" "1" "October 25, 2015" "1.4.0" "nghttp2" +.TH "NGHTTPX" "1" "November 12, 2015" "1.4.1-DEV" "nghttp2" .SH NAME nghttpx \- HTTP/2 proxy . From c6ef1c02b9c26bfb01e28c259c122ff55944f78e Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 13 Nov 2015 00:53:29 +0900 Subject: [PATCH 22/23] Switch to clang-format-3.6 --- doc/sources/contribute.rst | 4 +- examples/client.c | 11 ++--- lib/includes/nghttp2/nghttp2.h | 71 +++++++++++++++-------------- lib/nghttp2_helper.h | 2 +- lib/nghttp2_session.c | 7 ++- src/asio_client_session_impl.cc | 18 ++++---- src/asio_client_session_tcp_impl.cc | 12 ++--- src/asio_client_session_tls_impl.cc | 42 ++++++++--------- src/asio_common.cc | 27 +++++------ src/asio_server.cc | 58 +++++++++++------------ src/asio_server_connection.h | 39 ++++++++-------- src/asio_server_http2_handler.cc | 6 +-- src/http2.h | 10 ++-- src/includes/nghttp2/asio_http2.h | 4 +- src/memchunk.h | 2 +- src/memchunk_test.cc | 12 ++--- src/nghttp.cc | 27 +++++------ src/shrpx_connection_handler.cc | 16 +++---- src/shrpx_downstream_test.cc | 8 ++-- src/shrpx_http2_session.cc | 4 +- src/shrpx_http2_upstream.cc | 9 ++-- src/shrpx_log.cc | 2 +- src/shrpx_log_config.cc | 4 +- src/shrpx_spdy_upstream.cc | 12 ++--- src/shrpx_ssl.cc | 4 +- src/shrpx_ssl_test.cc | 13 +++--- src/template.h | 8 ++-- src/util.cc | 36 +++++++-------- src/util.h | 10 ++-- tests/nghttp2_hd_test.c | 39 ++++++---------- tests/nghttp2_pq_test.c | 4 +- tests/nghttp2_session_test.c | 8 ++-- 32 files changed, 255 insertions(+), 274 deletions(-) diff --git a/doc/sources/contribute.rst b/doc/sources/contribute.rst index 14a11e2e..4cbd341b 100644 --- a/doc/sources/contribute.rst +++ b/doc/sources/contribute.rst @@ -27,7 +27,7 @@ We use clang-format to format source code consistently. The clang-format configuration file .clang-format is located at the root directory. Since clang-format produces slightly different results between versions, we currently use clang-format which comes with -clang-3.5. +clang-3.6. To detect any violation to the coding style, we recommend to setup git pre-commit hook to check coding style of the changes you introduced. @@ -35,7 +35,7 @@ The pre-commit file is located at the root directory. Copy it under .git/hooks and make sure that it is executable. The pre-commit script uses clang-format-diff.py to detect any style errors. If it is not in your PATH or it exists under different name (e.g., -clang-format-diff-3.5 in debian), either add it to PATH variable or +clang-format-diff-3.6 in debian), either add it to PATH variable or add git option ``clangformatdiff.binary`` to point to the script. For emacs users, integrating clang-format to emacs is very easy. diff --git a/examples/client.c b/examples/client.c index 09bf7593..4280dd52 100644 --- a/examples/client.c +++ b/examples/client.c @@ -459,12 +459,11 @@ static void ctl_poll(struct pollfd *pollfd, struct Connection *connection) { static void submit_request(struct Connection *connection, struct Request *req) { int32_t stream_id; /* Make sure that the last item is NULL */ - const nghttp2_nv nva[] = {MAKE_NV(":method", "GET"), - MAKE_NV_CS(":path", req->path), - MAKE_NV(":scheme", "https"), - MAKE_NV_CS(":authority", req->hostport), - MAKE_NV("accept", "*/*"), - MAKE_NV("user-agent", "nghttp2/" NGHTTP2_VERSION)}; + const nghttp2_nv nva[] = { + MAKE_NV(":method", "GET"), MAKE_NV_CS(":path", req->path), + MAKE_NV(":scheme", "https"), MAKE_NV_CS(":authority", req->hostport), + MAKE_NV("accept", "*/*"), + MAKE_NV("user-agent", "nghttp2/" NGHTTP2_VERSION)}; stream_id = nghttp2_submit_request(connection->session, NULL, nva, sizeof(nva) / sizeof(nva[0]), NULL, req); diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 82749969..7515555c 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -2339,8 +2339,8 @@ NGHTTP2_EXTERN int nghttp2_session_send(nghttp2_session *session); * buffer up small chunks of data as necessary to avoid this * situation. */ -NGHTTP2_EXTERN ssize_t nghttp2_session_mem_send(nghttp2_session *session, - const uint8_t **data_ptr); +NGHTTP2_EXTERN ssize_t +nghttp2_session_mem_send(nghttp2_session *session, const uint8_t **data_ptr); /** * @function @@ -2538,7 +2538,7 @@ nghttp2_session_set_stream_user_data(nghttp2_session *session, * include the deferred DATA frames. */ NGHTTP2_EXTERN size_t - nghttp2_session_get_outbound_queue_size(nghttp2_session *session); +nghttp2_session_get_outbound_queue_size(nghttp2_session *session); /** * @function @@ -2554,8 +2554,9 @@ NGHTTP2_EXTERN size_t * * This function returns -1 if it fails. */ -NGHTTP2_EXTERN int32_t nghttp2_session_get_stream_effective_recv_data_length( - nghttp2_session *session, int32_t stream_id); +NGHTTP2_EXTERN int32_t +nghttp2_session_get_stream_effective_recv_data_length(nghttp2_session *session, + int32_t stream_id); /** * @function @@ -2567,8 +2568,9 @@ NGHTTP2_EXTERN int32_t nghttp2_session_get_stream_effective_recv_data_length( * * This function returns -1 if it fails. */ -NGHTTP2_EXTERN int32_t nghttp2_session_get_stream_effective_local_window_size( - nghttp2_session *session, int32_t stream_id); +NGHTTP2_EXTERN int32_t +nghttp2_session_get_stream_effective_local_window_size(nghttp2_session *session, + int32_t stream_id); /** * @function @@ -2585,7 +2587,7 @@ NGHTTP2_EXTERN int32_t nghttp2_session_get_stream_effective_local_window_size( * This function returns -1 if it fails. */ NGHTTP2_EXTERN int32_t - nghttp2_session_get_effective_recv_data_length(nghttp2_session *session); +nghttp2_session_get_effective_recv_data_length(nghttp2_session *session); /** * @function @@ -2598,7 +2600,7 @@ NGHTTP2_EXTERN int32_t * This function returns -1 if it fails. */ NGHTTP2_EXTERN int32_t - nghttp2_session_get_effective_local_window_size(nghttp2_session *session); +nghttp2_session_get_effective_local_window_size(nghttp2_session *session); /** * @function @@ -2615,8 +2617,8 @@ NGHTTP2_EXTERN int32_t * This function returns -1 if it fails. */ NGHTTP2_EXTERN int32_t - nghttp2_session_get_stream_remote_window_size(nghttp2_session *session, - int32_t stream_id); +nghttp2_session_get_stream_remote_window_size(nghttp2_session *session, + int32_t stream_id); /** * @function @@ -2626,7 +2628,7 @@ NGHTTP2_EXTERN int32_t * This function always succeeds. */ NGHTTP2_EXTERN int32_t - nghttp2_session_get_remote_window_size(nghttp2_session *session); +nghttp2_session_get_remote_window_size(nghttp2_session *session); /** * @function @@ -2752,8 +2754,8 @@ NGHTTP2_EXTERN int nghttp2_submit_shutdown_notice(nghttp2_session *session); * :enum:`nghttp2_settings_id`. */ NGHTTP2_EXTERN uint32_t - nghttp2_session_get_remote_settings(nghttp2_session *session, - nghttp2_settings_id id); +nghttp2_session_get_remote_settings(nghttp2_session *session, + nghttp2_settings_id id); /** * @function @@ -2782,7 +2784,7 @@ NGHTTP2_EXTERN int nghttp2_session_set_next_stream_id(nghttp2_session *session, * function returns 1 << 31. */ NGHTTP2_EXTERN uint32_t - nghttp2_session_get_next_stream_id(nghttp2_session *session); +nghttp2_session_get_next_stream_id(nghttp2_session *session); /** * @function @@ -2970,8 +2972,8 @@ NGHTTP2_EXTERN int nghttp2_session_upgrade2(nghttp2_session *session, * The provided |buflen| size is too small to hold the output. */ NGHTTP2_EXTERN ssize_t - nghttp2_pack_settings_payload(uint8_t *buf, size_t buflen, - const nghttp2_settings_entry *iv, size_t niv); +nghttp2_pack_settings_payload(uint8_t *buf, size_t buflen, + const nghttp2_settings_entry *iv, size_t niv); /** * @function @@ -3082,11 +3084,11 @@ nghttp2_priority_spec_check_default(const nghttp2_priority_spec *pri_spec); * */ NGHTTP2_EXTERN int32_t - nghttp2_submit_request(nghttp2_session *session, - const nghttp2_priority_spec *pri_spec, - const nghttp2_nv *nva, size_t nvlen, - const nghttp2_data_provider *data_prd, - void *stream_user_data); +nghttp2_submit_request(nghttp2_session *session, + const nghttp2_priority_spec *pri_spec, + const nghttp2_nv *nva, size_t nvlen, + const nghttp2_data_provider *data_prd, + void *stream_user_data); /** * @function @@ -3297,11 +3299,10 @@ NGHTTP2_EXTERN int nghttp2_submit_trailer(nghttp2_session *session, * */ NGHTTP2_EXTERN int32_t - nghttp2_submit_headers(nghttp2_session *session, uint8_t flags, - int32_t stream_id, - const nghttp2_priority_spec *pri_spec, - const nghttp2_nv *nva, size_t nvlen, - void *stream_user_data); +nghttp2_submit_headers(nghttp2_session *session, uint8_t flags, + int32_t stream_id, const nghttp2_priority_spec *pri_spec, + const nghttp2_nv *nva, size_t nvlen, + void *stream_user_data); /** * @function @@ -3504,9 +3505,9 @@ NGHTTP2_EXTERN int nghttp2_submit_settings(nghttp2_session *session, * */ NGHTTP2_EXTERN int32_t - nghttp2_submit_push_promise(nghttp2_session *session, uint8_t flags, - int32_t stream_id, const nghttp2_nv *nva, - size_t nvlen, void *promised_stream_user_data); +nghttp2_submit_push_promise(nghttp2_session *session, uint8_t flags, + int32_t stream_id, const nghttp2_nv *nva, + size_t nvlen, void *promised_stream_user_data); /** * @function @@ -3595,7 +3596,7 @@ NGHTTP2_EXTERN int nghttp2_submit_goaway(nghttp2_session *session, * This function always succeeds. */ NGHTTP2_EXTERN int32_t - nghttp2_session_get_last_proc_stream_id(nghttp2_session *session); +nghttp2_session_get_last_proc_stream_id(nghttp2_session *session); /** * @function @@ -3880,8 +3881,8 @@ nghttp2_hd_deflate_change_table_size(nghttp2_hd_deflater *deflater, * The provided |buflen| size is too small to hold the output. */ NGHTTP2_EXTERN ssize_t - nghttp2_hd_deflate_hd(nghttp2_hd_deflater *deflater, uint8_t *buf, - size_t buflen, const nghttp2_nv *nva, size_t nvlen); +nghttp2_hd_deflate_hd(nghttp2_hd_deflater *deflater, uint8_t *buf, + size_t buflen, const nghttp2_nv *nva, size_t nvlen); /** * @function @@ -4239,7 +4240,7 @@ typedef enum { * :enum:`NGHTTP2_STREAM_STATE_IDLE`. */ NGHTTP2_EXTERN nghttp2_stream_proto_state - nghttp2_stream_get_state(nghttp2_stream *stream); +nghttp2_stream_get_state(nghttp2_stream *stream); /** * @function @@ -4302,7 +4303,7 @@ NGHTTP2_EXTERN int32_t nghttp2_stream_get_weight(nghttp2_stream *stream); * Returns the sum of the weight for |stream|'s children. */ NGHTTP2_EXTERN int32_t - nghttp2_stream_get_sum_dependency_weight(nghttp2_stream *stream); +nghttp2_stream_get_sum_dependency_weight(nghttp2_stream *stream); #ifdef __cplusplus } diff --git a/lib/nghttp2_helper.h b/lib/nghttp2_helper.h index 9e2a71d3..1fff65c0 100644 --- a/lib/nghttp2_helper.h +++ b/lib/nghttp2_helper.h @@ -41,7 +41,7 @@ #define lstreq(A, B, N) ((sizeof((A)) - 1) == (N) && memcmp((A), (B), (N)) == 0) #define nghttp2_struct_of(ptr, type, member) \ - ((type *)(void *)((char *)(ptr) - offsetof(type, member))) + ((type *)(void *)((char *)(ptr)-offsetof(type, member))) /* * Copies 2 byte unsigned integer |n| in host byte order to |buf| in diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index dfb29a0d..b84b6d1a 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -610,10 +610,9 @@ void nghttp2_session_del(nghttp2_session *session) { nghttp2_mem_free(mem, session); } -int -nghttp2_session_reprioritize_stream(nghttp2_session *session, - nghttp2_stream *stream, - const nghttp2_priority_spec *pri_spec_in) { +int nghttp2_session_reprioritize_stream( + nghttp2_session *session, nghttp2_stream *stream, + const nghttp2_priority_spec *pri_spec_in) { int rv; nghttp2_stream *dep_stream = NULL; nghttp2_priority_spec pri_spec_default; diff --git a/src/asio_client_session_impl.cc b/src/asio_client_session_impl.cc index 7baa385e..d3a57a1e 100644 --- a/src/asio_client_session_impl.cc +++ b/src/asio_client_session_impl.cc @@ -59,13 +59,13 @@ void session_impl::start_resolve(const std::string &host, resolver_.async_resolve({host, service}, [this](const boost::system::error_code &ec, tcp::resolver::iterator endpoint_it) { - if (ec) { - not_connected(ec); - return; - } + if (ec) { + not_connected(ec); + return; + } - start_connect(endpoint_it); - }); + start_connect(endpoint_it); + }); } void session_impl::connected(tcp::resolver::iterator endpoint_it) { @@ -462,9 +462,9 @@ const request *session_impl::submit(boost::system::error_code &ec, [](nghttp2_session *session, int32_t stream_id, uint8_t *buf, size_t length, uint32_t *data_flags, nghttp2_data_source *source, void *user_data) -> ssize_t { - auto strm = static_cast(source->ptr); - return strm->request().impl().call_on_read(buf, length, data_flags); - }; + auto strm = static_cast(source->ptr); + return strm->request().impl().call_on_read(buf, length, data_flags); + }; prdptr = &prd; } diff --git a/src/asio_client_session_tcp_impl.cc b/src/asio_client_session_tcp_impl.cc index 41195bad..c20a2303 100644 --- a/src/asio_client_session_tcp_impl.cc +++ b/src/asio_client_session_tcp_impl.cc @@ -41,13 +41,13 @@ void session_tcp_impl::start_connect(tcp::resolver::iterator endpoint_it) { boost::asio::async_connect(socket_, endpoint_it, [this](const boost::system::error_code &ec, tcp::resolver::iterator endpoint_it) { - if (ec) { - not_connected(ec); - return; - } + if (ec) { + not_connected(ec); + return; + } - connected(endpoint_it); - }); + connected(endpoint_it); + }); } tcp::socket &session_tcp_impl::socket() { return socket_; } diff --git a/src/asio_client_session_tls_impl.cc b/src/asio_client_session_tls_impl.cc index 8db75c17..8dc32d57 100644 --- a/src/asio_client_session_tls_impl.cc +++ b/src/asio_client_session_tls_impl.cc @@ -45,31 +45,31 @@ session_tls_impl::session_tls_impl(boost::asio::io_service &io_service, session_tls_impl::~session_tls_impl() {} void session_tls_impl::start_connect(tcp::resolver::iterator endpoint_it) { - boost::asio::async_connect(socket(), endpoint_it, - [this](const boost::system::error_code &ec, + boost::asio::async_connect( + socket(), endpoint_it, [this](const boost::system::error_code &ec, tcp::resolver::iterator endpoint_it) { - if (ec) { - not_connected(ec); - return; - } + if (ec) { + not_connected(ec); + return; + } - socket_.async_handshake( - boost::asio::ssl::stream_base::client, - [this, endpoint_it](const boost::system::error_code &ec) { - if (ec) { - not_connected(ec); - return; - } + socket_.async_handshake( + boost::asio::ssl::stream_base::client, + [this, endpoint_it](const boost::system::error_code &ec) { + if (ec) { + not_connected(ec); + return; + } - if (!tls_h2_negotiated(socket_)) { - not_connected( - make_error_code(NGHTTP2_ASIO_ERR_TLS_NO_APP_PROTO_NEGOTIATED)); - return; - } + if (!tls_h2_negotiated(socket_)) { + not_connected(make_error_code( + NGHTTP2_ASIO_ERR_TLS_NO_APP_PROTO_NEGOTIATED)); + return; + } - connected(endpoint_it); - }); - }); + connected(endpoint_it); + }); + }); } tcp::socket &session_tls_impl::socket() { return socket_.next_layer(); } diff --git a/src/asio_common.cc b/src/asio_common.cc index f8567a5f..0319169e 100644 --- a/src/asio_common.cc +++ b/src/asio_common.cc @@ -90,8 +90,9 @@ generator_cb string_generator(std::string data) { } generator_cb deferred_generator() { - return [](uint8_t *buf, size_t len, - uint32_t *data_flags) { return NGHTTP2_ERR_DEFERRED; }; + return [](uint8_t *buf, size_t len, uint32_t *data_flags) { + return NGHTTP2_ERR_DEFERRED; + }; } template @@ -114,20 +115,20 @@ generator_cb file_generator_from_fd(int fd) { return [fd, d](uint8_t *buf, size_t len, uint32_t *data_flags) -> generator_cb::result_type { - ssize_t n; - while ((n = read(fd, buf, len)) == -1 && errno == EINTR) - ; + ssize_t n; + while ((n = read(fd, buf, len)) == -1 && errno == EINTR) + ; - if (n == -1) { - return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; - } + if (n == -1) { + return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; + } - if (n == 0) { - *data_flags |= NGHTTP2_DATA_FLAG_EOF; - } + if (n == 0) { + *data_flags |= NGHTTP2_DATA_FLAG_EOF; + } - return n; - }; + return n; + }; } bool check_path(const std::string &path) { return util::check_path(path); } diff --git a/src/asio_server.cc b/src/asio_server.cc index 11c6c31d..531677f3 100644 --- a/src/asio_server.cc +++ b/src/asio_server.cc @@ -123,44 +123,46 @@ void server::start_accept(boost::asio::ssl::context &tls_context, auto new_connection = std::make_shared>( mux, io_service_pool_.get_io_service(), tls_context); - acceptor.async_accept(new_connection->socket().lowest_layer(), - [this, &tls_context, &acceptor, &mux, new_connection]( - const boost::system::error_code &e) { - if (!e) { - new_connection->socket().lowest_layer().set_option(tcp::no_delay(true)); - new_connection->socket().async_handshake( - boost::asio::ssl::stream_base::server, - [new_connection](const boost::system::error_code &e) { - if (e) { - return; - } + acceptor.async_accept( + new_connection->socket().lowest_layer(), + [this, &tls_context, &acceptor, &mux, new_connection]( + const boost::system::error_code &e) { + if (!e) { + new_connection->socket().lowest_layer().set_option( + tcp::no_delay(true)); + new_connection->socket().async_handshake( + boost::asio::ssl::stream_base::server, + [new_connection](const boost::system::error_code &e) { + if (e) { + return; + } - if (!tls_h2_negotiated(new_connection->socket())) { - return; - } + if (!tls_h2_negotiated(new_connection->socket())) { + return; + } - new_connection->start(); - }); - } + new_connection->start(); + }); + } - start_accept(tls_context, acceptor, mux); - }); + start_accept(tls_context, acceptor, mux); + }); } void server::start_accept(tcp::acceptor &acceptor, serve_mux &mux) { auto new_connection = std::make_shared>( mux, io_service_pool_.get_io_service()); - acceptor.async_accept(new_connection->socket(), - [this, &acceptor, &mux, new_connection]( - const boost::system::error_code &e) { - if (!e) { - new_connection->socket().set_option(tcp::no_delay(true)); - new_connection->start(); - } + acceptor.async_accept( + new_connection->socket(), [this, &acceptor, &mux, new_connection]( + const boost::system::error_code &e) { + if (!e) { + new_connection->socket().set_option(tcp::no_delay(true)); + new_connection->start(); + } - start_accept(acceptor, mux); - }); + start_accept(acceptor, mux); + }); } void server::stop() { io_service_pool_.stop(); } diff --git a/src/asio_server_connection.h b/src/asio_server_connection.h index 0d58b882..3aba7336 100644 --- a/src/asio_server_connection.h +++ b/src/asio_server_connection.h @@ -83,29 +83,30 @@ public: void do_read() { auto self = this->shared_from_this(); - socket_.async_read_some(boost::asio::buffer(buffer_), - [this, self](const boost::system::error_code &e, - std::size_t bytes_transferred) { - if (!e) { - if (handler_->on_read(buffer_, bytes_transferred) != 0) { - return; - } + socket_.async_read_some( + boost::asio::buffer(buffer_), + [this, self](const boost::system::error_code &e, + std::size_t bytes_transferred) { + if (!e) { + if (handler_->on_read(buffer_, bytes_transferred) != 0) { + return; + } - do_write(); + do_write(); - if (!writing_ && handler_->should_stop()) { - return; - } + if (!writing_ && handler_->should_stop()) { + return; + } - do_read(); - } + do_read(); + } - // If an error occurs then no new asynchronous operations are - // started. This means that all shared_ptr references to the - // connection object will disappear and the object will be - // destroyed automatically after this handler returns. The - // connection class's destructor closes the socket. - }); + // If an error occurs then no new asynchronous operations are + // started. This means that all shared_ptr references to the + // connection object will disappear and the object will be + // destroyed automatically after this handler returns. The + // connection class's destructor closes the socket. + }); } void do_write() { diff --git a/src/asio_server_http2_handler.cc b/src/asio_server_http2_handler.cc index c55cb329..47cd18c5 100644 --- a/src/asio_server_http2_handler.cc +++ b/src/asio_server_http2_handler.cc @@ -332,9 +332,9 @@ int http2_handler::start_response(stream &strm) { [](nghttp2_session *session, int32_t stream_id, uint8_t *buf, size_t length, uint32_t *data_flags, nghttp2_data_source *source, void *user_data) -> ssize_t { - auto &strm = *static_cast(source->ptr); - return strm.response().impl().call_read(buf, length, data_flags); - }; + auto &strm = *static_cast(source->ptr); + return strm.response().impl().call_read(buf, length, data_flags); + }; prd_ptr = &prd; } rv = nghttp2_submit_response(session_, strm.get_stream_id(), nva.data(), diff --git a/src/http2.h b/src/http2.h index 2669dd49..dedee285 100644 --- a/src/http2.h +++ b/src/http2.h @@ -119,20 +119,20 @@ nghttp2_nv make_nv_nocopy(const std::string &name, const std::string &value, // Create nghttp2_nv from string literal |name| and |value|. template -constexpr nghttp2_nv make_nv_ll(const char (&name)[N], const char (&value)[M]) { +constexpr nghttp2_nv make_nv_ll(const char(&name)[N], const char(&value)[M]) { return {(uint8_t *)name, (uint8_t *)value, N - 1, M - 1, NGHTTP2_NV_FLAG_NO_COPY_NAME | NGHTTP2_NV_FLAG_NO_COPY_VALUE}; } // Create nghttp2_nv from string literal |name| and c-string |value|. template -nghttp2_nv make_nv_lc(const char (&name)[N], const char *value) { +nghttp2_nv make_nv_lc(const char(&name)[N], const char *value) { return {(uint8_t *)name, (uint8_t *)value, N - 1, strlen(value), NGHTTP2_NV_FLAG_NO_COPY_NAME}; } template -nghttp2_nv make_nv_lc_nocopy(const char (&name)[N], const char *value) { +nghttp2_nv make_nv_lc_nocopy(const char(&name)[N], const char *value) { return {(uint8_t *)name, (uint8_t *)value, N - 1, strlen(value), NGHTTP2_NV_FLAG_NO_COPY_NAME | NGHTTP2_NV_FLAG_NO_COPY_VALUE}; } @@ -140,13 +140,13 @@ nghttp2_nv make_nv_lc_nocopy(const char (&name)[N], const char *value) { // Create nghttp2_nv from string literal |name| and std::string // |value|. template -nghttp2_nv make_nv_ls(const char (&name)[N], const std::string &value) { +nghttp2_nv make_nv_ls(const char(&name)[N], const std::string &value) { return {(uint8_t *)name, (uint8_t *)value.c_str(), N - 1, value.size(), NGHTTP2_NV_FLAG_NO_COPY_NAME}; } template -nghttp2_nv make_nv_ls_nocopy(const char (&name)[N], const std::string &value) { +nghttp2_nv make_nv_ls_nocopy(const char(&name)[N], const std::string &value) { return {(uint8_t *)name, (uint8_t *)value.c_str(), N - 1, value.size(), NGHTTP2_NV_FLAG_NO_COPY_NAME | NGHTTP2_NV_FLAG_NO_COPY_VALUE}; } diff --git a/src/includes/nghttp2/asio_http2.h b/src/includes/nghttp2/asio_http2.h index a95d7b95..0a4044aa 100644 --- a/src/includes/nghttp2/asio_http2.h +++ b/src/includes/nghttp2/asio_http2.h @@ -89,8 +89,8 @@ typedef std::function close_cb; // are not available right now, return NGHTTP2_ERR_DEFERRED. In case // of the error and request/response must be closed, return // NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE. -typedef std::function< - ssize_t(uint8_t *buf, std::size_t len, uint32_t *data_flags)> generator_cb; +typedef std::function generator_cb; // Convenient function to create function to read file denoted by // |path|. This can be passed to response::end(). diff --git a/src/memchunk.h b/src/memchunk.h index 4b29bc29..6245f61f 100644 --- a/src/memchunk.h +++ b/src/memchunk.h @@ -161,7 +161,7 @@ template struct Memchunks { return count; } - template size_t append(const char (&s)[N]) { + template size_t append(const char(&s)[N]) { return append(s, N - 1); } size_t append(const std::string &s) { return append(s.c_str(), s.size()); } diff --git a/src/memchunk_test.cc b/src/memchunk_test.cc index c206d153..071fbbb1 100644 --- a/src/memchunk_test.cc +++ b/src/memchunk_test.cc @@ -226,8 +226,7 @@ void test_peek_memchunks_append(void) { std::array b{{ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '0', '1', '2', '3', '4', - '5', '6', '7', '8', '9', '0', '1', - }}, + '5', '6', '7', '8', '9', '0', '1', }}, d; pchunks.append(b.data(), b.size()); @@ -261,8 +260,7 @@ void test_peek_memchunks_disable_peek_drain(void) { std::array b{{ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '0', '1', '2', '3', '4', - '5', '6', '7', '8', '9', '0', '1', - }}, + '5', '6', '7', '8', '9', '0', '1', }}, d; pchunks.append(b.data(), b.size()); @@ -289,8 +287,7 @@ void test_peek_memchunks_disable_peek_no_drain(void) { std::array b{{ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '0', '1', '2', '3', '4', - '5', '6', '7', '8', '9', '0', '1', - }}, + '5', '6', '7', '8', '9', '0', '1', }}, d; pchunks.append(b.data(), b.size()); @@ -317,8 +314,7 @@ void test_peek_memchunks_reset(void) { std::array b{{ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', '0', '1', '2', '3', '4', - '5', '6', '7', '8', '9', '0', '1', - }}, + '5', '6', '7', '8', '9', '0', '1', }}, d; pchunks.append(b.data(), b.size()); diff --git a/src/nghttp.cc b/src/nghttp.cc index 625f0905..e48d6592 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -89,8 +89,7 @@ enum { namespace { constexpr auto anchors = std::array{{ - {3, 0, 201}, {5, 0, 101}, {7, 0, 1}, {9, 7, 1}, {11, 3, 1}, -}}; + {3, 0, 201}, {5, 0, 101}, {7, 0, 1}, {9, 7, 1}, {11, 3, 1}, }}; } // namespace Config::Config() @@ -554,10 +553,11 @@ int HttpClient::initiate_connection() { // If the user overrode the :authority or host header, use that // value for the SNI extension const char *host_string = nullptr; - auto i = std::find_if(std::begin(config.headers), - std::end(config.headers), [](const Header &nv) { - return ":authority" == nv.name || "host" == nv.name; - }); + auto i = + std::find_if(std::begin(config.headers), std::end(config.headers), + [](const Header &nv) { + return ":authority" == nv.name || "host" == nv.name; + }); if (i != std::end(config.headers)) { host_string = (*i).value.c_str(); } else { @@ -1933,12 +1933,12 @@ void print_stats(const HttpClient &client) { std::sort(std::begin(reqs), std::end(reqs), [](const Request *lhs, const Request *rhs) { - const auto <iming = lhs->timing; - const auto &rtiming = rhs->timing; - return ltiming.response_end_time < rtiming.response_end_time || - (ltiming.response_end_time == rtiming.response_end_time && - ltiming.request_start_time < rtiming.request_start_time); - }); + const auto <iming = lhs->timing; + const auto &rtiming = rhs->timing; + return ltiming.response_end_time < rtiming.response_end_time || + (ltiming.response_end_time == rtiming.response_end_time && + ltiming.request_start_time < rtiming.request_start_time); + }); std::cout << R"( Request timing: @@ -2027,7 +2027,8 @@ namespace { int communicate( const std::string &scheme, const std::string &host, uint16_t port, std::vector> - requests, const nghttp2_session_callbacks *callbacks) { + requests, + const nghttp2_session_callbacks *callbacks) { int result = 0; auto loop = EV_DEFAULT; SSL_CTX *ssl_ctx = nullptr; diff --git a/src/shrpx_connection_handler.cc b/src/shrpx_connection_handler.cc index 76e1edc6..536b3e16 100644 --- a/src/shrpx_connection_handler.cc +++ b/src/shrpx_connection_handler.cc @@ -175,12 +175,12 @@ int ConnectionHandler::create_single_worker() { , nb_.get() #endif // HAVE_NEVERBLEED - ); + ); auto cl_ssl_ctx = ssl::setup_client_ssl_context( #ifdef HAVE_NEVERBLEED nb_.get() #endif // HAVE_NEVERBLEED - ); + ); if (cl_ssl_ctx) { all_ssl_ctx_.push_back(cl_ssl_ctx); @@ -207,12 +207,12 @@ int ConnectionHandler::create_worker_thread(size_t num) { , nb_.get() #endif // HAVE_NEVERBLEED - ); + ); auto cl_ssl_ctx = ssl::setup_client_ssl_context( #ifdef HAVE_NEVERBLEED nb_.get() #endif // HAVE_NEVERBLEED - ); + ); if (cl_ssl_ctx) { all_ssl_ctx_.push_back(cl_ssl_ctx); @@ -401,8 +401,8 @@ void ConnectionHandler::accept_pending_connection() { } } -void -ConnectionHandler::set_ticket_keys(std::shared_ptr ticket_keys) { +void ConnectionHandler::set_ticket_keys( + std::shared_ptr ticket_keys) { ticket_keys_ = std::move(ticket_keys); if (single_worker_) { single_worker_->set_ticket_keys(ticket_keys_); @@ -740,8 +740,8 @@ void ConnectionHandler::on_tls_ticket_key_get_success( set_ticket_keys_to_worker(ticket_keys); } -void -ConnectionHandler::schedule_next_tls_ticket_key_memcached_get(ev_timer *w) { +void ConnectionHandler::schedule_next_tls_ticket_key_memcached_get( + ev_timer *w) { ev_timer_set(w, get_config()->tls_ticket_key_memcached_interval, 0.); ev_timer_start(loop_, w); } diff --git a/src/shrpx_downstream_test.cc b/src/shrpx_downstream_test.cc index 871cac70..f74647ea 100644 --- a/src/shrpx_downstream_test.cc +++ b/src/shrpx_downstream_test.cc @@ -120,10 +120,10 @@ void test_downstream_crumble_request_cookie(void) { Headers cookies; std::transform(std::begin(nva), std::end(nva), std::back_inserter(cookies), [](const nghttp2_nv &nv) { - return Header(std::string(nv.name, nv.name + nv.namelen), - std::string(nv.value, nv.value + nv.valuelen), - nv.flags & NGHTTP2_NV_FLAG_NO_INDEX); - }); + return Header(std::string(nv.name, nv.name + nv.namelen), + std::string(nv.value, nv.value + nv.valuelen), + nv.flags & NGHTTP2_NV_FLAG_NO_INDEX); + }); Headers ans = {{"cookie", "alpha"}, {"cookie", "bravo"}, diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index 6f255cf9..2258903c 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -553,8 +553,8 @@ void Http2Session::add_downstream_connection(Http2DownstreamConnection *dconn) { dconns_.append(dconn); } -void -Http2Session::remove_downstream_connection(Http2DownstreamConnection *dconn) { +void Http2Session::remove_downstream_connection( + Http2DownstreamConnection *dconn) { dconns_.remove(dconn); dconn->detach_stream_data(); } diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 05d0f70a..1c6af673 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -1391,8 +1391,8 @@ int Http2Upstream::error_reply(Downstream *downstream, return 0; } -void -Http2Upstream::add_pending_downstream(std::unique_ptr downstream) { +void Http2Upstream::add_pending_downstream( + std::unique_ptr downstream) { downstream_queue_.add_pending(std::move(downstream)); } @@ -1670,9 +1670,8 @@ int Http2Upstream::consume(int32_t stream_id, size_t len) { return 0; } -void -Http2Upstream::log_response_headers(Downstream *downstream, - const std::vector &nva) const { +void Http2Upstream::log_response_headers( + Downstream *downstream, const std::vector &nva) const { std::stringstream ss; for (auto &nv : nva) { ss << TTY_HTTP_HD << nv.name << TTY_RST << ": " << nv.value << "\n"; diff --git a/src/shrpx_log.cc b/src/shrpx_log.cc index b1f435e6..59095b51 100644 --- a/src/shrpx_log.cc +++ b/src/shrpx_log.cc @@ -184,7 +184,7 @@ std::pair copy(const std::string &src, size_t avail, namespace { template -std::pair copy_l(const char (&src)[N], size_t avail, +std::pair copy_l(const char(&src)[N], size_t avail, OutputIterator oitr) { return copy(src, N - 1, avail, oitr); } diff --git a/src/shrpx_log_config.cc b/src/shrpx_log_config.cc index 3d77673d..2222425c 100644 --- a/src/shrpx_log_config.cc +++ b/src/shrpx_log_config.cc @@ -52,8 +52,8 @@ static LogConfig *config = new LogConfig(); LogConfig *log_config(void) { return config; } #endif // NOTHREADS -void -LogConfig::update_tstamp(const std::chrono::system_clock::time_point &now) { +void LogConfig::update_tstamp( + const std::chrono::system_clock::time_point &now) { auto t0 = std::chrono::system_clock::to_time_t(time_str_updated_); auto t1 = std::chrono::system_clock::to_time_t(now); if (t0 == t1) { diff --git a/src/shrpx_spdy_upstream.cc b/src/shrpx_spdy_upstream.cc index 6b45a2e6..2ce0491c 100644 --- a/src/shrpx_spdy_upstream.cc +++ b/src/shrpx_spdy_upstream.cc @@ -888,13 +888,11 @@ int SpdyUpstream::error_reply(Downstream *downstream, std::string content_length = util::utos(html.size()); std::string status_string = http2::get_status_string(status_code); - const char *nv[] = {":status", status_string.c_str(), - ":version", "http/1.1", - "content-type", "text/html; charset=UTF-8", - "server", get_config()->server_name, - "content-length", content_length.c_str(), - "date", lgconf->time_http_str.c_str(), - nullptr}; + const char *nv[] = {":status", status_string.c_str(), ":version", "http/1.1", + "content-type", "text/html; charset=UTF-8", "server", + get_config()->server_name, "content-length", + content_length.c_str(), "date", + lgconf->time_http_str.c_str(), nullptr}; rv = spdylay_submit_response(session_, downstream->get_stream_id(), nv, &data_prd); diff --git a/src/shrpx_ssl.cc b/src/shrpx_ssl.cc index d204e2d6..8c4b4166 100644 --- a/src/shrpx_ssl.cc +++ b/src/shrpx_ssl.cc @@ -1145,7 +1145,7 @@ SSL_CTX *setup_server_ssl_context(std::vector &all_ssl_ctx, auto ssl_ctx = ssl::create_ssl_context(get_config()->private_key_file.get(), get_config()->cert_file.get() #ifdef HAVE_NEVERBLEED - , + , nb #endif // HAVE_NEVERBLEED ); @@ -1166,7 +1166,7 @@ SSL_CTX *setup_server_ssl_context(std::vector &all_ssl_ctx, auto ssl_ctx = ssl::create_ssl_context(keycert.first.c_str(), keycert.second.c_str() #ifdef HAVE_NEVERBLEED - , + , nb #endif // HAVE_NEVERBLEED ); diff --git a/src/shrpx_ssl_test.cc b/src/shrpx_ssl_test.cc index 22bac630..ff646b85 100644 --- a/src/shrpx_ssl_test.cc +++ b/src/shrpx_ssl_test.cc @@ -43,13 +43,12 @@ void test_shrpx_ssl_create_lookup_tree(void) { SSL_CTX_new(SSLv23_method()), SSL_CTX_new(SSLv23_method()), SSL_CTX_new(SSLv23_method()), SSL_CTX_new(SSLv23_method())}; - const char *hostnames[] = {"example.com", "www.example.org", - "*www.example.org", "x*.host.domain", - "*yy.host.domain", "nghttp2.sourceforge.net", - "sourceforge.net", - "sourceforge.net", // duplicate - "*.foo.bar", // oo.bar is suffix of *.foo.bar - "oo.bar"}; + const char *hostnames[] = { + "example.com", "www.example.org", "*www.example.org", "x*.host.domain", + "*yy.host.domain", "nghttp2.sourceforge.net", "sourceforge.net", + "sourceforge.net", // duplicate + "*.foo.bar", // oo.bar is suffix of *.foo.bar + "oo.bar"}; int num = array_size(ctxs); for (int i = 0; i < num; ++i) { tree->add_cert(ctxs[i], hostnames[i], strlen(hostnames[i])); diff --git a/src/template.h b/src/template.h index 5300902e..3f92bfac 100644 --- a/src/template.h +++ b/src/template.h @@ -60,11 +60,11 @@ make_array(T &&... t) { sizeof...(T)>{{std::forward(t)...}}; } -template constexpr size_t array_size(T (&)[N]) { +template constexpr size_t array_size(T(&)[N]) { return N; } -template constexpr size_t str_size(T (&)[N]) { +template constexpr size_t str_size(T(&)[N]) { return N - 1; } @@ -76,8 +76,8 @@ template struct Defer { Defer(Defer &&o) : f(std::move(o.f)) {} ~Defer() { f(); } - using ResultType = typename std::result_of< - typename std::decay::type(typename std::decay::type...)>::type; + using ResultType = typename std::result_of::type( + typename std::decay::type...)>::type; std::function f; }; diff --git a/src/util.cc b/src/util.cc index 0870b20e..1a040cf5 100644 --- a/src/util.cc +++ b/src/util.cc @@ -433,15 +433,15 @@ std::string format_hex(const unsigned char *s, size_t len) { void to_token68(std::string &base64str) { std::transform(std::begin(base64str), std::end(base64str), std::begin(base64str), [](char c) { - switch (c) { - case '+': - return '-'; - case '/': - return '_'; - default: - return c; - } - }); + switch (c) { + case '+': + return '-'; + case '/': + return '_'; + default: + return c; + } + }); base64str.erase(std::find(std::begin(base64str), std::end(base64str), '='), std::end(base64str)); } @@ -449,15 +449,15 @@ void to_token68(std::string &base64str) { void to_base64(std::string &token68str) { std::transform(std::begin(token68str), std::end(token68str), std::begin(token68str), [](char c) { - switch (c) { - case '-': - return '+'; - case '_': - return '/'; - default: - return c; - } - }); + switch (c) { + case '-': + return '+'; + case '_': + return '/'; + default: + return c; + } + }); if (token68str.size() & 0x3) { token68str.append(4 - (token68str.size() & 0x3), '='); } diff --git a/src/util.h b/src/util.h index 3a835e75..8eb88cdd 100644 --- a/src/util.h +++ b/src/util.h @@ -220,7 +220,7 @@ std::string quote_string(const std::string &target); std::string format_hex(const unsigned char *s, size_t len); -template std::string format_hex(const unsigned char (&s)[N]) { +template std::string format_hex(const unsigned char(&s)[N]) { return format_hex(s, N); } @@ -366,11 +366,11 @@ inline bool strieq(const char *a, const std::string &b) { } template -bool strieq_l(const char (&a)[N], InputIt b, size_t blen) { +bool strieq_l(const char(&a)[N], InputIt b, size_t blen) { return strieq(a, N - 1, b, blen); } -template bool strieq_l(const char (&a)[N], const std::string &b) { +template bool strieq_l(const char(&a)[N], const std::string &b) { return strieq(a, N - 1, std::begin(b), b.size()); } @@ -400,11 +400,11 @@ inline bool streq(const char *a, const char *b) { } template -bool streq_l(const char (&a)[N], InputIt b, size_t blen) { +bool streq_l(const char(&a)[N], InputIt b, size_t blen) { return streq(a, N - 1, b, blen); } -template bool streq_l(const char (&a)[N], const std::string &b) { +template bool streq_l(const char(&a)[N], const std::string &b) { return streq(a, N - 1, std::begin(b), b.size()); } diff --git a/tests/nghttp2_hd_test.c b/tests/nghttp2_hd_test.c index c71117ce..5a4d7c5f 100644 --- a/tests/nghttp2_hd_test.c +++ b/tests/nghttp2_hd_test.c @@ -1007,25 +1007,20 @@ void test_nghttp2_hd_deflate_inflate(void) { nghttp2_hd_deflater deflater; nghttp2_hd_inflater inflater; nghttp2_nv nv1[] = { - MAKE_NV(":status", "200 OK"), - MAKE_NV("access-control-allow-origin", "*"), + MAKE_NV(":status", "200 OK"), MAKE_NV("access-control-allow-origin", "*"), MAKE_NV("cache-control", "private, max-age=0, must-revalidate"), - MAKE_NV("content-length", "76073"), - MAKE_NV("content-type", "text/html"), + MAKE_NV("content-length", "76073"), MAKE_NV("content-type", "text/html"), MAKE_NV("date", "Sat, 27 Jul 2013 06:22:12 GMT"), MAKE_NV("expires", "Sat, 27 Jul 2013 06:22:12 GMT"), - MAKE_NV("server", "Apache"), - MAKE_NV("vary", "foobar"), + MAKE_NV("server", "Apache"), MAKE_NV("vary", "foobar"), MAKE_NV("via", "1.1 alphabravo (squid/3.x.x), 1.1 nghttpx"), MAKE_NV("x-cache", "MISS from alphabravo"), - MAKE_NV("x-cache-action", "MISS"), - MAKE_NV("x-cache-age", "0"), + MAKE_NV("x-cache-action", "MISS"), MAKE_NV("x-cache-age", "0"), MAKE_NV("x-cache-lookup", "MISS from alphabravo:3128"), MAKE_NV("x-lb-nocache", "true"), }; nghttp2_nv nv2[] = { - MAKE_NV(":status", "304 Not Modified"), - MAKE_NV("age", "0"), + MAKE_NV(":status", "304 Not Modified"), MAKE_NV("age", "0"), MAKE_NV("cache-control", "max-age=56682045"), MAKE_NV("content-type", "text/css"), MAKE_NV("date", "Sat, 27 Jul 2013 06:22:12 GMT"), @@ -1036,8 +1031,7 @@ void test_nghttp2_hd_deflate_inflate(void) { MAKE_NV("x-cache", "HIT from alphabravo"), MAKE_NV("x-cache-lookup", "HIT from alphabravo:3128")}; nghttp2_nv nv3[] = { - MAKE_NV(":status", "304 Not Modified"), - MAKE_NV("age", "0"), + MAKE_NV(":status", "304 Not Modified"), MAKE_NV("age", "0"), MAKE_NV("cache-control", "max-age=56682072"), MAKE_NV("content-type", "text/css"), MAKE_NV("date", "Sat, 27 Jul 2013 06:22:12 GMT"), @@ -1049,8 +1043,7 @@ void test_nghttp2_hd_deflate_inflate(void) { MAKE_NV("x-cache-lookup", "HIT from alphabravo:3128"), }; nghttp2_nv nv4[] = { - MAKE_NV(":status", "304 Not Modified"), - MAKE_NV("age", "0"), + MAKE_NV(":status", "304 Not Modified"), MAKE_NV("age", "0"), MAKE_NV("cache-control", "max-age=56682022"), MAKE_NV("content-type", "text/css"), MAKE_NV("date", "Sat, 27 Jul 2013 06:22:12 GMT"), @@ -1062,8 +1055,7 @@ void test_nghttp2_hd_deflate_inflate(void) { MAKE_NV("x-cache-lookup", "HIT from alphabravo:3128"), }; nghttp2_nv nv5[] = { - MAKE_NV(":status", "304 Not Modified"), - MAKE_NV("age", "0"), + MAKE_NV(":status", "304 Not Modified"), MAKE_NV("age", "0"), MAKE_NV("cache-control", "max-age=4461139"), MAKE_NV("content-type", "application/x-javascript"), MAKE_NV("date", "Sat, 27 Jul 2013 06:22:12 GMT"), @@ -1075,8 +1067,7 @@ void test_nghttp2_hd_deflate_inflate(void) { MAKE_NV("x-cache-lookup", "HIT from alphabravo:3128"), }; nghttp2_nv nv6[] = { - MAKE_NV(":status", "304 Not Modified"), - MAKE_NV("age", "0"), + MAKE_NV(":status", "304 Not Modified"), MAKE_NV("age", "0"), MAKE_NV("cache-control", "max-age=18645951"), MAKE_NV("content-type", "application/x-javascript"), MAKE_NV("date", "Sat, 27 Jul 2013 06:22:12 GMT"), @@ -1088,8 +1079,7 @@ void test_nghttp2_hd_deflate_inflate(void) { MAKE_NV("x-cache-lookup", "HIT from alphabravo:3128"), }; nghttp2_nv nv7[] = { - MAKE_NV(":status", "304 Not Modified"), - MAKE_NV("age", "0"), + MAKE_NV(":status", "304 Not Modified"), MAKE_NV("age", "0"), MAKE_NV("cache-control", "max-age=31536000"), MAKE_NV("content-type", "application/javascript"), MAKE_NV("date", "Sat, 27 Jul 2013 06:22:12 GMT"), @@ -1101,8 +1091,7 @@ void test_nghttp2_hd_deflate_inflate(void) { MAKE_NV("x-cache-lookup", "HIT from alphabravo:3128"), }; nghttp2_nv nv8[] = { - MAKE_NV(":status", "304 Not Modified"), - MAKE_NV("age", "0"), + MAKE_NV(":status", "304 Not Modified"), MAKE_NV("age", "0"), MAKE_NV("cache-control", "max-age=31536000"), MAKE_NV("content-type", "application/javascript"), MAKE_NV("date", "Sat, 27 Jul 2013 06:22:12 GMT"), @@ -1114,8 +1103,7 @@ void test_nghttp2_hd_deflate_inflate(void) { MAKE_NV("x-cache-lookup", "HIT from alphabravo:3128"), }; nghttp2_nv nv9[] = { - MAKE_NV(":status", "304 Not Modified"), - MAKE_NV("age", "0"), + MAKE_NV(":status", "304 Not Modified"), MAKE_NV("age", "0"), MAKE_NV("cache-control", "max-age=31536000"), MAKE_NV("content-type", "application/javascript"), MAKE_NV("date", "Sat, 27 Jul 2013 06:22:12 GMT"), @@ -1127,8 +1115,7 @@ void test_nghttp2_hd_deflate_inflate(void) { MAKE_NV("x-cache-lookup", "HIT from alphabravo:3128"), }; nghttp2_nv nv10[] = { - MAKE_NV(":status", "304 Not Modified"), - MAKE_NV("age", "0"), + MAKE_NV(":status", "304 Not Modified"), MAKE_NV("age", "0"), MAKE_NV("cache-control", "max-age=56682045"), MAKE_NV("content-type", "text/css"), MAKE_NV("date", "Sat, 27 Jul 2013 06:22:12 GMT"), diff --git a/tests/nghttp2_pq_test.c b/tests/nghttp2_pq_test.c index c37b156a..b0e694de 100644 --- a/tests/nghttp2_pq_test.c +++ b/tests/nghttp2_pq_test.c @@ -45,9 +45,7 @@ static string_entry *string_entry_new(const char *s) { return ent; } -static void string_entry_del(string_entry *ent) { - free(ent); -} +static void string_entry_del(string_entry *ent) { free(ent); } static int pq_less(const void *lhs, const void *rhs) { return strcmp(((string_entry *)lhs)->s, ((string_entry *)rhs)->s) < 0; diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 758ada16..e990e611 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -8401,9 +8401,9 @@ void test_nghttp2_http_mandatory_headers(void) { MAKE_NV(":authority", "localhost"), MAKE_NV(":path", "/"), MAKE_NV("content-length", "-1")}; const nghttp2_nv dupcl_reqnv[] = { - MAKE_NV(":scheme", "https"), MAKE_NV(":method", "POST"), + MAKE_NV(":scheme", "https"), MAKE_NV(":method", "POST"), MAKE_NV(":authority", "localhost"), MAKE_NV(":path", "/"), - MAKE_NV("content-length", "0"), MAKE_NV("content-length", "0")}; + MAKE_NV("content-length", "0"), MAKE_NV("content-length", "0")}; const nghttp2_nv badhd_reqnv[] = { MAKE_NV(":scheme", "https"), MAKE_NV(":method", "GET"), MAKE_NV(":authority", "localhost"), MAKE_NV(":path", "/"), @@ -8996,8 +8996,8 @@ void test_nghttp2_http_ignore_regular_header(void) { my_user_data ud; const nghttp2_nv bad_reqnv[] = { MAKE_NV(":authority", "localhost"), MAKE_NV(":scheme", "https"), - MAKE_NV(":path", "/"), MAKE_NV(":method", "GET"), - MAKE_NV("foo", "\x0zzz"), MAKE_NV("bar", "buzz"), + MAKE_NV(":path", "/"), MAKE_NV(":method", "GET"), + MAKE_NV("foo", "\x0zzz"), MAKE_NV("bar", "buzz"), }; const nghttp2_nv bad_ansnv[] = { MAKE_NV(":authority", "localhost"), MAKE_NV(":scheme", "https"), From fe8720341d3d2c486a1aaceef63039cdeecbb77f Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 13 Nov 2015 23:59:36 +0900 Subject: [PATCH 23/23] nghttpx: Fix crash on non-final response from backend server --- src/shrpx_http2_upstream.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 1c6af673..42a347fb 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -1471,9 +1471,9 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream) { nva.push_back(http2::make_nv_ls(":status", response_status)); } - http2::copy_headers_to_nva_nocopy(nva, downstream->get_response_headers()); - if (downstream->get_non_final_response()) { + http2::copy_headers_to_nva(nva, downstream->get_response_headers()); + if (LOG_ENABLED(INFO)) { log_response_headers(downstream, nva); } @@ -1492,6 +1492,8 @@ int Http2Upstream::on_downstream_header_complete(Downstream *downstream) { return 0; } + http2::copy_headers_to_nva_nocopy(nva, downstream->get_response_headers()); + if (!get_config()->http2_proxy && !get_config()->client_proxy) { nva.push_back( http2::make_nv_lc_nocopy("server", get_config()->server_name));