Fix bug that data are not consumed for connection in race condition

When we know that stream is closed at time we read DATA frame header,
we use NGHTTP2_IB_IGN_DATA, and consume data for connection if
nghttp2_option_set_no_auto_window_update() is used.  However, if
stream is closed while we are in NGHTTP2_IB_READ_DATA, those bytes are
not consumed for connection, nor notified to application via callback,
so it eventually fills up connection window and connection will
freeze.  This commit fixes this issue by consuming these data for
connection when stream is closed or does not exist.

lib/nghttp2_session.c

@@ -5760,49 +5760,58 @@ ssize_t nghttp2_session_mem_recv(nghttp2_session *session, const uint8_t *in,
           if (nghttp2_is_fatal(rv)) {
             return rv;
           }
-        }
 
-        data_readlen = inbound_frame_effective_readlen(
-            iframe, iframe->payloadleft, readlen);
+          data_readlen = inbound_frame_effective_readlen(
+              iframe, iframe->payloadleft, readlen);
 
-        padlen = readlen - data_readlen;
+          padlen = readlen - data_readlen;
 
-        if (padlen > 0) {
-          /* Padding is considered as "consumed" immediately */
-          rv = nghttp2_session_consume(session, iframe->frame.hd.stream_id,
-                                       padlen);
+          if (padlen > 0) {
+            /* Padding is considered as "consumed" immediately */
+            rv = nghttp2_session_consume(session, iframe->frame.hd.stream_id,
+                                         padlen);
 
-          if (nghttp2_is_fatal(rv)) {
-            return rv;
+            if (nghttp2_is_fatal(rv)) {
+              return rv;
+            }
           }
-        }
 
-        DEBUGF(fprintf(stderr, "recv: data_readlen=%zd\n", data_readlen));
+          DEBUGF(fprintf(stderr, "recv: data_readlen=%zd\n", data_readlen));
+
+          if (data_readlen > 0) {
+            if (session_enforce_http_messaging(session)) {
+              if (nghttp2_http_on_data_chunk(stream, data_readlen) != 0) {
+                rv = nghttp2_session_add_rst_stream(session,
+                                                    iframe->frame.hd.stream_id,
+                                                    NGHTTP2_PROTOCOL_ERROR);
+                if (nghttp2_is_fatal(rv)) {
+                  return rv;
+                }
+                busy = 1;
+                iframe->state = NGHTTP2_IB_IGN_DATA;
+                break;
+              }
+            }
+            if (session->callbacks.on_data_chunk_recv_callback) {
+              rv = session->callbacks.on_data_chunk_recv_callback(
+                  session, iframe->frame.hd.flags, iframe->frame.hd.stream_id,
+                  in - readlen, data_readlen, session->user_data);
+              if (rv == NGHTTP2_ERR_PAUSE) {
+                return in - first;
+              }
 
-        if (stream && data_readlen > 0) {
-          if (session_enforce_http_messaging(session)) {
-            if (nghttp2_http_on_data_chunk(stream, data_readlen) != 0) {
-              rv = nghttp2_session_add_rst_stream(
-                  session, iframe->frame.hd.stream_id, NGHTTP2_PROTOCOL_ERROR);
               if (nghttp2_is_fatal(rv)) {
-                return rv;
+                return NGHTTP2_ERR_CALLBACK_FAILURE;
               }
-              busy = 1;
-              iframe->state = NGHTTP2_IB_IGN_DATA;
-              break;
             }
           }
-          if (session->callbacks.on_data_chunk_recv_callback) {
-            rv = session->callbacks.on_data_chunk_recv_callback(
-                session, iframe->frame.hd.flags, iframe->frame.hd.stream_id,
-                in - readlen, data_readlen, session->user_data);
-            if (rv == NGHTTP2_ERR_PAUSE) {
-              return in - first;
-            }
+        } else if (session->opt_flags & NGHTTP2_OPTMASK_NO_AUTO_WINDOW_UPDATE) {
+          /* stream was closed or does not exist.  Consume all data
+             for connection immediately here */
+          rv = session_update_connection_consumed_size(session, readlen);
 
-            if (nghttp2_is_fatal(rv)) {
-              return NGHTTP2_ERR_CALLBACK_FAILURE;
-            }
+          if (nghttp2_is_fatal(rv)) {
+            return rv;
           }
         }
       }

tests/main.c

@@ -80,6 +80,8 @@ int main(int argc _U_, char *argv[] _U_) {
       !CU_add_test(pSuite, "session_recv_eof", test_nghttp2_session_recv_eof) ||
       !CU_add_test(pSuite, "session_recv_data",
                    test_nghttp2_session_recv_data) ||
+      !CU_add_test(pSuite, "session_recv_data_no_auto_flow_control",
+                   test_nghttp2_session_recv_data_no_auto_flow_control) ||
       !CU_add_test(pSuite, "session_recv_continuation",
                    test_nghttp2_session_recv_continuation) ||
       !CU_add_test(pSuite, "session_recv_headers_with_priority",

tests/nghttp2_session_test.c

@@ -800,6 +800,62 @@ void test_nghttp2_session_recv_data(void) {
   nghttp2_session_del(session);
 }
 
+void test_nghttp2_session_recv_data_no_auto_flow_control(void) {
+  nghttp2_session *session;
+  nghttp2_session_callbacks callbacks;
+  my_user_data ud;
+  nghttp2_option *option;
+  nghttp2_frame_hd hd;
+  size_t padlen;
+  uint8_t data[8192];
+  ssize_t rv;
+  size_t sendlen;
+
+  memset(&callbacks, 0, sizeof(nghttp2_session_callbacks));
+  callbacks.send_callback = null_send_callback;
+
+  nghttp2_option_new(&option);
+  nghttp2_option_set_no_auto_window_update(option, 1);
+
+  nghttp2_session_server_new2(&session, &callbacks, &ud, option);
+
+  /* Create DATA frame with length 4KiB + 11 bytes padding*/
+  padlen = 11;
+  memset(data, 0, sizeof(data));
+  hd.length = 4096 + 1 + padlen;
+  hd.type = NGHTTP2_DATA;
+  hd.flags = NGHTTP2_FLAG_PADDED;
+  hd.stream_id = 1;
+  nghttp2_frame_pack_frame_hd(data, &hd);
+  data[NGHTTP2_FRAME_HDLEN] = padlen;
+
+  /* First create stream 1, then close it.  Check that data is
+     consumed for connection in this situation */
+  open_stream(session, 1);
+
+  /* Receive first 100 bytes */
+  sendlen = 100;
+  rv = nghttp2_session_mem_recv(session, data, sendlen);
+  CU_ASSERT((ssize_t)sendlen == rv);
+
+  /* close stream here */
+  nghttp2_submit_rst_stream(session, NGHTTP2_FLAG_NONE, 1, NGHTTP2_NO_ERROR);
+  nghttp2_session_send(session);
+
+  /* stream 1 has been closed, and we disabled auto flow-control, so
+     data must be immediately consumed for connection. */
+  rv = nghttp2_session_mem_recv(session, data + sendlen,
+                                NGHTTP2_FRAME_HDLEN + hd.length - sendlen);
+  CU_ASSERT((ssize_t)(NGHTTP2_FRAME_HDLEN + hd.length - sendlen) == rv);
+
+  /* We already consumed pad length field (1 byte), so do +1 here */
+  CU_ASSERT((int32_t)(NGHTTP2_FRAME_HDLEN + hd.length - sendlen + 1) ==
+            session->consumed_size);
+
+  nghttp2_session_del(session);
+  nghttp2_option_del(option);
+}
+
 void test_nghttp2_session_recv_continuation(void) {
   nghttp2_session *session;
   nghttp2_session_callbacks callbacks;

tests/nghttp2_session_test.h

@@ -30,6 +30,7 @@ void test_nghttp2_session_recv_invalid_stream_id(void);
 void test_nghttp2_session_recv_invalid_frame(void);
 void test_nghttp2_session_recv_eof(void);
 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_premature_headers(void);