Add nghttp2_error_callback to tell application human readable error m…

…essage

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"])

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 \

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
  *

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;
+}

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 */

lib/nghttp2_session.c

@@ -28,6 +28,7 @@
 #include <stddef.h>
 #include <stdio.h>
 #include <assert.h>
+#include <stdarg.h>
 
 #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);

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(

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

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 <typename TimePoint>

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(

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;
 }