diff --git a/configure.ac b/configure.ac index 493995ba..cf9e60fd 100644 --- a/configure.ac +++ b/configure.ac @@ -723,6 +723,9 @@ if test "x$werror" != "xno"; then # Only work with Clang for the moment AX_CHECK_COMPILE_FLAG([-Wheader-guard], [CFLAGS="$CFLAGS -Wheader-guard"]) + # This is required because we pass format string as "const char*. + AX_CHECK_COMPILE_FLAG([-Wno-format-nonliteral], [CFLAGS="$CFLAGS -Wno-format-nonliteral"]) + # For C++ compiler AC_LANG_PUSH(C++) AX_CHECK_COMPILE_FLAG([-Wall], [CXXFLAGS="$CXXFLAGS -Wall"]) diff --git a/doc/Makefile.am b/doc/Makefile.am index c450a029..c1e9baac 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -73,6 +73,7 @@ APIDOCS= \ nghttp2_session_callbacks_new.rst \ nghttp2_session_callbacks_set_before_frame_send_callback.rst \ nghttp2_session_callbacks_set_data_source_read_length_callback.rst \ + nghttp2_session_callbacks_set_error_callback.rst \ nghttp2_session_callbacks_set_on_begin_frame_callback.rst \ nghttp2_session_callbacks_set_on_begin_headers_callback.rst \ nghttp2_session_callbacks_set_on_data_chunk_recv_callback.rst \ diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index 0e888fa7..8cd00ca6 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -1872,6 +1872,31 @@ typedef ssize_t (*nghttp2_pack_extension_callback)(nghttp2_session *session, const nghttp2_frame *frame, void *user_data); +/** + * @functypedef + * + * Callback function invoked when library provides the error message + * intended for human consumption. This callback is solely for + * debugging purpose. The |msg| is typically NULL-terminated string + * of length |len|. |len| does not include the sentinel NULL + * character. + * + * The format of error message may change between nghttp2 library + * versions. The application should not depend on the particular + * format. + * + * Normally, application should return 0 from this callback. If fatal + * error occurred while doing something in this callback, application + * should return :enum:`NGHTTP2_ERR_CALLBACK_FAILURE`. In this case, + * library will return immediately with return value + * :enum:`NGHTTP2_ERR_CALLBACK_FAILURE`. Currently, if nonzero value + * is returned from this callback, they are treated as + * :enum:`NGHTTP2_ERR_CALLBACK_FAILURE`, but application should not + * rely on this details. + */ +typedef int (*nghttp2_error_callback)(nghttp2_session *session, const char *msg, + size_t len, void *user_data); + struct nghttp2_session_callbacks; /** @@ -2108,6 +2133,15 @@ nghttp2_session_callbacks_set_on_extension_chunk_recv_callback( nghttp2_session_callbacks *cbs, nghttp2_on_extension_chunk_recv_callback on_extension_chunk_recv_callback); +/** + * @function + * + * Sets callback function invoked when library tells error message to + * the application. + */ +NGHTTP2_EXTERN void nghttp2_session_callbacks_set_error_callback( + nghttp2_session_callbacks *cbs, nghttp2_error_callback error_callback); + /** * @functypedef * diff --git a/lib/nghttp2_callbacks.c b/lib/nghttp2_callbacks.c index 4bf0e7a5..4d5211a1 100644 --- a/lib/nghttp2_callbacks.c +++ b/lib/nghttp2_callbacks.c @@ -151,3 +151,8 @@ void nghttp2_session_callbacks_set_on_extension_chunk_recv_callback( nghttp2_on_extension_chunk_recv_callback on_extension_chunk_recv_callback) { cbs->on_extension_chunk_recv_callback = on_extension_chunk_recv_callback; } + +void nghttp2_session_callbacks_set_error_callback( + nghttp2_session_callbacks *cbs, nghttp2_error_callback error_callback) { + cbs->error_callback = error_callback; +} diff --git a/lib/nghttp2_callbacks.h b/lib/nghttp2_callbacks.h index 80971c54..5f08474a 100644 --- a/lib/nghttp2_callbacks.h +++ b/lib/nghttp2_callbacks.h @@ -111,6 +111,7 @@ struct nghttp2_session_callbacks { nghttp2_pack_extension_callback pack_extension_callback; nghttp2_unpack_extension_callback unpack_extension_callback; nghttp2_on_extension_chunk_recv_callback on_extension_chunk_recv_callback; + nghttp2_error_callback error_callback; }; #endif /* NGHTTP2_CALLBACKS_H */ diff --git a/lib/nghttp2_session.c b/lib/nghttp2_session.c index 44ed7203..9d3bf1e6 100644 --- a/lib/nghttp2_session.c +++ b/lib/nghttp2_session.c @@ -28,6 +28,7 @@ #include #include #include +#include #include "nghttp2_helper.h" #include "nghttp2_net.h" @@ -141,6 +142,62 @@ static int session_detect_idle_stream(nghttp2_session *session, return 0; } +static int session_call_error_callback(nghttp2_session *session, + const char *fmt, ...) { + size_t bufsize; + va_list ap; + char *buf; + int rv; + nghttp2_mem *mem; + + if (!session->callbacks.error_callback) { + return 0; + } + + mem = &session->mem; + + va_start(ap, fmt); + rv = vsnprintf(NULL, 0, fmt, ap); + va_end(ap); + + if (rv < 0) { + return NGHTTP2_ERR_NOMEM; + } + + bufsize = (size_t)(rv + 1); + + buf = nghttp2_mem_malloc(mem, bufsize); + if (buf == NULL) { + return NGHTTP2_ERR_NOMEM; + } + + va_start(ap, fmt); + rv = vsnprintf(buf, bufsize, fmt, ap); + va_end(ap); + + if (rv < 0) { + nghttp2_mem_free(mem, buf); + /* vsnprintf may return error because of various things we can + imagine, but typically we don't want to drop session just for + debug callback. */ + DEBUGF(fprintf(stderr, + "error_callback: vsnprintf failed. The template was %s\n", + fmt)); + return 0; + } + + rv = session->callbacks.error_callback(session, buf, (size_t)rv, + session->user_data); + + nghttp2_mem_free(mem, buf); + + if (rv != 0) { + return NGHTTP2_ERR_CALLBACK_FAILURE; + } + + return 0; +} + static int session_terminate_session(nghttp2_session *session, int32_t last_stream_id, uint32_t error_code, const char *reason) { @@ -3384,6 +3441,16 @@ static int inflate_header_block(nghttp2_session *session, nghttp2_frame *frame, frame->hd.type, subject_stream->stream_id, (int)nv.name->len, nv.name->base, (int)nv.value->len, nv.value->base)); + rv = session_call_error_callback( + session, "Invalid HTTP header field was received: frame type: " + "%u, stream: %d, name: [%.*s], value: [%.*s]", + frame->hd.type, frame->hd.stream_id, (int)nv.name->len, + nv.name->base, (int)nv.value->len, nv.value->base); + + if (nghttp2_is_fatal(rv)) { + return rv; + } + rv = session_handle_invalid_stream2(session, subject_stream->stream_id, frame, NGHTTP2_ERR_HTTP_HEADER); diff --git a/src/HttpServer.cc b/src/HttpServer.cc index 5c65295d..74e9a9b6 100644 --- a/src/HttpServer.cc +++ b/src/HttpServer.cc @@ -1689,6 +1689,9 @@ void fill_callback(nghttp2_session_callbacks *callbacks, const Config *config) { if (config->verbose) { nghttp2_session_callbacks_set_on_invalid_frame_recv_callback( callbacks, verbose_on_invalid_frame_recv_callback); + + nghttp2_session_callbacks_set_error_callback(callbacks, + verbose_error_callback); } nghttp2_session_callbacks_set_on_data_chunk_recv_callback( diff --git a/src/app_helper.cc b/src/app_helper.cc index ba84052b..603151cf 100644 --- a/src/app_helper.cc +++ b/src/app_helper.cc @@ -414,6 +414,15 @@ int verbose_on_data_chunk_recv_callback(nghttp2_session *session, uint8_t flags, return 0; } +int verbose_error_callback(nghttp2_session *session, const char *msg, + size_t len, void *user_data) { + print_timer(); + fprintf(outfile, " [ERROR] %.*s\n", (int)len, msg); + fflush(outfile); + + return 0; +} + namespace { std::chrono::steady_clock::time_point base_tv; } // namespace diff --git a/src/app_helper.h b/src/app_helper.h index 408adac2..263bb99c 100644 --- a/src/app_helper.h +++ b/src/app_helper.h @@ -60,6 +60,9 @@ int verbose_on_data_chunk_recv_callback(nghttp2_session *session, uint8_t flags, int32_t stream_id, const uint8_t *data, size_t len, void *user_data); +int verbose_error_callback(nghttp2_session *session, const char *msg, + size_t len, void *user_data); + // Returns difference between |a| and |b| in milliseconds, assuming // |a| is more recent than |b|. template diff --git a/src/nghttp.cc b/src/nghttp.cc index d0fc8ece..d219d1da 100644 --- a/src/nghttp.cc +++ b/src/nghttp.cc @@ -2258,6 +2258,9 @@ int run(char **uris, int n) { nghttp2_session_callbacks_set_on_invalid_frame_recv_callback( callbacks, verbose_on_invalid_frame_recv_callback); + + nghttp2_session_callbacks_set_error_callback(callbacks, + verbose_error_callback); } nghttp2_session_callbacks_set_on_data_chunk_recv_callback( diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 32905876..6ad4ac24 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -837,6 +837,11 @@ nghttp2_session_callbacks *create_http2_upstream_callbacks() { callbacks, http::select_padding_callback); } + if (get_config()->http2.upstream.debug.frame_debug) { + nghttp2_session_callbacks_set_error_callback(callbacks, + verbose_error_callback); + } + return callbacks; }