Handle response in nghttp2_on_begin_frame_callback

Previously, nghttp2_session_end_request_headers_received assumes
stream is still writable (in other words, local endpoint has not sent
END_STREAM).  But this assumption is false, because application can
send response in nghttp2_on_begin_frame_callback.  Probably, this
assumption was made before the callback was introduced.  This commit
addresses this issue.  Since all
nghttp2_session_end_*_headers_received functions are identical, we
refactored them into one function.

lib/nghttp2_session.c

@@ -3292,9 +3292,7 @@ static int inflate_header_block(nghttp2_session *session, nghttp2_frame *frame,
 }
 
 /*
- * Decompress header blocks of incoming request HEADERS and also call
+ * Call this function when HEADERS frame was completely received.
- * additional callbacks. This function can be called again if this
- * function returns NGHTTP2_ERR_PAUSE.
  *
  * This function returns 0 if it succeeds, or one of negative error
  * codes:
@@ -3304,69 +3302,20 @@ static int inflate_header_block(nghttp2_session *session, nghttp2_frame *frame,
  * NGHTTP2_ERR_NOMEM
  *     Out of memory.
  */
-int nghttp2_session_end_request_headers_received(nghttp2_session *session _U_,
+static int session_end_stream_headers_received(nghttp2_session *session,
-                                                 nghttp2_frame *frame,
-                                                 nghttp2_stream *stream) {
-  if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) {
-    nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_RD);
-  }
-  /* Here we assume that stream is not shutdown in NGHTTP2_SHUT_WR */
-  return 0;
-}
-
-/*
- * Decompress header blocks of incoming (push-)response HEADERS and
- * also call additional callbacks. This function can be called again
- * if this function returns NGHTTP2_ERR_PAUSE.
- *
- * This function returns 0 if it succeeds, or one of negative error
- * codes:
- *
- * NGHTTP2_ERR_CALLBACK_FAILURE
- *     The callback function failed.
- * NGHTTP2_ERR_NOMEM
- *     Out of memory.
- */
-int nghttp2_session_end_response_headers_received(nghttp2_session *session,
                                                nghttp2_frame *frame,
                                                nghttp2_stream *stream) {
   int rv;
-  if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) {
+  if ((frame->hd.flags & NGHTTP2_FLAG_END_STREAM) == 0) {
-    /* This is the last frame of this stream, so disallow
+    return 0;
-       further receptions. */
+  }
+
   nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_RD);
   rv = nghttp2_session_close_stream_if_shut_rdwr(session, stream);
   if (nghttp2_is_fatal(rv)) {
     return rv;
   }
-  }
-  return 0;
-}
 
-/*
- * Decompress header blocks of incoming HEADERS and also call
- * additional callbacks. This function can be called again if this
- * function returns NGHTTP2_ERR_PAUSE.
- *
- * This function returns 0 if it succeeds, or one of negative error
- * codes:
- *
- * NGHTTP2_ERR_CALLBACK_FAILURE
- *     The callback function failed.
- * NGHTTP2_ERR_NOMEM
- *     Out of memory.
- */
-int nghttp2_session_end_headers_received(nghttp2_session *session,
-                                         nghttp2_frame *frame,
-                                         nghttp2_stream *stream) {
-  int rv;
-  if (frame->hd.flags & NGHTTP2_FLAG_END_STREAM) {
-    nghttp2_stream_shutdown(stream, NGHTTP2_SHUT_RD);
-    rv = nghttp2_session_close_stream_if_shut_rdwr(session, stream);
-    if (nghttp2_is_fatal(rv)) {
-      return rv;
-    }
-  }
   return 0;
 }
 
@@ -3447,19 +3396,7 @@ static int session_after_header_block_received(nghttp2_session *session) {
     return 0;
   }
 
