Call on_invalid_frame_recv_callback on bad HTTP messaging

lib/includes/nghttp2/nghttp2.h

@@ -348,6 +348,11 @@ typedef enum {
    * `nghttp2_session_terminate_session()` is called.
    */
   NGHTTP2_ERR_SESSION_CLOSING = -530,
+  /**
+   * Invalid HTTP header field was received and stream is going to be
+   * closed.
+   */
+  NGHTTP2_ERR_HTTP_HEADER = -531,
   /**
    * The errors < :enum:`NGHTTP2_ERR_FATAL` mean that the library is
    * under unexpected condition and processing was terminated (e.g.,

lib/nghttp2_http.c

@@ -232,7 +232,7 @@ static int http_request_on_header(nghttp2_stream *stream, nghttp2_nv *nv,
   if (nv->name[0] == ':') {
     if (trailer ||
         (stream->http_flags & NGHTTP2_HTTP_FLAG_PSEUDO_HEADER_DISALLOWED)) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
   }
 
@@ -241,55 +241,55 @@ static int http_request_on_header(nghttp2_stream *stream, nghttp2_nv *nv,
   switch (token) {
   case NGHTTP2_TOKEN__AUTHORITY:
     if (!check_pseudo_header(stream, nv, NGHTTP2_HTTP_FLAG__AUTHORITY)) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     break;
   case NGHTTP2_TOKEN__METHOD:
     if (!check_pseudo_header(stream, nv, NGHTTP2_HTTP_FLAG__METHOD)) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     if (streq("HEAD", nv->value, nv->valuelen)) {
       stream->http_flags |= NGHTTP2_HTTP_FLAG_METH_HEAD;
     } else if (streq("CONNECT", nv->value, nv->valuelen)) {
       if (stream->stream_id % 2 == 0) {
         /* we won't allow CONNECT for push */
-        return -1;
+        return NGHTTP2_ERR_HTTP_HEADER;
       }
       stream->http_flags |= NGHTTP2_HTTP_FLAG_METH_CONNECT;
       if (stream->http_flags &
           (NGHTTP2_HTTP_FLAG__PATH | NGHTTP2_HTTP_FLAG__SCHEME)) {
-        return -1;
+        return NGHTTP2_ERR_HTTP_HEADER;
       }
     }
     break;
   case NGHTTP2_TOKEN__PATH:
     if (stream->http_flags & NGHTTP2_HTTP_FLAG_METH_CONNECT) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     if (!check_pseudo_header(stream, nv, NGHTTP2_HTTP_FLAG__PATH)) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     break;
   case NGHTTP2_TOKEN__SCHEME:
     if (stream->http_flags & NGHTTP2_HTTP_FLAG_METH_CONNECT) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     if (!check_pseudo_header(stream, nv, NGHTTP2_HTTP_FLAG__SCHEME)) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     break;
   case NGHTTP2_TOKEN_HOST:
     if (!check_pseudo_header(stream, nv, NGHTTP2_HTTP_FLAG_HOST)) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     break;
   case NGHTTP2_TOKEN_CONTENT_LENGTH: {
     if (stream->content_length != -1) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     stream->content_length = parse_uint(nv->value, nv->valuelen);
     if (stream->content_length == -1) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     break;
   }
