Fix bug that on_header callback is still called after stream is closed

lib/nghttp2_session.c

@@ -3619,71 +3619,73 @@ static int inflate_header_block(nghttp2_session *session, nghttp2_frame *frame,
 
     if (call_header_cb && (inflate_flags & NGHTTP2_HD_INFLATE_EMIT)) {
       rv = 0;
-      if (subject_stream && session_enforce_http_messaging(session)) {
+      if (subject_stream) {
-        rv = nghttp2_http_on_header(session, subject_stream, frame, &nv,
+        if (session_enforce_http_messaging(session)) {
-                                    trailer);
+          rv = nghttp2_http_on_header(session, subject_stream, frame, &nv,
+                                      trailer);
 
-        if (rv == NGHTTP2_ERR_IGN_HTTP_HEADER) {
+          if (rv == NGHTTP2_ERR_IGN_HTTP_HEADER) {
-          /* Don't overwrite rv here */
+            /* Don't overwrite rv here */
-          int rv2;
+            int rv2;
 
-          rv2 = session_call_on_invalid_header(session, frame, &nv);
+            rv2 = session_call_on_invalid_header(session, frame, &nv);
-          if (rv2 == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) {
+            if (rv2 == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) {
-            rv = NGHTTP2_ERR_HTTP_HEADER;
+              rv = NGHTTP2_ERR_HTTP_HEADER;
-          } else {
+            } else {
-            if (rv2 != 0) {
+              if (rv2 != 0) {
-              return rv2;
+                return rv2;
+              }
+
+              /* header is ignored */
+              DEBUGF("recv: HTTP ignored: type=%u, id=%d, header %.*s: %.*s\n",
+                     frame->hd.type, frame->hd.stream_id, (int)nv.name->len,
+                     nv.name->base, (int)nv.value->len, nv.value->base);
+
+              rv2 = session_call_error_callback(
+                  session, NGHTTP2_ERR_HTTP_HEADER,
+                  "Ignoring received invalid HTTP header field: 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(rv2)) {
+                return rv2;
+              }
             }
+          }
 
-            /* header is ignored */
+          if (rv == NGHTTP2_ERR_HTTP_HEADER) {
-            DEBUGF("recv: HTTP ignored: type=%u, id=%d, header %.*s: %.*s\n",
+            DEBUGF("recv: HTTP error: type=%u, id=%d, header %.*s: %.*s\n",
                    frame->hd.type, frame->hd.stream_id, (int)nv.name->len,
                    nv.name->base, (int)nv.value->len, nv.value->base);
 
-            rv2 = session_call_error_callback(
+            rv = session_call_error_callback(
                 session, NGHTTP2_ERR_HTTP_HEADER,
-                "Ignoring received invalid HTTP header field: frame type: "
+                "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(rv2)) {
+            if (nghttp2_is_fatal(rv)) {
-              return rv2;
+              return rv;
             }
+
+            rv = session_handle_invalid_stream2(session,
+                                                subject_stream->stream_id,
+                                                frame, NGHTTP2_ERR_HTTP_HEADER);
+            if (nghttp2_is_fatal(rv)) {
+              return rv;
+            }
+            return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
           }
         }
-
+        if (rv == 0) {
-        if (rv == NGHTTP2_ERR_HTTP_HEADER) {
+          rv = session_call_on_header(session, frame, &nv);
-          DEBUGF("recv: HTTP error: type=%u, id=%d, header %.*s: %.*s\n",
+          /* This handles NGHTTP2_ERR_PAUSE and
-                 frame->hd.type, frame->hd.stream_id, (int)nv.name->len,
+             NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE as well */
-                 nv.name->base, (int)nv.value->len, nv.value->base);
+          if (rv != 0) {
-
-          rv = session_call_error_callback(
-              session, NGHTTP2_ERR_HTTP_HEADER,
-              "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);
-          if (nghttp2_is_fatal(rv)) {
-            return rv;
-          }
-          return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
-        }
-      }
-      if (rv == 0) {
-        rv = session_call_on_header(session, frame, &nv);
-        /* This handles NGHTTP2_ERR_PAUSE and
-           NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE as well */
-        if (rv != 0) {
-          return rv;
         }
       }
     }

tests/main.c

@@ -91,6 +91,8 @@ int main() {
                    test_nghttp2_session_recv_headers_with_padding) ||
       !CU_add_test(pSuite, "session_recv_headers_early_response",
                    test_nghttp2_session_recv_headers_early_response) ||
+      !CU_add_test(pSuite, "session_recv_headers_for_closed_stream",
+                   test_nghttp2_session_recv_headers_for_closed_stream) ||
       !CU_add_test(pSuite, "session_server_recv_push_response",
                    test_nghttp2_session_server_recv_push_response) ||
       !CU_add_test(pSuite, "session_recv_premature_headers",

tests/nghttp2_session_test.c

@@ -1743,6 +1743,91 @@ void test_nghttp2_session_recv_headers_early_response(void) {
   nghttp2_bufs_free(&bufs);
 }
 
+void test_nghttp2_session_recv_headers_for_closed_stream(void) {
+  nghttp2_session *session;
+  nghttp2_session_callbacks callbacks;
+  nghttp2_nv *nva;
+  size_t nvlen;
+  nghttp2_frame frame;
+  nghttp2_bufs bufs;
+  nghttp2_buf *buf;
+  ssize_t rv;
+  my_user_data ud;
+  nghttp2_hd_deflater deflater;
+  nghttp2_stream *stream;
+  nghttp2_mem *mem;
+  const uint8_t *data;
+
+  mem = nghttp2_mem_default();
+  frame_pack_bufs_init(&bufs);
+
+  memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
+  callbacks.on_frame_recv_callback = on_frame_recv_callback;
+  callbacks.on_header_callback = on_header_callback;
+
+  nghttp2_session_server_new(&session, &callbacks, &ud);
+
+  nghttp2_hd_deflate_init(&deflater, mem);
+
+  /* Make sure that on_header callback never be invoked for closed
+     stream */
+  nvlen = ARRLEN(reqnv);
+  nghttp2_nv_array_copy(&nva, reqnv, nvlen, mem);
+
+  nghttp2_frame_headers_init(&frame.headers, NGHTTP2_FLAG_END_HEADERS, 1,
+                             NGHTTP2_HCAT_HEADERS, NULL, nva, nvlen);
+
+  rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater);
+
+  CU_ASSERT(0 == rv);
+  CU_ASSERT(nghttp2_bufs_len(&bufs) > 0);
+
+  nghttp2_frame_headers_free(&frame.headers, mem);
+
+  buf = &bufs.head->buf;
+  assert(nghttp2_bufs_len(&bufs) == nghttp2_buf_len(buf));
+
+  ud.header_cb_called = 0;
+  ud.frame_recv_cb_called = 0;
+
+  rv = nghttp2_session_mem_recv(session, buf->pos, NGHTTP2_FRAME_HDLEN);
+
+  CU_ASSERT(NGHTTP2_FRAME_HDLEN == rv);
+  CU_ASSERT(0 == ud.header_cb_called);
+  CU_ASSERT(0 == ud.frame_recv_cb_called);
+
+  stream = nghttp2_session_get_stream(session, 1);
+
+  CU_ASSERT(NULL != stream);
+
+  rv = nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, 1,
+                                 NGHTTP2_NO_ERROR);
+
+  CU_ASSERT(0 == rv);
+
+  rv = nghttp2_session_mem_send(session, &data);
+
+  CU_ASSERT(rv > 0);
+
+  stream = nghttp2_session_get_stream(session, 1);
+
+  CU_ASSERT(NULL == stream);
+
+  ud.header_cb_called = 0;
+  ud.frame_recv_cb_called = 0;
+
+  rv = nghttp2_session_mem_recv(session, buf->pos + NGHTTP2_FRAME_HDLEN,
+                                nghttp2_buf_len(buf) - NGHTTP2_FRAME_HDLEN);
+
+  CU_ASSERT((ssize_t)nghttp2_buf_len(buf) - NGHTTP2_FRAME_HDLEN == rv);
+  CU_ASSERT(0 == ud.header_cb_called);
+  CU_ASSERT(0 == ud.frame_recv_cb_called);
+
+  nghttp2_bufs_free(&bufs);
+  nghttp2_hd_deflate_free(&deflater);
+  nghttp2_session_del(session);
+}
+
 void test_nghttp2_session_server_recv_push_response(void) {
   nghttp2_session *session;
   nghttp2_session_callbacks callbacks;

tests/nghttp2_session_test.h

@@ -39,6 +39,7 @@ void test_nghttp2_session_recv_continuation(void);
 void test_nghttp2_session_recv_headers_with_priority(void);
 void test_nghttp2_session_recv_headers_with_padding(void);
 void test_nghttp2_session_recv_headers_early_response(void);
+void test_nghttp2_session_recv_headers_for_closed_stream(void);
 void test_nghttp2_session_server_recv_push_response(void);
 void test_nghttp2_session_recv_premature_headers(void);
 void test_nghttp2_session_recv_unknown_frame(void);