From 12dad328902bdccdfbde8d803b73bd82295b7e47 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 12 Mar 2016 15:05:20 +0900 Subject: [PATCH] Add nghttp2_on_header_callback2 --- doc/Makefile.am | 4 +++ lib/includes/nghttp2/nghttp2.h | 49 ++++++++++++++++++++++++--- lib/nghttp2_callbacks.c | 6 ++++ lib/nghttp2_callbacks.h | 1 + lib/nghttp2_session.c | 22 +++++++----- src/shrpx_downstream.cc | 9 +++++ src/shrpx_downstream.h | 4 +++ src/shrpx_http2_session.cc | 61 +++++++++++++++++++--------------- src/shrpx_http2_upstream.cc | 41 ++++++++++++----------- 9 files changed, 139 insertions(+), 58 deletions(-) diff --git a/doc/Makefile.am b/doc/Makefile.am index 35f1390b..c450a029 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -65,6 +65,9 @@ APIDOCS= \ nghttp2_priority_spec_check_default.rst \ nghttp2_priority_spec_default_init.rst \ nghttp2_priority_spec_init.rst \ + nghttp2_rcbuf_decref.rst \ + nghttp2_rcbuf_get_buf.rst \ + nghttp2_rcbuf_incref.rst \ nghttp2_select_next_protocol.rst \ nghttp2_session_callbacks_del.rst \ nghttp2_session_callbacks_new.rst \ @@ -78,6 +81,7 @@ APIDOCS= \ nghttp2_session_callbacks_set_on_frame_recv_callback.rst \ nghttp2_session_callbacks_set_on_frame_send_callback.rst \ nghttp2_session_callbacks_set_on_header_callback.rst \ + nghttp2_session_callbacks_set_on_header_callback2.rst \ nghttp2_session_callbacks_set_on_invalid_frame_recv_callback.rst \ nghttp2_session_callbacks_set_on_stream_close_callback.rst \ nghttp2_session_callbacks_set_pack_extension_callback.rst \ diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 2a3674fb..0e888fa7 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -450,7 +450,7 @@ typedef struct nghttp2_rcbuf nghttp2_rcbuf; * * Increments the reference count of |rcbuf| by 1. */ -void nghttp2_rcbuf_incref(nghttp2_rcbuf *rcbuf); +NGHTTP2_EXTERN void nghttp2_rcbuf_incref(nghttp2_rcbuf *rcbuf); /** * @function @@ -459,12 +459,14 @@ void nghttp2_rcbuf_incref(nghttp2_rcbuf *rcbuf); * count becomes zero, the object pointed by |rcbuf| will be freed. * In this case, application must not use |rcbuf| again. */ -void nghttp2_rcbuf_decref(nghttp2_rcbuf *rcbuf); +NGHTTP2_EXTERN void nghttp2_rcbuf_decref(nghttp2_rcbuf *rcbuf); /** + * @function + * * Returns the underlying buffer managed by |rcbuf|. */ -nghttp2_vec nghttp2_rcbuf_get_buf(nghttp2_rcbuf *rcbuf); +NGHTTP2_EXTERN nghttp2_vec nghttp2_rcbuf_get_buf(nghttp2_rcbuf *rcbuf); /** * @enum @@ -1674,6 +1676,32 @@ typedef int (*nghttp2_on_header_callback)(nghttp2_session *session, const uint8_t *value, size_t valuelen, uint8_t flags, void *user_data); +/** + * @functypedef + * + * Callback function invoked when a header name/value pair is received + * for the |frame|. The |name| is header name. The |value| is header + * value. The |flags| is bitwise OR of one or more of + * :type:`nghttp2_nv_flag`. + * + * This callback behaves like :type:`nghttp2_on_header_callback`, + * except that |name| and |value| are stored in reference counted + * buffer. If application wishes to keep these references without + * copying them, use `nghttp2_rcbuf_incref()` to increment their + * reference count. It is the application's responsibility to call + * `nghttp2_rcbuf_decref()` if they called `nghttp2_rcbuf_incref()` so + * as not to leak memory. If the |session| is created by + * `nghttp2_session_server_new3()` or `nghttp2_session_client_new3()`, + * the function to free memory is the one belongs to the mem + * parameter. As long as this free function alives, |name| and + * |value| can live after |session| was destroyed. + */ +typedef int (*nghttp2_on_header_callback2)(nghttp2_session *session, + const nghttp2_frame *frame, + nghttp2_rcbuf *name, + nghttp2_rcbuf *value, uint8_t flags, + void *user_data); + /** * @functypedef * @@ -1988,12 +2016,25 @@ NGHTTP2_EXTERN void nghttp2_session_callbacks_set_on_begin_headers_callback( * @function * * Sets callback function invoked when a header name/value pair is - * received. + * received. If both + * `nghttp2_session_callbacks_set_on_header_callback()` and + * `nghttp2_session_callbacks_set_on_header_callback2()` are used to + * set callbacks, the latter has the precedence. */ NGHTTP2_EXTERN void nghttp2_session_callbacks_set_on_header_callback( nghttp2_session_callbacks *cbs, nghttp2_on_header_callback on_header_callback); +/** + * @function + * + * Sets callback function invoked when a header name/value pair is + * received. + */ +NGHTTP2_EXTERN void nghttp2_session_callbacks_set_on_header_callback2( + nghttp2_session_callbacks *cbs, + nghttp2_on_header_callback2 on_header_callback2); + /** * @function * diff --git a/lib/nghttp2_callbacks.c b/lib/nghttp2_callbacks.c index 3b0369c1..4bf0e7a5 100644 --- a/lib/nghttp2_callbacks.c +++ b/lib/nghttp2_callbacks.c @@ -104,6 +104,12 @@ void nghttp2_session_callbacks_set_on_header_callback( cbs->on_header_callback = on_header_callback; } +void nghttp2_session_callbacks_set_on_header_callback2( + nghttp2_session_callbacks *cbs, + nghttp2_on_header_callback2 on_header_callback2) { + cbs->on_header_callback2 = on_header_callback2; +} + void nghttp2_session_callbacks_set_select_padding_callback( nghttp2_session_callbacks *cbs, nghttp2_select_padding_callback select_padding_callback) { diff --git a/lib/nghttp2_callbacks.h b/lib/nghttp2_callbacks.h index 664bf35b..80971c54 100644 --- a/lib/nghttp2_callbacks.h +++ b/lib/nghttp2_callbacks.h @@ -91,6 +91,7 @@ struct nghttp2_session_callbacks { * received. */ nghttp2_on_header_callback on_header_callback; + nghttp2_on_header_callback2 on_header_callback2; /** * Callback function invoked when the library asks application how * many padding bytes are required for the transmission of the given diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 92b1c482..44ed7203 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -3123,19 +3123,23 @@ static int session_call_on_begin_headers(nghttp2_session *session, static int session_call_on_header(nghttp2_session *session, const nghttp2_frame *frame, const nghttp2_hd_nv *nv) { - int rv; - if (session->callbacks.on_header_callback) { + int rv = 0; + if (session->callbacks.on_header_callback2) { + rv = session->callbacks.on_header_callback2( + session, frame, nv->name, nv->value, nv->flags, session->user_data); + } else if (session->callbacks.on_header_callback) { rv = session->callbacks.on_header_callback( session, frame, nv->name->base, nv->name->len, nv->value->base, nv->value->len, nv->flags, session->user_data); - if (rv == NGHTTP2_ERR_PAUSE || - rv == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) { - return rv; - } - if (rv != 0) { - return NGHTTP2_ERR_CALLBACK_FAILURE; - } } + + if (rv == NGHTTP2_ERR_PAUSE || rv == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) { + return rv; + } + if (rv != 0) { + return NGHTTP2_ERR_CALLBACK_FAILURE; + } + return 0; } diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index 51931aad..0cdd81b8 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -182,6 +182,10 @@ Downstream::~Downstream() { // explicitly. dconn_.reset(); + for (auto rcbuf : rcbufs_) { + nghttp2_rcbuf_decref(rcbuf); + } + if (LOG_ENABLED(INFO)) { DLOG(INFO, this) << "Deleted"; } @@ -905,4 +909,9 @@ int32_t Downstream::get_assoc_stream_id() const { return assoc_stream_id_; } BlockAllocator &Downstream::get_block_allocator() { return balloc_; } +void Downstream::add_rcbuf(nghttp2_rcbuf *rcbuf) { + nghttp2_rcbuf_incref(rcbuf); + rcbufs_.push_back(rcbuf); +} + } // namespace shrpx diff --git a/src/shrpx_downstream.h b/src/shrpx_downstream.h index a5024169..0d987517 100644 --- a/src/shrpx_downstream.h +++ b/src/shrpx_downstream.h @@ -374,6 +374,8 @@ public: BlockAllocator &get_block_allocator(); + void add_rcbuf(nghttp2_rcbuf *rcbuf); + enum { EVENT_ERROR = 0x1, EVENT_TIMEOUT = 0x2, @@ -395,6 +397,8 @@ public: private: BlockAllocator balloc_; + std::vector rcbufs_; + Request req_; Response resp_; diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc index 25a7f996..84258896 100644 --- a/src/shrpx_http2_session.cc +++ b/src/shrpx_http2_session.cc @@ -801,10 +801,9 @@ void Http2Session::stop_settings_timer() { } namespace { -int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, - const uint8_t *name, size_t namelen, - const uint8_t *value, size_t valuelen, uint8_t flags, - void *user_data) { +int on_header_callback2(nghttp2_session *session, const nghttp2_frame *frame, + nghttp2_rcbuf *name, nghttp2_rcbuf *value, + uint8_t flags, void *user_data) { auto http2session = static_cast(user_data); auto sd = static_cast( nghttp2_session_get_stream_user_data(session, frame->hd.stream_id)); @@ -816,22 +815,25 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, return 0; } + auto namebuf = nghttp2_rcbuf_get_buf(name); + auto valuebuf = nghttp2_rcbuf_get_buf(value); + auto &resp = downstream->response(); auto &httpconf = get_config()->http; - auto &balloc = downstream->get_block_allocator(); switch (frame->hd.type) { case NGHTTP2_HEADERS: { auto trailer = frame->headers.cat == NGHTTP2_HCAT_HEADERS && !downstream->get_expect_final_response(); - if (resp.fs.buffer_size() + namelen + valuelen > + if (resp.fs.buffer_size() + namebuf.len + valuebuf.len > httpconf.response_header_field_buffer || resp.fs.num_fields() >= httpconf.max_response_header_fields) { if (LOG_ENABLED(INFO)) { - DLOG(INFO, downstream) << "Too large or many header field size=" - << resp.fs.buffer_size() + namelen + valuelen - << ", num=" << resp.fs.num_fields() + 1; + DLOG(INFO, downstream) + << "Too large or many header field size=" + << resp.fs.buffer_size() + namebuf.len + valuebuf.len + << ", num=" << resp.fs.num_fields() + 1; } if (trailer) { @@ -843,20 +845,23 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; } - auto token = http2::lookup_token(name, namelen); + auto token = http2::lookup_token(namebuf.base, namebuf.len); auto no_index = flags & NGHTTP2_NV_FLAG_NO_INDEX; + downstream->add_rcbuf(name); + downstream->add_rcbuf(value); + if (trailer) { // just store header fields for trailer part - resp.fs.add_trailer_token( - make_string_ref(balloc, StringRef{name, namelen}), - make_string_ref(balloc, StringRef{value, valuelen}), no_index, token); + resp.fs.add_trailer_token(StringRef{namebuf.base, namebuf.len}, + StringRef{valuebuf.base, valuebuf.len}, + no_index, token); return 0; } - resp.fs.add_header_token( - make_string_ref(balloc, StringRef{name, namelen}), - make_string_ref(balloc, StringRef{value, valuelen}), no_index, token); + resp.fs.add_header_token(StringRef{namebuf.base, namebuf.len}, + StringRef{valuebuf.base, valuebuf.len}, no_index, + token); return 0; } case NGHTTP2_PUSH_PROMISE: { @@ -870,30 +875,34 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, auto promised_downstream = promised_sd->dconn->get_downstream(); + auto namebuf = nghttp2_rcbuf_get_buf(name); + auto valuebuf = nghttp2_rcbuf_get_buf(value); + assert(promised_downstream); auto &promised_req = promised_downstream->request(); - auto &promised_balloc = promised_downstream->get_block_allocator(); // We use request header limit for PUSH_PROMISE - if (promised_req.fs.buffer_size() + namelen + valuelen > + if (promised_req.fs.buffer_size() + namebuf.len + valuebuf.len > httpconf.request_header_field_buffer || promised_req.fs.num_fields() >= httpconf.max_request_header_fields) { if (LOG_ENABLED(INFO)) { DLOG(INFO, downstream) << "Too large or many header field size=" - << promised_req.fs.buffer_size() + namelen + valuelen + << promised_req.fs.buffer_size() + namebuf.len + valuebuf.len << ", num=" << promised_req.fs.num_fields() + 1; } return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE; } - auto token = http2::lookup_token(name, namelen); - promised_req.fs.add_header_token( - make_string_ref(promised_balloc, StringRef{name, namelen}), - make_string_ref(promised_balloc, StringRef{value, valuelen}), - flags & NGHTTP2_NV_FLAG_NO_INDEX, token); + promised_downstream->add_rcbuf(name); + promised_downstream->add_rcbuf(value); + + auto token = http2::lookup_token(namebuf.base, namebuf.len); + promised_req.fs.add_header_token(StringRef{namebuf.base, namebuf.len}, + StringRef{valuebuf.base, valuebuf.len}, + flags & NGHTTP2_NV_FLAG_NO_INDEX, token); return 0; } @@ -1402,8 +1411,8 @@ nghttp2_session_callbacks *create_http2_downstream_callbacks() { nghttp2_session_callbacks_set_on_frame_not_send_callback( callbacks, on_frame_not_send_callback); - nghttp2_session_callbacks_set_on_header_callback(callbacks, - on_header_callback); + nghttp2_session_callbacks_set_on_header_callback2(callbacks, + on_header_callback2); nghttp2_session_callbacks_set_on_begin_headers_callback( callbacks, on_begin_headers_callback); diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index a3c78572..79fac5de 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -154,13 +154,15 @@ void Http2Upstream::stop_settings_timer() { } namespace { -int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, - const uint8_t *name, size_t namelen, - const uint8_t *value, size_t valuelen, uint8_t flags, - void *user_data) { +int on_header_callback2(nghttp2_session *session, const nghttp2_frame *frame, + nghttp2_rcbuf *name, nghttp2_rcbuf *value, + uint8_t flags, void *user_data) { + auto namebuf = nghttp2_rcbuf_get_buf(name); + auto valuebuf = nghttp2_rcbuf_get_buf(value); + if (get_config()->http2.upstream.debug.frame_debug) { - verbose_on_header_callback(session, frame, name, namelen, value, valuelen, - flags, user_data); + verbose_on_header_callback(session, frame, namebuf.base, namebuf.len, + valuebuf.base, valuebuf.len, flags, user_data); } if (frame->hd.type != NGHTTP2_HEADERS) { return 0; @@ -176,9 +178,7 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, auto &httpconf = get_config()->http; - auto &balloc = downstream->get_block_allocator(); - - if (req.fs.buffer_size() + namelen + valuelen > + if (req.fs.buffer_size() + namebuf.len + valuebuf.len > httpconf.request_header_field_buffer || req.fs.num_fields() >= httpconf.max_request_header_fields) { if (downstream->get_response_state() == Downstream::MSG_COMPLETE) { @@ -187,7 +187,7 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, if (LOG_ENABLED(INFO)) { ULOG(INFO, upstream) << "Too large or many header field size=" - << req.fs.buffer_size() + namelen + valuelen + << req.fs.buffer_size() + namebuf.len + valuebuf.len << ", num=" << req.fs.num_fields() + 1; } @@ -203,20 +203,23 @@ int on_header_callback(nghttp2_session *session, const nghttp2_frame *frame, return 0; } - auto token = http2::lookup_token(name, namelen); + auto token = http2::lookup_token(namebuf.base, namebuf.len); auto no_index = flags & NGHTTP2_NV_FLAG_NO_INDEX; + downstream->add_rcbuf(name); + downstream->add_rcbuf(value); + if (frame->headers.cat == NGHTTP2_HCAT_HEADERS) { // just store header fields for trailer part - req.fs.add_trailer_token( - make_string_ref(balloc, StringRef{name, namelen}), - make_string_ref(balloc, StringRef{value, valuelen}), no_index, token); + req.fs.add_trailer_token(StringRef{namebuf.base, namebuf.len}, + StringRef{valuebuf.base, valuebuf.len}, no_index, + token); return 0; } - req.fs.add_header_token(make_string_ref(balloc, StringRef{name, namelen}), - make_string_ref(balloc, StringRef{value, valuelen}), - no_index, token); + req.fs.add_header_token(StringRef{namebuf.base, namebuf.len}, + StringRef{valuebuf.base, valuebuf.len}, no_index, + token); return 0; } } // namespace @@ -820,8 +823,8 @@ nghttp2_session_callbacks *create_http2_upstream_callbacks() { nghttp2_session_callbacks_set_on_frame_not_send_callback( callbacks, on_frame_not_send_callback); - nghttp2_session_callbacks_set_on_header_callback(callbacks, - on_header_callback); + nghttp2_session_callbacks_set_on_header_callback2(callbacks, + on_header_callback2); nghttp2_session_callbacks_set_on_begin_headers_callback( callbacks, on_begin_headers_callback);