@@ -299,15 +299,15 @@ static int http_request_on_header(nghttp2_stream *stream, nghttp2_nv *nv,
   case NGHTTP2_TOKEN_PROXY_CONNECTION:
   case NGHTTP2_TOKEN_TRANSFER_ENCODING:
   case NGHTTP2_TOKEN_UPGRADE:
-    return -1;
+    return NGHTTP2_ERR_HTTP_HEADER;
   case NGHTTP2_TOKEN_TE:
     if (!strieq("trailers", nv->value, nv->valuelen)) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     break;
   default:
     if (nv->name[0] == ':') {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
   }
 
@@ -325,7 +325,7 @@ static int http_response_on_header(nghttp2_stream *stream, nghttp2_nv *nv,
   if (nv->name[0] == ':') {
     if (trailer ||
         (stream->http_flags & NGHTTP2_HTTP_FLAG_PSEUDO_HEADER_DISALLOWED)) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
   }
 
@@ -334,24 +334,24 @@ static int http_response_on_header(nghttp2_stream *stream, nghttp2_nv *nv,
   switch (token) {
   case NGHTTP2_TOKEN__STATUS: {
     if (!check_pseudo_header(stream, nv, NGHTTP2_HTTP_FLAG__STATUS)) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     if (nv->valuelen != 3) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     stream->status_code = parse_uint(nv->value, nv->valuelen);
     if (stream->status_code == -1) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     break;
   }
   case NGHTTP2_TOKEN_CONTENT_LENGTH: {
     if (stream->content_length != -1) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     stream->content_length = parse_uint(nv->value, nv->valuelen);
     if (stream->content_length == -1) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     break;
   }
@@ -361,15 +361,15 @@ static int http_response_on_header(nghttp2_stream *stream, nghttp2_nv *nv,
   case NGHTTP2_TOKEN_PROXY_CONNECTION:
   case NGHTTP2_TOKEN_TRANSFER_ENCODING:
   case NGHTTP2_TOKEN_UPGRADE:
-    return -1;
+    return NGHTTP2_ERR_HTTP_HEADER;
   case NGHTTP2_TOKEN_TE:
     if (!strieq("trailers", nv->value, nv->valuelen)) {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     break;
   default:
     if (nv->name[0] == ':') {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
   }
 
@@ -392,31 +392,32 @@ int nghttp2_http_on_header(nghttp2_session *session, nghttp2_stream *stream,
   if (!nghttp2_check_header_name(nv->name, nv->namelen)) {
     size_t i;
     if (nv->namelen > 0 && nv->name[0] == ':') {
-      return -1;
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     /* header field name must be lower-cased without exception */
     for (i = 0; i < nv->namelen; ++i) {
       char c = nv->name[i];
       if ('A' <= c && c <= 'Z') {
-        return -1;
+        return NGHTTP2_ERR_HTTP_HEADER;
       }
     }
     /* When ignoring regular headers, we set this flag so that we
        still enforce header field ordering rule for pseudo header
        fields. */
     stream->http_flags |= NGHTTP2_HTTP_FLAG_PSEUDO_HEADER_DISALLOWED;
-    return 1;
+    return NGHTTP2_ERR_IGN_HTTP_HEADER;
   }
 
   if (!nghttp2_check_header_value(nv->value, nv->valuelen)) {
-    if (nv->namelen > 0 && nv->name[0] == ':') {
+    assert(nv->namelen > 0);
-      return -1;
+    if (nv->name[0] == ':') {
+      return NGHTTP2_ERR_HTTP_HEADER;
     }
     /* When ignoring regular headers, we set this flag so that we
        still enforce header field ordering rule for pseudo header
        fields. */
     stream->http_flags |= NGHTTP2_HTTP_FLAG_PSEUDO_HEADER_DISALLOWED;
-    return 1;
+    return NGHTTP2_ERR_IGN_HTTP_HEADER;
   }
 
   if (session->server || frame->hd.type == NGHTTP2_PUSH_PROMISE) {

lib/nghttp2_http.h

@@ -36,10 +36,16 @@
 /*
  * This function is called when HTTP header field |nv| in |frame| is
  * received for |stream|.  This function will validate |nv| against
- * the current state of stream.  This function returns 0 if it
+ * the current state of stream.
- * succeeds, or 1 if the header field should be ignored, or -1 if the
+ *
- * header field contains totally unforgivable piece of junk and stream
+ * This function returns 0 if it succeeds, or one of the following
- * should be killed.
+ * negative error codes:
+ *
+ * NGHTTP2_ERR_HTTP_HEADER
+ *     Invalid HTTP header field was received.
+ * NGHTTP2_ERR_IGN_HTTP_HEADER
+ *     Invalid HTTP header field was received but it can be treated as
+ *     if it was not received because of compatibility reasons.
  */
 int nghttp2_http_on_header(nghttp2_session *session, nghttp2_stream *stream,
                            nghttp2_frame *frame, nghttp2_nv *nv, int trailer);

lib/nghttp2_int.h

@@ -46,7 +46,12 @@ typedef int (*nghttp2_compar)(const void *lhs, const void *rhs);
 typedef enum {
   NGHTTP2_ERR_CREDENTIAL_PENDING = -101,
   NGHTTP2_ERR_IGN_HEADER_BLOCK = -103,
-  NGHTTP2_ERR_IGN_PAYLOAD = -104
+  NGHTTP2_ERR_IGN_PAYLOAD = -104,
+  /*
+   * Invalid HTTP header field was received but it can be treated as
+   * if it was not received because of compatibility reasons.
+   */
+  NGHTTP2_ERR_IGN_HTTP_HEADER = -105,
 } nghttp2_internal_error;
 
 #endif /* NGHTTP2_INT_H */

lib/nghttp2_session.c

@@ -3023,11 +3023,12 @@ static int session_handle_frame_size_error(nghttp2_session *session,
   return nghttp2_session_terminate_session(session, NGHTTP2_FRAME_SIZE_ERROR);
 }
 
-static int session_handle_invalid_stream(nghttp2_session *session,
+static int session_handle_invalid_stream2(nghttp2_session *session,
+                                          int32_t stream_id,
                                           nghttp2_frame *frame,
                                           uint32_t error_code) {
   int rv;
-  rv = nghttp2_session_add_rst_stream(session, frame->hd.stream_id, error_code);
+  rv = nghttp2_session_add_rst_stream(session, stream_id, error_code);
   if (rv != 0) {
     return rv;
   }
@@ -3040,6 +3041,13 @@ static int session_handle_invalid_stream(nghttp2_session *session,
   return 0;
 }
 
+static int session_handle_invalid_stream(nghttp2_session *session,
+                                         nghttp2_frame *frame,
+                                         uint32_t error_code) {
+  return session_handle_invalid_stream2(session, frame->hd.stream_id, frame,
+                                        error_code);
+}
+
 static int session_inflate_handle_invalid_stream(nghttp2_session *session,
                                                  nghttp2_frame *frame,
                                                  uint32_t error_code) {
@@ -3170,18 +3178,22 @@ static int inflate_header_block(nghttp2_session *session, nghttp2_frame *frame,
       if (subject_stream && session_enforce_http_messaging(session)) {
         rv = nghttp2_http_on_header(session, subject_stream, frame, &nv,
                                     trailer);
-        if (rv == -1) {
+        if (rv == NGHTTP2_ERR_HTTP_HEADER) {
           DEBUGF(fprintf(
               stderr, "recv: HTTP error: type=%d, id=%d, header %.*s: %.*s\n",
               frame->hd.type, subject_stream->stream_id, (int)nv.namelen,
               nv.name, (int)nv.valuelen, nv.value));
-          rv = nghttp2_session_add_rst_stream(
+
-              session, subject_stream->stream_id, NGHTTP2_PROTOCOL_ERROR);
+          rv =
+              session_handle_invalid_stream2(session, subject_stream->stream_id,
+                                             frame, NGHTTP2_PROTOCOL_ERROR);
           if (nghttp2_is_fatal(rv)) {
             return rv;
           }
           return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
-        } else if (rv == 1) {
+        }
+
+        if (rv == NGHTTP2_ERR_IGN_HTTP_HEADER) {
           /* header is ignored */
           DEBUGF(fprintf(
               stderr, "recv: HTTP ignored: type=%d, id=%d, header %.*s: %.*s\n",
@@ -3189,7 +3201,7 @@ static int inflate_header_block(nghttp2_session *session, nghttp2_frame *frame,
               nv.name, (int)nv.valuelen, nv.value));
         }
       }
-      if (rv == 0 && call_header_cb) {
+      if (rv == 0) {
         rv = session_call_on_header(session, frame, &nv);
         /* This handles NGHTTP2_ERR_PAUSE and
            NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE as well */
@@ -3348,7 +3360,7 @@ static int session_after_header_block_received(nghttp2_session *session) {
 
       call_cb = 0;
 
-      rv = nghttp2_session_add_rst_stream(session, stream_id,
+      rv = session_handle_invalid_stream2(session, stream_id, frame,
                                           NGHTTP2_PROTOCOL_ERROR);
       if (nghttp2_is_fatal(rv)) {
         return rv;

tests/nghttp2_session_test.c

@@ -6820,10 +6820,13 @@ static void check_nghttp2_http_recv_headers_fail(
   ssize_t rv;
   nghttp2_outbound_item *item;
   nghttp2_bufs bufs;
+  my_user_data *ud;
 
   mem = nghttp2_mem_default();
   frame_pack_bufs_init(&bufs);
 
+  ud = session->user_data;
+
   if (stream_state != -1) {
     nghttp2_session_open_stream(session, stream_id, NGHTTP2_STREAM_FLAG_NONE,
                                 &pri_spec_default, stream_state, NULL);
@@ -6833,6 +6836,8 @@ static void check_nghttp2_http_recv_headers_fail(
                     nvlen, mem);
   CU_ASSERT(0 == rv);
 
+  ud->invalid_frame_recv_cb_called = 0;
+
   rv = nghttp2_session_mem_recv(session, bufs.head->buf.pos,
                                 nghttp2_buf_len(&bufs.head->buf));
 
@@ -6841,6 +6846,7 @@ static void check_nghttp2_http_recv_headers_fail(
   item = nghttp2_session_get_next_ob_item(session);
 
   CU_ASSERT(NGHTTP2_RST_STREAM == item->frame.hd.type);
+  CU_ASSERT(1 == ud->invalid_frame_recv_cb_called);
 
   CU_ASSERT(0 == nghttp2_session_send(session));
 
@@ -6852,6 +6858,7 @@ void test_nghttp2_http_mandatory_headers(void) {
   nghttp2_session_callbacks callbacks;
   nghttp2_hd_deflater deflater;
   nghttp2_mem *mem;
+  my_user_data ud;
   /* test case for response */
   const nghttp2_nv nostatus_resnv[] = {MAKE_NV("server", "foo")};
   const nghttp2_nv dupstatus_resnv[] = {MAKE_NV(":status", "200"),
@@ -6907,8 +6914,9 @@ void test_nghttp2_http_mandatory_headers(void) {
 
   memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
   callbacks.send_callback = null_send_callback;
+  callbacks.on_invalid_frame_recv_callback = on_invalid_frame_recv_callback;
 
-  nghttp2_session_client_new(&session, &callbacks, NULL);
+  nghttp2_session_client_new(&session, &callbacks, &ud);
 
   nghttp2_hd_deflate_init(&deflater, mem);
 
@@ -6957,7 +6965,7 @@ void test_nghttp2_http_mandatory_headers(void) {
   nghttp2_session_del(session);
 
   /* check server side */
-  nghttp2_session_server_new(&session, &callbacks, NULL);
+  nghttp2_session_server_new(&session, &callbacks, &ud);
 
   nghttp2_hd_deflate_init(&deflater, mem);