From a59cd3be826b0995881d28b5a3a1ec7169dc1984 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Thu, 29 Aug 2013 21:03:39 +0900 Subject: [PATCH] Add int return value to nghttp2_on_frame_recv_callback --- lib/includes/nghttp2/nghttp2.h | 7 ++- lib/nghttp2_session.c | 79 +++++++++++++++++++++++----------- src/HttpServer.cc | 7 +-- src/app_helper.cc | 3 +- src/app_helper.h | 2 +- src/nghttp.cc | 3 +- src/shrpx_http2_upstream.cc | 13 +++--- src/shrpx_spdy_session.cc | 9 ++-- tests/nghttp2_session_test.c | 7 +-- 9 files changed, 86 insertions(+), 44 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 2dcfc186..25a152d6 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -779,8 +779,13 @@ typedef ssize_t (*nghttp2_recv_callback) * * Callback function invoked by `nghttp2_session_recv()` when a * non-DATA frame is received. + * + * The implementation of this function must return 0 if it + * succeeds. If nonzero is returned, it is treated as fatal error and + * `nghttp2_session_recv()` and `nghttp2_session_send()` functions + * return :enum:`NGHTTP2_ERR_CALLBACK_FAILURE`. */ -typedef void (*nghttp2_on_frame_recv_callback) +typedef int (*nghttp2_on_frame_recv_callback) (nghttp2_session *session, nghttp2_frame *frame, void *user_data); /** diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 98a15752..60d459c4 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -1567,13 +1567,18 @@ static void nghttp2_session_call_on_request_recv } } -static void nghttp2_session_call_on_frame_received +static int nghttp2_session_call_on_frame_received (nghttp2_session *session, nghttp2_frame *frame) { + int rv; if(session->callbacks.on_frame_recv_callback) { - session->callbacks.on_frame_recv_callback(session, frame, - session->user_data); + rv = session->callbacks.on_frame_recv_callback(session, frame, + session->user_data); + if(rv != 0) { + return NGHTTP2_ERR_CALLBACK_FAILURE; + } } + return 0; } /* @@ -1709,7 +1714,10 @@ int nghttp2_session_on_request_headers_received(nghttp2_session *session, if(flags & NGHTTP2_FLAG_END_STREAM) { nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_RD); } - nghttp2_session_call_on_frame_received(session, frame); + r = nghttp2_session_call_on_frame_received(session, frame); + if(r != 0) { + return r; + } if(flags & NGHTTP2_FLAG_END_STREAM) { nghttp2_session_call_on_request_recv(session, frame->hd.stream_id); } @@ -1723,6 +1731,7 @@ int nghttp2_session_on_response_headers_received(nghttp2_session *session, nghttp2_frame *frame, nghttp2_stream *stream) { + int rv; /* This function is only called if stream->state == NGHTTP2_STREAM_OPENING and stream_id is local side initiated. */ assert(stream->state == NGHTTP2_STREAM_OPENING && @@ -1738,7 +1747,10 @@ int nghttp2_session_on_response_headers_received(nghttp2_session *session, } if((stream->shut_flags & NGHTTP2_SHUT_RD) == 0) { stream->state = NGHTTP2_STREAM_OPENED; - nghttp2_session_call_on_frame_received(session, frame); + rv = nghttp2_session_call_on_frame_received(session, frame); + if(rv != 0) { + return rv; + } if(frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { /* This is the last frame of this stream, so disallow further receptions. */ @@ -1783,8 +1795,7 @@ int nghttp2_session_on_push_response_headers_received(nghttp2_session *session, } nghttp2_stream_promise_fulfilled(stream); ++session->num_incoming_streams; - nghttp2_session_call_on_frame_received(session, frame); - return 0; + return nghttp2_session_call_on_frame_received(session, frame); } int nghttp2_session_on_headers_received(nghttp2_session *session, @@ -1807,7 +1818,10 @@ int nghttp2_session_on_headers_received(nghttp2_session *session, if(nghttp2_session_is_my_stream_id(session, frame->hd.stream_id)) { if(stream->state == NGHTTP2_STREAM_OPENED) { valid = 1; - nghttp2_session_call_on_frame_received(session, frame); + r = nghttp2_session_call_on_frame_received(session, frame); + if(r != 0) { + return r; + } if(frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_RD); nghttp2_session_close_stream_if_shut_rdwr(session, stream); @@ -1825,7 +1839,10 @@ int nghttp2_session_on_headers_received(nghttp2_session *session, condition. */ valid = 1; if(stream->state != NGHTTP2_STREAM_CLOSING) { - nghttp2_session_call_on_frame_received(session, frame); + r = nghttp2_session_call_on_frame_received(session, frame); + if(r != 0) { + return r; + } if(frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { nghttp2_session_call_on_request_recv(session, frame->hd.stream_id); nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_RD); @@ -1858,6 +1875,7 @@ int nghttp2_session_on_headers_received(nghttp2_session *session, int nghttp2_session_on_priority_received(nghttp2_session *session, nghttp2_frame *frame) { + int rv; nghttp2_stream *stream; if(frame->hd.stream_id == 0) { return nghttp2_session_handle_invalid_connection(session, frame, @@ -1870,7 +1888,10 @@ int nghttp2_session_on_priority_received(nghttp2_session *session, nghttp2_session_reprioritize_stream(session, stream, frame->priority.pri); } - nghttp2_session_call_on_frame_received(session, frame); + rv = nghttp2_session_call_on_frame_received(session, frame); + if(rv != 0) { + return rv; + } } return 0; } @@ -1878,11 +1899,15 @@ int nghttp2_session_on_priority_received(nghttp2_session *session, int nghttp2_session_on_rst_stream_received(nghttp2_session *session, nghttp2_frame *frame) { + int rv; if(frame->hd.stream_id == 0) { return nghttp2_session_handle_invalid_connection(session, frame, NGHTTP2_PROTOCOL_ERROR); } - nghttp2_session_call_on_frame_received(session, frame); + rv = nghttp2_session_call_on_frame_received(session, frame); + if(rv != 0) { + return rv; + } nghttp2_session_close_stream(session, frame->hd.stream_id, frame->rst_stream.error_code); return 0; @@ -2167,13 +2192,13 @@ int nghttp2_session_on_settings_received(nghttp2_session *session, } session->remote_settings[entry->settings_id] = entry->value; } - nghttp2_session_call_on_frame_received(session, frame); - return 0; + return nghttp2_session_call_on_frame_received(session, frame); } int nghttp2_session_on_push_promise_received(nghttp2_session *session, nghttp2_frame *frame) { + int rv; nghttp2_stream *stream; if(session->server || frame->hd.stream_id == 0) { return nghttp2_session_handle_invalid_connection(session, frame, @@ -2222,7 +2247,10 @@ int nghttp2_session_on_push_promise_received(nghttp2_session *session, if(!promised_stream) { return NGHTTP2_ERR_NOMEM; } - nghttp2_session_call_on_frame_received(session, frame); + rv = nghttp2_session_call_on_frame_received(session, frame); + if(rv != 0) { + return rv; + } } } else { if(session->callbacks.on_invalid_frame_recv_callback) { @@ -2255,9 +2283,11 @@ int nghttp2_session_on_ping_received(nghttp2_session *session, /* Peer sent ping, so ping it back */ r = nghttp2_session_add_ping(session, NGHTTP2_FLAG_PONG, frame->ping.opaque_data); + if(r != 0) { + return r; + } } - nghttp2_session_call_on_frame_received(session, frame); - return r; + return nghttp2_session_call_on_frame_received(session, frame); } int nghttp2_session_on_goaway_received(nghttp2_session *session, @@ -2265,8 +2295,7 @@ int nghttp2_session_on_goaway_received(nghttp2_session *session, { session->last_stream_id = frame->goaway.last_stream_id; session->goaway_flags |= NGHTTP2_GOAWAY_RECV; - nghttp2_session_call_on_frame_received(session, frame); - return 0; + return nghttp2_session_call_on_frame_received(session, frame); } static int nghttp2_push_back_deferred_data_func(nghttp2_map_entry *entry, @@ -2307,6 +2336,7 @@ static int nghttp2_session_push_back_deferred_data(nghttp2_session *session) int nghttp2_session_on_window_update_received(nghttp2_session *session, nghttp2_frame *frame) { + int rv; if(frame->hd.stream_id == 0) { /* Handle connection-level flow control */ if(session->remote_flow_control == 0) { @@ -2314,8 +2344,7 @@ int nghttp2_session_on_window_update_received(nghttp2_session *session, WINDOW_UPDATE are asynchronous, so it is hard to determine that the peer is misbehaving or not without measuring RTT. For now, we just ignore such frames. */ - nghttp2_session_call_on_frame_received(session, frame); - return 0; + return nghttp2_session_call_on_frame_received(session, frame); } if(NGHTTP2_MAX_WINDOW_SIZE - frame->window_update.window_size_increment < session->remote_window_size) { @@ -2323,7 +2352,10 @@ int nghttp2_session_on_window_update_received(nghttp2_session *session, (session, frame, NGHTTP2_FLOW_CONTROL_ERROR); } session->remote_window_size += frame->window_update.window_size_increment; - nghttp2_session_call_on_frame_received(session, frame); + rv = nghttp2_session_call_on_frame_received(session, frame); + if(rv != 0) { + return rv; + } /* To queue the DATA deferred by connection-level flow-control, we have to check all streams. Bad. */ if(session->remote_window_size > 0) { @@ -2341,8 +2373,7 @@ int nghttp2_session_on_window_update_received(nghttp2_session *session, } if(stream->remote_flow_control == 0) { /* Same reason with connection-level flow control */ - nghttp2_session_call_on_frame_received(session, frame); - return 0; + return nghttp2_session_call_on_frame_received(session, frame); } if(NGHTTP2_MAX_WINDOW_SIZE - frame->window_update.window_size_increment < stream->remote_window_size) { @@ -2368,7 +2399,7 @@ int nghttp2_session_on_window_update_received(nghttp2_session *session, return r; } } - nghttp2_session_call_on_frame_received(session, frame); + return nghttp2_session_call_on_frame_received(session, frame); } } } diff --git a/src/HttpServer.cc b/src/HttpServer.cc index 3418d968..fa912b17 100644 --- a/src/HttpServer.cc +++ b/src/HttpServer.cc @@ -685,7 +685,7 @@ const char *REQUIRED_HEADERS[] = { } // namespace namespace { -void hd_on_frame_recv_callback +int hd_on_frame_recv_callback (nghttp2_session *session, nghttp2_frame *frame, void *user_data) { auto hd = reinterpret_cast(user_data); @@ -701,7 +701,7 @@ void hd_on_frame_recv_callback if(!http2::check_http2_headers(frame->headers.nva, frame->headers.nvlen)) { nghttp2_submit_rst_stream(session, stream_id, NGHTTP2_PROTOCOL_ERROR); - return; + return 0; } for(size_t i = 0; REQUIRED_HEADERS[i]; ++i) { if(!http2::get_unique_header(frame->headers.nva, @@ -709,7 +709,7 @@ void hd_on_frame_recv_callback REQUIRED_HEADERS[i])) { nghttp2_submit_rst_stream(session, stream_id, NGHTTP2_PROTOCOL_ERROR); - return; + return 0; } } auto req = util::make_unique(stream_id); @@ -724,6 +724,7 @@ void hd_on_frame_recv_callback default: break; } + return 0; } } // namespace diff --git a/src/app_helper.cc b/src/app_helper.cc index d2440db2..562a9162 100644 --- a/src/app_helper.cc +++ b/src/app_helper.cc @@ -323,13 +323,14 @@ void print_frame(print_type ptype, nghttp2_frame *frame) } } // namespace -void on_frame_recv_callback +int on_frame_recv_callback (nghttp2_session *session, nghttp2_frame *frame, void *user_data) { print_timer(); printf(" recv "); print_frame(PRINT_RECV, frame); fflush(stdout); + return 0; } void on_invalid_frame_recv_callback diff --git a/src/app_helper.h b/src/app_helper.h index 291f9711..1de0b008 100644 --- a/src/app_helper.h +++ b/src/app_helper.h @@ -39,7 +39,7 @@ namespace nghttp2 { void print_nv(char **nv); -void on_frame_recv_callback +int on_frame_recv_callback (nghttp2_session *session, nghttp2_frame *frame, void *user_data); void on_invalid_frame_recv_callback diff --git a/src/nghttp.cc b/src/nghttp.cc index 379484ad..bfb3f9cd 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -1024,7 +1024,7 @@ void check_response_header } } -void on_frame_recv_callback2 +int on_frame_recv_callback2 (nghttp2_session *session, nghttp2_frame *frame, void *user_data) { if(frame->hd.type == NGHTTP2_HEADERS && @@ -1041,6 +1041,7 @@ void on_frame_recv_callback2 if(config.verbose) { on_frame_recv_callback(session, frame, user_data); } + return 0; } void on_stream_close_callback diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index b87ac819..42650dbf 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -173,7 +173,7 @@ int Http2Upstream::upgrade_upstream(HttpsUpstream *http) } namespace { -void on_frame_recv_callback +int on_frame_recv_callback (nghttp2_session *session, nghttp2_frame *frame, void *user_data) { auto upstream = reinterpret_cast(user_data); @@ -212,7 +212,7 @@ void on_frame_recv_callback // Assuming that nva is sorted by name. if(!http2::check_http2_headers(nva, nvlen)) { upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); - return; + return 0; } for(size_t i = 0; i < nvlen; ++i) { @@ -233,7 +233,7 @@ void on_frame_recv_callback http2::value_lws(method) || (!is_connect && (!scheme || http2::value_lws(scheme)))) { upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); - return; + return 0; } if(!is_connect && (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) == 0) { @@ -242,7 +242,7 @@ void on_frame_recv_callback // If content-length is missing, // Downstream::push_upload_data_chunk will fail and upstream->rst_stream(downstream, NGHTTP2_PROTOCOL_ERROR); - return; + return 0; } } @@ -270,12 +270,12 @@ void on_frame_recv_callback // If downstream connection fails, issue RST_STREAM. upstream->rst_stream(downstream, NGHTTP2_INTERNAL_ERROR); downstream->set_request_state(Downstream::CONNECT_FAIL); - return; + return 0; } rv = downstream->push_request_headers(); if(rv != 0) { upstream->rst_stream(downstream, NGHTTP2_INTERNAL_ERROR); - return; + return 0; } downstream->set_request_state(Downstream::HEADER_COMPLETE); if(frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { @@ -286,6 +286,7 @@ void on_frame_recv_callback default: break; } + return 0; } } // namespace diff --git a/src/shrpx_spdy_session.cc b/src/shrpx_spdy_session.cc index 702d4032..7e76ed5d 100644 --- a/src/shrpx_spdy_session.cc +++ b/src/shrpx_spdy_session.cc @@ -722,7 +722,7 @@ void on_stream_close_callback } // namespace namespace { -void on_frame_recv_callback +int on_frame_recv_callback (nghttp2_session *session, nghttp2_frame *frame, void *user_data) { int rv; @@ -755,7 +755,7 @@ void on_frame_recv_callback NGHTTP2_PROTOCOL_ERROR); downstream->set_response_state(Downstream::MSG_RESET); call_downstream_readcb(spdy, downstream); - return; + return 0; } for(size_t i = 0; i < nvlen; ++i) { @@ -771,7 +771,7 @@ void on_frame_recv_callback NGHTTP2_PROTOCOL_ERROR); downstream->set_response_state(Downstream::MSG_RESET); call_downstream_readcb(spdy, downstream); - return; + return 0; } downstream->set_response_http_status (strtoul(http2::value_to_str(status).c_str(), nullptr, 10)); @@ -826,7 +826,7 @@ void on_frame_recv_callback if(upstream->resume_read(SHRPX_MSG_BLOCK, downstream) != 0) { // If resume_read fails, just drop connection. Not ideal. delete upstream->get_client_handler(); - return; + return 0; } downstream->set_request_state(Downstream::HEADER_COMPLETE); if(LOG_ENABLED(INFO)) { @@ -890,6 +890,7 @@ void on_frame_recv_callback default: break; } + return 0; } } // namespace diff --git a/tests/nghttp2_session_test.c b/tests/nghttp2_session_test.c index 1ddde737..5bc040ab 100644 --- a/tests/nghttp2_session_test.c +++ b/tests/nghttp2_session_test.c @@ -126,12 +126,13 @@ static ssize_t accumulator_send_callback(nghttp2_session *session, return len; } -static void on_frame_recv_callback(nghttp2_session *session, - nghttp2_frame *frame, - void *user_data) +static int on_frame_recv_callback(nghttp2_session *session, + nghttp2_frame *frame, + void *user_data) { my_user_data *ud = (my_user_data*)user_data; ++ud->frame_recv_cb_called; + return 0; } static void on_invalid_frame_recv_callback(nghttp2_session *session,