-  switch (frame->headers.cat) {
+  return session_end_stream_headers_received(session, frame, stream);
-  case NGHTTP2_HCAT_REQUEST:
-    return nghttp2_session_end_request_headers_received(session, frame, stream);
-  case NGHTTP2_HCAT_RESPONSE:
-  case NGHTTP2_HCAT_PUSH_RESPONSE:
-    return nghttp2_session_end_response_headers_received(session, frame,
-                                                         stream);
-  case NGHTTP2_HCAT_HEADERS:
-    return nghttp2_session_end_headers_received(session, frame, stream);
-  default:
-    assert(0);
-  }
-  return 0;
 }
 
 int nghttp2_session_on_request_headers_received(nghttp2_session *session,

lib/nghttp2_session.h

@@ -560,18 +560,6 @@ int nghttp2_session_adjust_idle_stream(nghttp2_session *session);
 int nghttp2_session_close_stream_if_shut_rdwr(nghttp2_session *session,
                                               nghttp2_stream *stream);
 
-int nghttp2_session_end_request_headers_received(nghttp2_session *session,
-                                                 nghttp2_frame *frame,
-                                                 nghttp2_stream *stream);
-
-int nghttp2_session_end_response_headers_received(nghttp2_session *session,
-                                                  nghttp2_frame *frame,
-                                                  nghttp2_stream *stream);
-
-int nghttp2_session_end_headers_received(nghttp2_session *session,
-                                         nghttp2_frame *frame,
-                                         nghttp2_stream *stream);
-
 int nghttp2_session_on_request_headers_received(nghttp2_session *session,
                                                 nghttp2_frame *frame);
 

tests/main.c

@@ -87,6 +87,8 @@ int main(int argc _U_, char *argv[] _U_) {
                    test_nghttp2_session_recv_continuation) ||
       !CU_add_test(pSuite, "session_recv_headers_with_priority",
                    test_nghttp2_session_recv_headers_with_priority) ||
+      !CU_add_test(pSuite, "session_recv_headers_early_response",
+                   test_nghttp2_session_recv_headers_early_response) ||
       !CU_add_test(pSuite, "session_recv_premature_headers",
                    test_nghttp2_session_recv_premature_headers) ||
       !CU_add_test(pSuite, "session_recv_unknown_frame",

tests/nghttp2_session_test.c

@@ -1356,6 +1356,85 @@ void test_nghttp2_session_recv_headers_with_priority(void) {
   nghttp2_session_del(session);
 }
 
+static int response_on_begin_frame_callback(nghttp2_session *session,
+                                            const nghttp2_frame_hd *hd,
+                                            void *user_data _U_) {
+  int rv;
+
+  if (hd->type != NGHTTP2_HEADERS) {
+    return 0;
+  }
+
+  rv = nghttp2_submit_response(session, hd->stream_id, resnv, ARRLEN(resnv),
+                               NULL);
+
+  CU_ASSERT(0 == rv);
+
+  return 0;
+}
+
+void test_nghttp2_session_recv_headers_early_response(void) {
+  nghttp2_session *session;
+  nghttp2_session_callbacks callbacks;
+  nghttp2_bufs bufs;
+  nghttp2_buf *buf;
+  nghttp2_hd_deflater deflater;
+  nghttp2_mem *mem;
+  nghttp2_nv *nva;
+  size_t nvlen;
+  nghttp2_frame frame;
+  ssize_t rv;
+  nghttp2_stream *stream;
+
+  mem = nghttp2_mem_default();
+  frame_pack_bufs_init(&bufs);
+
+  memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
+  callbacks.send_callback = null_send_callback;
+  callbacks.on_begin_frame_callback = response_on_begin_frame_callback;
+
+  nghttp2_session_server_new(&session, &callbacks, NULL);
+
+  nghttp2_hd_deflate_init(&deflater, mem);
+
+  nvlen = ARRLEN(reqnv);
+  nghttp2_nv_array_copy(&nva, reqnv, nvlen, mem);
+  nghttp2_frame_headers_init(&frame.headers,
+                             NGHTTP2_FLAG_END_HEADERS | NGHTTP2_FLAG_END_STREAM,
+                             1, NGHTTP2_HCAT_REQUEST, NULL, nva, nvlen);
+
+  rv = nghttp2_frame_pack_headers(&bufs, &frame.headers, &deflater);
+
+  CU_ASSERT(0 == rv);
+
+  nghttp2_frame_headers_free(&frame.headers, mem);
+
+  buf = &bufs.head->buf;
+
+  /* Only receive 9 bytes headers, and invoke
+     on_begin_frame_callback */
+  rv = nghttp2_session_mem_recv(session, buf->pos, 9);
+
+  CU_ASSERT(9 == rv);
+
+  rv = nghttp2_session_send(session);
+
+  CU_ASSERT(0 == rv);
+
+  rv =
+      nghttp2_session_mem_recv(session, buf->pos + 9, nghttp2_buf_len(buf) - 9);
+
+  CU_ASSERT((ssize_t)nghttp2_buf_len(buf) - 9 == rv);
+
+  stream = nghttp2_session_get_stream_raw(session, 1);
+
+  CU_ASSERT(stream->flags & NGHTTP2_STREAM_FLAG_CLOSED);
+
+  nghttp2_hd_deflate_free(&deflater);
+  nghttp2_session_del(session);
+  nghttp2_bufs_free(&bufs);
+}
+
 void test_nghttp2_session_recv_premature_headers(void) {
   nghttp2_session *session;
   nghttp2_session_callbacks callbacks;

tests/nghttp2_session_test.h

@@ -33,6 +33,7 @@ void test_nghttp2_session_recv_data(void);
 void test_nghttp2_session_recv_data_no_auto_flow_control(void);
 void test_nghttp2_session_recv_continuation(void);
 void test_nghttp2_session_recv_headers_with_priority(void);
+void test_nghttp2_session_recv_headers_early_response(void);
 void test_nghttp2_session_recv_premature_headers(void);
 void test_nghttp2_session_recv_unknown_frame(void);
 void test_nghttp2_session_recv_unexpected_continuation(void);